feat(audience): pricing rules wizard + reader-segment foundation#465
feat(audience): pricing rules wizard + reader-segment foundation#465miguelpeixe wants to merge 23 commits into
Conversation
There was a problem hiding this comment.
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. |
|
Addressed Copilot's review:
|
…into feat/audience-pricing-rules-ui
|
@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?
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. |
|
@kmwilkerson — confirmed and fixed; thanks for catching this, you're right on all three counts. The contradiction, verified: spec 15 ( Fix (merged to the 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
left a comment
There was a problem hiding this comment.
@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 select → int[]); 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 aPricing_Contextwith 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_optionswhile the engine's rule REST is gated atedit_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.
|
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 Release coordination (engine PR #2): Understood — this ships alongside the Engine follow-ups (product/shop-page disclosure, capability mismatch): Agreed these are engine-side, not #465 — no change here; tracking separately. |
dkoo
left a comment
There was a problem hiding this comment.
@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.
dkoo
left a comment
There was a problem hiding this comment.
@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. 🎉
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-pricingengine plugin is active (the engine owns the rules REST API).Also includes the reader-segment foundation the eligibility builds on:
utils/segments.js,segmentation.js); a server-sideReader_Data::get_matched_segments()accessor reads it.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:
matched_segmentsreader-data) and the preview reflects the segment.Other information: