Skip to content

Normalize fixes REST API namespace - #1841

Open
benitoalba wants to merge 1 commit into
equalizedigital:developfrom
benitoalba:fix/1736-normalize-fixes-rest-namespace
Open

Normalize fixes REST API namespace#1841
benitoalba wants to merge 1 commit into
equalizedigital:developfrom
benitoalba:fix/1736-normalize-fixes-rest-namespace

Conversation

@benitoalba

@benitoalba benitoalba commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Register the fixes routes under the canonical accessibility-checker/v1 namespace.
  • Preserve edac/v1 as a backward-compatible alias with the same callbacks, permissions, methods, and payloads.
  • Point GET /fixes at the existing settings getter so both canonical and legacy routes have a callable handler.
  • Update the issue modal and localized admin/frontend consumers to use only the canonical namespace.
  • Add PHPUnit and Jest regression coverage for route registration and responses, localized URLs, custom REST prefixes, and FixCard requests.

Testing

  • npm run test:jest -- --runInBand — 53 suites and 949 tests passed.
  • docker compose exec -T phpunit vendor/bin/phpunit — 876 tests and 1,891 assertions passed; 15 skipped.
  • Changed JavaScript lint and touched PHP PHPCS passed.
  • npm run build passed with the existing asset-size and Browserslist data warnings.
  • git diff --check passed.
  • Targeted REST tests confirmed authorized canonical and legacy GET /fixes requests return identical 200 responses.
  • Manual WordPress testing confirmed issue-modal load/save and frontend-highlighter save requests use accessibility-checker/v1 and return 200, legacy fix-fields and update aliases return 200, and anonymous requests to both namespaces remain rejected with 401.

Fixes #1736

Checklist

  • PR is linked to the main issue in the repo
  • Tests are added that cover changes

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Fixes REST routes now support the canonical accessibility-checker/v1 namespace while retaining edac/v1. Admin, frontend, and FixCard requests use the canonical namespace, with PHPUnit and Jest coverage updated accordingly.

Changes

Fixes REST namespace

Layer / File(s) Summary
Register canonical and legacy fix routes
includes/classes/Fixes/FixesManager.php, tests/phpunit/includes/classes/Fixes/FixesManagerTest.php
Shared route definitions register fixes endpoints under both namespaces and verify matching handlers, permissions, and methods.
Point clients to the canonical namespace
admin/class-enqueue-admin.php, includes/classes/class-enqueue-frontend.php, src/issueModal/components/FixCard.js
Localized URLs and FixCard GET/POST requests now use accessibility-checker/v1.
Validate namespace URLs and requests
tests/jest/issueModal/FixCard.test.js, tests/phpunit/Admin/EnqueueAdminTest.php, tests/phpunit/includes/classes/EnqueueFrontendTest.php
Tests verify canonical URLs, REST prefix handling, request payloads, and FixCard save behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: pattonwebz, stevejonesdev

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR updates the fixes endpoint to accessibility-checker/v1 and keeps edac/v1 as a backward-compatible alias, matching #1736.
Out of Scope Changes check ✅ Passed The extra test and refactor changes are directly tied to the namespace migration and its regression coverage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: normalizing the fixes REST API namespace to the canonical endpoint.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@benitoalba
benitoalba marked this pull request as ready for review July 18, 2026 18:10

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the REST API namespace for fixes from "edac/v1" to "accessibility-checker/v1" across admin scripts, frontend scripts, and React components, while retaining the legacy namespace for backward compatibility. It also introduces corresponding Jest and PHPUnit tests. The review feedback highlights a critical issue where the registered callback get_fixes is not defined in FixesManager, which would cause a fatal error. Additionally, it suggests a more reliable way to initialize the REST server in PHPUnit tests to avoid potential issues with do_action( 'rest_api_init' ).

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.

Comment thread includes/classes/Fixes/FixesManager.php Outdated
$routes = [
'/fixes' => [
'methods' => 'GET',
'callback' => [ $this, 'get_fixes' ],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The callback [ $this, 'get_fixes' ] is registered for the /fixes route, but the get_fixes method is not defined anywhere in the FixesManager class. This will result in a fatal error (Uncaught Error: Call to undefined method ...) when a GET request is made to the /fixes endpoint. Please implement the get_fixes method or map it to an existing method (such as returning get_fixes_settings()).

Comment on lines +144 to +146
do_action( 'rest_api_init' );

$registered_routes = rest_get_server()->get_routes();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Calling do_action( 'rest_api_init' ) before rest_get_server() is initialized can be problematic. In WordPress, if the global $wp_rest_server is not yet instantiated, register_rest_route() calls will return false and do nothing. When rest_get_server() is subsequently called, it will initialize the server and trigger rest_api_init again. To ensure a clean and reliable test environment, initialize the REST server first using rest_get_server() and let it handle the action firing.

		$server            = rest_get_server();
		$registered_routes = $server->get_routes();

@benitoalba
benitoalba force-pushed the fix/1736-normalize-fixes-rest-namespace branch from ca917f6 to a4f87e8 Compare July 18, 2026 18:21

@pattonwebz pattonwebz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fixes rest api endpoint uses edac instead of accessibility-checker

2 participants