-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release v1.18.0 #837
Release v1.18.0 #837
Conversation
Backport: Release/1.17.0
…-on-summary-generation Add a flag to skip cache when getting scan stats
…ew_window_warning-fix-key-on-uninstall Add new_window_warning fix key to the items cleared on uninstall
…-remove-admin-notices removed: unused admin notices functionality from FixesPage and Admin Notices classes
This now allows posts that no longer exist to be flushed out of the table of issues/
…ear-rest-endpoint-able-to-clear-issues-for-post-that-no-longer-exists Make the endpoint for clearing issues less strict
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces version 1.18.0 of the Accessibility Checker plugin. The changes include updating version numbers across multiple files, removing certain methods related to admin notices and slugs, modifying the REST API error handling, and adding a new option for plugin uninstallation. The modifications appear to be focused on refining the plugin's functionality, improving error messages, and preparing for a new release. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
uninstall.php (1)
Line range hint
22-68
: Consider consolidating similar options and improving organization.The options list contains several items that appear to be variations or older versions of the same setting:
edac_fix_prevent_links_opening_in_new_windows
edac_fix_prevent_links_opening_new_windows
edac_fix_prevent-links-opening-new-windows
Consider:
- Consolidating duplicate options
- Organizing options into logical groups using arrays
- Standardizing the naming convention (choose between underscores vs hyphens)
Example refactor:
- $options = [ + $core_options = [ 'edac_db_version', 'edac_activation_date', - 'edac_simplified_summary_position', + 'edac_delete_data', + ]; + + $display_options = [ + 'edac_simplified_summary_position', 'edac_post_types', 'edac_frontend_highlighter_position', - 'edac_delete_data', + ]; + + $notice_options = [ 'edac_review_notice', - 'edac_authorization_password', - 'edac_authorization_username', 'edac_gaad_notice_dismiss', 'edac_black_friday_2023_notice_dismiss', ]; - $fix_options = [ + + $auth_options = [ + 'edac_authorization_password', + 'edac_authorization_username', + ]; + + $accessibility_statement_options = [ + 'edac_add_footer_accessibility_statement', + 'edac_accessibility_policy_page', + 'edac_include_accessibility_statement_link', + ]; + + $fix_options = [ 'edac_add_label_to_unlabeled_form_fields', - 'edac_add_label_to_unlabelled_form_fields', // Remove duplicate - 'edac_fix_add_label_to_unlabelled_form_fields', // Remove duplicate + 'edac_fix_new_window_warning', 'edac_fix_prevent_links_opening_in_new_windows', // Keep only one version - 'edac_fix_prevent_links_opening_new_windows', // Remove duplicate - 'edac_fix_prevent-links-opening-new-windows', // Remove duplicate ]; - $options_to_clear = array_merge( $options, $fix_options ); + $options_to_clear = array_merge( + $core_options, + $display_options, + $notice_options, + $auth_options, + $accessibility_statement_options, + $fix_options + );.github/workflows/phpunit.yml (1)
79-82
: LGTM! Consider adding error handling and version check.The SVN installation step is correctly positioned and uses appropriate commands. Consider these improvements:
- Add version check to skip if SVN is already installed
- Add error handling
- Add comment explaining why SVN is needed
# Some images won't have svn available. Install it if that's the case. - name: Install SVN - run: sudo apt-get update && sudo apt-get install -y subversion + run: | + if ! command -v svn &> /dev/null; then + echo "Installing SVN..." + sudo apt-get update || { echo "Failed to update package lists"; exit 1; } + sudo apt-get install -y subversion || { echo "Failed to install SVN"; exit 1; } + else + echo "SVN is already installed" + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/phpunit.yml
(1 hunks)accessibility-checker.php
(2 hunks)admin/AdminPage/FixesPage.php
(0 hunks)admin/class-admin-notices.php
(0 hunks)admin/class-scans-stats.php
(1 hunks)includes/classes/class-rest-api.php
(1 hunks)package.json
(1 hunks)readme.txt
(1 hunks)tests/phpunit/helper-functions/OrdinalTest.php
(1 hunks)uninstall.php
(1 hunks)
💤 Files with no reviewable changes (2)
- admin/class-admin-notices.php
- admin/AdminPage/FixesPage.php
✅ Files skipped from review due to trivial changes (3)
- package.json
- readme.txt
- accessibility-checker.php
🧰 Additional context used
🪛 phpstan (2.0.3)
includes/classes/class-rest-api.php
219-219: Call to method get_json_params() on an unknown class EDAC\Inc\WP_REST_Request.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(class.notFound)
237-237: Variable $post_id might not be defined.
(variable.undefined)
244-244: Variable $post_id might not be defined.
(variable.undefined)
🔇 Additional comments (6)
uninstall.php (1)
68-68
: LGTM! The new option follows the existing pattern.The addition of
edac_fix_new_window_warning
to the cleanup list is implemented correctly and aligns with the PR objective of ensuring this setting is removed during uninstallation..github/workflows/phpunit.yml (1)
79-82
: Verify SVN requirement for WordPress test installation.Let's verify if SVN is actually required for the WordPress test installation script.
✅ Verification successful
SVN is required for WordPress test installation
The test installation script uses SVN to fetch WordPress core and test files from wordpress.org repositories. Found multiple critical SVN operations at:
- Line 67:
svn export
for WordPress trunk- Lines 111-112:
svn export
for test files and data🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the WordPress test installation script requires SVN # Expected: Find SVN commands or references in the installation script # Check the WordPress test installation script for SVN usage if [ -f "tests/bin/install-wp-tests.sh" ]; then echo "Checking install-wp-tests.sh for SVN usage:" grep -n "svn" tests/bin/install-wp-tests.sh else echo "WordPress test installation script not found at expected location" fiLength of output: 656
tests/phpunit/helper-functions/OrdinalTest.php (1)
135-135
: LGTM! Test expectations updated correctly.The changes correctly update the expected ordinal values for Arabic and Greek locales by removing the trailing period, which aligns with the proper ordinal representation in these languages.
Also applies to: 143-143
admin/class-scans-stats.php (1)
112-113
: LGTM! Cache control enhancement.The addition of the
$skip_cache
parameter provides a clean way to bypass the cache when needed, while maintaining backward compatibility with the defaultfalse
value.Also applies to: 115-115, 123-123
includes/classes/class-rest-api.php (2)
216-216
: LGTM! Improved error message clarity.The error message has been updated to be more specific about which parameter is missing.
219-231
: LGTM! Enhanced post validation logic.The changes improve the validation logic by:
- Making post existence check configurable via
skip_post_exists_check
- Adding proper post type validation
- Providing clear error messages for each validation step
🧰 Tools
🪛 phpstan (2.0.3)
219-219: Call to method get_json_params() on an unknown class EDAC\Inc\WP_REST_Request.
Learn more at https://phpstan.org/user-guide/discovering-symbols(class.notFound)
William Patton (5):
Summary by CodeRabbit
Release Notes v1.18.0
Version Update
Uninstall
edac_fix_new_window_warning
during plugin removalAdmin
Testing
Miscellaneous