test: add template_id checkout validation tests and fix Dashboard_Widgets_Test#801
Merged
superdav42 merged 1 commit intomainfrom Apr 12, 2026
Merged
test: add template_id checkout validation tests and fix Dashboard_Widgets_Test#801superdav42 merged 1 commit intomainfrom
superdav42 merged 1 commit intomainfrom
Conversation
…gets_Test Add 7 unit tests covering the template_selection → template_id validation mapping fix from PR #800: - template_selection required attribute maps to template_id rule key - min:1 guard rejects template_id=0 during checkout - positive template_id passes validation - no min:1 added when template_selection field is absent - template_id=0 allowed with base rule (admin/network context) - non-template required fields still map to themselves Fix Dashboard_Widgets_Test failure caused by PR #785 adding is_network_admin() guard to enqueue_scripts(). The test now uses set_current_screen('dashboard-network') to simulate the network admin context. Also adds a new test confirming the per-site dashboard does NOT enqueue wu-activity-stream.
superdav42
added
origin:interactive
Contributor
📝 WalkthroughWalkthroughThis PR adds comprehensive PHPUnit test coverage for Checkout validation rules tied to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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 docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/WP_Ultimo/Dashboard_Widgets_Test.php (1)
125-160:⚠️ Potential issue | 🟡 MinorHarden global cleanup to avoid cross-test leakage on failure.
Line 131/177 resets
$wp_scripts->done, and Line 137/183 changes screen context, but cleanup only restores$wp_scripts->queueand$pagenow. If an assertion throws, subsequent tests can inherit mutated globals/screen state.Suggested fix (apply to both tests)
global $pagenow, $wp_scripts; $original = $pagenow; - $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; + $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; + $original_done = isset($wp_scripts) ? $wp_scripts->done : []; + $original_screen = function_exists('get_current_screen') ? get_current_screen() : null; + $original_screen_id = $original_screen ? $original_screen->id : null; $pagenow = 'index.php'; - if (isset($wp_scripts)) { - $wp_scripts->queue = []; - $wp_scripts->done = []; - } + try { + if (isset($wp_scripts)) { + $wp_scripts->queue = []; + $wp_scripts->done = []; + } - set_current_screen('dashboard-network'); + set_current_screen('dashboard-network'); - \WP_Ultimo\Scripts::get_instance()->register_default_scripts(); + \WP_Ultimo\Scripts::get_instance()->register_default_scripts(); - $instance = $this->get_instance(); - $instance->enqueue_scripts(); + $instance = $this->get_instance(); + $instance->enqueue_scripts(); - $this->assertTrue( - wp_script_is('wu-activity-stream', 'enqueued'), - 'wu-activity-stream should be enqueued on the network admin index.php' - ); + $this->assertTrue( + wp_script_is('wu-activity-stream', 'enqueued'), + 'wu-activity-stream should be enqueued on the network admin index.php' + ); + } finally { + if (isset($wp_scripts)) { + $wp_scripts->queue = $original_queue; + $wp_scripts->done = $original_done; + } + if ($original_screen_id) { + set_current_screen($original_screen_id); + } + $pagenow = $original; + }Also applies to: 171-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Dashboard_Widgets_Test.php` around lines 125 - 160, Wrap the test setup and assertions in a try/finally so globals are always restored; capture the original screen (e.g. $original_screen = get_current_screen()), original $pagenow and original $wp_scripts->queue/done, then in finally restore $pagenow, restore $wp_scripts->queue and $wp_scripts->done (checking isset($wp_scripts) first), and restore the screen via set_current_screen($original_screen) so failures in enqueue_scripts()/assertions cannot leak mutated state; apply the same pattern to both tests referencing enqueue_scripts(), $pagenow, $wp_scripts and set_current_screen().
🧹 Nitpick comments (1)
tests/WP_Ultimo/Checkout/Checkout_Test.php (1)
4657-4671: Tighten this test to exercise generated checkout rules, not only a hardcoded rule string.Line 4669 currently validates
integer|min:1directly, which is good for rule semantics but weaker for checkout-rule integration.♻️ Suggested adjustment
public function test_validate_accepts_positive_template_id(): void { $checkout = Checkout::get_instance(); - $checkout->step = ['fields' => []]; + $checkout->step = [ + 'fields' => [ + ['id' => 'template_selection', 'required' => true], + ], + ]; $checkout->steps = []; $checkout->step_name = null; $this->ensure_session($checkout); $_REQUEST['template_id'] = 5; + unset($_REQUEST['pre-flight'], $_REQUEST['checkout_form']); - // min:1 should pass for value 5 - $result = $checkout->validate(['template_id' => 'integer|min:1']); + $rules = $checkout->get_validation_rules(); + $this->assertStringContainsString('min:1', $rules['template_id']); + $result = $checkout->validate(['template_id' => 'integer|required|min:1']); $this->assertTrue($result); unset($_REQUEST['template_id']); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Checkout/Checkout_Test.php` around lines 4657 - 4671, The test is currently validating a hardcoded rule string; instead fetch the checkout's generated rules and exercise them via the Checkout::validate method. In test_validate_accepts_positive_template_id, obtain the rules from the Checkout instance (e.g. $checkout->rules() or the method that returns generated validation rules), extract the 'template_id' rule from that rules array, then call $checkout->validate(['template_id' => 5], $rulesSubset) or validate against the full rules array to ensure the generated checkout rules (not a hardcoded string) accept a positive template_id; keep using Checkout::get_instance(), the test method name, and validate() to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/WP_Ultimo/Dashboard_Widgets_Test.php`:
- Around line 125-160: Wrap the test setup and assertions in a try/finally so
globals are always restored; capture the original screen (e.g. $original_screen
= get_current_screen()), original $pagenow and original $wp_scripts->queue/done,
then in finally restore $pagenow, restore $wp_scripts->queue and
$wp_scripts->done (checking isset($wp_scripts) first), and restore the screen
via set_current_screen($original_screen) so failures in
enqueue_scripts()/assertions cannot leak mutated state; apply the same pattern
to both tests referencing enqueue_scripts(), $pagenow, $wp_scripts and
set_current_screen().
---
Nitpick comments:
In `@tests/WP_Ultimo/Checkout/Checkout_Test.php`:
- Around line 4657-4671: The test is currently validating a hardcoded rule
string; instead fetch the checkout's generated rules and exercise them via the
Checkout::validate method. In test_validate_accepts_positive_template_id, obtain
the rules from the Checkout instance (e.g. $checkout->rules() or the method that
returns generated validation rules), extract the 'template_id' rule from that
rules array, then call $checkout->validate(['template_id' => 5], $rulesSubset)
or validate against the full rules array to ensure the generated checkout rules
(not a hardcoded string) accept a positive template_id; keep using
Checkout::get_instance(), the test method name, and validate() to locate and
update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41eda309-aae8-4794-852e-7eb34abe0540
📒 Files selected for processing (2)
tests/WP_Ultimo/Checkout/Checkout_Test.phptests/WP_Ultimo/Dashboard_Widgets_Test.php
|
Performance Test Results Performance test results for 849c73f are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
Collaborator
Author
Summary
Test results
Do not mergeThis PR is labelled Merged via PR #801 to main. aidevops.sh v3.6.244 spent 8m on this as a headless bash routine. |
superdav42
added a commit
that referenced
this pull request
Apr 13, 2026
Addresses CodeRabbit review feedback from PR #801: - Capture $original_done and screen state before mutating globals - Wrap test body in try/finally so globals always restored on failure - Restore $wp_scripts->done and screen context in finally block Applies to both test_enqueue_scripts_enqueues_activity_stream_on_index and test_enqueue_scripts_skips_activity_stream_on_per_site_dashboard to prevent cross-test leakage when assertions throw. Fixes #806
superdav42
added a commit
that referenced
this pull request
Apr 13, 2026
…821) Addresses CodeRabbit review feedback from PR #801 (issue #806). - Capture $original_done and screen state before mutating globals - Wrap test body in try/finally so globals always restored on failure - Restore $wp_scripts->queue, $wp_scripts->done, screen context, and $pagenow in finally Also includes: fix: sanitize subdomain slug in wu_create_site (issue #812) Resolves #806
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
template_selection→template_idvalidation mapping fix from PR t509: fix: template_selection field validation maps to template_id rule key #800 (checkout allowedtemplate_id=0to pass when the template selection field was required, because therequiredrule was applied to the wrong validation key)Dashboard_Widgets_Test::test_enqueue_scripts_enqueues_activity_stream_on_indexfailure caused by PR fix: skip activity-stream assets on non-network admin dashboard #785 adding anis_network_admin()guard — test now usesset_current_screen('dashboard-network')to simulate the network admin contextwu-activity-streamis NOT enqueued on the per-site dashboardTest results
Do not merge
This PR is labelled
do-not-mergefor manual review before merging.Summary by CodeRabbit