Skip to content

feat(audience): pricing rules wizard + reader-segment foundation#465

Open
miguelpeixe wants to merge 23 commits into
mainfrom
feat/audience-pricing-rules-ui
Open

feat(audience): pricing rules wizard + reader-segment foundation#465
miguelpeixe wants to merge 23 commits into
mainfrom
feat/audience-pricing-rules-ui

Conversation

@miguelpeixe

Copy link
Copy Markdown
Member

All Submissions:

Changes proposed in this Pull Request:

Adds a new "Pricing Rules" management page under Audience for the dynamic-pricing engine's rules — a DataViews list plus a recipe-first editor (intent/path picker, conditions, scope targets, price schedule/steps, catalog-impact preview, and reader-segment eligibility). The page registers only when the woocommerce-dynamic-pricing engine plugin is active (the engine owns the rules REST API).

Also includes the reader-segment foundation the eligibility builds on:

  • Matched-segments snapshot — newspack-popups computes and persists the reader's matching segment IDs (utils/segments.js, segmentation.js); a server-side Reader_Data::get_matched_segments() accessor reads it.
  • WooCommerce bridges (dynamic-pricing-bridges, available-deals-bridge) and the reader-segment condition matcher + eligibility helper.

PHP and JS unit tests for the bridges, reader-segment, reader-data, available-deals, and recipe/model-sentence logic are included.

Second of a three-way split of feat/rsm-pricing-rules-dataviews: (1) Plans, (2) Pricing Rules (this PR), (3) the engine-pricing integration. Independent of the Plans PR (any merge order).

How to test the changes in this Pull Request:

  1. With the woocommerce-dynamic-pricing plugin active, go to Audience — a Pricing Rules submenu appears. Without the engine it does not register.
  2. Create/edit a rule: pick a path (intent), set conditions, scope (products/categories), and a price schedule; confirm the catalog-impact preview renders and the rule saves to the engine's REST API.
  3. Add a reader-segment eligibility condition; as a logged-in reader matching that segment, confirm the snapshot persists (matched_segments reader-data) and the preview reflects the segment.
  4. Run the unit suites: dynamic-pricing-bridges, reader-segment-eligibility, reader-data-matched-segments, available-deals-read-only-key, the audience-pricing-rules wizard test, and the recipes / model-sentence / segments JS tests.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Copilot AI review requested due to automatic review settings June 30, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an Audience → Pricing Rules wizard (DataViews list + recipe-first editor) that integrates with the woocommerce-dynamic-pricing engine’s REST API, and introduces the reader-segment snapshot foundation used for dynamic-pricing eligibility and “available deals” targeting.

Changes:

  • Adds a new Pricing Rules wizard shell + React UI (list, editor, impact previews, recipes/model sentence helpers).
  • Persists a logged-in reader’s matched segment IDs from newspack-popups to reader-data and exposes a PHP accessor.
  • Adds WooCommerce/dynamic-pricing bridges (exclusions, modal-checkout annotation, reader-segment matcher, available-deals bridge) plus PHPUnit/JS unit tests.

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
plugins/newspack-popups/src/view/utils/segments.test.js Adds unit tests for matching-segment ID snapshot and sync behavior.
plugins/newspack-popups/src/view/utils/segments.js Implements getMatchingSegmentIds + syncMatchedSegments snapshot persistence.
plugins/newspack-popups/src/view/segmentation.js Hooks snapshot persistence into the segmentation flow.
plugins/newspack-plugin/tests/unit-tests/wizards/class-audience-pricing-rules-test.php Verifies Pricing Rules wizard slug/name.
plugins/newspack-plugin/tests/unit-tests/test-reader-segment-eligibility-assumed.php Tests preview-mode segment intersection helper.
plugins/newspack-plugin/tests/unit-tests/reader-segment-eligibility.php Tests segment eligibility against stored snapshot.
plugins/newspack-plugin/tests/unit-tests/reader-data-matched-segments.php Tests Reader_Data::get_matched_segments() parsing/coercion behavior.
plugins/newspack-plugin/tests/unit-tests/plugins/woocommerce/class-dynamic-pricing-bridges.php Tests dynamic-pricing exclusion bridge behavior.
plugins/newspack-plugin/tests/unit-tests/available-deals-read-only-key.php Ensures available_deals is registered as read-only reader-data.
plugins/newspack-plugin/tests/mocks/wc-mocks.php Extends WooCommerce test shims used by PHPUnit suite.
plugins/newspack-plugin/src/wizards/index.tsx Registers the Pricing Rules wizard entry.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/types.d.ts Adds ambient TS types for the Pricing Rules UI contracts.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/style.scss Adds styling for Pricing Rules UI, tables, and goal cards.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/scope-targets.tsx Adds product/category scope target picker via WP REST.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-preview.tsx Adds debounced per-rule preview panel (impact + audience line).
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-form.tsx Adds create/edit form for common pricing-rule fields + preview.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-edit.tsx Adds route wrapper to load vocab + rule and render form.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/recipes.ts Adds “intent/path” recipes that preset lifecycle/application/scope.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/recipes.test.js Adds unit tests for recipe behavior/visibility rules.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/model-sentence.ts Adds human-readable pricing-model sentence builder for list column.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/model-sentence.test.js Adds unit tests for model sentence formatting.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/list.tsx Adds DataViews list of rules + actions + segment labeling.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/index.tsx Adds wizard container/sections for list + editor routes.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/impact-table.tsx Shared impact table for catalog and per-rule previews.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/impact-format.ts Shared formatting helpers for impact preview rendering.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/datetime.ts Converts UTC timestamps ↔ datetime-local input values.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/conditions.tsx Renders eligibility conditions based on REST vocab field types.
plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/catalog-impact.tsx Adds catalog-wide impact preview panel above list.
plugins/newspack-plugin/includes/wizards/audience/class-audience-pricing-rules.php Adds PHP wizard shell + submenu + localized config.
plugins/newspack-plugin/includes/reader-activation/class-reader-data.php Adds Reader_Data::get_matched_segments() accessor.
plugins/newspack-plugin/includes/plugins/woocommerce/class-reader-segment-eligibility.php Adds engine-free segment eligibility helper.
plugins/newspack-plugin/includes/plugins/woocommerce/class-reader-segment-condition-matcher.php Adds dynamic-pricing engine condition matcher for reader segments.
plugins/newspack-plugin/includes/plugins/woocommerce/class-dynamic-pricing-bridges.php Adds dynamic-pricing engine bridges + modal checkout annotation + preview segment groups.
plugins/newspack-plugin/includes/plugins/woocommerce/class-available-deals-bridge.php Adds available-deals computation bridge + segmentation criterion.
plugins/newspack-plugin/includes/class-wizards.php Registers Pricing Rules wizard only when engine plugin is active.
plugins/newspack-plugin/includes/class-newspack.php Includes the new wizard + dynamic pricing bridges at bootstrap.

Comment thread plugins/newspack-plugin/tests/mocks/wc-mocks.php
Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-form.tsx Outdated
Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-preview.tsx Outdated
@miguelpeixe

Copy link
Copy Markdown
Member Author

Addressed Copilot's review:

  • wc-mocks.php wc_clean() dropped — restored (it had been removed); fixed alongside the unit-test WC-mock gate fix in e1b410503.
  • rule-form.tsx conditions typed too narrow — widened to the shared ConditionsMap (6e09430c1).
  • rule-form.tsx form stuck "saving" — reset isSaving(false) before the stepped-validation early return (6e09430c1).
  • available-deals-bridge.php "advertisable/publicize" copy — docblock + criterion description updated to match the actual behavior (all active rules; the segment selection is the targeting) (6e09430c1).
  • rule-preview.tsx useRef() no initial value — explicit useRef<…|undefined>(undefined) (6e09430c1).
  • reader-data.php matched_segments spoofable — kept client-asserted by design (segments are computed client-side), and documented at the eligibility use-site that the gate is best-effort targeting, not a hard entitlement — do not use it as a security boundary (f353a5d67).

@miguelpeixe
miguelpeixe marked this pull request as ready for review July 2, 2026 16:15
@miguelpeixe
miguelpeixe requested a review from a team as a code owner July 2, 2026 16:15
@miguelpeixe
miguelpeixe requested a review from dkoo July 2, 2026 16:15
@miguelpeixe miguelpeixe self-assigned this Jul 2, 2026
@kmwilkerson

kmwilkerson commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@miguelpeixe — flagging a spec-vs-code contradiction on Win-back that surfaced during the QA-plan validation. It's not a bug in this PR's code, but it gates the Win-back goal this wizard exposes, so raising it here.

The conflict — does a lapsed reader using WCS' "Resubscribe" get the win-back deal?

  • Spec 15 says yes. [specs/15-save-and-winback.md]: "Delivery: nothing new. WCS resubscribe flows through the cart → the engine prices it → lock-at-purchase pins it" (line 31), and its own test plan asserts "resubscribe a cancelled customer under a win-back rule → the resubscribe gets the deal" (line 77).
  • The shipped engine says no. WooProduct_Surface::is_eligible_cart_item() (includes/class-woo-product-surface.php:471) excludes subscription_resubscribe outright, and the class header (lines 32–36) calls it "a cancel-and-resubscribe loophole around stepped pricing" — a deliberate block. So a reader clicking WCS's Resubscribe action pays regular; the win-back rule only fires on a fresh add-to-cart purchase.

But win-back should price a WCS Resubscribe. A lapsed reader reactivating from My Account is the low-friction claim path we're optimizing for, and the cancel-and-resubscribe abuse case doesn't really apply to win-back — they have to genuinely lapse first, and a returner already fails the new-subscriber gate, so this isn't a stepped-intro loophole. Can we make subscription_resubscribe eligible when a win-back/lapsed_subscriber rule matches, while keeping the exclusion for renewals, switches, and stepped-schedule cycle-reset? Spec 15 stands; the current engine surface block (WooProduct_Surface::is_eligible_cart_item) needs to become goal-aware rather than blanket. Let's confirm the reader claim path (email/prompt → My Account → Reactivate → win-back price applied) works end-to-end.

@miguelpeixe

Copy link
Copy Markdown
Member Author

@kmwilkerson — confirmed and fixed; thanks for catching this, you're right on all three counts.

The contradiction, verified: spec 15 (specs/15-save-and-winback.md:31) says "WCS resubscribe flows through the cart → the engine prices it → lock-at-purchase pins it" (and its test plan, L77, asserts the resubscribe gets the deal), but WooProduct_Surface::is_eligible_cart_item() blanket-excluded subscription_resubscribe, so the surface never priced it. Reproduced in-env: fresh add-to-cart → win-back $4/mo; My Account → Resubscribe → $10/mo (regular).

Fix (merged to the woocommerce-dynamic-pricing engine trunk, 2113090): made the exclusion goal/strategy-aware, as you proposed. subscription_resubscribe is now eligible; the cancel-and-resubscribe-resets-stepped-pricing abuse — the exclusion's real purpose — moves to the strategy layer: context() flags is_resubscribe and Stepped_By_Cycle_Strategy abstains, so flat Win-back deals price a resubscribe while stepped schedules don't. Renewals and switches stay excluded.

Verified: engine PHPUnit 353/353 (new resubscribe cases included); env repro now prices a resubscribe at the win-back $4/mo, and a resubscribe under a stepped rule does not reset to the intro step. The reader claim path (email/prompt → My Account → Resubscribe → win-back price applied) works end-to-end. Spec 15's delivery note is reconciled with a dated note.

Scope: the fix lives in the engine, not this PR — #465's wizard correctly exposes the Win-back goal; the engine surface was under-delivering it. Also added the resubscribe path to the QA harness so §5 can't pass on the fresh-add path alone again.

@dkoo dkoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miguelpeixe Reviewed as the human pass — code review + hands-on manual QA in an isolated integrated env (#464+#465+#466, WCS 9.0 + Stripe test). Approving. #465's Newspack code is well-built, secure, and correct: the reader-segment foundation carries an explicit "client-asserted, never an entitlement gate" caveat, the WooCommerce bridges are clean engine-seam code, the DataViews rule builder follows admin-wizard conventions, and both in-scope regression tickets (NPPD-1854, NPPD-1853) are implemented and verified in-browser. It also resolves #466's F1 — the donation-exclusion bridge correctly degrades donations to base. Only suggestions/nits remain (inline below).

⚑ Release coordination (important)

Reader-segment eligibility depends on an engine fix. #465's reader_segment matcher is the first array-valued (select) condition, and the engine's REST sanitize_condition_value previously dropped the array (PR #1's E6 hardening rejected non-scalars) — so a reader-segment rule silently saved with no segment. That's fixed in engine PR #2 (fix/rest-select-condition-sanitize, sanitizes selectint[]); I verified live that it resolves both persistence and the M2 hydration below, with the harness §9 leg now green. Please make sure #465 ships alongside that engine fix.

Engine follow-ups (not #465 code — flagging for coordination)

  • Product/shop-page disclosure (medium): a segment-gated discount applies on the single-product/shop page but silently. The engine's Product_Schedule_Display::render()Product_Rules_Inspector::matching_rules_for() builds a Pricing_Context with a null customer, so the segment-gated rule is excluded from the disclosure list — yet the price path uses the real customer and substitutes the price. Net: correct price, but no reader label / "regular price" footnote (the publicize help text promises the comparison shows "on the product page"). #465's label wiring is correct; this is engine-side. (The modal-checkout leg is a separate timing gap that needs #453.)
  • Capability mismatch (nit): the wizard inherits manage_options while the engine's rule REST is gated at edit_products, so a shop_manager can hit the REST but not see the menu. Engine/product-team call.

Subjective UX suggestion (M3)

The impact preview highlights the "Resulting price" column (#f0b849) when no segment is selected, but doesn't highlight the matching-segment columns when one is — consider highlighting those too.

Checked & clean

Security: the engine REST is the auth boundary (edit_products + absint ids); the wizard is a page shell that owns no REST; available_deals is registered read-only so the client can't forge it; modal annotation emits plain text. The parse_form() $_POST read is safe — the engine verifies nonce + cap before calling it. The one console warning (a WP-components Tooltip ref) is pre-existing, not #465.

Thanks — solid work, especially the explicit security caveats on the client-asserted segment data.

Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-form.tsx Outdated
Comment thread plugins/newspack-plugin/includes/reader-activation/class-reader-data.php Outdated
Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/style.scss Outdated
Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/list.tsx Outdated
@github-actions github-actions Bot added the [Status] Approved Pull request has been approved label Jul 16, 2026
@miguelpeixe

Copy link
Copy Markdown
Member Author

Thanks for the thorough review, @dkoo — all five inline suggestions are addressed in 6f17df9 (replies on each thread). Notes on the review-body items:

M3 — impact preview highlight: Done. The matching-segment columns now get a faint column highlight in the per-rule preview (a light tint of the same $alert-yellow hue as the changed-cell highlight), mirroring how the sole "Resulting price" column reads when no segment is selected. A changed cell within a segment column still gets the solid highlight on top.

Release coordination (engine PR #2): Understood — this ships alongside the selectint[] sanitize fix (fix/rest-select-condition-sanitize). The M2 client-side coercion above is the belt-and-suspenders that also heals legacy string-saved segment rules independently of the server value type.

Engine follow-ups (product/shop-page disclosure, capability mismatch): Agreed these are engine-side, not #465 — no change here; tracking separately.

@dkoo dkoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miguelpeixe Thanks for the quick turnaround — the fixes look good: the segment token now hydrates on edit (M2, nice seedConditions() coerce), get_matched_segments drops non-scalars with a test (N2), the datetime normalize + save-time validation resolve the silent-drop (M4 — the @wordpress/components swap was the soft "and/or" option, no worries), and the $white + DataViews trash-modal nits are done. Nice touch on the segment-column highlight, too.

I'm flipping this back to Request changes for one issue — and it's on us: we missed it in our prior approval. During follow-up manual testing of the effective-price surfaces I hit an edit-view routing bug that predates your fix commit (so it isn't something you introduced — we just didn't catch it the first time):

Edit-view URL routing shows the wrong rule. Editing a rule by clicking it from the DataViews table renders the correct rule, but navigating directly to a different rule's ID in the URL shows the previous rule's settings. Root cause: RuleEdit renders <RuleForm … /> (rule-edit.tsx:75) with no key, and the fetch effect never resets isLoading on an id change — so RuleForm (whose state is seeded by mount-only useState initializers) doesn't remount and keeps the prior rule's state. Fix: key={ id ?? 'new' } on <RuleForm> (or setIsLoading( true ) as the first line of the fetch effect). Details inline.

Apologies for the late catch — everything else is good to go once this lands. Happy to re-verify.

Comment thread plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-edit.tsx Outdated
@github-actions github-actions Bot added [Status] Needs Changes or Feedback Pull request needs changes or feedback and removed [Status] Approved Pull request has been approved labels Jul 17, 2026
@miguelpeixe
miguelpeixe requested a review from dkoo July 17, 2026 13:23
@miguelpeixe miguelpeixe added [Status] Needs Review and removed [Status] Needs Changes or Feedback Pull request needs changes or feedback labels Jul 17, 2026

@dkoo dkoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miguelpeixe B1 is fixed and verified live — the edit view now remounts on a rule-id change (the key={ id ?? 'new' } + the setIsLoading( true ) reset both do the job), so navigating directly between rule ids in the URL shows the correct rule, and the added routing test locks it in. All the earlier items are resolved too (M2 segment coerce, N2 is_scalar, N5/N6 nits, and the M4 functional fixes — the @wordpress/components swap was the soft option). Approving — thanks for the quick turnarounds. 🎉

@github-actions github-actions Bot added [Status] Approved Pull request has been approved and removed [Status] Needs Review labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Status] Approved Pull request has been approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants