Add initial support for handling of virtual posts - #1171
Conversation
[PRO-169]
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces scattered option reads with Settings::get_scannable_post_types(), adds edac_is_virtual_page() and edac_is_pro(), hides Readability UI for virtual pages, introduces origin-URL/filter and frontend post_id filter, fires edac_before_delete_cpt_posts, expands settings UI/sanitizers, and adds PHPUnit tests for virtual-page detection. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST_Api
participant Settings
participant Validator
Client->>REST_Api: POST set_post_scan_results / clear_issues_for_post
REST_Api->>Settings: get_scannable_post_types()
Settings-->>REST_Api: scannable_post_types
REST_Api->>Validator: check post_type ∈ scannable_post_types
alt allowed
REST_Api->>REST_Api: perform save/clear
REST_Api-->>Client: 200/204
else forbidden
REST_Api-->>Client: 400
end
sequenceDiagram
participant EditorUI
participant Admin_Ajax
participant WP_Filter
participant WP_Core
EditorUI->>Admin_Ajax: request details (post_id)
Admin_Ajax->>WP_Filter: apply_filters('edac_get_origin_url_for_virtual_page', post_id)
WP_Filter-->>Admin_Ajax: origin_url or null
alt origin_url is string
Admin_Ajax->>Admin_Ajax: build view link = origin_url + edac params
else
Admin_Ajax->>WP_Core: get_the_permalink(post_id)
WP_Core-->>Admin_Ajax: permalink
Admin_Ajax->>Admin_Ajax: build view link = permalink + edac params
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds filtering capability for post types at save time by applying a filter to the post types configuration. This allows for dynamic modification of which post types should be scanned during REST API operations.
Key changes:
- Applies a filter to post types configuration in the REST API class to enable dynamic post type filtering
Comments suppressed due to low confidence (1)
includes/classes/class-rest-api.php:300
- [nitpick] The filter name 'edacp_fill_site_scan_scannable_post_types' appears to be inconsistent with the context. It mentions 'fill_site_scan' but this code is in the REST API's set_post_scan_results method. Consider renaming to something more contextually appropriate like 'edacp_rest_api_scannable_post_types' or 'edacp_post_scan_results_post_types'.
$post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @pattonwebz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a change that introduces a new filter hook to the system. This enhancement provides greater flexibility by allowing external code to dynamically modify the list of post types that are designated as scannable. This means that the set of post types checked during a scan can now be customized without directly altering the core logic, making the system more adaptable to various content configurations.
Highlights
- Post Type Filtering: I've added a new filter hook,
edacp_fill_site_scan_scannable_post_types, to theset_post_scan_resultsmethod inclass-rest-api.php. This filter allows external code to modify the array of post types that are considered scannable before they are checked.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds a filter to allow modifying the list of scannable post types when saving scan results, which is a good enhancement for extensibility. While the change itself is positive, I've identified a potential issue with inconsistency. The same check for scannable post types is performed in the clear_issues_for_post method within the same file, but it has not been updated to use the new filter. This will cause issues for any post types added via the filter, as they can be scanned but their results cannot be cleared. It's important to apply this filter consistently wherever scannable post types are checked. I've also added a specific comment suggesting an improvement to the filter's name for clarity and consistency.
|
|
||
| $post_type = get_post_type( $post ); | ||
| $post_types = Helpers::get_option_as_array( 'edac_post_types' ); | ||
| $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) ); |
There was a problem hiding this comment.
The filter name edacp_fill_site_scan_scannable_post_types is quite long and potentially misleading. The context here is saving results for a single post scan, not a 'site scan'. A more concise and accurate name would improve readability and maintainability. Consider renaming it to something like edacp_scannable_post_types to better reflect its purpose and align with the naming conventions of other hooks in the project.
$post_types = apply_filters( 'edacp_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/classes/class-rest-api.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/classes/class-rest-api.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
includes/classes/class-rest-api.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/classes/class-rest-api.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-rest-api.php
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Profile and optimize accessibility scans for large posts/pages
📚 Learning: applies to **/*.php : use edac_ prefix for custom hooks and filters...
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
Applied to files:
includes/classes/class-rest-api.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). (1)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
|
|
||
| $post_type = get_post_type( $post ); | ||
| $post_types = Helpers::get_option_as_array( 'edac_post_types' ); | ||
| $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) ); |
There was a problem hiding this comment.
Use edac_ prefix for the custom filter hook.
The filter name edacp_fill_site_scan_scannable_post_types doesn't follow the established edac_ prefix convention used throughout the codebase for custom hooks and filters.
Apply this diff to use the correct prefix:
- $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );
+ $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) ); | |
| $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) ); |
🤖 Prompt for AI Agents
In includes/classes/class-rest-api.php at line 300, the filter hook name uses
the incorrect prefix 'edacp_'. Change the filter name from
'edacp_fill_site_scan_scannable_post_types' to
'edac_fill_site_scan_scannable_post_types' to follow the established 'edac_'
prefix convention used in the codebase.
[PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
admin/class-meta-boxes.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
admin/class-meta-boxes.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-meta-boxes.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
admin/class-meta-boxes.php
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Profile and optimize accessibility scans for large posts/pages
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
📚 Learning: applies to **/*.php : use edac_ prefix for custom hooks and filters...
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
Applied to files:
admin/class-meta-boxes.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). (4)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
| */ | ||
| public function register_meta_boxes(): void { | ||
| $post_types = get_option( 'edac_post_types' ); | ||
| $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Filter name doesn't follow coding guidelines.
The filter name uses edacp_ prefix instead of the required edac_ prefix as specified in the coding guidelines.
Apply this diff to use the correct prefix:
- $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) );
+ $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) );Please verify that this filter name change is applied consistently across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check for consistent usage of the filter name across the codebase
# Expected: All occurrences should use the same filter name
echo "Searching for the current filter name with edacp_ prefix:"
rg "edacp_fill_site_scan_scannable_post_types" -A 2 -B 2
echo -e "\nSearching for similar filter names with edac_ prefix:"
rg "edac_fill_site_scan_scannable_post_types" -A 2 -B 2
echo -e "\nSearching for all edacp_ prefixed filters:"
rg "edacp_" -A 1 -B 1Length of output: 4640
Filter hook prefix should use edac_ instead of edacp_
Coding guidelines require a edac_ prefix for custom hooks and filters. We currently have two occurrences of edacp_fill_site_scan_scannable_post_types that need updating:
• includes/classes/class-rest-api.php
• admin/class-meta-boxes.php
Proposed diffs:
diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php
--- a/includes/classes/class-rest-api.php
+++ b/includes/classes/class-rest-api.php
- $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );
+ $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', Helpers::get_option_as_array( 'edac_post_types' ) );diff --git a/admin/class-meta-boxes.php b/admin/class-meta-boxes.php
--- a/admin/class-meta-boxes.php
+++ b/admin/class-meta-boxes.php
- $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) );
+ $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) );Also, search the codebase for any remaining edacp_fill_site_scan_scannable_post_types references (documentation, tests, external integrations) to ensure consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) ); | |
| // admin/class-meta-boxes.php | |
| - $post_types = apply_filters( 'edacp_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) ); | |
| + $post_types = apply_filters( 'edac_fill_site_scan_scannable_post_types', get_option( 'edac_post_types' ) ); |
🤖 Prompt for AI Agents
In admin/class-meta-boxes.php at line 34, the filter hook prefix is incorrectly
using 'edacp_' instead of the required 'edac_'. Change the filter hook name from
'edacp_fill_site_scan_scannable_post_types' to
'edac_fill_site_scan_scannable_post_types'. Additionally, search the entire
codebase for any other occurrences of
'edacp_fill_site_scan_scannable_post_types' including in
includes/classes/class-rest-api.php, documentation, tests, and external
integrations, and update them to use the correct 'edac_' prefix for consistency.
This change allows the post view link to be modified via a filter, enhancing flexibility in how links are generated. [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
admin/class-ajax.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
admin/class-ajax.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-ajax.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
admin/class-ajax.php
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Profile and optimize accessibility scans for large posts/pages
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use descriptive hook names with edac_ prefix
📚 Learning: applies to **/*.php : use edac_ prefix for custom hooks and filters...
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
Applied to files:
admin/class-ajax.php
📚 Learning: applies to **/*.php : use descriptive hook names with edac_ prefix...
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use descriptive hook names with edac_ prefix
Applied to files:
admin/class-ajax.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). (1)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
| $post_view_link = apply_filters( | ||
| 'edac_details_post_view_link', | ||
| $postid | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add PHPDoc documentation for the new filter hook.
The filter hook implementation is correct and follows the edac_ prefix convention, but it lacks proper documentation. According to the coding guidelines, all custom hooks should be documented with clear descriptions and parameter types.
Add PHPDoc documentation above the filter:
+ /**
+ * Filters the URL used for the "View on page" link in accessibility issue details.
+ *
+ * This filter allows customization of the URL, which is particularly useful for
+ * virtual pages or custom post types that may need special URL handling.
+ *
+ * @since [version]
+ *
+ * @param int $postid The post ID for which to generate the view link.
+ * @return string The URL to use for the "View on page" link.
+ */
$post_view_link = apply_filters(
'edac_details_post_view_link',
$postid
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $post_view_link = apply_filters( | |
| 'edac_details_post_view_link', | |
| $postid | |
| ); | |
| /** | |
| * Filters the URL used for the "View on page" link in accessibility issue details. | |
| * | |
| * This filter allows customization of the URL, which is particularly useful for | |
| * virtual pages or custom post types that may need special URL handling. | |
| * | |
| * @since [version] | |
| * | |
| * @param int $postid The post ID for which to generate the view link. | |
| * @return string The URL to use for the "View on page" link. | |
| */ | |
| $post_view_link = apply_filters( | |
| 'edac_details_post_view_link', | |
| $postid | |
| ); |
🤖 Prompt for AI Agents
In admin/class-ajax.php around lines 491 to 494, the new filter hook
'edac_details_post_view_link' lacks PHPDoc documentation. Add a PHPDoc block
above the apply_filters call describing the filter's purpose, the expected
parameter type (post ID as an integer), and the return type. Follow the existing
coding guidelines for documenting custom hooks with clear descriptions and
parameter annotations.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/classes/class-enqueue-frontend.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/classes/class-enqueue-frontend.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
includes/classes/class-enqueue-frontend.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/classes/class-enqueue-frontend.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/classes/class-enqueue-frontend.php
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
⏰ 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: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
| // Don't load on the frontend if we don't have a post to work with. | ||
| global $post; | ||
| $post_id = is_object( $post ) ? $post->ID : null; | ||
| $post_id = apply_filters( 'edac_filter_frontend_highlight_post_id', is_object( $post ) ? $post->ID : null ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add PHPDoc documentation for the new filter.
The filter implementation is correct and follows WordPress conventions with the proper edac_ prefix. However, according to the coding guidelines, custom hooks should be documented with clear descriptions and parameter types.
Add PHPDoc documentation above the filter:
+ /**
+ * Filter the post ID used for frontend highlighting.
+ *
+ * Allows external code to override which post ID is used for frontend
+ * highlighting functionality, supporting virtual posts for archive/taxonomy pages.
+ *
+ * @since 1.15.0
+ *
+ * @param int|null $post_id The post ID to use for frontend highlighting. Default is the current post ID or null.
+ */
$post_id = apply_filters( 'edac_filter_frontend_highlight_post_id', is_object( $post ) ? $post->ID : null );🤖 Prompt for AI Agents
In includes/classes/class-enqueue-frontend.php around line 53, there is a new
filter call that lacks PHPDoc; add a PHPDoc block immediately above the
apply_filters line documenting the filter name
edac_filter_frontend_highlight_post_id, a short description of what the filter
does, the expected parameter and types (int|null $post_id — the current post ID
or null, and optionally WP_Post|null $post if you want to document the original
variable), the return type (int|null), and tags like @since and @param/@return
following WP coding standards; ensure the comment is concise and placed directly
above the apply_filters call.
Updated the filter name from 'edac_details_post_view_link' to 'edac_get_origin_url_for_virtual_page' for better clarity and consistency. [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
includes/helper-functions.php (1)
789-791: Add rel="noopener noreferrer" when opening in a new tab to prevent tabnabbingWhen
target="_blank"is used, also includerel="noopener noreferrer"for security.Apply this diff:
- $target_attr = $target_blank ? ' target="_blank"' : ''; + $target_attr = $target_blank ? ' target="_blank" rel="noopener noreferrer"' : '';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
admin/class-ajax.php(1 hunks)includes/helper-functions.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- admin/class-ajax.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/helper-functions.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/helper-functions.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/helper-functions.php
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
🔇 Additional comments (1)
includes/helper-functions.php (1)
781-783: Nonce Verification Confirmed in LazyloadFilterThe
edac_noncefor theedac_highlightaction is already validated in the front-end filter:
- File:
includes/classes/class-lazyload-filter.php
Method:perfmatters()(lines 33–35)
Checks forisset( $_GET['edac_nonce'] )and calls
wp_verify_nonce( sanitize_text_field( $_GET['edac_nonce'] ), 'edac_highlight' )No additional
check_ajax_referer()orwp_verify_nonce()calls are required here.
| $link = apply_filters( | ||
| 'edac_get_origin_url_for_virtual_page', | ||
| $post_id | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use permalink as the filter’s default value and document the hook; remove type ambiguity and simplify fallback
Passing $post_id as the value to be filtered is non-standard for apply_filters and introduces type ambiguity (int vs string). Provide the permalink as the default value, pass useful context, and keep a defensive fallback. Also, per coding guidelines, document custom hooks with clear descriptions and parameter types.
Apply this diff:
- $link = apply_filters(
- 'edac_get_origin_url_for_virtual_page',
- $post_id
- );
+ $base_link = get_the_permalink( $post_id );
+
+ /**
+ * Filter the base URL used for "View on page"/landmark links.
+ *
+ * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies).
+ *
+ * @since 1.x.x
+ *
+ * @param string $base_link Default base URL (permalink for $post_id).
+ * @param int $post_id Post ID represented by the link (may be a virtual/ghost post).
+ * @param string $landmark_selector Unencoded CSS selector for the landmark being highlighted.
+ * @param string $landmark Human-readable landmark label.
+ *
+ * @return string Filtered base URL.
+ */
+ $link = apply_filters( 'edac_get_origin_url_for_virtual_page', $base_link, $post_id, $landmark_selector, $landmark );
+ if ( ! is_string( $link ) || '' === $link ) {
+ $link = $base_link;
+ }
@@
- is_string( $link ) ? $link : get_the_permalink( $post_id )
+ $linkNote: The hook name follows the edac_ prefix per team conventions (good). The added docblock aligns with “Document all custom hooks” guidance.
Also applies to: 783-784
🤖 Prompt for AI Agents
In includes/helper-functions.php around lines 773–777 (and similarly at
783–784), the filter call currently passes $post_id as the value which creates a
type ambiguity; change the apply_filters usage to use the permalink (e.g.
get_permalink($post_id) or a computed $permalink) as the default value and pass
$post_id as an additional context argument, keep a defensive fallback to a
string (e.g. get_permalink($post_id) ?? home_url('/')), and add a docblock
immediately above the filter showing the hook name, description, and
parameter/return types (string $url, int $post_id): describe that it filters the
origin URL for a virtual page and returns a string URL.
This change enhances the flexibility of post type handling by applying a filter to retrieve the scannable post types. [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
partials/custom-meta-box.php (2)
8-10: Make post type detection robust and centralize the virtual post type slugUsing get_the_ID() can be unreliable in some admin contexts. Also, hardcoding the slug risks drift; expose it via a filter for consistency and extensibility.
Proposed change:
-// get the post type of the current editor page. -$is_virtual_page = ( get_post_type( get_the_ID() ) === 'edac_virtual_page' ); +// Get the post type of the current editor page. +global $post; +$post_type = $post instanceof WP_Post ? get_post_type( $post ) : get_post_type( get_the_ID() ); +// Allow the virtual post type slug to be customized if ever needed. +$virtual_post_type = apply_filters( 'edac_virtual_post_type', 'edac_virtual_page' ); +$is_virtual_page = ( $post_type === $virtual_post_type );Follow-ups:
- Please verify this partial is always executed in admin where get_current_screen()/global $post are available; otherwise we can guard further.
- Per org standard (learnings), new hooks should use edac_ prefix. If this PR adds any edacp_* hooks elsewhere (e.g., edacp_fill_site_scan_scannable_post_types mentioned in context), consider renaming to edac_* for consistency.
37-37: Avoid inline styles; hide the tab accessibly and keep ARIA/tablist consistentInline styles inside attributes are harder to maintain and may be flagged by WPCS. Prefer semantic hiding and ensure the corresponding tabpanel state matches.
Apply within this line range:
- <li class="edac-tab" <?php echo $is_virtual_page ? 'style="display: none;"' : ''; ?>> + <li class="edac-tab" <?php if ( $is_virtual_page ) : ?>hidden aria-hidden="true"<?php endif; ?>>Additionally (outside the selected lines), mirror the state on the panel so ARIA relationships remain coherent when hidden:
<div role="tabpanel" aria-labelledby="edac-readability-tab" id="edac-readability-panel" class="edac-panel edac-readability" <?php if ( $is_virtual_page ) : ?>hidden aria-hidden="true"<?php endif; ?> style="display: none;" ></div>Notes:
- If any JS assumes the readability tab always exists, please verify that adding hidden does not break focus management or tab-count assumptions. If it does, we can alternatively conditionally omit rendering both the tab and panel when $is_virtual_page is true.
- If you prefer, I can provide a follow-up patch to conditionally not render the tab and panel at all for virtual pages and audit the related JS initialization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
admin/class-ajax.php(4 hunks)includes/classes/class-rest-api.php(2 hunks)partials/custom-meta-box.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- includes/classes/class-rest-api.php
- admin/class-ajax.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
partials/custom-meta-box.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
partials/custom-meta-box.php
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
⏰ 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). (1)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
It has more than a dozen releases since this was changed
edac_before_delete_cpt_posts [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
includes/helper-functions.php (1)
776-780: Use permalink as the default filtered value; pass context and document the hook (and keep a defensive fallback)Passing $post_id as the filter value is non-standard and type-ambiguous (int vs string). Provide the permalink as the default value, pass useful context ($post_id, $landmark_selector, $landmark), and keep a fallback to a valid URL. Also, document the custom filter per guidelines.
Apply:
- $link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - $post_id - ); + $base_link = get_the_permalink( $post_id ); + + /** + * Filter the base URL used for "View on page"/landmark links. + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.31.0 + * + * @param string $base_link Default base URL (permalink for $post_id). + * @param int $post_id Post ID represented by the link (may be a virtual/ghost post). + * @param string $landmark_selector Unencoded CSS selector for the landmark being highlighted. + * @param string $landmark Human-readable landmark label. + * + * @return string Filtered base URL. + */ + $link = apply_filters( 'edac_get_origin_url_for_virtual_page', $base_link, $post_id, $landmark_selector, $landmark ); + if ( ! is_string( $link ) || '' === $link ) { + $link = $base_link; + } @@ - is_string( $link ) ? $link : get_the_permalink( $post_id ) + $linkAlso applies to: 786-786
🧹 Nitpick comments (2)
includes/helper-functions.php (1)
165-168: Return consistent post type slugs from edac_custom_post_typesMerging in an associative pair (
'edac_virtual_page' => 'Archives') injects the label into the values, so consumers like the options page end up with"Archives"instead of the slug. Since bothedac_post_types()andedac_custom_post_types()should return a flat list of slugs, update the helper to emit just the slug.• File: includes/helper-functions.php
• Lines: 165–168- return array_merge( - get_post_types( $args, $output, $operator ), - [ 'edac_virtual_page' => 'Archives' ] - ); + return array_merge( + get_post_types( $args, $output, $operator ), + [ 'edac_virtual_page' ] // always return slugs, not labels + );admin/class-settings.php (1)
50-69: Expose a filter on the computed scannable post types for consistency with callersOther parts of the PR apply edacp_fill_site_scan_scannable_post_types to the scannable list. Apply the same filter here so the single source of truth (Settings::get_scannable_post_types) is extensible and consistent.
Apply:
- return $post_types; + /** + * Filter the list of post types that are scannable. + * + * Mirrors usage in REST and meta boxes so external code can adjust the list globally. + * + * @since 1.31.0 + * + * @param array $post_types Scannable post types. + */ + return apply_filters( 'edacp_fill_site_scan_scannable_post_types', $post_types );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
admin/class-purge-post-data.php(1 hunks)admin/class-settings.php(1 hunks)includes/helper-functions.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
admin/class-purge-post-data.phpadmin/class-settings.phpincludes/helper-functions.php
**/class-*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-purge-post-data.phpadmin/class-settings.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
admin/class-purge-post-data.phpadmin/class-settings.phpincludes/helper-functions.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/helper-functions.php
🧬 Code Graph Analysis (1)
admin/class-settings.php (1)
admin/class-helpers.php (1)
get_option_as_array(132-140)
⏰ 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: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
🔇 Additional comments (2)
admin/class-purge-post-data.php (1)
85-93: Good addition: pre-delete hook with docblock and edac_ prefixThe edac_before_delete_cpt_posts action is well-placed, properly prefixed, and documented with parameter type and since tag. This enables integrations to react before purging data for a CPT.
admin/class-settings.php (1)
43-48: Early delegation to Pro settings is clean and safeThe class_exists + method_exists guard and namespaced static call are correct and maintain compatibility. LGTM.
…pt-slug Updated to use CPT slug from Pro plugin
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
includes/helper-functions.php (1)
776-787: Pass permalink as the filter’s default and document the hook (repeat from prior review)apply_filters currently passes $post_id as the value to be filtered, causing type ambiguity and forcing callbacks to special-case types. Provide a string URL as default, pass useful context, and keep a defensive fallback. Also document the custom hook per guidelines.
Apply this diff:
- $link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - $post_id - ); + $base_link = get_the_permalink( $post_id ); + /** + * Filter the base URL used for landmark links ("View on page"). + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.x.x + * + * @param string $base_link Default base URL (permalink for $post_id). + * @param int $post_id Post ID represented by the link (may be a virtual/ghost post). + * @param string $landmark_selector Unencoded CSS selector for the landmark being highlighted. + * @param string $landmark Human-readable landmark label. + * + * @return string Filtered base URL. + */ + $link = apply_filters( 'edac_get_origin_url_for_virtual_page', $base_link, $post_id, $landmark_selector, $landmark ); + if ( ! is_string( $link ) || '' === $link ) { + $link = $base_link; + } @@ - is_string( $link ) ? $link : get_the_permalink( $post_id ) + $link
🧹 Nitpick comments (4)
partials/custom-meta-box.php (1)
37-37: Avoid inline style; prefer boolean hidden attribute or conditional renderInline styles are harder to override and don’t communicate semantics to AT. Use the hidden attribute (or conditionally omit the LI entirely) to keep the tab out of the tablist and focus order.
Apply this diff:
- <li class="edac-tab" <?php echo $is_virtual_page ? 'style="display: none;"' : ''; ?>> + <li class="edac-tab" <?php echo $is_virtual_page ? 'hidden' : ''; ?>>If the readability panel should never be reachable for virtual pages, consider conditionally not rendering the panel div as well to avoid stray tabpanel markup.
includes/helper-functions.php (1)
817-825: Add type hints and defensive guard to edac_is_virtual_pageMinor hardening: ensure the function signature is explicit and invalid IDs short-circuit before calling get_post_type.
-function edac_is_virtual_page( $post_id ) { +function edac_is_virtual_page( int $post_id ): bool { + $post_id = absint( $post_id ); + if ( 0 === $post_id ) { + return false; + }tests/phpunit/helper-functions/IsVirtualPageTest.php (2)
36-44: Avoid leaking the temporary CPT across testsTo minimize cross-test side effects, unregister the temporary CPT in tearDown(). Example addition (outside this hunk):
protected function tearDown(): void { if ( post_type_exists( 'edac_virtual_item' ) ) { unregister_post_type( 'edac_virtual_item' ); } parent::tearDown(); }
89-101: Eval in tests is acceptable here; naming could align with production for clarityDefining the mock class via eval is fine in the test context (you’ve suppressed PHPCS). Consider using the production slug edac_virtual_page for consistency with documentation and UI to reduce cognitive overhead when cross-referencing.
- class VirtualPageType { - const POST_TYPE = "edac_virtual_item"; - } + class VirtualPageType { + const POST_TYPE = "edac_virtual_page"; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
admin/class-ajax.php(4 hunks)includes/helper-functions.php(3 hunks)partials/custom-meta-box.php(2 hunks)tests/phpunit/helper-functions/IsVirtualPageTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- admin/class-ajax.php
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
tests/phpunit/helper-functions/IsVirtualPageTest.phpincludes/helper-functions.phppartials/custom-meta-box.php
**/[A-Z]*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/[A-Z]*.php: Class names use CamelCase (ClassNameConvention) for new classes
File names use ClassNameConvention.php for new classes (CamelCase)
Classes: ClassName.php
Files:
tests/phpunit/helper-functions/IsVirtualPageTest.php
**/*.{php,js}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
tests/phpunit/helper-functions/IsVirtualPageTest.phpincludes/helper-functions.phppartials/custom-meta-box.php
tests/phpunit/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/phpunit/**/*.php: Write unit tests for new PHP functions and classes
Add integration tests for major features and accessibility rules
Files:
tests/phpunit/helper-functions/IsVirtualPageTest.php
includes/**/*.php
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/helper-functions.php
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: equalizedigital/accessibility-checker#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-04T16:46:23.515Z
Learning: Applies to **/*.php : Use edac_ prefix for custom hooks and filters
🧬 Code Graph Analysis (2)
tests/phpunit/helper-functions/IsVirtualPageTest.php (1)
includes/helper-functions.php (1)
edac_is_virtual_page(817-825)
partials/custom-meta-box.php (1)
includes/helper-functions.php (1)
edac_is_virtual_page(817-825)
⏰ 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). (3)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
🔇 Additional comments (2)
partials/custom-meta-box.php (1)
8-10: LGTM: Virtual page detection is cleanly integratedUsing edac_is_virtual_page(get_the_ID()) up-front is straightforward and keeps the template logic simple.
includes/helper-functions.php (1)
165-168: Fix custom post types merge to append only the slug
The currentarray_mergeofget_post_types()with[ 'edac_virtual_page' => 'Archives' ]appends the literal string
'Archives'into the returned list of slugs. Replace this with code that conditionally appends the virtual CPT’s slug constant, removes duplicates, and reindexes:
- includes/helper-functions.php (edac_custom_post_types, around lines 165–168)
- includes/options-page.php (edac_post_types_cb): confirmed to iterate only slug values, so no callers expect a slug→label map
--- a/includes/helper-functions.php +++ b/includes/helper-functions.php @@ -162,7 +162,16 @@ function edac_custom_post_types() { - return array_merge( - get_post_types( $args, $output, $operator ), - [ 'edac_virtual_page' => 'Archives' ] - ); + $types = get_post_types( $args, $output, $operator ); + // Append virtual CPT slug when Pro is active. + if ( class_exists( '\EqualizeDigital\AccessibilityCheckerPro\VirtualContent\VirtualPageType' ) ) { + $virtual = \EqualizeDigital\AccessibilityCheckerPro\VirtualContent\VirtualPageType::POST_TYPE; + if ( is_string( $virtual ) && '' !== $virtual ) { + $types[] = $virtual; + } + } + return array_values( array_unique( $types ) );
[PRO-169]
…trieval
Updated multiple files to replace direct calls to get_option('edac_post_types') with Settings::get_scannable_post_types() for consistency and maintainability.
NOTE: if 🐛 happen it was probably this change.
[PRO-169]
…lliam/no-issue/move-pro-settings-to-free-with-upsell
…ro-settings-to-free-with-upsell Move some settings definitions into free plugin
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin/class-enqueue-admin.php (1)
64-74: That OR loads scripts everywhere — tighten the gate, fool!Using “|| 'site-editor.php' !== $pagenow” makes the whole condition true on almost every admin screen, bloating performance. This should be an AND.
- if ( - ( - is_array( $post_types ) && - count( $post_types ) && - ( - in_array( $current_post_type, $post_types, true ) || - in_array( $page, $enabled_pages, true ) - ) - ) || - 'site-editor.php' !== $pagenow - ) { + if ( + is_array( $post_types ) && + count( $post_types ) && + ( + in_array( $current_post_type, $post_types, true ) || + in_array( $page, $enabled_pages, true ) + ) && + 'site-editor.php' !== $pagenow + ) {
♻️ Duplicate comments (2)
admin/class-enqueue-admin.php (1)
115-125: Validate filtered origin URLs and document the hook — no javascript: traps.Echoing earlier feedback: pass a default URL into the filter, validate scheme, and fall back safely. Don’t let a bad filter feed a bogus protocol.
- $post_view_link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - $post_id - ); - - $scan_url = add_query_arg( - [ - 'edac_pageScanner' => 1, - ], - is_string( $post_view_link ) ? $post_view_link : get_preview_post_link( $post_id ) - ); + $default_base = get_preview_post_link( $post_id ); + /** + * Filter the base URL used when launching the page scanner from the editor. + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.29.0 + * + * @param string $default_base Default preview URL for the post. + * @param int $post_id Current post ID (can be virtual/ghost). + * @return string Filtered base URL. + */ + $post_view_link = apply_filters( 'edac_get_origin_url_for_virtual_page', $default_base, $post_id ); + $scan_base = ( is_string( $post_view_link ) && '' !== $post_view_link ) ? $post_view_link : $default_base; + $parsed = wp_parse_url( $scan_base ); + if ( empty( $parsed['scheme'] ) || ! in_array( $parsed['scheme'], [ 'http', 'https' ], true ) ) { + $scan_base = $default_base; + } + $scan_url = add_query_arg( [ 'edac_pageScanner' => 1 ], $scan_base );Also applies to: 120-125
includes/helper-functions.php (1)
773-786: Pass a URL (not a post ID) to the filter and document the hookYou’re filtering with
$post_idas the value, which is non-standard and ambiguous (int vs string). Feed the permalink as the default, pass useful context, and keep the defensive fallback. Don’t make me pity a type confusion bug later.This mirrors a prior remark on this exact spot.
Apply:
- $link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - $post_id - ); + $base_link = get_the_permalink( $post_id ); + + /** + * Filter the base URL used for "View on page"/landmark links. + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.31.0 + * + * @param string $base_link Default base URL (permalink for $post_id). + * @param int $post_id Post ID represented by the link (may be a virtual/ghost post). + * @param string $landmark_selector Unencoded CSS selector for the landmark being highlighted. + * @param string $landmark Human-readable landmark label. + * + * @return string Filtered base URL. + */ + $link = apply_filters( 'edac_get_origin_url_for_virtual_page', $base_link, $post_id, $landmark_selector, $landmark ); + if ( ! is_string( $link ) || '' === $link ) { + $link = $base_link; + } @@ - is_string( $link ) ? $link : get_the_permalink( $post_id ) + $linkRun to verify docblocks exist where the hook is used:
#!/bin/bash # Expect: a docblock immediately above each apply_filters('edac_get_origin_url_for_virtual_page', ...) rg -n -C2 "apply_filters\(\s*'edac_get_origin_url_for_virtual_page'" --type=php
🧹 Nitpick comments (15)
src/common/sass/_fix-settings.scss (2)
248-269: Keep the old selector too — don’t strand legacy markup.You renamed the class to edac-setting--upsell-link, but older screens or cached HTML may still emit edac-fix--upsell-link. Support both to avoid visual regressions.
-.edac-setting--upsell-link { +.edac-setting--upsell-link, +.edac-fix--upsell-link { font-weight: 600; background: #f3cd1e; border-radius: 18px; color: #072446; display: inline-block; font-size: 0.75rem; line-height: 1; padding: 4px 8px; text-decoration: none; &:hover, &:focus { color: #072446; text-decoration: underline; } &:focus { outline: revert !important; outline-offset: revert !important; } }
5-5: Typo in comment — keep it tight.“alement” → “element”.
- /* Styles on a parent alement set `all` to none making everything invisible, need to revert that here. */ + /* Styles on a parent element set `all` to none making everything invisible; revert that here. */src/common/settings-pro-callout.js (1)
27-33: Don’t bail when there’s no table header — fall back gracefully.Some settings UIs won’t live inside a table header. Instead of returning early, append the link to the upsell container as a fallback.
- const tableHead = element.closest( 'tr' )?.querySelector( 'th' ); - if ( ! tableHead ) { - return; - } - tableHead.appendChild( document.createTextNode( ' ' ) ); - tableHead.appendChild( upsellLink ); + const tableHead = element.closest( 'tr' )?.querySelector( 'th' ); + if ( ! tableHead ) { + element.appendChild( document.createTextNode( ' ' ) ); + element.appendChild( upsellLink ); + } else { + tableHead.appendChild( document.createTextNode( ' ' ) ); + tableHead.appendChild( upsellLink ); + }admin/class-enqueue-admin.php (1)
128-144: Escape scanUrl before localizing — keep it clean.Minor hardening: escape the computed URL you pass into JS.
- 'scanUrl' => $scan_url, + 'scanUrl' => esc_url_raw( $scan_url ),src/admin/index.js (1)
664-665: Define the dependency helper before it’s used — avoid TDZ surprises.It’s called on window load, so it’s effectively safe today, but moving the function above the addEventListener or wrapping the call in a DOMContentLoaded handler near the function keeps things future-proof if bundling order changes.
-// window load handler... -window.addEventListener( 'load', function() { - ... - initArchivesScanningDependency(); -} ); - -/** - * Initialize the interdependency between archives scanning and taxonomy scanning settings - */ -const initArchivesScanningDependency = () => { +/** + * Initialize the interdependency between archives scanning and taxonomy scanning settings + */ +const initArchivesScanningDependency = () => { // ... unchanged ... }; + +// window load handler... +window.addEventListener( 'load', function() { + ... + initArchivesScanningDependency(); +} );Also applies to: 933-960
includes/helper-functions.php (1)
807-825: Docblock mismatch and optional typing for edac_is_virtual_page()Doc mentions “VirtualPageType” but code uses VirtualItemType. Fix the reference to avoid confusion. Optional: add an int type-hint if callers never pass WP_Post.
Doc fix:
- * This function checks if a post is a virtual page using the pro plugin's - * VirtualPageType constant. + * This function checks if a post is a virtual page using the Pro plugin's + * VirtualItemType::POST_TYPE constant.Optional signature tightening (only if callers always pass an int):
-function edac_is_virtual_page( $post_id ) { +function edac_is_virtual_page( int $post_id ) {includes/options-page.php (9)
153-161: Scan Speed field UI — solid; consider minor hardeningUI looks good and is gated behind Pro. Minor: ensure sanitizer enforces allowed values (you added one; great). Optionally add a title attribute for extra context.
216-223: Simplified Summary Heading field — add sensible limitsAdd maxlength to prevent absurdly long headings wrecking layouts, fool.
- <input + <input <?php echo edac_is_pro() ? '' : 'class="edac-setting--upsell"'; ?> type="text" name="edacp_simplified_summary_heading" id="edacp_simplified_summary_heading" value="<?php echo esc_attr( $simplified_summary_heading ); ?>" <?php disabled( ! edac_is_pro() ); ?> + maxlength="120" >
298-304: Registering Pro-compat settings in Free — good migration pathThe wrapper sanitizers preserve values when Pro is inactive. Consider adding @SInCE tags to these options in code comments for maintainability.
493-499: Sanitizer should use strict typingTighten the signature to match the doc and reduce ambiguity.
-function edac_sanitize_scan_speed( $speed ) { +function edac_sanitize_scan_speed( string $speed ): string { if ( in_array( $speed, [ '250', '1000', '5000', '30000' ], true ) ) { return $speed; } return '1000'; }
808-824: edac_sanitize_pro_setting(): behavior is correct; add @SInCE and detailNice guard. Add a short doc note that when Pro is inactive, user input is ignored and existing value is returned unchanged.
I can add @SInCE tags for all new sanitizers in one sweep if you’d like.
878-885: Ignore roles wrapper sanitizerSane default of ['administrator'] when Pro is inactive. Consider documenting the default in the settings description text.
913-931: Simplified Summary Heading input — accessibility polishConsider adding aria-describedby pointing to a short description (if you add one) and maxlength as suggested earlier. Otherwise looks good.
938-967: Roles list rendering: use wp_roles() for robustnessUsing the global works, but wp_roles() is more robust across contexts. Also, don’t forget to escape role names (you did — nice).
- global $wp_roles; - // phpcs:ignore Universal.Operators.DisallowShortTernary.Found -- ternary is more readable here. - $selected_roles = get_option( 'edacp_ignore_user_roles' ) ?: []; - $roles = $wp_roles->roles; + $roles_api = function_exists( 'wp_roles' ) ? wp_roles() : $GLOBALS['wp_roles']; + $selected_roles = get_option( 'edacp_ignore_user_roles' ) ?: []; + $roles = is_object( $roles_api ) ? $roles_api->roles : [];
975-989: Sanitizer should always return an array and can be typedGuarantee an array return and add strict types to reduce footguns, fool.
-function edac_sanitize_ignore_user_roles( $selected_roles ) { +function edac_sanitize_ignore_user_roles( array $selected_roles ): array { @@ - if ( $selected_roles ) { + if ( ! empty( $selected_roles ) ) { foreach ( $selected_roles as $key => $selected_role ) { if ( ! in_array( $selected_role, (array) $roles, true ) ) { unset( $selected_roles[ $key ] ); } } } - return $selected_roles; + return array_values( (array) $selected_roles );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
admin/class-enqueue-admin.php(3 hunks)includes/activation.php(1 hunks)includes/classes/class-enqueue-frontend.php(3 hunks)includes/classes/class-rest-api.php(2 hunks)includes/helper-functions.php(4 hunks)includes/options-page.php(12 hunks)src/admin/index.js(4 hunks)src/common/sass/_fix-settings.scss(1 hunks)src/common/settings-pro-callout.js(1 hunks)tests/phpunit/helper-functions/IsVirtualPageTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- includes/classes/class-rest-api.php
- includes/classes/class-enqueue-frontend.php
- tests/phpunit/helper-functions/IsVirtualPageTest.php
🧰 Additional context used
📓 Path-based instructions (8)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/activation.phpadmin/class-enqueue-admin.phpincludes/helper-functions.phpincludes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/activation.phpsrc/admin/index.jssrc/common/settings-pro-callout.jsadmin/class-enqueue-admin.phpincludes/helper-functions.phpincludes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/activation.phpincludes/helper-functions.phpincludes/options-page.php
**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.js: Strings in JavaScript also need translation support using wp.i18n functions
Gracefully handle JavaScript errors to avoid breaking accessibility features
Files:
src/admin/index.jssrc/common/settings-pro-callout.js
**/*.{css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Support RTL languages where applicable
Files:
src/admin/index.jssrc/common/settings-pro-callout.js
src/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.js: Focus management implemented
Minimize DOM operations in JavaScript for frontend scanning
Files:
src/admin/index.jssrc/common/settings-pro-callout.js
**/[a-z]*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
JavaScript Utilities: utilityName.js
Files:
src/admin/index.jssrc/common/settings-pro-callout.js
**/class-*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-enqueue-admin.php
🧠 Learnings (2)
📚 Learning: 2025-07-20T09:54:46.746Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1089
File: includes/classes/Fixes/Fix/AddSpacebarSupportToLinksWithButtonRoleFix.php:83-83
Timestamp: 2025-07-20T09:54:46.746Z
Learning: In the Accessibility Checker plugin, fix classes use a consistent pattern `isset( $this->is_pro ) && $this->is_pro ? false : true` for the upsell field in their settings arrays. The `is_pro` property is dynamically set and the `isset()` check prevents PHP notices when the property doesn't exist. This is the standard pattern used across all fix classes in the codebase.
Applied to files:
src/common/settings-pro-callout.js
📚 Learning: 2025-08-26T11:25:48.196Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.196Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/helper-functions.php
🧬 Code graph analysis (4)
src/admin/index.js (1)
src/common/settings-pro-callout.js (2)
inlineSettingsProUpsell(3-34)inlineSettingsProUpsell(3-34)
admin/class-enqueue-admin.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_generate_link_type(558-607)
includes/helper-functions.php (1)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)
includes/options-page.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
⏰ 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: Integration Test: PHP 8.2 | WP latest (+ ms)
- GitHub Check: Integration Test: PHP 8.1 | WP 6.2
🔇 Additional comments (24)
src/common/settings-pro-callout.js (1)
11-25: Solid i18n and safe URL handling — nice work, sucka.Encoding the dynamic name, using translated text with the correct domain, and adding noopener/noreferrer is all on point.
includes/activation.php (1)
23-24: No Option Key Mismatch – All Hooks Use “edacp_ignore_user_roles”Fool! I checked the code: the option is added, registered, sanitized, retrieved, and output via the checkbox name all as
edacp_ignore_user_roles:
- add_option( 'edacp_ignore_user_roles', … ); (includes/activation.php:24)
- register_setting( …, 'edacp_ignore_user_roles', … ); (includes/options-page.php:302)
- get_option( 'edacp_ignore_user_roles', … ); (includes/options-page.php:26 & 883 & 942)
- name="edacp_ignore_user_roles[]" in the callback’s
<input>(includes/options-page.php:952)The lone “edac_ignore_user_roles” appears only as the callback name and the label_for attribute (used for HTML IDs), not as an option key. The UI correctly reads and writes the same
edacp_ignore_user_roleskey, so there’s no silent break.Ignore the original mismatch concern.
Likely an incorrect or invalid review comment.
admin/class-enqueue-admin.php (1)
100-101: Pro detection constant looks off — confirm the right flag.Elsewhere you use EDACP_KEY_VALID to detect Pro (see edac_generate_link_type). Here you check EDAC_KEY_VALID with EDACP_VERSION. Pick one scheme and standardize.
- $pro = defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID; + $pro = ( defined( 'EDACP_KEY_VALID' ) && EDACP_KEY_VALID );If Pro uses a different constant, align with that instead across the codebase.
src/admin/index.js (2)
9-9: Import rename looks good — consistent module boundary.Swapping to the common settings upsell is tidy and reduces duplication.
23-25: Smart gating before injecting upsell links — nice.You check for either upsell class before calling the initializer. Low risk of unnecessary work.
includes/helper-functions.php (3)
8-9: Centralizing scannable post types via Settings is on pointGood move importing EDAC\Admin\Settings to keep post-type logic in one place. Tightens cohesion and avoids option drift.
413-417: edac_get_posts_count() now uses Settings::get_scannable_post_types() — solidThis aligns with the new single source of truth and respects Pro pass-through. No issues spotted.
831-833: Pro detection is robust — matches established patternUsing defined checks for both EDACP_VERSION and EDAC_KEY_VALID avoids notices and false positives. Nice.
includes/options-page.php (16)
9-9: Importing Settings here keeps sanitizers and UI in syncThumbs up — this prevents the UI from diverging from the scanning engine.
110-110: Section rename to “Scan Settings”Clearer label; improves IA. All good.
136-141: New “System Settings” sectionGood separation of concerns; “Delete Data” belongs here. Proceed.
162-169: Enable Archive Scanning UIWell-structured with aria-describedby and Pro gate. No issues.
171-178: Scan All Taxonomy Terms UICorrectly disabled unless archives enabled and Pro active. Copy sets expectations; looks good.
194-196: Move “Delete Data” to System SettingsRight call; matches user mental model.
376-384: System Settings section callbackClear and translatable description. No issues.
390-423: Scan Speed control rendering
- Proper gating with disabled() and upsell class.
- aria-describedby points to an explanatory paragraph.
All good.
432-453: Archive scanning checkboxEverything escapes correctly; gating is consistent. LGTM.
462-483: “Scan all taxonomy terms” checkboxGood UX with dependency on archive scanning. No further notes.
831-836: edac_sanitize_pro_scan_speed(): uses core sanitizer — goodWrapper delegates to the stricter sanitizer only when Pro is active. LGTM.
845-850: edac_sanitize_pro_checkbox(): correct boolean guardConsistent behavior across Pro states. Good.
858-860: Archive scanning wrapper sanitizerConsistent with checkbox wrapper; no issues.
868-870: Taxonomy terms wrapper sanitizerSame pattern; all good.
892-897: Summary heading wrapper sanitizerCorrect use of sanitize_text_field and a translatable default. Approved.
77-85: Double-check Pro’s hook priorityI pity the fool who assumes Pro hooks at priority 11 without verifying! We couldn’t find any registration of
edacp_register_settingin this codebase, so please manually confirm in the Pro plugin (or vendor directory) that it indeed does:
- add_action( 'admin_init', 'edacp_register_setting', 11 );
If Pro uses a class method or array callback (e.g.
[ $this, 'edacp_register_setting' ]), adjust your search and ensure the remove_action at priority 10 will reliably preempt it.
The previous comparison was an order compare not a set compare. Pre-sorting them ensures the same order so it's a real compare. [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
includes/options-page.php (1)
703-705: Fix broken comparison: sort() returns bool — cache never clears (or thrashes). I pity that bug!Using sort() in the comparison mutates the arrays and returns bool, so you’re comparing booleans, not normalized arrays. This can incorrectly suppress or trigger cache clears. Normalize and compare sets.
Apply this diff:
- $previous = Settings::get_scannable_post_types(); - if ( sort( $previous ) !== sort( $selected_post_types ) ) { + $prev = array_values( array_unique( (array) Settings::get_scannable_post_types() ) ); + $next = array_values( array_unique( (array) $selected_post_types ) ); + sort( $prev ); + sort( $next ); + if ( $prev !== $next ) {Alternative set-compare (order-agnostic, no mutation):
- $previous = Settings::get_scannable_post_types(); - if ( sort( $previous ) !== sort( $selected_post_types ) ) { + $prev = array_unique( (array) Settings::get_scannable_post_types() ); + $next = array_unique( (array) $selected_post_types ); + $changed = array_diff( $prev, $next ) || array_diff( $next, $prev ); + if ( $changed ) {Follow-up: please add a unit test that reorders selected types and asserts Scans_Stats::clear_cache() is NOT called. Reorder + add/remove should call it.
🧹 Nitpick comments (6)
includes/options-page.php (6)
153-160: New settings UI looks solid; one copy tweak for clarityThe upsell-gated controls and ARIA hooks are consistent. Consider renaming “Ignore Permissions” to something clearer like “Roles allowed to ignore issues,” and add aria-describedby helper text to the roles control for parity with other fields.
- __( 'Ignore Permissions', 'accessibility-checker' ), + __( 'Roles Allowed to Ignore Issues', 'accessibility-checker' ),Also applies to: 162-169, 171-178, 180-187
385-424: Scan speed control: consider adding type hints to the sanitizer and aligning typesYou cast the option to int here (good for selected()), but edac_sanitize_scan_speed currently accepts/returns strings. Add strict type hints to keep the contract crisp.
- function edac_sanitize_scan_speed( $speed ) { + function edac_sanitize_scan_speed( string $speed ): string {Optional: if consumers treat it as an integer later, consider storing as string but casting on read consistently.
563-567: Sanitizers should return a default instead of nullBoth position/prompt sanitizers fall off without returning a value on invalid input. Return the registered defaults to avoid saving null/empty.
function edac_sanitize_simplified_summary_position( $position ) { if ( in_array( $position, [ 'before', 'after', 'none' ], true ) ) { return $position; } + return 'after'; } function edac_sanitize_simplified_summary_prompt( $prompt ) { if ( in_array( $prompt, [ 'when required', 'always', 'none' ], true ) ) { return $prompt; } + return 'when required'; }Also applies to: 616-620
919-932: Optional a11y nicety: reflect disabled state for screen readersYou already disable the input when not Pro. Consider adding aria-disabled on the input or helper text clarifying Pro requirement, mirroring other fields’ descriptions.
939-990: Ignore Roles: validation looks good; minor hardeningValidation against $wp_roles is correct. For resilience (e.g., custom roles modified at runtime), consider guarding for empty/undefined roles and returning an empty array.
- $roles = array_keys( $wp_roles->roles ); + $roles = isset( $wp_roles->roles ) ? array_keys( (array) $wp_roles->roles ) : [];
18-30: Return type doc says bool, but function returns array|falseedac_user_can_ignore() returns the intersect array or false. Either cast to bool on return or update the docblock to reflect reality. Keep it honest, fool!
- * @return bool + * @return bool ... - return ( $interset ); + return (bool) $interset;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
includes/options-page.php(12 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/options-page.php
🧬 Code graph analysis (1)
includes/options-page.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
⏰ 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). (1)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
🔇 Additional comments (7)
includes/options-page.php (7)
9-9: Good move centralizing post-type logic behind SettingsImporting EDAC\Admin\Settings keeps scannable post type resolution in one place. Tight.
110-110: Section reorg reads wellRenaming “General” to “Scan Settings” and adding “System Settings” clarifies intent. No concerns.
Also applies to: 136-141
194-196: “Delete Data” under System Settings is the right homeNice IA improvement.
216-223: Heading option is tidy and fully i18n’dUI/escaping look good and disabled state matches edac_is_pro().
298-304: Pro-aware sanitizers wired correctlyUsing wrappers that pass-through on Pro and freeze values on Free avoids surprise changes. Clean pattern.
376-384: System section copy: short and helpfulNo issues.
77-85: Hook removal runs too late; won’t stop Pro from registering settings this requestYou add the remover on admin_menu via edac_add_options_page, but admin_init fires earlier. By the time this code attaches, edacp_register_setting on admin_init (prio 11) has already run. Move the remover so it’s registered before admin_init executes.
Apply within this block (minimal change, but still too late if kept inside this function):
- add_action( - 'admin_init', - function () { - // Remove the pro plugin's settings registration entirely. - remove_action( 'admin_init', 'edacp_register_setting', 11 ); - } - ); + // Register the remover as early as possible. + add_action( + 'admin_init', + function () { + remove_action( 'admin_init', 'edacp_register_setting', 11 ); + }, + 1 + );Recommended fix (move out of edac_add_options_page so it registers before admin_init on every admin request):
// Place at file load or on 'plugins_loaded' (priority 0) in a bootstrap file, not inside edac_add_options_page(). add_action( 'plugins_loaded', function () { remove_action( 'admin_init', 'edacp_register_setting', 11 ); }, 0 );⛔ Skipped due to learnings
Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. The timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. This timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.
…ST_TYPE constant [PRO-169]
[PRO-232]
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
includes/options-page.php (2)
319-336: Don’t read EDAC_KEY_VALID directly; use edac_is_pro()Accessing EDAC_KEY_VALID directly can emit notices if not defined. The project standard (per prior learnings) is to gate with edac_is_pro(), which checks constant existence safely.
Apply this diff:
- if ( EDAC_KEY_VALID === false ) { + if ( ! edac_is_pro() ) {Note: This aligns with the retrieved learnings about robust Pro detection used elsewhere.
656-663: Refactor all direct EDAC_KEY_VALID checks to use edac_is_pro()
I pity the fool who still checks theEDAC_KEY_VALIDconstant directly—let’s useedac_is_pro()everywhere to prevent PHP notices and centralize “pro” detection.Key locations needing updates (replace any
EDAC_KEY_VALIDordefined( 'EDACP_VERSION' ) && EDAC_KEY_VALIDchecks):
- includes/options-page.php
- Lines ~321–324, ~656–663
- partials/settings-page.php
- Lines ~56, ~88–90, ~99–101
- partials/admin-page/fixes-page.php
- Lines ~11–12, ~25–27, ~33–35
- partials/welcome-page.php
- Lines ~20–22, ~130–132, ~188–190, ~196–198
- includes/classes/class-admin-toolbar.php
- Lines ~107–109
- accessibility-checker.php
- Definition of
EDAC_KEY_VALID(leave define, but don’t read it directly elsewhere)- admin/class-upgrade-promotion.php
- Lines ~182–183
- admin/class-enqueue-admin.php
- Line ~100
- admin/site-health/class-checks.php
- Lines ~93–95
- admin/class-welcome-page.php
- Lines ~33–35
- admin/class-widgets.php
- Lines ~57–59, ~79–81, ~219–221, ~274–276
- admin/class-plugin-action-links.php
- Lines ~54–56
- admin/class-admin-footer-text.php
- Lines ~86–87
Example refactor in
includes/options-page.php:- <?php if ( EDAC_KEY_VALID === false ) { ?> + <?php if ( ! edac_is_pro() ) { ?>And for positive checks:
- <?php if ( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID ) : ?> + <?php if ( edac_is_pro() ) : ?>Tests that define/mock
EDAC_KEY_VALIDcan remain, but you’ll need to adjust any plugin code they exercise to calledac_is_pro()(tests can stub that function or define the constant beforehand).
♻️ Duplicate comments (1)
includes/options-page.php (1)
706-715: Array comparison is broken; sort() returns bool, mutates arrayssort($previous) !== sort($selected_post_types) compares booleans, not arrays. It also mutates $selected_post_types, potentially reordering the saved option. Compare sorted copies instead. This also addresses the prior “compare sets, not order” feedback.
Apply this diff:
- $previous = Settings::get_scannable_post_types(); - if ( sort( $previous ) !== sort( $selected_post_types ) ) { + $prev = (array) Settings::get_scannable_post_types(); + $next = array_values( (array) $selected_post_types ); + sort( $prev ); + sort( $next ); + if ( $prev !== $next ) {
🧹 Nitpick comments (5)
src/common/sass/_fix-settings.scss (1)
248-250: Backward-compatible selector: solidTargeting both .edac-fix--upsell-link and .edac-setting--upsell-link keeps old markup working while enabling the new class. Nice.
If variables.$color-yellow is the canonical brand color, consider using it here instead of hardcoding #f3cd1e for consistency with the rest of the stylesheet.
includes/options-page.php (4)
160-167: New settings fields look good; add aria-describedby on taxonomy toggleThe Pro-gated fields and labeling are clean. For edacp_scan_all_taxonomy_terms, wire up aria-describedby to its description for better a11y, like you did for archive scanning.
Apply this diff:
- <input + <input type="checkbox" name="edacp_scan_all_taxonomy_terms" id="edacp_scan_all_taxonomy_terms" + aria-describedby="edac_scan_all_taxonomy_terms_desc" value="1" <?php checked( $scan_all_taxonomies, 1 ); ?> <?php disabled( ! $enable_archives || ! edac_is_pro() ); ?> > @@ - <p class="edac-description"> + <p id="edac_scan_all_taxonomy_terms_desc" class="edac-description">Also applies to: 169-176, 178-185, 187-195, 223-231
393-427: Scan Speed control: clean; minor type consistencyLGTM. If you want to be extra tidy, keep the option as a string consistently (don’t cast to int) since sanitizer returns strings; selected() handles strings fine.
- $full_site_scan_speed = (int) get_option( 'edacp_full_site_scan_speed', 1000 ); + $full_site_scan_speed = (string) get_option( 'edacp_full_site_scan_speed', '1000' ); @@ - <?php selected( $full_site_scan_speed, (int) $value, false ); ?> + <?php selected( $full_site_scan_speed, (string) $value, false ); ?>
812-902: Pro sanitizers: DRY opportunityThe wrappers are correct and safe. You can centralize the “preserve existing value when not Pro” pattern to reduce duplication.
Example refactor:
// Helper to preserve existing option value when Pro is disabled. function edac_pro_or_existing( $input, string $option, callable $sanitize_when_pro, $default ) { return edac_is_pro() ? $sanitize_when_pro( $input ) : get_option( $option, $default ); } // Then: function edac_sanitize_pro_scan_speed( $input ) { return edac_pro_or_existing( $input, 'edacp_full_site_scan_speed', 'edac_sanitize_scan_speed', '1000' ); } function edac_sanitize_pro_archive_scanning( $input ) { return edac_pro_or_existing( $input, 'edacp_enable_archive_scanning', 'edac_sanitize_checkbox', 0 ); }
942-971: Use WP APIs for roles, not $wp_roles globalLeverage wp_roles() or get_editable_roles() to avoid relying on the global and to respect capabilities.
Apply this diff:
- global $wp_roles; - // phpcs:ignore Universal.Operators.DisallowShortTernary.Found -- ternary is more readable here. - $selected_roles = get_option( 'edacp_ignore_user_roles' ) ?: []; - $roles = $wp_roles->roles; + // phpcs:ignore Universal.Operators.DisallowShortTernary.Found -- ternary is more readable here. + $selected_roles = get_option( 'edacp_ignore_user_roles' ) ?: []; + $roles = get_editable_roles(); @@ - <?php echo esc_html( $role['name'] ); ?> + <?php echo esc_html( $role['name'] ); ?>And for sanitization:
- global $wp_roles; - $roles = array_keys( $wp_roles->roles ); + $roles = array_keys( get_editable_roles() );Also applies to: 979-993
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
includes/helper-functions.php(4 hunks)includes/options-page.php(13 hunks)src/common/sass/_fix-settings.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/helper-functions.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/options-page.php
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/options-page.php
🧬 Code graph analysis (1)
includes/options-page.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
⏰ 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). (1)
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
🔇 Additional comments (3)
includes/options-page.php (3)
197-203: Moving “Delete Data” into System Settings: sensible IARe-homing Delete Data under the new System section improves grouping. No issues spotted.
9-9: Centralizing post-type resolution via Settings: good moveImporting EDAC\Admin\Settings and using its API elsewhere in this file keeps behavior consistent across Free/Pro. Thumbs up.
77-85: Ignore: remove_action placement is actually correct due to WP hook timingThe original concern assumes that code hooked within
admin_menuruns afteradmin_inithas completed, but in WordPress core theadmin_menuaction is fired during the execution ofadmin_init. This means your nestedadd_action( 'admin_init', function () { remove_action( 'admin_init', 'edacp_register_setting', 11 ); } );inside
edac_add_options_page()(hooked toadmin_menu) will still register anadmin_initcallback at priority 10. That callback fires earlier in the sameadmin_initpass—precisely before the Pro plugin’sedacp_register_settingat priority 11—so it successfully prevents the Pro settings from registering. Learned from WordPress hook internals: “The admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.” [retrieved_learnings]No changes are required here.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
includes/options-page.php (2)
319-335: Avoid undefined-constant notices; use edac_is_pro()Directly reading EDAC_KEY_VALID can trigger PHP notices when Pro isn’t loaded. We learned this the hard way; the robust pattern is to call edac_is_pro() which checks constant existence first. Don’t let warnings clown you.
Apply this diff:
- if ( EDAC_KEY_VALID === false ) { + if ( ! edac_is_pro() ) {Also apply the same change in edac_post_types_cb where the upsell text is gated by EDAC_KEY_VALID. If you want, I can draft that patch too.
705-718: Prevent cache thrash: compare effective “prev vs next” scannable sets symmetricallyGreat move sorting/uniquing to avoid order-based churn. But mixing “prev” from Settings::get_scannable_post_types() with “next” from raw $selected_post_types can still cause false positives (especially when Pro augments types independently). Compare the effective scannable sets for both “before” and “after” the save using the same resolver. That keeps the cache from getting cleared on every settings save. Don’t make the cache suffer, fool.
Apply this diff:
- // clear cached stats if selected posts types change. - $prev = array_values( array_unique( (array) Settings::get_scannable_post_types() ) ); - $next = array_values( array_unique( (array) $selected_post_types ) ); - sort( $prev ); - sort( $next ); + // Clear cached stats if the effective scannable post-type set changes. + // Compute "prev" using the current option. + $prev = array_values( array_unique( (array) Settings::get_scannable_post_types() ) ); + + // Compute "next" by temporarily overriding the option with the posted value + // so Settings::get_scannable_post_types() resolves the same way as runtime. + $__edac_pre_option_cb = static function () use ( $selected_post_types ) { + return (array) $selected_post_types; + }; + // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores -- Core filter name. + add_filter( 'pre_option_edac_post_types', $__edac_pre_option_cb ); + $next = array_values( array_unique( (array) Settings::get_scannable_post_types() ) ); + // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores -- Core filter name. + remove_filter( 'pre_option_edac_post_types', $__edac_pre_option_cb ); + + sort( $prev ); + sort( $next ); if ( $prev !== $next ) { $scan_stats = new \EDAC\Admin\Scans_Stats(); $scan_stats->clear_cache(); // EDACP\Scans is the old namespace, kept for back compat but should be removed after a few releases. if ( class_exists( '\EDACP\Scans' ) || class_exists( '\EqualizeDigital\AccessibilityCheckerPro\Admin\Scans' ) ) { delete_option( 'edacp_fullscan_completed_at' ); } }
🧹 Nitpick comments (3)
includes/options-page.php (3)
77-85: Back-compat hook is solid; name the callback so it can be unhooked and documentedAnonymous callbacks are hard to unhook and reference. Give this a named function with an edac_ prefix. Keep it tight.
Apply this diff:
- add_action( - 'admin_init', - function () { - // Remove the pro plugin's settings registration entirely. - remove_action( 'admin_init', 'edacp_register_setting', 11 ); - } - ); + add_action( 'admin_init', 'edac_backcompat_deregister_pro_setting', 10 ); + + /** + * Deregister Pro's settings registration for back-compat (free-only sites with outdated Pro). + * + * @since 1.31.0 + * @return void + */ + function edac_backcompat_deregister_pro_setting() { + // Remove the pro plugin's settings registration entirely. + remove_action( 'admin_init', 'edacp_register_setting', 11 ); + }
305-311: Register settings with explicit args for type/defaultTo play nice with REST and ensure stable defaults, prefer the args array form with type/default. It also documents intent. Pity the ambiguity.
Example for each (repeat pattern for the others):
-register_setting( 'edac_settings', 'edacp_full_site_scan_speed', 'edac_sanitize_pro_scan_speed' ); +register_setting( + 'edac_settings', + 'edacp_full_site_scan_speed', + [ + 'type' => 'string', // '250' | '1000' | '5000' | '30000' + 'default' => '1000', + 'sanitize_callback' => 'edac_sanitize_pro_scan_speed', + ] +);
- edacp_enable_archive_scanning / edacp_scan_all_taxonomy_terms: use type 'integer' (since sanitize returns 1/0) and default 0.
- edacp_ignore_user_roles: use type 'array' and default [].
- edacp_simplified_summary_heading: use type 'string' and a sensible default (e.g., 'Simplified Summary').
393-426: Scan speed control is accessible and safe
- Uses aria-describedby and disabled() correctly.
- Values/labels are escaped.
- Works with Pro gating.
Minor nit: you’re casting to int but your sanitize returns strings; it’s fine, but aligning on strings would avoid the extra casts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
includes/helper-functions.php(4 hunks)includes/options-page.php(13 hunks)src/common/sass/_fix-settings.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/common/sass/_fix-settings.scss
- includes/helper-functions.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/options-page.php
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/options-page.php
🧬 Code graph analysis (1)
includes/options-page.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
🔇 Additional comments (22)
includes/options-page.php (22)
110-120: Section rename to “Scan Settings” and new “Permissions” section look goodGood structure and i18n. Clearer UX for where scan-related options live. I like that, fool.
143-149: “System Settings” section registration is cleanConsistent with WPCS and translatable. No concerns.
160-167: Scan Speed field registration reads wellLabel, callback, and section pairing are correct. No jive.
169-176: Archive scanning field registration is correctGood use of a dedicated callback and label_for.
178-185: “Scan All Taxonomy Terms” field registration is correctLooks consistent and ready for the UI logic.
187-194: Ignore Roles field registration is correctProperly grouped under the Permissions section.
201-203: Moving “Delete Data” to System section improves IANice UX touch. Approved.
223-230: Simplified Summary Heading field registration is tidyMatches the new callback you added below. Good.
379-387: System section copy: clear and translatableAll good. Short and helpful.
435-456: Archive scanning checkbox: good gating and descriptionDisabled state and upsell class are applied correctly; description aids understanding.
465-486: Scan-all-taxonomy-terms checkbox: dependency enforced correctlyDisabled when archive scanning is off or Pro is absent. Clear description. Solid.
496-502: Scan speed sanitizer is tightStrict allowlist with a safe default. Nicely done.
815-831: Pro-setting sanitizer wrapper is the right guardrailPassing through when Pro is active and freezing values otherwise avoids sneaky POSTs changing upsell options. Strong.
838-844: Scan speed pro-wrapper sanitizer is correctDefers to the base sanitizer when Pro is active; otherwise returns current value with a sane default.
852-857: Generic pro-checkbox sanitizer wrapper is neatCentralized logic reduces duplication.
865-867: Archive scanning pro-wrapper sanitizer is correctThin wrapper around the generic helper. Approved.
875-877: Taxonomy-terms pro-wrapper sanitizer is correctSame pattern; reads clean.
885-892: Ignore-roles pro-wrapper sanitizer: good defaultFalling back to administrator only keeps power restricted. Looks good.
899-905: Summary heading pro-wrapper sanitizer is correctSanitization plus a friendly default. Ship it.
920-939: Simplified Summary Heading field is well-escaped and gatedProper escaping and disabled state. Nice consistency.
945-974: Ignore Roles UI is correct and safe
- Roles are escaped.
- Values match sanitized output.
- Gating matches Pro status.
All good.
999-1007: Permissions section copy is clearSuccinct and translatable. No changes needed.
[PRO-169]
Updated the way scannable post types are retrieved and cached, simplifying the instantiation of the Scans_Stats class. [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/options-page.php (1)
320-336: Replace all direct EDAC_KEY_VALID/EDACP_KEY_VALID checks with edac_is_pro()
To ensure consistent “Pro” detection and avoid PHP notices, every direct reference to the EDAC_KEY_VALID or EDACP_KEY_VALID constants in production code should be replaced with calls toedac_is_pro()(or its logical inverse). Test files may continue to define and inspect the constants directly to simulate different license states.Please update the following code locations (all line numbers approximate):
- includes/options-page.php
• Line 322:if ( EDAC_KEY_VALID === false ) {→if ( ! edac_is_pro() ) {- includes/options-page.php
• Line 657:<?php if ( EDAC_KEY_VALID === false ) { ?>→<?php if ( ! edac_is_pro() ) { ?>- partials/settings-page.php
• Line 56:<div class="wrap edac-settings <?php echo EDAC_KEY_VALID ? '' : 'pro-callout-wrapper'; ?>">→<?php echo edac_is_pro() ? '' : 'pro-callout-wrapper'; ?>
• Line 88:if ( EDAC_KEY_VALID === false ) {→if ( ! edac_is_pro() ) {
• Line 99:<?php if ( EDAC_KEY_VALID === false ) { ?>→<?php if ( ! edac_is_pro() ) { ?>- partials/welcome-page.php
• Line 20:if ( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID === true ) {→if ( edac_is_pro() ) {
• Line 130:if ( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID ) {→if ( edac_is_pro() ) {
• Line 188:if ( ! defined( 'EDACP_VERSION' ) || ! EDAC_KEY_VALID ) {→if ( ! edac_is_pro() ) {
• Line 196:if ( ! defined( 'EDACP_VERSION' ) || ! EDAC_KEY_VALID ) {→if ( ! edac_is_pro() ) {- partials/admin-page/fixes-page.php
• Line 11:<div id="edac-fixes-page" class="wrap edac-settings <?php echo EDAC_KEY_VALID ? '' : 'pro-callout-wrapper'; ?>">→ useedac_is_pro()
• Line 25:<div class="edac-settings-general <?php echo EDAC_KEY_VALID ? '' : 'edac-show-pro-callout'; ?>">→ useedac_is_pro()
• Line 33:<?php if ( EDAC_KEY_VALID === false ) { ?>→<?php if ( ! edac_is_pro() ) { ?>- admin/class-enqueue-admin.php
• Line 100:$pro = defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID;→$pro = edac_is_pro();- admin/site-health/class-checks.php
• Line 93:if ( defined( 'EDACP_VERSION' ) && defined( 'EDAC_KEY_VALID' ) && EDAC_KEY_VALID ) {→if ( edac_is_pro() ) {- admin/class-upgrade-promotion.php
• Line 182:return defined( 'EDACP_VERSION' ) && defined( 'EDAC_KEY_VALID' ) && (bool) EDAC_KEY_VALID;→return edac_is_pro();- admin/class-widgets.php
• Line 57–59:( ! defined( 'EDACP_VERSION' ) || false === EDAC_KEY_VALID )→! edac_is_pro()
• Line 79–81:( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID )→edac_is_pro()
• Line 219–221: same as above →edac_is_pro()
• Line 274–276:if ( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID ) {→if ( edac_is_pro() ) {- admin/class-plugin-action-links.php
• Line 54:if ( ! defined( 'EDACP_VERSION' ) || ! EDAC_KEY_VALID ) {→if ( ! edac_is_pro() ) {- admin/class-admin-footer-text.php
• Line 86:return defined( 'EDACP_VERSION' ) && defined( 'EDAC_KEY_VALID' ) && EDAC_KEY_VALID;→return edac_is_pro();- admin/class-welcome-page.php
• Line 33:<?php if ( defined( 'EDACP_VERSION' ) && EDAC_KEY_VALID ) : ?>→<?php if ( edac_is_pro() ) : ?>- includes/helper-functions.php
• Line 579:'software' => defined( 'EDACP_KEY_VALID' ) && EDACP_KEY_VALID ? 'pro' : 'free',→ useedac_is_pro() ? 'pro' : 'free'
• (edac_is_pro() is already defined at line 832)Leave all occurrences inside tests/ untouched—they’re intentionally driving the constant values for test scenarios and won’t change when switching production code to use the helper function.
♻️ Duplicate comments (1)
includes/options-page.php (1)
706-719: Nice: set-wise comparison prevents cache thrashSorting/unique before comparing fixes the false-invalidations we flagged earlier. Clean and correct.
🧹 Nitpick comments (1)
includes/options-page.php (1)
320-336: Don’t poke EDAC_KEY_VALID directly—use edac_is_pro() to avoid noticesAlign with your own pattern elsewhere and the repo learning. Checking the constant without defined() risks a notice. Use the helper, sucka.
- if ( EDAC_KEY_VALID === false ) { + if ( ! edac_is_pro() ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
includes/options-page.php(13 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/options-page.php
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/options-page.php
🧬 Code graph analysis (1)
includes/options-page.php (3)
admin/class-scans-stats.php (1)
Scans_Stats(15-469)admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
🔇 Additional comments (8)
includes/options-page.php (8)
9-11: Imports are on-pointUsing Settings::get_scannable_post_types() and Scans_Stats centralizes behavior and keeps this page thin. Solid.
116-121: Sections restructure reads betterNew “Permissions” and “System Settings” sections are clear and follow WPCS/i18n. Ship it.
Also applies to: 144-149
161-168: New fields wired correctly with labels, descriptions, and callbacksA11y and i18n look good. Label_for usage and upsell classes are consistent.
Also applies to: 170-177, 179-186, 188-195, 224-231
306-312: Pro-safe sanitizers registered properlyRegistering edacp_* with wrapper sanitizers preserves values when Pro is absent. Good compatibility story.
380-388: UI callbacks and sanitizer are tidy
- System section copy is clear.
- Scan Speed dropdown: aria-describedby, disabled state, and sane defaults—nice.
- Archive scanning + “scan all terms” dependency and disabled logic are right.
- edac_sanitize_scan_speed whitelists values and defaults cleanly.
Also applies to: 389-427, 436-457, 466-487, 497-503
816-899: Wrapper sanitizers for Pro settings are robustGraceful degradation with value preservation and a pre-save action hook. Good extensibility.
919-932: Simplified summary heading and ignore-roles UX look goodEscaping, disabled state, and role validation against $wp_roles are handled properly. Copy is crisp.
Also applies to: 939-967, 976-990, 995-1001
78-86: Unhook isn’t effective; remove_action runs too late on admin_initAt priority 10, you try to remove a callback scheduled for admin_init 11, but by the time this closure runs on the same hook chain, the 11-callback will still execute in this request. Remove it before admin_init fires (e.g., on plugins_loaded) or at admin_init with an earlier priority than the target (and also before the chain starts). Do it right, fool.
Apply one of these diffs (belt-and-suspenders version runs in both places safely):
- add_action( - 'admin_init', - function () { - // Remove the pro plugin's settings registration entirely. - remove_action( 'admin_init', 'edacp_register_setting', 11 ); - } - ); + // Ensure the Pro registration is unhooked before admin_init runs. + add_action( + 'plugins_loaded', + function () { + remove_action( 'admin_init', 'edacp_register_setting', 11 ); + }, + 0 + ); + // Extra safety: also unhook at the start of admin_init (before 11 runs). + add_action( + 'admin_init', + function () { + remove_action( 'admin_init', 'edacp_register_setting', 11 ); + }, + 0 + );⛔ Skipped due to learnings
Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. The timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. This timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.
[PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
includes/options-page.php (5)
161-168: Scan Speed field — OK; clarify units.Consider mentioning “milliseconds” in the label/description to reduce ambiguity.
-__( 'Scan Speed', 'accessibility-checker' ), +__( 'Scan Speed (milliseconds between requests)', 'accessibility-checker' ),
497-503: Sanitizer for scan speed — constrain inputs; add type hints.Add strict types to meet PHP 7.4 guidance across sanitizers.
-function edac_sanitize_scan_speed( $speed ) { +function edac_sanitize_scan_speed( string $speed ): string {
893-899: Summary heading sanitizer — unslash before sanitize.Settings API submits slashed; unslash to avoid backslashes in saved value.
- return sanitize_text_field( $input ); + return sanitize_text_field( wp_unslash( $input ) );
976-990: Ignore roles sanitizer — validate + also sanitize keys.Add sanitize_key on each role for belt-and-suspenders hardening.
- if ( ! in_array( $selected_role, (array) $roles, true ) ) { + $role_key = sanitize_key( $selected_role ); + if ( ! in_array( $role_key, (array) $roles, true ) ) { unset( $selected_roles[ $key ] ); }
657-665: Stop checking EDAC_KEY_VALID directly — use edac_is_pro()!I pity the fool who relies on raw constants and risks unexpected notices.
edac_is_pro()wraps bothEDACP_VERSIONandEDAC_KEY_VALIDchecks into one reliable helper.This snippet in includes/options-page.php (at line 657) should be updated as follows:
- <?php if ( EDAC_KEY_VALID === false ) { ?> + <?php if ( ! edac_is_pro() ) { ?>We also spotted numerous other direct uses of
EDAC_KEY_VALIDthroughout the plugin—consider standardizing these to useedac_is_pro()for consistency and notice-free detection:
- partials/settings-page.php: lines 56, 88, 99
- partials/admin-page/fixes-page.php: lines 11, 25, 33
- partials/welcome-page.php: lines 20, 130, 188, 196
- admin/class-enqueue-admin.php: line 100
- admin/class-widgets.php: lines 57, 79, 219, 274
- includes/classes/class-admin-toolbar.php: line 107
- admin/site-health/class-checks.php: line 93
- admin/class-welcome-page.php: line 33
- admin/class-upgrade-promotion.php: line 182
- admin/class-plugin-action-links.php: line 54
- admin/class-admin-footer-text.php: line 86
Pity the fool who doesn’t keep it consistent!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
includes/options-page.php(13 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/options-page.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/options-page.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/options-page.php
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/options-page.php
🧬 Code graph analysis (1)
includes/options-page.php (3)
admin/class-scans-stats.php (1)
Scans_Stats(15-469)admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_is_pro(831-833)
⏰ 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). (1)
- GitHub Check: Integration Test: PHP 8.1 | WP latest (+ ms)
🔇 Additional comments (24)
includes/options-page.php (24)
9-11: Centralized imports for Settings/Scans_Stats — solid.Good move consolidating dependencies; aligns with the new single-source-of-truth approach for scannable post types and stats.
111-111: Section rename to “Scan Settings” — good UX.
116-121: New “Permissions” section — looks right.Callback provided and i18n in place.
144-149: New “System Settings” section — clean separation.
170-177: Enable Archive Scanning control — aligns with PRO-169.Gated with edac_is_pro() and sanitized via wrapper. Good.
179-186: Scan All Taxonomy Terms — dependency gating is correct.Disabled unless archives are enabled and Pro is active. Nice.
188-195: Ignore Permissions control — wiring + sanitizer present.
197-204: Moved “Delete Data” under System — sensible IA.
224-231: Simplified Summary Heading — proper gating + sanitize.
306-312: Pro-aware register_setting wrappers — solid back-compat.
320-336: General section copy — i18n/escaping/link handling are tight.
380-388: System section copy — clear and localized.
394-427: Scan Speed UI — a11y and defaults are handled well.aria-describedby, disabled gating, and selected() casting are correct.
436-457: Archive scanning checkbox — gating and description look good.
466-487: Taxonomy “scan all terms” checkbox — dependency + messaging are on point.
707-714: Nice: order-insensitive compare prevents cache thrash.Sorting before compare avoids unnecessary stats clears. Sharp.
816-827: Pro scan speed sanitizer — preserves value when Pro disabled.Behavior matches intent and avoids surprise resets.
829-851: Generic Pro checkbox sanitizer + pre-save action — flexible and safe.Good hook naming (edac_ prefix) and i18n-friendly path.
859-871: Archive/taxonomy wrappers — consistent with checkbox wrapper.
879-886: Ignore roles wrapper — sensible default to administrator.
919-932: Summary heading field — gating + default are correct.
939-968: Ignore roles UI — roles list, escaping, and gating are correct.
995-1001: Permissions section copy — clear guidance, localized.
78-86: Bug: remove_action is registered too late; admin_init already fired.This runs inside edac_add_options_page (typically hooked on admin_menu), so your admin_init callback won’t ever execute in the same request. Register the removal earlier (plugins_loaded/init) at file load, not inside this function.
Apply this diff to delete the late registration:
- // Deregister the pro setting registration - this is added for backwards compatibility for users - // that don't update pro. Added here in 1.31.0 and released with pro 1.16.0. - add_action( - 'admin_init', - function () { - // Remove the pro plugin's settings registration entirely. - remove_action( 'admin_init', 'edacp_register_setting', 11 ); - } - );Add this early hook near the top-level (after use statements), so removal is guaranteed before admin_init:
// Back-compat: remove Pro's settings registration if present (added in 1.31.0; Pro 1.16.0). add_action( 'init', static function () { remove_action( 'admin_init', 'edacp_register_setting', 11 ); }, 0 );⛔ Skipped due to learnings
Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. The timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it. This means remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks. This timing works correctly for backwards compatibility scenarios where you need to remove Pro plugin hooks.Learnt from: pattonwebz PR: equalizedigital/accessibility-checker#1208 File: includes/options-page.php:76-84 Timestamp: 2025-08-26T13:24:26.788Z Learning: In WordPress, the admin_menu hook fires during the admin_init process, not after it, so remove_action calls placed inside functions hooked to admin_menu will execute at the appropriate time to affect other admin_init callbacks.
This makes the pattern of sending string and returning string more clear, the id is still passed through as well to get the virtual url for it If no virtual url is found the original string is passed back, the original string is usually a permalink [PRO-169]
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin/class-enqueue-admin.php (2)
64-74: This condition enqueues on almost every admin page; switch OR to ANDUsing OR with 'site-editor.php' !== $pagenow makes the whole condition true for nearly all pages. That’s bloat city—tighten it up so we skip Site Editor while still gating by post type/page.
- ) || - 'site-editor.php' !== $pagenow + ) && + 'site-editor.php' !== $pagenow
129-145: Escape scanUrl before localizingEven with validation, always ship sanitized URLs to JS.
- 'scanUrl' => $scan_url, + 'scanUrl' => esc_url_raw( $scan_url ),
♻️ Duplicate comments (2)
admin/class-enqueue-admin.php (1)
115-126: Harden origin-URL filter: validate scheme, fallback to preview, and document the hookA poisoned filter could return javascript: or empty; don’t let bad URLs punk your editor. Add docblock, fallback, and http/https scheme check.
- $post_view_link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - get_preview_post_link( $post_id ), - $post_id - ); - - $scan_url = add_query_arg( - [ - 'edac_pageScanner' => 1, - ], - $post_view_link - ); + $default_base = get_preview_post_link( $post_id ); + /** + * Filter the base URL used when launching the page scanner from the editor. + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.31.0 + * + * @param string $default_base Default preview URL for the post. + * @param int $post_id Current post ID (can be virtual/ghost). + * @return string Filtered base URL. + */ + $post_view_link = apply_filters( 'edac_get_origin_url_for_virtual_page', $default_base, $post_id ); + $scan_base = ( is_string( $post_view_link ) && '' !== $post_view_link ) ? $post_view_link : $default_base; + $parsed = wp_parse_url( $scan_base ); + if ( empty( $parsed['scheme'] ) || ! in_array( $parsed['scheme'], [ 'http', 'https' ], true ) ) { + $scan_base = $default_base; + } + $scan_url = add_query_arg( [ 'edac_pageScanner' => 1 ], $scan_base );includes/helper-functions.php (1)
773-787: Document the filter, pass context, and add a defensive fallbackAdd a docblock and pass useful context args; also guard against non-string returns. Don’t get sloppy with untrusted filter outputs.
- $link = apply_filters( - 'edac_get_origin_url_for_virtual_page', - get_the_permalink( $post_id ), - $post_id - ); + $base_link = get_the_permalink( $post_id ); + /** + * Filter the base URL used for "View on page"/landmark links. + * + * Allows mapping virtual/ghost posts to their canonical origin URL (e.g., archives/taxonomies). + * + * @since 1.x.x + * + * @param string $base_link Default base URL (permalink for $post_id). + * @param int $post_id Post ID represented by the link (may be a virtual/ghost post). + * @param string $landmark_selector Unencoded CSS selector for the landmark being highlighted. + * @param string $landmark Human-readable landmark label. + * + * @return string Filtered base URL. + */ + $link = apply_filters( 'edac_get_origin_url_for_virtual_page', $base_link, $post_id, $landmark_selector, $landmark ); + if ( ! is_string( $link ) || '' === $link ) { + $link = $base_link; + }
🧹 Nitpick comments (4)
admin/class-enqueue-admin.php (3)
85-90: Script vars look tight; UTM switch and proUrl rename LGTMThe associative UTM args and proUrl key align with helper behavior. Keep it rolling.
Optional: prefer the native REST helper to avoid string concat:
- 'edacApiUrl' => esc_url_raw( rest_url() . 'accessibility-checker/v1' ), + 'edacApiUrl' => esc_url_raw( rest_url( 'accessibility-checker/v1' ) ),
96-99: Avoid double-fetching scannable post typesYou already computed $post_types above. Reuse it here to avoid an extra option read/filter pass.
108-110: Minor: use [] for deps for clarityFalse works, but an empty array reads cleaner per WPCS expectations.
- wp_enqueue_script( 'edac-editor-app', plugin_dir_url( EDAC_PLUGIN_FILE ) . 'build/editorApp.bundle.js', false, EDAC_VERSION, false ); + wp_enqueue_script( 'edac-editor-app', plugin_dir_url( EDAC_PLUGIN_FILE ) . 'build/editorApp.bundle.js', [], EDAC_VERSION, false );includes/helper-functions.php (1)
832-834: Pro detection is solid and aligned with our playbookYou’re checking both constants exist before reading EDAC_KEY_VALID. Good hustle.
Optional: add a return type.
-function edac_is_pro() { +function edac_is_pro(): bool {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
admin/class-ajax.php(4 hunks)admin/class-enqueue-admin.php(3 hunks)includes/helper-functions.php(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- admin/class-ajax.php
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress coding standards (WPCS) in all PHP files
Use PSR-4 autoloading with EqualizeDigital\AccessibilityChecker namespace for new classes
Use WordPress hooks and filters appropriately
Use edac_ prefix for custom hooks and filters
Minimum PHP 7.4 compatibility
Use type hints where appropriate in PHP code
Follow WordPress security best practices (sanitization, validation, nonces)
Use WordPress database API (wpdb) for database operations
Prefix all functions and classes with edac_ when in global namespace
Use WordPress hooks (actions/filters) for extensibility
Use WordPress transients for caching
Sanitize all user inputs
Validate and escape all outputs
Use WordPress nonces for form submissions
Implement proper capability checks
Provide appropriate hooks for extensibility when adding new functionality
Use descriptive hook names with edac_ prefix
Document all custom hooks in docblocks
Document custom hooks and filters with clear descriptions and parameter types
Use PHPDoc for all public classes, methods, and properties
Escape all output, especially in admin screens and user-generated content
Use WordPress error handling functions (e.g., WP_Error) for PHP errors
Files:
includes/helper-functions.phpadmin/class-enqueue-admin.php
**/*.{php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{php,js}: Follow WordPress internationalization (i18n) practices and use accessibility-checker text domain
All user-facing text must be translatable
Add inline comments for complex accessibility logic or non-obvious code
Use semantic HTML structure
ARIA attributes used correctly
Keyboard navigation supported
Images have descriptive alt text
Heading hierarchy logical
Screen reader compatibility
Forms are accessible and labeled
Files:
includes/helper-functions.phpadmin/class-enqueue-admin.php
includes/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
includes/**/*.php: Validate all AJAX requests and REST endpoints with nonces and capability checks
Avoid blocking queries in PHP, especially during scans
Files:
includes/helper-functions.php
**/class-*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/class-*.php: Legacy files are autoloaded using EDAC namespace
Legacy class names use WordPress style (Class_Name_Convention)
Legacy file names use class-class-name.php (WordPress style)
Legacy Classes: class-class-name.php
Deprecate legacy code with clear docblocks and migration notes
Files:
admin/class-enqueue-admin.php
🧠 Learnings (1)
📚 Learning: 2025-08-26T11:25:48.236Z
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#1208
File: includes/options-page.php:401-410
Timestamp: 2025-08-26T11:25:48.236Z
Learning: In the Accessibility Checker plugin, the edac_is_pro() function uses a robust detection pattern by checking both EDACP_VERSION (which comes from the pro plugin) and EDAC_KEY_VALID existence before looking at its value. This prevents PHP notices and ensures reliable pro feature detection.
Applied to files:
includes/helper-functions.php
🧬 Code graph analysis (2)
includes/helper-functions.php (1)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)
admin/class-enqueue-admin.php (2)
admin/class-settings.php (2)
Settings(13-98)get_scannable_post_types(42-71)includes/helper-functions.php (1)
edac_generate_link_type(558-607)
⏰ 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: Integration Test: PHP 8.1 | WP 6.2
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
🔇 Additional comments (5)
admin/class-enqueue-admin.php (3)
51-51: Good move: centralized scannable post typesSwapping to Settings::get_scannable_post_types() is the right abstraction. Rock solid.
178-180: Nice: safe, typed page slug helpersanitize_key + wp_unslash + typed return. Clean.
100-100: Ignore the constant swap; EDAC_KEY_VALID is correctThe
EDAC_KEY_VALIDconstant is defined inaccessibility-checker.phpand used throughout the plugin for license‐valid checks. There is noEDACP_KEY_VALIDdefinition anywhere in the codebase, so switching toEDACP_KEY_VALIDwould break Pro detection rather than fix it:
- accessibility-checker.php defines
define('EDAC_KEY_VALID', 'valid' === get_option('edacp_license_status'));- All existing checks use
EDAC_KEY_VALID(e.g.,edac_is_pro()and various admin classes) without issue- No
define('EDACP_KEY_VALID', …)is present, soEDACP_KEY_VALIDwould always be undefinedRemove the suggestion to replace
EDAC_KEY_VALIDwithEDACP_KEY_VALID—the current implementation is correct.Likely an incorrect or invalid review comment.
includes/helper-functions.php (2)
413-417: Good consolidation to Settings::get_scannable_post_types()Centralizing scannable types is the right call; reduces drift and keeps filters consistent. I pity the duplicate config.
8-8: Namespace and Settings class declaration verifiedI checked
admin/class-settings.phpand it declaresnamespace EDAC\Admin;on line 8 and definesclass Settingson line 13, so theuse EDAC\Admin\Settings;import inincludes/helper-functions.phpis safe. I pity the fool who doubts it!
pattonwebz
changed the title
[PRO-169]
Checklist
Summary by CodeRabbit
New Features
Settings
UI
Hooks
Tests
Chores