t473: test(admin): write unit tests for Membership_Edit_Admin_Page#634
Merged
superdav42 merged 3 commits intomainfrom Mar 28, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 35 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughComprehensive unit test suite added for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 7
🧹 Nitpick comments (1)
tests/Admin_Pages/Membership_Edit_Admin_Page_Test.php (1)
658-691:remove_all_filters()is too broad for test cleanup.These tests wipe every callback on
wu_data_json_success_delete_membership_modal, not just the one added byregister_forms(). That can create order-dependent failures in later tests if anything else hooks the same filter. Prefer removing the exact callback or snapshotting/restoring the hook state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Admin_Pages/Membership_Edit_Admin_Page_Test.php` around lines 658 - 691, Snapshot the hook state for 'wu_data_json_success_delete_membership_modal' before calling $this->page->register_forms(), run the assertions, and then restore that snapshot instead of calling remove_all_filters(); specifically, capture $original = $wp_filter['wu_data_json_success_delete_membership_modal'] (or use has_filter/apply_filters to copy the callbacks array) before register_forms(), call $this->page->register_forms() and your tests (apply_filters/asserts), and finally restore $wp_filter['wu_data_json_success_delete_membership_modal'] = $original to remove only the callback(s) added by register_forms() while preserving any other hooks.
🤖 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/Admin_Pages/Membership_Edit_Admin_Page_Test.php`:
- Around line 760-773: Replace the no-op assertion in
test_register_widgets_in_swap_preview_mode with an assertion that the "events"
widget is not registered when is_swap_preview is true: after calling
$this->page->register_widgets(), confirm the events widget is absent by
asserting the events key isn't present in the page's widget storage (or by using
the page's widget accessor method, e.g.
get_widget('events')/has_widget('events') if available). Keep references to
register_widgets, is_swap_preview and the test method name to locate the change.
- Around line 1330-1355: The test currently only expects a WPDieException which
can be thrown for both success and error; change
test_handle_edit_membership_product_modal_adds_product to capture the JSON
payload returned by handle_edit_membership_product_modal (the WPDieException
message produced by wp_send_json_success/wp_send_json_error), decode it and
assert it indicates success and contains the added product id (use
$product->get_id()), or alternatively reload the membership object
($this->membership) after calling handle_edit_membership_product_modal and
assert the membership's products include the new product id; reference the
handle_edit_membership_product_modal call,
wp_send_json_success/wp_send_json_error responses, and $this->membership /
$product->get_id() when implementing the assertion.
- Around line 1093-1112: The test asserts handle_save() returns false but
doesn't ensure the convert-to-lifetime branch was executed; make the expectation
specific to handle_convert_to_lifetime by adding an expectation on the mock
Membership that set_date_expiration(null) is called (or assert another side
effect unique to handle_convert_to_lifetime) so when $this->page->handle_save()
runs it must route into handle_convert_to_lifetime and invoke
set_date_expiration on the $mock_membership; update the test method
test_handle_save_routes_to_convert_to_lifetime to set that expectation on
$mock_membership (or assert the unique side effect) before calling
handle_save().
In `@tests/WP_Ultimo/Functions/Membership_Functions_Test.php`:
- Around line 60-65: Update the assertions to verify actual search behavior
instead of only type: in test_get_memberships_with_search_no_customers replace
the generic $this->assertIsArray($result) with an assertion that $result is
empty (e.g. assertEmpty) when searching for 'nonexistent_xyz_abc_123'; and in
the corresponding positive test (the one that creates a user/customer around
lines 70-82) assert that wu_get_memberships(['search' => '<matching_term>'])
returns a non-empty array and contains the expected membership entry (e.g. check
for the created user's ID, email or membership ID in the result) so the test
verifies the positive path rather than just array type.
- Around line 521-572: The tests
test_membership_create_new_payment_cancels_pending (and the related
no_cancel_pending and keep_non_recurring cases) only assert the returned type
from wu_membership_create_new_payment and do not verify the expected
side-effects; update each test to reload the original Payment model after
calling wu_membership_create_new_payment and assert its status/flags (e.g.,
cancelled vs pending) to confirm cancellation behavior, and for the
keep_non_recurring case create a non-recurring plan or set
remove_non_recurring=false on the membership so the non-recurring branch is
exercised, then assert the original payment remains pending; reference the test
names (test_membership_create_new_payment_cancels_pending,
test_membership_create_new_payment_no_cancel_pending,
test_membership_create_new_payment_keep_non_recurring) and the function
wu_membership_create_new_payment when adding these assertions.
- Around line 388-426: The test currently only asserts that
wu_get_membership_new_cart($membership) returns a Cart but doesn't verify the
adjustment branch; update test_get_membership_new_cart_with_amount_difference to
inspect the cart lines (e.g., via $cart->get_lines() or $cart->get_items()) and
assert that one line exists with type 'ADJUSTMENT' (or the ADJUSTMENT constant)
and that its amount equals the difference between the membership amount (15.00)
and the product amount (10.00) so the adjustment entry is actually created by
wu_get_membership_new_cart.
- Around line 206-210: Update the
test_get_membership_by_customer_gateway_id_with_amount to actually validate the
amount filter: create/seed a pending membership record with gateway_customer_id
'cus_test_amount' (or the same ID used in the test) and initial_amount set to a
different value (e.g., 50.00), then call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe']) to
assert the unfiltered lookup returns the seeded membership, call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe'], 99.99)
to assert it returns false (amount mismatch), and finally call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe'], 50.00)
to assert it returns the membership; reference the test method
test_get_membership_by_customer_gateway_id_with_amount, the function under test
wu_get_membership_by_customer_gateway_id, and the membership field
initial_amount when adding assertions.
---
Nitpick comments:
In `@tests/Admin_Pages/Membership_Edit_Admin_Page_Test.php`:
- Around line 658-691: Snapshot the hook state for
'wu_data_json_success_delete_membership_modal' before calling
$this->page->register_forms(), run the assertions, and then restore that
snapshot instead of calling remove_all_filters(); specifically, capture
$original = $wp_filter['wu_data_json_success_delete_membership_modal'] (or use
has_filter/apply_filters to copy the callbacks array) before register_forms(),
call $this->page->register_forms() and your tests (apply_filters/asserts), and
finally restore $wp_filter['wu_data_json_success_delete_membership_modal'] =
$original to remove only the callback(s) added by register_forms() while
preserving any other hooks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a4c2f30-bcc0-49af-8b66-d264512acb29
📒 Files selected for processing (2)
tests/Admin_Pages/Membership_Edit_Admin_Page_Test.phptests/WP_Ultimo/Functions/Membership_Functions_Test.php
Comment on lines
+60
to
+65
| public function test_get_memberships_with_search_no_customers(): void { | ||
|
|
||
| $result = wu_get_memberships(['search' => 'nonexistent']); | ||
| $result = wu_get_memberships(['search' => 'nonexistent_xyz_abc_123']); | ||
|
|
||
| $this->assertIsArray($result); | ||
| } |
Contributor
There was a problem hiding this comment.
The search cases do not verify search behavior yet.
wu_get_memberships() returns memberships, but the “finds customers” case only creates a user/customer and then checks that the result is an array. That still passes when the search returns [], so the positive path is not actually covered. The “no customers” case has the same problem in reverse: it should assert empty, not just array type.
Also applies to: 70-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Functions/Membership_Functions_Test.php` around lines 60 -
65, Update the assertions to verify actual search behavior instead of only type:
in test_get_memberships_with_search_no_customers replace the generic
$this->assertIsArray($result) with an assertion that $result is empty (e.g.
assertEmpty) when searching for 'nonexistent_xyz_abc_123'; and in the
corresponding positive test (the one that creates a user/customer around lines
70-82) assert that wu_get_memberships(['search' => '<matching_term>']) returns a
non-empty array and contains the expected membership entry (e.g. check for the
created user's ID, email or membership ID in the result) so the test verifies
the positive path rather than just array type.
Comment on lines
+206
to
+210
| public function test_get_membership_by_customer_gateway_id_with_amount(): void { | ||
|
|
||
| $result = wu_get_membership_by_customer_gateway_id('cus_nonexistent', ['stripe'], 99.99); | ||
|
|
||
| $this->assertFalse($result); |
Contributor
There was a problem hiding this comment.
This does not validate the amount filter.
Using a nonexistent gateway customer ID makes the lookup return false regardless of whether initial_amount is honored, so this test can pass even if the amount filter regresses. Seed a pending membership with the same gateway customer ID and a different initial_amount, then compare the filtered and unfiltered lookups.
Suggested test shape
public function test_get_membership_by_customer_gateway_id_with_amount(): void {
-
- $result = wu_get_membership_by_customer_gateway_id('cus_nonexistent', ['stripe'], 99.99);
-
- $this->assertFalse($result);
+ $customer = wu_create_customer([
+ 'user_id' => self::factory()->user->create(),
+ 'skip_validation' => true,
+ ]);
+ $gateway_customer_id = 'cus_test_' . wp_rand();
+ $membership = wu_create_membership([
+ 'customer_id' => $customer->get_id(),
+ 'status' => 'pending',
+ 'amount' => 29.99,
+ 'initial_amount' => 29.99,
+ 'currency' => 'USD',
+ 'gateway' => 'stripe',
+ 'gateway_customer_id' => $gateway_customer_id,
+ 'skip_validation' => true,
+ ]);
+
+ $this->assertNotWPError($membership);
+ $this->assertFalse(
+ wu_get_membership_by_customer_gateway_id($gateway_customer_id, ['stripe'], 99.99)
+ );
+ $this->assertSame(
+ $membership->get_id(),
+ wu_get_membership_by_customer_gateway_id($gateway_customer_id, ['stripe'])->get_id()
+ );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Functions/Membership_Functions_Test.php` around lines 206 -
210, Update the test_get_membership_by_customer_gateway_id_with_amount to
actually validate the amount filter: create/seed a pending membership record
with gateway_customer_id 'cus_test_amount' (or the same ID used in the test) and
initial_amount set to a different value (e.g., 50.00), then call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe']) to
assert the unfiltered lookup returns the seeded membership, call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe'], 99.99)
to assert it returns false (amount mismatch), and finally call
wu_get_membership_by_customer_gateway_id('cus_test_amount', ['stripe'], 50.00)
to assert it returns the membership; reference the test method
test_get_membership_by_customer_gateway_id_with_amount, the function under test
wu_get_membership_by_customer_gateway_id, and the membership field
initial_amount when adding assertions.
Comment on lines
+388
to
+426
| public function test_get_membership_new_cart_with_amount_difference(): void { | ||
|
|
||
| $customer = wu_create_customer([ | ||
| 'user_id' => self::factory()->user->create(), | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $product = wu_create_product([ | ||
| 'name' => 'Adjustment Plan', | ||
| 'slug' => 'adjustment-plan-' . wp_rand(), | ||
| 'type' => 'plan', | ||
| 'amount' => 10.00, | ||
| 'recurring' => true, | ||
| 'duration' => 1, | ||
| 'duration_unit' => 'month', | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $this->assertNotWPError($product); | ||
|
|
||
| // Set amount different from product price to trigger adjustment line item | ||
| $membership = wu_create_membership([ | ||
| 'customer_id' => $customer->get_id(), | ||
| 'plan_id' => $product->get_id(), | ||
| 'status' => 'active', | ||
| 'amount' => 15.00, | ||
| 'initial_amount' => 15.00, | ||
| 'currency' => 'USD', | ||
| 'recurring' => true, | ||
| 'duration' => 1, | ||
| 'duration_unit' => 'month', | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $this->assertNotWPError($membership); | ||
|
|
||
| $cart = wu_get_membership_new_cart($membership); | ||
|
|
||
| $this->assertInstanceOf(\WP_Ultimo\Checkout\Cart::class, $cart); |
Contributor
There was a problem hiding this comment.
The adjustment branch is not observed here.
This function always returns a Cart, so the instance check on Line 426 does not tell us whether the amount-difference path actually added the adjustment line item. As written, the test still passes if that branch stops creating the ADJUSTMENT entry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Functions/Membership_Functions_Test.php` around lines 388 -
426, The test currently only asserts that
wu_get_membership_new_cart($membership) returns a Cart but doesn't verify the
adjustment branch; update test_get_membership_new_cart_with_amount_difference to
inspect the cart lines (e.g., via $cart->get_lines() or $cart->get_items()) and
assert that one line exists with type 'ADJUSTMENT' (or the ADJUSTMENT constant)
and that its amount equals the difference between the membership amount (15.00)
and the product amount (10.00) so the adjustment entry is actually created by
wu_get_membership_new_cart.
Comment on lines
+521
to
+572
| public function test_membership_create_new_payment_cancels_pending(): void { | ||
|
|
||
| $customer = wu_create_customer([ | ||
| 'user_id' => self::factory()->user->create(), | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $product = wu_create_product([ | ||
| 'name' => 'Cancel Pending Plan', | ||
| 'slug' => 'cancel-pending-plan-' . wp_rand(), | ||
| 'type' => 'plan', | ||
| 'amount' => 29.99, | ||
| 'recurring' => true, | ||
| 'duration' => 1, | ||
| 'duration_unit' => 'month', | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $this->assertNotWPError($product); | ||
|
|
||
| $membership = wu_create_membership([ | ||
| 'customer_id' => $customer->get_id(), | ||
| 'plan_id' => $product->get_id(), | ||
| 'status' => 'active', | ||
| 'amount' => 29.99, | ||
| 'initial_amount' => 29.99, | ||
| 'currency' => 'USD', | ||
| 'recurring' => true, | ||
| 'duration' => 1, | ||
| 'duration_unit' => 'month', | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| $this->assertNotWPError($membership); | ||
|
|
||
| // Create a pending payment first | ||
| wu_create_payment([ | ||
| 'customer_id' => $customer->get_id(), | ||
| 'membership_id' => $membership->get_id(), | ||
| 'currency' => 'USD', | ||
| 'subtotal' => 29.99, | ||
| 'total' => 29.99, | ||
| 'status' => 'pending', | ||
| 'skip_validation' => true, | ||
| ]); | ||
|
|
||
| // Now create a new payment — should cancel the pending one | ||
| $new_payment = wu_membership_create_new_payment($membership, true); | ||
|
|
||
| $this->assertNotWPError($new_payment); | ||
| $this->assertInstanceOf(\WP_Ultimo\Models\Payment::class, $new_payment); | ||
| } |
Contributor
There was a problem hiding this comment.
These payment branch tests only assert the return type.
cancels_pending, no_cancel_pending, and keep_non_recurring never verify the side effects they are named after. The pending-payment cases do not reload the original payment to check cancelled vs pending, and the non-recurring case uses only a recurring plan, so remove_non_recurring=false is not observable at all.
Also applies to: 577-660
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Functions/Membership_Functions_Test.php` around lines 521 -
572, The tests test_membership_create_new_payment_cancels_pending (and the
related no_cancel_pending and keep_non_recurring cases) only assert the returned
type from wu_membership_create_new_payment and do not verify the expected
side-effects; update each test to reload the original Payment model after
calling wu_membership_create_new_payment and assert its status/flags (e.g.,
cancelled vs pending) to confirm cancellation behavior, and for the
keep_non_recurring case create a non-recurring plan or set
remove_non_recurring=false on the membership so the non-recurring branch is
exercised, then assert the original payment remains pending; reference the test
names (test_membership_create_new_payment_cancels_pending,
test_membership_create_new_payment_no_cancel_pending,
test_membership_create_new_payment_keep_non_recurring) and the function
wu_membership_create_new_payment when adding these assertions.
superdav42
force-pushed
the
feature/t473-membership-edit-admin-page-tests
branch
from
586901b to
eaf1635
Compare
🔨 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: |
|
Performance Test Results Performance test results for 0e8d684 are in 🛎️! URL:
|
superdav42
force-pushed
the
feature/t473-membership-edit-admin-page-tests
branch
from
eaf1635 to
b997953
Compare
🔨 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: |
…mbership_Edit_Admin_Page Covers all public and protected methods: - Static properties (id, type, object_id, supported_panels, badge_count, highlight_menu_slug) - get_title(), get_menu_title(), action_links(), has_title() - get_labels() — all keys and values - get_object() — caching, preview-swap path, swap flag - add_swap_notices() — no swap, with swap, preview-swap param, date in message - page_loaded() — edit mode, object set, swap notices - register_forms() — filter registration and redirect URL - register_widgets() — basic, locked, swap preview, lifetime, gateway - payments_query_filter(), sites_query_filter(), customer_query_filter(), events_query_filter() - handle_save() — auto_renew, billing validation, convert_to_lifetime routing, swap preview - handle_convert_to_lifetime() — error path, success path - handle_transfer_membership_modal() — not confirmed, not found, same customer - render_transfer_membership_modal() — not found, renders - render_edit_membership_product_modal() — not found, renders - handle_edit_membership_product_modal() — not found errors - render_remove_membership_product() — not found, renders - handle_remove_membership_product() — not found errors - render_change_membership_plan_modal() — not found, renders - handle_change_membership_plan_modal() — not found, same plan errors - output_widget_products(), register_scripts() Uses factory()->user->create() pattern for reliable test data creation. Closes #626
superdav42
force-pushed
the
feature/t473-membership-edit-admin-page-tests
branch
from
b997953 to
f21f3b4
Compare
🔨 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: |
…ns on PR #634 - Fix 1: test_register_widgets_in_swap_preview_mode now asserts the events widget is absent from wp_meta_boxes when is_swap_preview=true, replacing the no-op assertTrue(true) - Fix 2: test_handle_save_routes_to_convert_to_lifetime uses expects($this->once()) on set_date_expiration(null) to prove routing to handle_convert_to_lifetime() - Fix 3: test_handle_edit_membership_product_modal_adds_product captures the WPDieException JSON payload and asserts wp_send_json_success was called; also reloads the membership and asserts the product is present - Fix 4 (nitpick): register_forms tests snapshot/restore the wu_data_json_success_delete_membership_modal hook state instead of remove_all_filters() to avoid order-dependent test failures - Fix 5: test_get_memberships_with_search_no_customers uses assertEmpty instead of assertIsArray to verify the search actually returns nothing - Fix 6: test_get_memberships_with_search_finds_customers creates a membership for the customer and asserts the result is non-empty and contains the expected membership ID - Fix 7: test_get_membership_by_customer_gateway_id_with_amount seeds a real membership and validates the amount filter (mismatch returns false, exact match returns the membership) - Fix 8: test_get_membership_new_cart_with_amount_difference asserts the ADJUSTMENT line item is present and its unit_price equals the difference - Fix 9: payment cancellation tests reload the original payment after wu_membership_create_new_payment and assert its status (cancelled vs pending); keep_non_recurring uses initial_amount != amount to create a non-recurring INITADJUSTMENT item and verifies it is kept/stripped correctly
🔨 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: |
🔨 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: |
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
Membership_Edit_Admin_Pageclassfactory()->user->create()pattern for reliable test data creationCloses #626
Test Coverage
id,type,object_id,supported_panels,badge_count,highlight_menu_slugget_title(),get_menu_title(),action_links(),has_title()get_labels(),get_object()(caching + preview-swap),add_swap_notices()page_loaded(),register_forms(),register_widgets()handle_save(),handle_convert_to_lifetime()Summary by CodeRabbit