fix(checkout): correct addon pricing to only charge for new products#354
Open
superdav42 wants to merge 5 commits intomainfrom
Open
fix(checkout): correct addon pricing to only charge for new products#354superdav42 wants to merge 5 commits intomainfrom
superdav42 wants to merge 5 commits intomainfrom
Conversation
…_templates Fixes #351 - Template validation failing with 'The selected template is not available for this product' error. Root cause: Template IDs stored as string keys in the limitations array were not being converted to integers, causing type mismatch with the validation rule which uses absint() on the submitted template ID. Changes: - Updated get_available_site_templates() to convert site_id to integer - Updated get_pre_selected_site_template() to convert site_id to integer - Added regression test to ensure template IDs are returned as integers - Test validates strict type checking with in_array(..., true) This ensures consistent integer comparison in the Site_Template validation rule (inc/helpers/validation-rules/class-site-template.php:104).
Previously, when customers added addon services to their existing membership,
the cart was incorrectly charging for the next billing period in advance:
- Added the existing plan at full price (€90)
- Added the new addon (€5)
- Subtracted a small pro-rata credit (~€5.41 for 2 days used)
- Result: Customer charged €89.59 instead of just €5
This commit fixes three related bugs:
1. **Pro-rata applied incorrectly for addon purchases**
- Changed wu_cart_addon_include_existing_plan filter default from true to false
- Addon purchases now only charge for the new addon products
- Existing plan continues to be billed on its regular subscription schedule
- Pro-rata credits are only applied when actually changing plans (upgrades/downgrades)
2. **Existing discount codes not applied to addons**
- Now applies membership discount codes to addon purchases when apply_to_renewals is enabled
- Ensures consistent pricing for customers with recurring discounts
3. **Plan removal from addon-only carts**
- In the second addon detection path (product count > 1 with no plan change),
explicitly removes the plan from products and line_items
- Prevents accidental plan charges in edge cases
Includes comprehensive unit tests covering:
- Addon-only pricing (should only charge for addon)
- Discount code application to addons
- Filter override capability for backward compatibility
- Plan upgrade/downgrade still using pro-rata correctly
- Setup fee handling for addon products
Fixes customer-reported issue where €5 addon was charging €89.59
Contributor
📝 WalkthroughWalkthroughAdds addon-specific cart behavior: by default, addon-only carts remove existing plans and skip prorate credits; introduces a filter to re-include plans, applies membership discount codes to addons (with a private helper to reapply discounts), normalizes site template IDs, updates membership swap logic, and adds tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cart
participant Membership
participant Billing
User->>Cart: add addon product
Cart->>Membership: get_discount_code() & validate membership renewals
Membership-->>Cart: discount object (if valid)
alt wu_cart_addon_include_existing_plan = true
Cart->>Cart: re-add existing plan line items
Cart->>Cart: calculate prorate credits
Cart->>Cart: reapply_discounts_to_existing_line_items()
else wu_cart_addon_include_existing_plan = false
Cart->>Cart: remove existing plan line items
Cart->>Cart: skip prorate credits
end
Cart->>Billing: recalc totals, taxes, apply discounts
Billing-->>User: present final cart total
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/WP_Ultimo/Objects/Limitations_Test.php (1)
914-950: Add direct assertion for pre-selected template integer typing.This test fully covers
get_available_site_templates(), but it doesn’t directly lock the newget_pre_selected_site_template()integer normalization path.Suggested test extension
public function test_available_site_templates_returns_integers(): void { @@ $this->assertTrue(in_array(123, $available, true), 'in_array with strict=true should find integer 123'); $this->assertTrue(in_array(456, $available, true), 'in_array with strict=true should find integer 456'); + + $pre_selected = $limitations->site_templates->get_pre_selected_site_template(); + $this->assertIsInt($pre_selected, 'Pre-selected template ID should be integer'); + $this->assertSame(456, $pre_selected, 'Pre-selected template ID should match expected integer'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Objects/Limitations_Test.php` around lines 914 - 950, Extend the test to also exercise the integer normalization path by calling Limitations->site_templates->get_pre_selected_site_template() and asserting it returns the pre-selected template as an integer (assertIsInt) and equals 456 (or assertSame(456, ...)/in_array with strict=true); this ensures get_pre_selected_site_template() normalizes string keys like '456' into an int and preserves strict comparisons alongside the existing get_available_site_templates() checks.tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php (1)
178-203: Usetry/finallyfor filter cleanup to avoid test leakage.If an assertion fails before Line 202, the filter remains active and can affect subsequent tests.
💡 Proposed fix
public function test_addon_filter_can_include_existing_plan() { // Add filter to force inclusion of existing plan add_filter('wu_cart_addon_include_existing_plan', '__return_true'); - - $cart = new Cart([ - 'customer_id' => self::$customer->get_id(), - 'membership_id' => self::$membership->get_id(), - 'products' => [self::$addon->get_id()], - ]); - - // Should have 2 product line items (plan + addon) - $line_items = $cart->get_line_items(); - $product_line_items = array_filter($line_items, function($item) { - return $item->get_type() === 'product'; - }); - - $this->assertCount(2, $product_line_items, 'Should have 2 product line items when filter returns true'); - - // Should have a pro-rata credit line item - $credit_line_items = array_filter($line_items, function($item) { - return $item->get_type() === 'credit'; - }); - $this->assertGreaterThan(0, count($credit_line_items), 'Should have pro-rata credit when plan is included'); - - // Remove filter - remove_filter('wu_cart_addon_include_existing_plan', '__return_true'); + try { + $cart = new Cart([ + 'customer_id' => self::$customer->get_id(), + 'membership_id' => self::$membership->get_id(), + 'products' => [self::$addon->get_id()], + ]); + + // Should have 2 product line items (plan + addon) + $line_items = $cart->get_line_items(); + $product_line_items = array_filter($line_items, function($item) { + return $item->get_type() === 'product'; + }); + + $this->assertCount(2, $product_line_items, 'Should have 2 product line items when filter returns true'); + + // Should have a pro-rata credit line item + $credit_line_items = array_filter($line_items, function($item) { + return $item->get_type() === 'credit'; + }); + $this->assertGreaterThan(0, count($credit_line_items), 'Should have pro-rata credit when plan is included'); + } finally { + remove_filter('wu_cart_addon_include_existing_plan', '__return_true'); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 178 - 203, Wrap the section that adds the filter and exercises the cart in a try/finally so the filter is always removed even if an assertion fails: call add_filter('wu_cart_addon_include_existing_plan', '__return_true') before creating the Cart and computing $line_items/$product_line_items/$credit_line_items, then put the assertions inside the try block and call remove_filter('wu_cart_addon_include_existing_plan', '__return_true') in the finally block to guarantee cleanup; reference the existing add_filter/remove_filter calls and the Cart instantiation to locate where to wrap the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/checkout/class-cart.php`:
- Around line 892-903: The membership discount is being set after addon products
are added (add_product), so existing line items aren't repriced; add a private
helper method reapply_discounts_to_existing_line_items() in the Cart class that
iterates $this->line_items, skips non-discountable items (is_discountable()),
re-runs apply_discounts_to_item() and, if is_taxable(), apply_taxes_to_item(),
and writes back each item to $this->line_items; then call this helper
immediately after add_discount_code($membership_discount_code) wherever you set
the membership discount (the current hunk and the similar block around lines
~1008-1012) so addons already in the cart are repriced.
- Around line 1023-1037: When removing the existing plan in class-cart.php
(inside the foreach over $this->products where
wu_is_plan_type($product->get_type()) && $product->get_id() ===
$membership->get_plan_id()), also remove any residual fee line items tied to
that plan: iterate $this->line_items and unset entries where get_product_id()
=== $product->get_id() OR where get_type() === 'fee' and metadata indicates
association with that product/plan; additionally ensure the filter added later
(around the product insertion path) is applied earlier or replicate its logic
here so fees are cleaned when the plan product is removed; keep resetting
$this->plan_id = 0 and breaking as before.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Line 1: Add the missing PHPDoc and fix multiline call formatting to satisfy
PHPCS: add a file-level docblock at the top, add class and all test method
docblocks for the Cart_Addon_Pricing_Test class and each test method referenced
(ensure summary, `@covers/`@group tags as appropriate), and reformat any multiline
function/method calls across lines to match the project's coding standard
(aligned parentheses, one argument per line where required, and proper
indentation). Update the docblocks and formatting for the methods and calls
covering the ranges mentioned (lines ~30–73) so the file passes PHPCS checks.
---
Nitpick comments:
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 178-203: Wrap the section that adds the filter and exercises the
cart in a try/finally so the filter is always removed even if an assertion
fails: call add_filter('wu_cart_addon_include_existing_plan', '__return_true')
before creating the Cart and computing
$line_items/$product_line_items/$credit_line_items, then put the assertions
inside the try block and call
remove_filter('wu_cart_addon_include_existing_plan', '__return_true') in the
finally block to guarantee cleanup; reference the existing
add_filter/remove_filter calls and the Cart instantiation to locate where to
wrap the code.
In `@tests/WP_Ultimo/Objects/Limitations_Test.php`:
- Around line 914-950: Extend the test to also exercise the integer
normalization path by calling
Limitations->site_templates->get_pre_selected_site_template() and asserting it
returns the pre-selected template as an integer (assertIsInt) and equals 456 (or
assertSame(456, ...)/in_array with strict=true); this ensures
get_pre_selected_site_template() normalizes string keys like '456' into an int
and preserves strict comparisons alongside the existing
get_available_site_templates() checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7b07357-253f-404b-8969-b200b56e0eda
📒 Files selected for processing (4)
inc/checkout/class-cart.phpinc/limitations/class-limit-site-templates.phptests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.phptests/WP_Ultimo/Objects/Limitations_Test.php
1. Add reapply_discounts_to_existing_line_items() helper method - Discounts were not being applied to addons already in cart - Helper reapplies discounts and taxes to all discountable line items - Called after add_discount_code() in both addon paths 2. Fix plan removal to also remove fee line items - When removing existing plan from addon cart, also remove 'fee' line items - Changed line item type check from 'product' only to ['product', 'fee'] - Prevents accidental setup fee charges for removed plans 3. Fix PHPCS violations in test file - Added file-level docblock - Added docblocks for all class properties - Reformatted multiline array calls to match coding standards - Used array() instead of [] and proper indentation 4. Add try/finally for filter cleanup in tests - Wrapped test_addon_filter_can_include_existing_plan() in try/finally - Ensures filter is always removed even if assertions fail - Prevents test leakage to other test methods These changes address all CodeRabbit review comments while maintaining the core functionality of the addon pricing fix.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php (1)
76-131:⚠️ Potential issue | 🟠 MajorPHPCS issues are still present in this test file and will keep CI red.
The multiline call formatting and alignment problems reported around Line 76-131, Line 153, and Line 199 should be fixed before merge.
💡 Example formatting fix pattern
- self::$customer = wu_create_customer([ + self::$customer = wu_create_customer( + [ 'username' => 'testuser_addon_pricing', 'email' => 'addon_pricing@example.com', 'password' => 'password123', - ]); + ] + );Also applies to: 153-153, 199-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 76 - 131, The PHP CS (PHPCS) failures are due to multiline array argument formatting/alignment in the test setup calls—fix the formatting of the wu_create_customer, wu_create_product (both plan and addon), wu_create_discount_code, and wu_create_membership calls so each array key => value pair is on its own line, aligned and indented consistently with the project's coding standard (e.g., PSR-12), and remove any trailing comma/spacing issues; specifically adjust the argument arrays passed to wu_create_customer, the two wu_create_product calls, wu_create_discount_code, and wu_create_membership (which references Membership_Status::ACTIVE and wu_date()) to match the same multiline indentation and alignment pattern used elsewhere in the test suite.inc/checkout/class-cart.php (1)
1045-1057:⚠️ Potential issue | 🟠 MajorLate
wu_apply_signup_feefilter registration is ineffective and leaks globally.Line 1056 registers a global filter after addon products were already added, so it does not affect this cart path and can affect subsequent cart operations in the same request.
💡 Proposed fix
- /* - * For addon purchases, only apply setup fees to NEW addon products. - * Skip setup fees if the membership has already been billed at least once, - * as the plan's setup fee was already paid. - * - * Note: Products were already added to the cart above (line 840), so setup - * fees for addons have already been processed. This filter mainly affects - * any future product additions in this request. - * - * `@since` 2.0.12 - */ - add_filter('wu_apply_signup_fee', fn() => $membership->get_times_billed() <= 0); + // No signup-fee filter here: products were already added above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 1045 - 1057, The wu_apply_signup_fee filter is registered too late (after addons were added) and left active for the rest of the request; change it so the filter is applied at the correct time and is scoped/removed immediately afterwards: instead of an anonymous one-off add_filter('wu_apply_signup_fee', fn() => $membership->get_times_billed() <= 0) at the end of this block, either (A) move the filter registration to before addons/products are added so it affects the addon setup-fee calculation (use the same callback logic referencing $membership->get_times_billed()), or (B) register the callback to a variable or named function (e.g., $apply_signup_fee_cb or function apply_signup_fee_for_membership) before adding addons and then call remove_filter('wu_apply_signup_fee', $apply_signup_fee_cb) right after addon processing to avoid leaking the filter globally; update code references around add_filter('wu_apply_signup_fee', ...), membership->get_times_billed(), and the addon/product addition logic to implement this.
🧹 Nitpick comments (1)
tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php (1)
143-147:customer_idin cart args is ignored; tests should set customer context explicitly.Cart setup does not consume
customer_id, so these tests depend on ambient current-user state. Please set the current customer/user explicitly in fixture setup and avoid passing unsupported cart args.Also applies to: 187-191, 276-280, 313-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 143 - 147, The Cart constructor ignores the 'customer_id' argument causing tests to rely on global current-user state; update the fixture setup to explicitly set the current user/context before creating Cart instances (e.g., call the test helper that sets the WP/current customer user to self::$customer->get_id() or use wp_set_current_user with self::$customer->get_id()) and remove/stop relying on the unsupported 'customer_id' cart arg in the Cart(...) calls (affects Cart creation at the shown location and also the instances at the other ranges referenced).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/checkout/class-cart.php`:
- Around line 899-904: The code calls $membership->get_discount_code() and
immediately invokes ->should_apply_to_renewals(), which can crash if
get_discount_code() returns a string or false; update the checks around
membership->get_discount_code() (used at the shown block and the other
occurrence around lines ~1009-1014) to first verify the returned value is an
object of the expected discount class (or at minimum is_object() and
method_exists('should_apply_to_renewals')) before calling
->should_apply_to_renewals(), and only then call
$this->add_discount_code($membership_discount_code) and
$this->reapply_discounts_to_existing_line_items().
---
Duplicate comments:
In `@inc/checkout/class-cart.php`:
- Around line 1045-1057: The wu_apply_signup_fee filter is registered too late
(after addons were added) and left active for the rest of the request; change it
so the filter is applied at the correct time and is scoped/removed immediately
afterwards: instead of an anonymous one-off add_filter('wu_apply_signup_fee',
fn() => $membership->get_times_billed() <= 0) at the end of this block, either
(A) move the filter registration to before addons/products are added so it
affects the addon setup-fee calculation (use the same callback logic referencing
$membership->get_times_billed()), or (B) register the callback to a variable or
named function (e.g., $apply_signup_fee_cb or function
apply_signup_fee_for_membership) before adding addons and then call
remove_filter('wu_apply_signup_fee', $apply_signup_fee_cb) right after addon
processing to avoid leaking the filter globally; update code references around
add_filter('wu_apply_signup_fee', ...), membership->get_times_billed(), and the
addon/product addition logic to implement this.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 76-131: The PHP CS (PHPCS) failures are due to multiline array
argument formatting/alignment in the test setup calls—fix the formatting of the
wu_create_customer, wu_create_product (both plan and addon),
wu_create_discount_code, and wu_create_membership calls so each array key =>
value pair is on its own line, aligned and indented consistently with the
project's coding standard (e.g., PSR-12), and remove any trailing comma/spacing
issues; specifically adjust the argument arrays passed to wu_create_customer,
the two wu_create_product calls, wu_create_discount_code, and
wu_create_membership (which references Membership_Status::ACTIVE and wu_date())
to match the same multiline indentation and alignment pattern used elsewhere in
the test suite.
---
Nitpick comments:
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 143-147: The Cart constructor ignores the 'customer_id' argument
causing tests to rely on global current-user state; update the fixture setup to
explicitly set the current user/context before creating Cart instances (e.g.,
call the test helper that sets the WP/current customer user to
self::$customer->get_id() or use wp_set_current_user with
self::$customer->get_id()) and remove/stop relying on the unsupported
'customer_id' cart arg in the Cart(...) calls (affects Cart creation at the
shown location and also the instances at the other ranges referenced).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e269727-048d-460d-a8b2-14f8b7a7518f
📒 Files selected for processing (2)
inc/checkout/class-cart.phptests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php
1. Fix get_discount_code() type safety - get_discount_code() can return Discount_Code object, string, or false - Added is_object() and method_exists() checks before calling should_apply_to_renewals() - Applied to both addon detection paths (lines 899-907, 1018-1026) - Prevents fatal errors if discount code is not an object 2. Fix test fixture creation errors - Added missing required fields: pricing_type, currency, active - Added is_wp_error() checks with self::fail() for all fixtures - Prevents TypeError when wu_create_* functions return WP_Error - Changed all array literals from [] to array() for PHPCS compliance 3. Test improvements - All Cart instantiations now use array() instead of [] - All multiline arrays properly formatted with proper indentation - Added docblock for tear_down_after_class() These fixes address: - TypeError: Cannot assign WP_Error to property (test failures) - CodeRabbit suggestion about discount code type safety - PHPCS violations for array syntax Tests should now pass successfully.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
inc/checkout/class-cart.php (2)
903-913:⚠️ Potential issue | 🟠 MajorResolve string membership discount codes before renewal checks.
These blocks only apply discounts when
get_discount_code()returns an object. If it returns a string code (which is a valid path), addon discounts are skipped.💡 Suggested fix
$membership_discount_code = $membership->get_discount_code(); + if (is_string($membership_discount_code)) { + $membership_discount_code = wu_get_discount_code_by_code($membership_discount_code); + } if ( - $membership_discount_code - && is_object($membership_discount_code) - && method_exists($membership_discount_code, 'should_apply_to_renewals') + $membership_discount_code instanceof \WP_Ultimo\Models\Discount_Code && $membership_discount_code->should_apply_to_renewals() ) { $this->add_discount_code($membership_discount_code); $this->reapply_discounts_to_existing_line_items(); }Also applies to: 1022-1032
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 903 - 913, The code only handles membership discount codes when membership->get_discount_code() returns an object, skipping valid string codes; update the renewal check to resolve string discount codes into discount objects before inspecting should_apply_to_renewals(): call get_discount_code(), if it's a string use the existing discount resolution logic (the same conversion used elsewhere when adding codes) to obtain the discount object, then check method_exists($discount, 'should_apply_to_renewals') and $discount->should_apply_to_renewals() and only then call $this->add_discount_code($discount) and $this->reapply_discounts_to_existing_line_items(); apply the same fix to the other occurrence that mirrors this block (the second get_discount_code() usage).
1063-1074:⚠️ Potential issue | 🟠 MajorRemove late
wu_apply_signup_feefilter in this addon edge-path.At this point, product insertion already happened, so this filter does not affect current line items and can bleed into unrelated operations later in the same request.
💡 Suggested fix
- /* - * For addon purchases, only apply setup fees to NEW addon products. - * Skip setup fees if the membership has already been billed at least once, - * as the plan's setup fee was already paid. - * - * Note: Products were already added to the cart above (line 840), so setup - * fees for addons have already been processed. This filter mainly affects - * any future product additions in this request. - * - * `@since` 2.0.12 - */ - add_filter('wu_apply_signup_fee', fn() => $membership->get_times_billed() <= 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 1063 - 1074, The late add_filter('wu_apply_signup_fee', fn() => $membership->get_times_billed() <= 0) is registered after products were already added and can leak into unrelated work later in the same request; make this a scoped/temporary filter by registering it only around the code that needs it (or store the closure in a variable and call remove_filter('wu_apply_signup_fee', $closure) immediately after the product-insert logic), or replace it with a direct conditional when creating addon items so you don't register the global 'wu_apply_signup_fee' filter at all; locate the add_filter call referencing wu_apply_signup_fee and membership->get_times_billed() and ensure it is removed or confined to the immediate operation.tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php (1)
84-85:⚠️ Potential issue | 🟠 MajorFix PHPCS-reported spacing/alignment to keep checks green.
Current style around
is_wp_error(...)and the discount-code array still violates the configured standard.💡 Suggested fix
- if ( is_wp_error( self::$customer ) ) { + if (is_wp_error(self::$customer)) { self::fail( 'Failed to create test customer' ); } @@ - if ( is_wp_error( self::$plan ) ) { + if (is_wp_error(self::$plan)) { self::fail( 'Failed to create test plan' ); } @@ - if ( is_wp_error( self::$addon ) ) { + if (is_wp_error(self::$addon)) { self::fail( 'Failed to create test addon' ); } @@ - 'name' => 'Test Discount', - 'code' => 'TEST10', - 'value' => 10, - 'type' => 'percentage', - 'uses' => 0, - 'max_uses' => 100, - 'apply_to_renewals' => true, + 'name' => 'Test Discount', + 'code' => 'TEST10', + 'value' => 10, + 'type' => 'percentage', + 'uses' => 0, + 'max_uses' => 100, + 'apply_to_renewals' => true,Also applies to: 105-106, 126-126, 133-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 84 - 85, Fix PHPCS spacing/alignment issues by normalizing the conditional and array formatting: remove or add spaces inside function-call parentheses to match the project standard for is_wp_error checks (update the occurrences using is_wp_error and self::$customer), and reformat the discount-code array entries so keys, arrows (=>) and values are consistently aligned and indented according to the coding standard; apply the same adjustments to the other occurrences noted (the other is_wp_error checks and the discount-code array blocks in this test class) so PHPCS passes.
🧹 Nitpick comments (1)
inc/checkout/class-cart.php (1)
1085-1085: Drop stale commented-out code marker.Prefer a normal explanatory comment over commented executable code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` at line 1085, Remove the stale commented-out code marker "$this->calculate_prorate_credits();" in class-cart.php and replace it with a short explanatory comment describing why prorate credit calculation was removed or where that logic now lives (e.g., mention the current method or hook responsible for prorate handling), so future readers understand intent instead of seeing commented executable code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 311-325: The test dereferences products returned by
wu_create_product (e.g., $upgraded_plan and $addon_with_fee) without checking
for creation errors; add is_wp_error() guards immediately after each
wu_create_product call (for $upgraded_plan and $addon_with_fee) and handle
failures by failing or skipping the test (use $this->fail(...) or
$this->markTestSkipped(...)) with a clear message so subsequent dereferences are
safe; locate the calls to wu_create_product in the test and insert the
is_wp_error check and failure/skip logic for each created product.
- Around line 72-73: In set_up_before_class and the other test locations that
build membership-change carts, ensure the test sets the WordPress current user
to the intended customer so Cart sanitization honors the customer_id: create the
customer user via the test factory (or use the existing test user IDs) and call
wp_set_current_user($customer_id) (or wp_set_current_user(self::$customer_id))
immediately before instantiating new Cart objects; alternatively, if Cart has a
static sanitization helper (e.g., Cart::sanitize_args or Cart::__construct),
call wp_set_current_user first so Cart's customer_id logic sees the correct
current user.
- Around line 76-87: The typed static properties (self::$customer, self::$plan,
self::$addon, self::$discount_code, self::$membership) are being assigned the
raw results of factory functions (wu_create_customer, wu_create_product,
wu_create_membership, wu_create_discount_code) which may return WP_Error and
will cause a TypeError; instead call each factory into a local variable first,
check is_wp_error(localVar) and call self::fail(...) on error, then assign the
verified value to the typed static property. Update all occurrences where these
factories are directly assigned (the blocks around the current assignments) to
follow this guard-then-assign pattern.
---
Duplicate comments:
In `@inc/checkout/class-cart.php`:
- Around line 903-913: The code only handles membership discount codes when
membership->get_discount_code() returns an object, skipping valid string codes;
update the renewal check to resolve string discount codes into discount objects
before inspecting should_apply_to_renewals(): call get_discount_code(), if it's
a string use the existing discount resolution logic (the same conversion used
elsewhere when adding codes) to obtain the discount object, then check
method_exists($discount, 'should_apply_to_renewals') and
$discount->should_apply_to_renewals() and only then call
$this->add_discount_code($discount) and
$this->reapply_discounts_to_existing_line_items(); apply the same fix to the
other occurrence that mirrors this block (the second get_discount_code() usage).
- Around line 1063-1074: The late add_filter('wu_apply_signup_fee', fn() =>
$membership->get_times_billed() <= 0) is registered after products were already
added and can leak into unrelated work later in the same request; make this a
scoped/temporary filter by registering it only around the code that needs it (or
store the closure in a variable and call remove_filter('wu_apply_signup_fee',
$closure) immediately after the product-insert logic), or replace it with a
direct conditional when creating addon items so you don't register the global
'wu_apply_signup_fee' filter at all; locate the add_filter call referencing
wu_apply_signup_fee and membership->get_times_billed() and ensure it is removed
or confined to the immediate operation.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 84-85: Fix PHPCS spacing/alignment issues by normalizing the
conditional and array formatting: remove or add spaces inside function-call
parentheses to match the project standard for is_wp_error checks (update the
occurrences using is_wp_error and self::$customer), and reformat the
discount-code array entries so keys, arrows (=>) and values are consistently
aligned and indented according to the coding standard; apply the same
adjustments to the other occurrences noted (the other is_wp_error checks and the
discount-code array blocks in this test class) so PHPCS passes.
---
Nitpick comments:
In `@inc/checkout/class-cart.php`:
- Line 1085: Remove the stale commented-out code marker
"$this->calculate_prorate_credits();" in class-cart.php and replace it with a
short explanatory comment describing why prorate credit calculation was removed
or where that logic now lives (e.g., mention the current method or hook
responsible for prorate handling), so future readers understand intent instead
of seeing commented executable code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2072d25c-a7b1-442b-933e-f98c5178295e
📒 Files selected for processing (2)
inc/checkout/class-cart.phptests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php
Comment on lines
+72
to
+73
| public static function set_up_before_class() { | ||
| parent::set_up_before_class(); |
Contributor
There was a problem hiding this comment.
Set current user before building membership-change carts.
These tests rely on membership ownership checks, but user context is never set. Also, customer_id cart arg is ignored by Cart arg sanitization.
💡 Suggested fix
class Cart_Addon_Pricing_Test extends WP_UnitTestCase {
+ public function set_up() {
+ parent::set_up();
+ wp_set_current_user(self::$customer->get_user_id(), self::$customer->get_username());
+ }Also applies to: 178-185, 231-237, 270-276, 327-333, 374-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 72 - 73,
In set_up_before_class and the other test locations that build membership-change
carts, ensure the test sets the WordPress current user to the intended customer
so Cart sanitization honors the customer_id: create the customer user via the
test factory (or use the existing test user IDs) and call
wp_set_current_user($customer_id) (or wp_set_current_user(self::$customer_id))
immediately before instantiating new Cart objects; alternatively, if Cart has a
static sanitization helper (e.g., Cart::sanitize_args or Cart::__construct),
call wp_set_current_user first so Cart's customer_id logic sees the correct
current user.
Comment on lines
+76
to
+87
| self::$customer = wu_create_customer( | ||
| array( | ||
| 'username' => 'testuser_addon_pricing', | ||
| 'email' => 'addon_pricing@example.com', | ||
| 'password' => 'password123', | ||
| ) | ||
| ); | ||
|
|
||
| if ( is_wp_error( self::$customer ) ) { | ||
| self::fail( 'Failed to create test customer' ); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check file size first
echo "=== File size check ==="
wc -l tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php
# Read the beginning of the file to see typed property declarations
echo ""
echo "=== Typed property declarations (first 60 lines) ==="
head -60 tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -n
# Read lines around 76-87 (the specific snippet mentioned)
echo ""
echo "=== Lines 76-87 (snippet in review) ==="
sed -n '76,87p' tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -n
# Read lines around 89-107
echo ""
echo "=== Lines 89-107 ==="
sed -n '89,107p' tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for factory function definitions to understand their return types
echo "=== Searching for wu_create_customer definition ==="
rg -n "function wu_create_customer" --type php -A 10
echo ""
echo "=== Searching for wu_create_product definition ==="
rg -n "function wu_create_product" --type php -A 10
echo ""
echo "=== Searching for `@return` WP_Error in factory functions ==="
rg -n "@return.*WP_Error|WP_Error.*@return" --type php -B 5 | head -100Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 6855
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the complete wu_create_customer function
echo "=== wu_create_customer function (inc/functions/customer.php) ==="
sed -n '124,250p' inc/functions/customer.php | cat -n
echo ""
echo "=== wu_create_product function (inc/functions/product.php) ==="
sed -n '117,250p' inc/functions/product.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 9784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for return statements in wu_create_customer
echo "=== Return statements in wu_create_customer ==="
rg -n "return " inc/functions/customer.php | head -20
echo ""
echo "=== Return statements in wu_create_product ==="
rg -n "return " inc/functions/product.php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Lines 110-128 (expected: wu_create_addon) ==="
sed -n '110,128p' tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -n
echo ""
echo "=== Lines 131-145 (expected: wu_create_membership) ==="
sed -n '131,145p' tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -n
echo ""
echo "=== Lines 148-167 (expected: wu_create_discount_code) ==="
sed -n '148,167p' tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check return types for remaining factory functions
echo "=== wu_create_membership return type ==="
rg -n "function wu_create_membership" inc/functions/*.php -A 3 | head -20
echo ""
echo "=== wu_create_discount_code return type ==="
rg -n "function wu_create_discount_code" inc/functions/*.php -A 3 | head -20
# Look for `@return` hints
echo ""
echo "=== Return type hints for membership and discount_code ==="
rg -B 2 "function wu_create_(membership|discount_code)" inc/functions/*.php | grep -E "@return|function wu_create"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1124
Guard factory results before assigning typed static properties.
PHP will throw TypeError when WP_Error is assigned to a typed property. The error checks at lines 76–87, 89–107, 110–128, 131–145, and 148–167 execute too late—after the property assignment fails.
The factory functions wu_create_customer(), wu_create_product(), wu_create_membership(), and wu_create_discount_code() are all documented to return WP_Error on failure. Assignments must be guarded first:
Suggested fix pattern
- self::$customer = wu_create_customer(
+ $customer = wu_create_customer(
array(
'username' => 'testuser_addon_pricing',
'email' => 'addon_pricing@example.com',
'password' => 'password123',
)
);
- if ( is_wp_error( self::$customer ) ) {
+ if (is_wp_error($customer)) {
self::fail( 'Failed to create test customer' );
}
+ self::$customer = $customer;Apply the same pattern to $plan, $addon, $discount_code, and $membership.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self::$customer = wu_create_customer( | |
| array( | |
| 'username' => 'testuser_addon_pricing', | |
| 'email' => 'addon_pricing@example.com', | |
| 'password' => 'password123', | |
| ) | |
| ); | |
| if ( is_wp_error( self::$customer ) ) { | |
| self::fail( 'Failed to create test customer' ); | |
| } | |
| $customer = wu_create_customer( | |
| array( | |
| 'username' => 'testuser_addon_pricing', | |
| 'email' => 'addon_pricing@example.com', | |
| 'password' => 'password123', | |
| ) | |
| ); | |
| if (is_wp_error($customer)) { | |
| self::fail( 'Failed to create test customer' ); | |
| } | |
| self::$customer = $customer; |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 85-85:
Expected 0 spaces before closing parenthesis; 1 found
[failure] 85-85:
Space after opening parenthesis of function call prohibited
[failure] 84-84:
Expected 0 spaces before closing parenthesis; 1 found
[failure] 84-84:
Space after opening parenthesis of function call prohibited
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 76 - 87,
The typed static properties (self::$customer, self::$plan, self::$addon,
self::$discount_code, self::$membership) are being assigned the raw results of
factory functions (wu_create_customer, wu_create_product, wu_create_membership,
wu_create_discount_code) which may return WP_Error and will cause a TypeError;
instead call each factory into a local variable first, check
is_wp_error(localVar) and call self::fail(...) on error, then assign the
verified value to the typed static property. Update all occurrences where these
factories are directly assigned (the blocks around the current assignments) to
follow this guard-then-assign pattern.
1. Fix Stripe 'coupon' parameter deprecated error - Use 'discounts' array instead of deprecated 'coupon' parameter - Fixes: 'Received unknown parameter: coupon' error when updating subscriptions 2. Fix addon products not showing in membership - Modified swap() to handle addon carts differently than plan changes - For addon carts: preserve existing addons and merge new ones - For addon carts: don't update recurring amount/duration 3. Fix discount code type safety (CodeRabbit feedback) - Resolve string discount codes to objects for legacy compatibility - Use instanceof check instead of is_object() + method_exists() 4. Remove late/ineffective wu_apply_signup_fee filter - Filter was registered after products were already added - Removed as it had no effect and could leak globally 5. Fix PHPCS violations - Auto-fixed spacing and indentation issues in test file and cart class
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php (3)
76-163:⚠️ Potential issue | 🟠 MajorGuard factory results before assigning typed static fixtures.
These helpers can return
WP_Error. Assigning the raw result into a typed static property will throw before the subsequentis_wp_error()check runs, so the test never reaches the intended failure path. Validate into locals first, then assign.🛡️ Suggested fix pattern
- self::$customer = wu_create_customer( + $customer = wu_create_customer( array( 'username' => 'testuser_addon_pricing', 'email' => 'addon_pricing@example.com', 'password' => 'password123', ) ); - if ( is_wp_error(self::$customer) ) { + if (is_wp_error($customer)) { self::fail('Failed to create test customer'); } + self::$customer = $customer;Apply the same guard-then-assign pattern to
self::$plan,self::$addon,self::$discount_code, andself::$membership.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 76 - 163, The test assigns factory return values (which may be WP_Error) directly to typed static fixtures (self::$customer, self::$plan, self::$addon, self::$discount_code, self::$membership) causing type errors before the is_wp_error() checks; fix by first storing each factory call (wu_create_customer, wu_create_product, wu_create_discount_code, wu_create_membership) into a local variable, call is_wp_error() and fail early if needed, then assign the validated object to the corresponding static property (self::$customer, self::$plan, self::$addon, self::$discount_code, self::$membership).
72-168:⚠️ Potential issue | 🟠 MajorSet the current WP user before building membership-change carts.
These tests pass
customer_id, butCartresolves ownership throughwu_get_current_customer(). Without a per-testwp_set_current_user(self::$customer->get_user_id()), the addon/upgrade carts can fail the permission gate and make the assertions flaky.🧪 Suggested fix
class Cart_Addon_Pricing_Test extends WP_UnitTestCase { + public function set_up() { + parent::set_up(); + wp_set_current_user(self::$customer->get_user_id()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 72 - 168, The test setup never sets the current WP user so Cart ownership (resolved via wu_get_current_customer()) can be wrong; after creating self::$customer in set_up_before_class() call wp_set_current_user(self::$customer->get_user_id()) so Cart/permission gates resolve to the test customer when building addon/upgrade carts (update any teardown if needed to reset the current user). Ensure this change is placed in set_up_before_class() immediately after the wu_create_customer block so functions like Cart and wu_get_current_customer() use the test user.
311-325:⚠️ Potential issue | 🟡 MinorCheck per-test product creation before dereferencing the result.
Both
wu_create_product()calls are used immediately viaget_id()anddelete(). If either factory returnsWP_Error, the test will fatal instead of failing with a clear message.🧪 Suggested fix
$upgraded_plan = wu_create_product( array( 'name' => 'Premium Plan', @@ 'active' => true, ) ); + if (is_wp_error($upgraded_plan)) { + $this->fail('Failed to create upgraded plan'); + } @@ $addon_with_fee = wu_create_product( array( 'name' => 'Addon with Fee', @@ 'active' => true, ) ); + if (is_wp_error($addon_with_fee)) { + $this->fail('Failed to create addon with setup fee'); + }Also applies to: 358-372
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php` around lines 311 - 325, The test currently assumes wu_create_product() returns a product object and immediately calls methods like get_id() and delete(), which will fatal if wu_create_product() returns a WP_Error; update the test to check the return value from wu_create_product() (for both $upgraded_plan and the other product created around the 358-372 region) with is_wp_error() or similar before dereferencing—if it is an error, fail the test with a clear message (or skip) instead of calling get_id() or delete(); ensure all uses of get_id() and delete() on the created product variables are guarded by this check.inc/checkout/class-cart.php (1)
942-959:⚠️ Potential issue | 🟠 MajorScope the temporary signup-fee override to this one plan insertion.
This
wu_apply_signup_feefilter is never removed, so opting into the legacy path can leak the override into lateradd_product()calls in the same request. Store the closure and remove it in afinallyafter re-adding the existing plan.♻️ Suggested fix
- add_filter('wu_apply_signup_fee', fn() => $membership->get_times_billed() <= 0); - - $this->add_product($membership->get_plan_id()); + $signup_fee_filter = static fn() => $membership->get_times_billed() <= 0; + add_filter('wu_apply_signup_fee', $signup_fee_filter); + try { + $this->add_product($membership->get_plan_id()); + } finally { + remove_filter('wu_apply_signup_fee', $signup_fee_filter); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 942 - 959, The temporary wu_apply_signup_fee filter must be scoped to the single add_product call to avoid leaking into later insertions: capture the closure used for add_filter (instead of passing an anonymous fn inline), call add_filter('wu_apply_signup_fee', $closure) before calling $this->add_product($membership->get_plan_id()), then ensure you remove it in a finally block with remove_filter('wu_apply_signup_fee', $closure) after re-adding the existing plan and running $this->calculate_prorate_credits(); this confines the override to only that plan insertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/models/class-membership.php`:
- Around line 764-777: The current branch skips updating the membership
recurring amount for addon carts which leaves Membership::get_amount() stale and
can cause wu_get_membership_new_cart() to compute incorrect adjustment; fix by
still merging the addon’s recurring delta into the membership before saving:
move/ensure $this->set_amount($order->get_recurring_total()),
$this->set_initial_amount($order->get_total()), and
$this->set_recurring($order->has_recurring()) run even when $is_addon_cart is
true, but keep $this->set_duration($order->get_duration()) and
$this->set_duration_unit($order->get_duration_unit()) inside the !
$is_addon_cart block so the billing period is unchanged; this ensures
Membership::save()/process_membership_update() rebuilds the subscription with
the updated amount while preserving duration/unit.
---
Duplicate comments:
In `@inc/checkout/class-cart.php`:
- Around line 942-959: The temporary wu_apply_signup_fee filter must be scoped
to the single add_product call to avoid leaking into later insertions: capture
the closure used for add_filter (instead of passing an anonymous fn inline),
call add_filter('wu_apply_signup_fee', $closure) before calling
$this->add_product($membership->get_plan_id()), then ensure you remove it in a
finally block with remove_filter('wu_apply_signup_fee', $closure) after
re-adding the existing plan and running $this->calculate_prorate_credits(); this
confines the override to only that plan insertion.
In `@tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php`:
- Around line 76-163: The test assigns factory return values (which may be
WP_Error) directly to typed static fixtures (self::$customer, self::$plan,
self::$addon, self::$discount_code, self::$membership) causing type errors
before the is_wp_error() checks; fix by first storing each factory call
(wu_create_customer, wu_create_product, wu_create_discount_code,
wu_create_membership) into a local variable, call is_wp_error() and fail early
if needed, then assign the validated object to the corresponding static property
(self::$customer, self::$plan, self::$addon, self::$discount_code,
self::$membership).
- Around line 72-168: The test setup never sets the current WP user so Cart
ownership (resolved via wu_get_current_customer()) can be wrong; after creating
self::$customer in set_up_before_class() call
wp_set_current_user(self::$customer->get_user_id()) so Cart/permission gates
resolve to the test customer when building addon/upgrade carts (update any
teardown if needed to reset the current user). Ensure this change is placed in
set_up_before_class() immediately after the wu_create_customer block so
functions like Cart and wu_get_current_customer() use the test user.
- Around line 311-325: The test currently assumes wu_create_product() returns a
product object and immediately calls methods like get_id() and delete(), which
will fatal if wu_create_product() returns a WP_Error; update the test to check
the return value from wu_create_product() (for both $upgraded_plan and the other
product created around the 358-372 region) with is_wp_error() or similar before
dereferencing—if it is an error, fail the test with a clear message (or skip)
instead of calling get_id() or delete(); ensure all uses of get_id() and
delete() on the created product variables are guarded by this check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff9bd976-8c42-4e85-808b-aa610cc1ecde
📒 Files selected for processing (4)
inc/checkout/class-cart.phpinc/gateways/class-base-stripe-gateway.phpinc/models/class-membership.phptests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php
Comment on lines
764
to
+777
| /* | ||
| * Finally, we have a couple of other parameters to set. | ||
| * For addon carts, don't update the recurring amount/duration | ||
| * since we're not changing the plan, just adding products. | ||
| * | ||
| * @since 2.0.12 | ||
| */ | ||
| $this->set_amount($order->get_recurring_total()); | ||
| $this->set_initial_amount($order->get_total()); | ||
| $this->set_recurring($order->has_recurring()); | ||
| if ( ! $is_addon_cart) { | ||
| $this->set_amount($order->get_recurring_total()); | ||
| $this->set_initial_amount($order->get_total()); | ||
| $this->set_recurring($order->has_recurring()); | ||
|
|
||
| $this->set_duration($order->get_duration()); | ||
| $this->set_duration_unit($order->get_duration_unit()); | ||
| $this->set_duration($order->get_duration()); | ||
| $this->set_duration_unit($order->get_duration_unit()); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t leave the membership amount stale on addon swaps.
This branch keeps addon_products in sync but leaves Membership::get_amount() at the pre-addon value. Membership::save() still calls process_membership_update() when products change, and that flow rebuilds the recurring subscription from membership state; because wu_get_membership_new_cart() derives an adjustment from the stored amount, the new addon can be canceled back out on future renewals. Keep the billing period unchanged for addon carts, but still merge the addon’s recurring delta into the membership amount before saving.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/models/class-membership.php` around lines 764 - 777, The current branch
skips updating the membership recurring amount for addon carts which leaves
Membership::get_amount() stale and can cause wu_get_membership_new_cart() to
compute incorrect adjustment; fix by still merging the addon’s recurring delta
into the membership before saving: move/ensure
$this->set_amount($order->get_recurring_total()),
$this->set_initial_amount($order->get_total()), and
$this->set_recurring($order->has_recurring()) run even when $is_addon_cart is
true, but keep $this->set_duration($order->get_duration()) and
$this->set_duration_unit($order->get_duration_unit()) inside the !
$is_addon_cart block so the billing period is unchanged; this ensures
Membership::save()/process_membership_update() rebuilds the subscription with
the updated amount while preserving duration/unit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the bug where adding addon services to an existing membership was incorrectly charging customers for the next billing period in advance instead of only charging for the addon.
Before: Customer adding a €5 addon to a €90/month membership was charged ~€89.59
After: Customer is only charged €5 for the addon
Problem Description
When customers tried to add addon services through the Account section, the cart pricing calculation was:
This happened because the system was treating addon purchases like plan upgrades, charging for the "next" billing period and then applying credits, rather than just charging for the new addon.
Changes Made
1. Fixed Pro-rata Application for Addon Purchases
wu_cart_addon_include_existing_planfilter default fromtruetofalseFiles modified:
inc/checkout/class-cart.php:876-948,inc/checkout/class-cart.php:984-10602. Added Discount Code Application to Addons
apply_to_renewalsenabled are now applied to addon purchasesFiles modified:
inc/checkout/class-cart.php:892-903,inc/checkout/class-cart.php:1001-10123. Explicit Plan Removal in Edge Cases
plan_idto 0 for addon-only cartsFiles modified:
inc/checkout/class-cart.php:1014-1038Testing
Added comprehensive unit tests in
tests/WP_Ultimo/Checkout/Cart_Addon_Pricing_Test.php:test_addon_only_charges_for_addon_product()- Verifies €5 addon only charges €5test_addon_applies_existing_discount_code()- Verifies discount codes are appliedtest_addon_filter_can_include_existing_plan()- Tests backward compatibility filtertest_plan_upgrade_still_uses_prorate()- Ensures upgrades/downgrades still work correctlytest_addon_does_not_reapply_setup_fees()- Verifies setup fee handlingBackward Compatibility
The
wu_cart_addon_include_existing_planfilter allows sites to revert to the old behavior if needed for specific use cases:However, the old behavior was incorrect for most use cases, so the new default is more appropriate.
Migration Notes
No migration needed. Existing subscriptions and memberships are unaffected. This only changes the behavior for new addon purchases going forward.
Related Issues
Fixes customer-reported issues:
Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Tests