Fix integrations #215
Fix integrations #215
Conversation
WalkthroughAdds script registration to the Hosting Integration Wizard admin page (with inline data), removes equivalent enqueue from the wizard test view, guards 404 template inclusion in checkout login obfuscation, adjusts installer error sanitization to wp_kses_post, and updates Domain model docblocks for return types and deprecated action parameters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as Admin Page (Hosting Integration Wizard)
participant WP as WordPress Enqueue
participant JS as wu-integration-test.js
Admin->>WP: register_scripts()
Note over Admin,WP: Parent scripts registered, then new asset enqueued
WP-->>JS: Enqueue script (deps: jquery, wu-vue, in footer)
WP->>WP: Add inline data (wu_integration_test_data)
JS-->>Admin: Runs with provided integration_id and waiting_message
sequenceDiagram
autonumber
participant Req as Request (Login URL)
participant Ck as maybe_obfuscate_login_url
participant WP as WP Template Loader
Req->>Ck: Handle obfuscated login
Ck->>WP: locate_template('404.php')
WP-->>Ck: template_path or empty
alt Template found
Ck->>Ck: include template_path
else No template
Note over Ck: Skip include
end
Ck->>Req: terminate (die)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
inc/admin-pages/class-hosting-integration-wizard-admin-page.php (2)
359-369: Remove redundant 'wu-vue' enqueue.
The new test script declares 'wu-vue' as a dependency; this explicit enqueue is now unnecessary.Apply this diff:
- wp_enqueue_script('wu-vue');
371-397: Gate script to the testing step and inline data via JSON.
Avoid loading the test bundle on non-testing steps; prevent fatals if $this->integration is unset; safer inline data.Apply this diff:
- /** - * Register the script for the test page. - * - * @return void - */ - public function register_scripts() { - parent::register_scripts(); - - wp_enqueue_script( - 'wu-integration-test', - wu_get_asset('integration-test.js', 'js'), - [ - 'jquery', - 'wu-vue', - ], - wu_get_version(), - true - ); - wp_add_inline_script( - 'wu-integration-test', - 'var wu_integration_test_data = { - integration_id: "' . esc_js($this->integration->get_id()) . '", - waiting_message: "' . esc_js(__('Waiting for results...', 'multisite-ultimate')) . '" - };', - 'before' - ); - } + /** + * Register scripts for the wizard page. + * + * Only enqueue the integration test bundle on the "testing" step. + * + * @return void + */ + public function register_scripts(): void { + parent::register_scripts(); + + if ($this->get_current_section() !== 'testing') { + return; + } + + wp_register_script( + 'wu-integration-test', + wu_get_asset('integration-test.js', 'js'), + [ 'jquery', 'wu-vue' ], + wu_get_version(), + true + ); + + $integration_id = (is_object($this->integration) && method_exists($this->integration, 'get_id')) + ? (string) $this->integration->get_id() + : ''; + $data = [ + 'integration_id' => $integration_id, + 'waiting_message' => __('Waiting for results...', 'multisite-ultimate'), + ]; + wp_add_inline_script( + 'wu-integration-test', + 'window.wu_integration_test_data = ' . wp_json_encode($data) . ';', + 'before' + ); + + wp_enqueue_script('wu-integration-test'); + }Verification ask:
- Confirm register_scripts() runs after $this->integration is set (typically on load-$hook before admin_enqueue_scripts). If not guaranteed, keep the defensive check above or set $this->integration earlier.
inc/checkout/signup-fields/class-signup-field-email-confirmation.php (1)
206-223: Add autocomplete hint to the confirmation input.
Improves UX and consistency.Apply this diff:
'html_attr' => [ - 'data-confirm-email' => 'email_address', + 'data-confirm-email' => 'email_address', + 'autocomplete' => 'email', ],Also, confirm there’s server-side validation ensuring both email fields match (front-end data attributes alone aren’t sufficient).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
inc/admin-pages/class-hosting-integration-wizard-admin-page.php(1 hunks)inc/admin-pages/class-wizard-admin-page.php(1 hunks)inc/checkout/class-checkout-pages.php(1 hunks)inc/checkout/signup-fields/class-signup-field-email-confirmation.php(4 hunks)inc/installers/class-base-installer.php(1 hunks)inc/models/class-domain.php(3 hunks)views/wizards/host-integrations/test.php(0 hunks)
💤 Files with no reviewable changes (1)
- views/wizards/host-integrations/test.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/admin-pages/class-hosting-integration-wizard-admin-page.php (3)
inc/functions/helper.php (1)
wu_get_version(21-24)inc/models/class-base-model.php (1)
get_id(427-430)inc/admin-pages/class-base-admin-page.php (1)
get_id(213-216)
inc/checkout/signup-fields/class-signup-field-email-confirmation.php (2)
inc/functions/helper.php (1)
wu_get_isset(66-73)inc/checkout/signup-fields/class-base-signup-field.php (1)
calculate_style_attr(220-237)
🔇 Additional comments (6)
inc/admin-pages/class-wizard-admin-page.php (1)
166-166: No-op whitespace change looks good.
No functional impact.inc/checkout/class-checkout-pages.php (1)
478-484: Good guard around 404 template include.
Prevents warnings when theme lacks 404.php; headers and query state already set.inc/installers/class-base-installer.php (1)
96-96: Allowing limited HTML in error messages is ok; verify sources aren’t sensitive.
wp_kses_post prevents XSS, but ensure exception messages can’t leak secrets/PII from external APIs.Would you like me to scan exception throw sites in installers to confirm messages don’t include tokens/keys?
inc/checkout/signup-fields/class-signup-field-email-confirmation.php (1)
154-158: Good: unique confirmation field ID.
Prevents ID collision with the primary email field.Please confirm any CSS/JS previously targeting the old confirmation field ID (if any) has been updated.
inc/models/class-domain.php (2)
455-456: Docblock now allows WP_Error on save(); ensure call sites handle it.
If callers assume bool, they should be updated to branch on is_wp_error().Do you want me to locate usages of Domain::save() and flag any boolean-only checks?
442-445: Confirm return type annotation for has_valid_ssl_certificate().
Method currently documents boolean; if Helper::has_valid_ssl_certificate() can return WP_Error, update the docblock here accordingly and audit call sites.I can scan for usages expecting strictly bool if you confirm Helper may return WP_Error.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
inc/checkout/class-checkout-pages.php (1)
485-485: Optional: Standardize on exit for consistencyThe rest of the class uses exit; consider aligning here for consistency.
Apply this diff:
- die; + exit;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/admin-pages/class-hosting-integration-wizard-admin-page.php(1 hunks)inc/checkout/class-checkout-pages.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/admin-pages/class-hosting-integration-wizard-admin-page.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (1)
inc/checkout/class-checkout-pages.php (1)
478-483: Guarding 404 template inclusion — LGTMGood defensive check to avoid including a non-existent theme 404 template while preserving proper 404 headers and query state.
Summary by CodeRabbit