Use a rest api url that works on subsites as well as if someone changes the rest api base - #1737
Conversation
…ave and clear calls
…es and also sites that change the rest base
…subsites and if people change the rest base
…cross various situations
|
Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughHardcoded WP REST paths were replaced with localized REST base URLs. PHP enqueue code now exposes ChangesDynamic REST URL Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded REST API paths with dynamically localized URLs (fixesRestUrl and restUrl) in both admin and frontend scripts, and adds comprehensive PHPUnit tests to verify their behavior. Feedback on these changes focuses on improving robustness: it is recommended to add guard clauses to prevent broken network requests if the localized URLs are undefined, and to access global variables via the window object to avoid potential runtime ReferenceError exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const fixesRestUrl = window.edacFrontendHighlighterApp?.fixesRestUrl ?? window.edac_script_vars?.fixesRestUrl; | ||
|
|
||
| // make a rest call to save the settings | ||
| fetch( '/wp-json/edac/v1/fixes/update/', { | ||
| fetch( `${ fixesRestUrl }/fixes/update/`, { |
There was a problem hiding this comment.
If fixesRestUrl is undefined or null (for example, if the script is loaded in a context where the localized variables are missing), the fetch call will attempt to request a relative URL starting with undefined/fixes/update/. Adding a guard clause to handle this scenario gracefully prevents broken network requests and improves robustness.
const fixesRestUrl = window.edacFrontendHighlighterApp?.fixesRestUrl ?? window.edac_script_vars?.fixesRestUrl;
if ( ! fixesRestUrl ) {
fixSettingsContainer.classList.remove( 'edac-fix-settings--saving' );
fixButtons.forEach( ( button ) => {
button.disabled = false;
} );
fixSettingsContainer.classList.add( 'edac-fix-settings--saved--error' );
if ( liveRegion ) {
liveRegion.innerText = __( 'Saving failed: Missing REST API URL.', 'accessibility-checker' );
}
return;
}
// make a rest call to save the settings
fetch( fixesRestUrl + '/fixes/update/', {There was a problem hiding this comment.
Fixed in ce8674d — added guard clause / switched to window. prefix throughout.
| saveScanResults( postId, nonce, violations, densityMetrics ) { | ||
| const self = this; | ||
| return fetch( '/wp-json/accessibility-checker/v1/post-scan-results/' + postId, { | ||
| return fetch( `${ edacFrontendHighlighterApp.restUrl }/post-scan-results/${ postId }`, { |
There was a problem hiding this comment.
If window.edacFrontendHighlighterApp is defined but restUrl is missing or undefined, the fetch call will attempt to request a relative URL starting with undefined/post-scan-results/.... Adding a check to ensure restUrl is present before making the API call prevents broken network requests.
const restUrl = window.edacFrontendHighlighterApp?.restUrl;
if ( ! restUrl ) {
return Promise.reject( new Error( 'Missing REST API URL.' ) );
}
return fetch( restUrl + '/post-scan-results/' + postId, {There was a problem hiding this comment.
Fixed in ce8674d — added guard clause / switched to window. prefix throughout.
|
|
||
| // Validate required parameters | ||
| if ( ! edacFrontendHighlighterApp?.edacUrl || ! edacFrontendHighlighterApp?.postID ) { | ||
| if ( ! edacFrontendHighlighterApp?.restUrl || ! edacFrontendHighlighterApp?.postID ) { |
There was a problem hiding this comment.
Accessing edacFrontendHighlighterApp directly can throw a ReferenceError at runtime if the variable is completely undeclared (e.g., if the script is loaded in a context where the localization didn't run). Accessing it via window.edacFrontendHighlighterApp is safer as it will evaluate to undefined instead of throwing an error.
| if ( ! edacFrontendHighlighterApp?.restUrl || ! edacFrontendHighlighterApp?.postID ) { | |
| if ( ! window.edacFrontendHighlighterApp?.restUrl || ! window.edacFrontendHighlighterApp?.postID ) { |
There was a problem hiding this comment.
Fixed in ce8674d — added guard clause / switched to window. prefix throughout.
| const summary = document.querySelector( '.edac-highlight-panel-controls-summary' ); | ||
|
|
||
| fetch( `${ edacFrontendHighlighterApp.edacUrl }/wp-json/accessibility-checker/v1/clear-issues/${ edacFrontendHighlighterApp.postID }`, { | ||
| fetch( `${ edacFrontendHighlighterApp.restUrl }/clear-issues/${ edacFrontendHighlighterApp.postID }`, { |
There was a problem hiding this comment.
For safety and consistency, access the localized variables via window.edacFrontendHighlighterApp to avoid potential ReferenceError at runtime.
| fetch( `${ edacFrontendHighlighterApp.restUrl }/clear-issues/${ edacFrontendHighlighterApp.postID }`, { | |
| fetch( window.edacFrontendHighlighterApp.restUrl + '/clear-issues/' + window.edacFrontendHighlighterApp.postID, { |
There was a problem hiding this comment.
Fixed in ce8674d — added guard clause / switched to window. prefix throughout.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
admin/class-enqueue-admin.php (1)
139-139: ⚡ Quick winConsider updating the editor app localization for consistency.
Line 139 still uses the old concatenation pattern (
rest_url() . 'accessibility-checker/v1') while the main admin script now uses the directrest_url('accessibility-checker/v1')pattern. For consistency and to ensure the editor app also benefits from proper multisite/custom REST base support, consider updating this line to match the new pattern.Suggested refactor
-'edacApiUrl' => esc_url_raw( rest_url() . 'accessibility-checker/v1' ), +'edacApiUrl' => esc_url_raw( rest_url( 'accessibility-checker/v1' ) ),🤖 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 `@admin/class-enqueue-admin.php` at line 139, Update the editor app localization entry for 'edacApiUrl' to use the REST helper with a path argument rather than string concatenation: replace the esc_url_raw( rest_url() . 'accessibility-checker/v1' ) pattern with esc_url_raw( rest_url('accessibility-checker/v1') ) so the 'edacApiUrl' uses multisite/custom REST base-aware URL generation (locate the 'edacApiUrl' array entry in admin/class-enqueue-admin.php).
🤖 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 `@admin/class-enqueue-admin.php`:
- Line 139: Update the editor app localization entry for 'edacApiUrl' to use the
REST helper with a path argument rather than string concatenation: replace the
esc_url_raw( rest_url() . 'accessibility-checker/v1' ) pattern with esc_url_raw(
rest_url('accessibility-checker/v1') ) so the 'edacApiUrl' uses multisite/custom
REST base-aware URL generation (locate the 'edacApiUrl' array entry in
admin/class-enqueue-admin.php).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be7c0d1c-844f-4c51-932b-28c29b9e6c53
📒 Files selected for processing (6)
admin/class-enqueue-admin.phpincludes/classes/class-enqueue-frontend.phpsrc/common/saveFixSettingsRest.jssrc/frontendHighlighterApp/index.jstests/phpunit/Admin/EnqueueAdminTest.phptests/phpunit/includes/classes/EnqueueFrontendTest.php
…ently Add guard clause to saveFixSettings() so that if fixesRestUrl resolves to undefined (script loaded outside a context where localisation ran), the function exits cleanly with a user-visible error message instead of sending a fetch to `undefined/fixes/update/`. Add equivalent guard to saveScanResults() returning a rejected Promise when restUrl is missing, matching the existing guard pattern in clearIssues(). Prefix all edacFrontendHighlighterApp accesses in clearIssues() and the new saveScanResults() guard with `window.` to avoid a ReferenceError if the global is completely undeclared rather than merely undefined. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed the Gemini Code Assist review findings in commit ce8674d:
|
|
Addressed the Gemini Code Assist review findings in commit ce8674d:
|
|
CodeRabbit review complete — no inline findings. All 5 pre-merge checks passed. The JS files were skipped as similar to previous changes; the PHP enqueue and test files passed cleanly. |
1 participant
This pull request updates how REST API endpoint URLs are handled throughout the plugin, ensuring they are dynamically passed from PHP to JavaScript rather than being hardcoded. This change improves compatibility with custom site configurations and makes the codebase more maintainable. The main updates involve adding new variables for REST URLs in PHP and refactoring JavaScript to use these variables.
REST API URL Handling Improvements:
Added
restUrlandfixesRestUrlvariables to the localized script data in both admin (maybe_enqueue_admin_and_editor_app_scripts) and frontend (maybe_enqueue_frontend_highlighter) PHP enqueue functions, ensuring all REST endpoint URLs are dynamically generated and available for JavaScript. [1] [2]Updated JavaScript (
src/frontendHighlighterApp/index.jsandsrc/common/saveFixSettingsRest.js) to use the newrestUrlandfixesRestUrlvariables instead of hardcoded REST endpoint paths, improving flexibility and reliability. [1] [2] [3]Validation and Error Handling:
restUrlinstead of the previously usededacUrl, ensuring required parameters are correctly checked before making API calls.Summary by CodeRabbit