fix: preserve multi-step checkout account fields#1611
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds shared checkout-step session persistence, routes both AJAX and native submission paths through it, and expands checkout test coverage around the new helper and related test-state cleanup. ChangesCheckout step session persistence
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/WP_Ultimo/Checkout/Checkout_Test.php (1)
523-579: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for the core persistence path.
Test correctly validates that account fields are persisted while control/nonce fields are excluded, matching
get_current_step_session_values()'s filtering contract ininc/checkout/class-checkout.php.One gap: the
pre-flightbranch ofpersist_current_step_to_session()(which stores values underpre_selectedinstead) isn't covered by any test. Given this is a distinct code path in the exact method under test, consider adding a follow-up case.💡 Suggested additional test case
public function test_persist_current_step_to_session_handles_pre_flight(): void { $checkout = Checkout::get_instance(); $reflection = new \ReflectionClass($checkout); $this->ensure_session($checkout); $session_prop = $this->get_session_prop($reflection); $session = $session_prop->getValue($checkout); $session->set('signup', []); $_POST['pre-flight'] = '1'; $_POST['email_address'] = 'pre-flight@example.com'; $method = $reflection->getMethod('persist_current_step_to_session'); if (PHP_VERSION_ID < 80100) { $method->setAccessible(true); } $saved = $method->invoke($checkout); $signup = $session->get('signup'); $this->assertArrayNotHasKey('pre-flight', $saved); $this->assertSame('pre-flight@example.com', $signup['pre_selected']['email_address']); $session->set('signup', []); unset($_POST['pre-flight'], $_POST['email_address']); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/WP_Ultimo/Checkout/Checkout_Test.php` around lines 523 - 579, Add a follow-up test for the pre-flight branch in persist_current_step_to_session(), since the current test only covers the normal account path. Create a new case around Checkout::persist_current_step_to_session that sets the pre-flight request input, invokes the method via reflection like the existing test, and asserts the returned data omits control fields while the session stores values under signup['pre_selected'] rather than the regular account keys. Reuse the same setup helpers and cleanup pattern from test_persist_current_step_to_session_saves_account_fields().inc/checkout/class-checkout.php (1)
3639-3655: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply input sanitization per coding guidelines.
get_current_step_session_values()returns raw$_POSTvalues without sanitization. Per coding guidelines, input should be sanitized usingwu_clean()or WordPress sanitization functions. Password fields (e.g.,password,password_conf) should be excluded from sanitization to avoid altering credentials.🛡️ Proposed sanitization wrapper
protected function get_current_step_session_values() { - return array_filter( - $_POST, // phpcs:ignore WordPress.Security.NonceVerification.Missing - fn($key) => ! str_starts_with((string) $key, 'checkout_') && ! str_starts_with((string) $key, '_'), - ARRAY_FILTER_USE_KEY - ); + $values = array_filter( + $_POST, // phpcs:ignore WordPress.Security.NonceVerification.Missing + fn($key) => ! str_starts_with((string) $key, 'checkout_') && ! str_starts_with((string) $key, '_'), + ARRAY_FILTER_USE_KEY + ); + + foreach ($values as $key => $value) { + if ( ! str_contains((string) $key, 'password')) { + $values[ $key ] = wu_clean($value); + } + } + + return $values; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/checkout/class-checkout.php` around lines 3639 - 3655, `get_current_step_session_values()` is returning raw `$_POST` data without sanitization. Update the method to sanitize each retained value before returning it, using `wu_clean()` or an appropriate WordPress sanitization function, while preserving the existing exclusion logic for checkout control/private nonce keys. Make sure password-related fields such as `password` and `password_conf` are left unsanitized so credentials are not altered, and keep the behavior localized to `get_current_step_session_values()`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@inc/checkout/class-checkout.php`:
- Around line 3639-3655: `get_current_step_session_values()` is returning raw
`$_POST` data without sanitization. Update the method to sanitize each retained
value before returning it, using `wu_clean()` or an appropriate WordPress
sanitization function, while preserving the existing exclusion logic for
checkout control/private nonce keys. Make sure password-related fields such as
`password` and `password_conf` are left unsanitized so credentials are not
altered, and keep the behavior localized to `get_current_step_session_values()`.
In `@tests/WP_Ultimo/Checkout/Checkout_Test.php`:
- Around line 523-579: Add a follow-up test for the pre-flight branch in
persist_current_step_to_session(), since the current test only covers the normal
account path. Create a new case around Checkout::persist_current_step_to_session
that sets the pre-flight request input, invokes the method via reflection like
the existing test, and asserts the returned data omits control fields while the
session stores values under signup['pre_selected'] rather than the regular
account keys. Reuse the same setup helpers and cleanup pattern from
test_persist_current_step_to_session_saves_account_fields().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc986c58-76b9-4b9c-acfc-93e86b5e4342
📒 Files selected for processing (2)
inc/checkout/class-checkout.phptests/WP_Ultimo/Checkout/Checkout_Test.php
🔨 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: |
superdav42
added
the
review-feedback-scanned
1 participant
Summary
email_address,username,password, andpassword_confin the signup session.Reproduction and verification
Created a local 2-step checkout form at
http://wordpress.local:8080/aidevops-two-step-repro/:Before the fix:
wu_validate_formAJAX returnedsuccess: true.wu_validate_formAJAX, without reposting step-1 account fields, returned required-field errors foremail_address,username, andpassword.After the fix:
success: trueon step 2.Tests
php -l inc/checkout/class-checkout.php && php -l tests/WP_Ultimo/Checkout/Checkout_Test.phpvendor/bin/phpcs inc/checkout/class-checkout.php tests/WP_Ultimo/Checkout/Checkout_Test.phpvendor/bin/phpstan analyse inc/checkout/class-checkout.php tests/WP_Ultimo/Checkout/Checkout_Test.php --no-progressvendor/bin/phpunit --filter test_persist_current_step_to_session_saves_account_fieldsvendor/bin/phpunit --filter Checkout_TestNotes:
Checkout_Test.aidevops.sh v3.32.2 plugin for OpenCode v1.17.13
Summary by CodeRabbit