Add edac_dismiss_reasons filter to allow customizing dismiss options - #1726
Conversation
Wraps the dismiss reasons array in apply_filters() so developers can add, remove, or reorder options without modifying core code. Closes #1511 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe ignore UI dismiss reasons now pass through the ChangesFilterable dismiss reasons
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the edac_dismiss_reasons filter to allow developers to customize the dismiss reasons in IgnoreUI::get_reasons(). The review feedback recommends adding defensive type checking to ensure the filtered value is an array before returning it, which prevents potential fatal TypeErrors if a filter callback returns an invalid type.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Falls back to default reasons if a filter callback returns an invalid type, preventing a fatal TypeError from the array return type hint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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
🧹 Nitpick comments (1)
admin/class-ignore-ui.php (1)
32-48: ⚖️ Poor tradeoffHandle legacy dismiss reasons when
edac_dismiss_reasonsfilters them out
IgnoreUI::render_reason_fieldset()only checks$current_reasonagainst the filtered$reasons; if a stored key (e.g.,remediated) is removed byedac_dismiss_reasons, no radio will be selected.- The React
DismissPanelalso builds its options from the same filteredIgnoreUI::get_reasons()payload, so the modal can similarly show no selected reason/label for legacy values.admin/class-ajax.phpsaves$_REQUEST['reason']after sanitization without validating against the current reasons list, so the legacy value can persist.Consider documenting that filtered-out reasons won’t show as selected for already-dismissed issues, or add a UI fallback like “Dismissed as: ” when the stored key isn’t present in the current options.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin/class-ignore-ui.php` around lines 32 - 48, IgnoreUI::render_reason_fieldset() and the React DismissPanel rely solely on the filtered IgnoreUI::get_reasons() list so stored legacy keys removed by the edac_dismiss_reasons filter won’t appear selected; fix by adding a fallback: when $current_reason (or incoming saved reason in admin/class-ajax.php) is not in IgnoreUI::get_reasons(), append a synthetic option like "legacy:<key>" with label "Dismissed as: <key or stored label>" to the reasons payload returned by IgnoreUI::get_reasons()/used by render_reason_fieldset() and include the same fallback key/value in the data sent to the React DismissPanel, and additionally validate/sanitize in admin/class-ajax.php that $_REQUEST['reason'] is either a current key or the allowed legacy fallback before saving.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin/class-ignore-ui.php`:
- Around line 32-48: The edac_dismiss_reasons filter return must be validated
and sanitized before returning: in the method that builds this default (the
block returning apply_filters('edac_dismiss_reasons', [...])) call the filter
into a variable, verify it's an array, iterate each reason key/value ensuring
each value is an array with 'label' and 'description' keys that are strings; for
any missing/malformed entry fall back to the original default for that key, and
sanitize the strings (e.g., sanitize_text_field or wp_kses_post as appropriate)
before returning; this protects render_reason_fieldset() from non-array or
malformed entries and guarantees safe, expected structure.
---
Nitpick comments:
In `@admin/class-ignore-ui.php`:
- Around line 32-48: IgnoreUI::render_reason_fieldset() and the React
DismissPanel rely solely on the filtered IgnoreUI::get_reasons() list so stored
legacy keys removed by the edac_dismiss_reasons filter won’t appear selected;
fix by adding a fallback: when $current_reason (or incoming saved reason in
admin/class-ajax.php) is not in IgnoreUI::get_reasons(), append a synthetic
option like "legacy:<key>" with label "Dismissed as: <key or stored label>" to
the reasons payload returned by IgnoreUI::get_reasons()/used by
render_reason_fieldset() and include the same fallback key/value in the data
sent to the React DismissPanel, and additionally validate/sanitize in
admin/class-ajax.php that $_REQUEST['reason'] is either a current key or the
allowed legacy fallback before saving.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8cf2fbeb-4af0-4492-a1d5-35ea97a9558d
📒 Files selected for processing (1)
admin/class-ignore-ui.php
Strip any entries missing label/description string keys rather than returning malformed data that would cause fatal errors in render_reason_fieldset(). Falls back to defaults if the validated result is empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
✅ Accessibility Checker build (primary only)
|
|
Here is an example filter to test with. add_filter( 'edac_dismiss_reasons', function( $reasons ) {
// Add custom reasons.
$reasons['needs_review'] = [
'label' => 'Needs review',
'description' => 'Flagged for manual review by the accessibility team.',
];
$reasons['wont_fix'] = [
'label' => "Won't fix",
'description' => 'Reviewed and determined not to be addressed.',
];
// Remove an existing reason.
unset( $reasons['remediated'] );
return $reasons;
} );
|
…ns-to-be-filterable-to-add-or
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin/class-ignore-ui.php (1)
32-46: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the parse error in
admin/class-ignore-ui.php.
$default_reasons = [is immediately followed byreturn [, so the method is syntactically invalid and the plugin cannot load. Remove the strayreturn [and close the$default_reasonsarray before applying the filter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin/class-ignore-ui.php` around lines 32 - 46, The method in class-ignore-ui.php has a syntax error because $default_reasons is started and then immediately interrupted by an unintended return array, preventing the plugin from loading. Fix the structure in the relevant ignore-reasons method by removing the stray return, properly closing the $default_reasons array, and then returning the filtered/default reasons using the existing filter call and unique symbols like $default_reasons and the ignore UI method name.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@admin/class-ignore-ui.php`:
- Around line 32-46: The method in class-ignore-ui.php has a syntax error
because $default_reasons is started and then immediately interrupted by an
unintended return array, preventing the plugin from loading. Fix the structure
in the relevant ignore-reasons method by removing the stray return, properly
closing the $default_reasons array, and then returning the filtered/default
reasons using the existing filter call and unique symbols like $default_reasons
and the ignore UI method name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3024e466-4a3b-4848-84c3-3804e7129372
📒 Files selected for processing (1)
admin/class-ignore-ui.php
The merge of develop into this branch left a stray "return [" inside the $default_reasons array literal, breaking PHP syntax and failing CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in fa7b9b5 — removed the stray 🤖 Addressed by Claude Code |
2 participants
Summary
IgnoreUI::get_reasons()return value inapply_filters( 'edac_dismiss_reasons', ... )so developers can add, remove, or reorder dismiss reasons without modifying core codeUsage example
Test plan
Closes #1511
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Documentation