Review and update codebase#1
Conversation
- Replace Grunt with native npm scripts for i18n and readme tasks - Add custom Node.js script for readme.txt to README.md conversion - Fix critical security vulnerability: replace preg_replace with str_replace to prevent ReDoS - Fix escaping issues: use esc_html_e and esc_attr for proper output escaping - Fix typo: register_opion -> register_option - Add comprehensive error handling for wp_parse_url - Implement domain caching for better performance - Add PHPDoc comments following WordPress standards - Add ABSPATH check for direct access prevention - Improve input validation and isset checks - Update text domain from 'algolia-headless-mode' to 'algolia-headless' - Add helpful placeholder and description to settings field
📝 WalkthroughWalkthroughRemoved Grunt-based build; added a Node readme converter and updated package.json; replaced the plugin bootstrap with new PHP classes for headless-domain handling, URL replacement, admin settings/preview, admin notices, Site Health checks, and WP‑CLI; expanded README/readme.txt and bumped version to 0.2.0. Changes
Sequence Diagram(s)sequenceDiagram
participant Index as Algolia Index
participant Replacer as Algolia_Headless_Replacer
participant Helper as Algolia_Headless_Helper
participant Settings as Algolia_Headless_Settings
participant WPCLI as WP-CLI
Note over Index,Replacer: Permalink from Algolia record
Index->>Replacer: apply permalink replacement filter
Replacer->>Helper: get_headless_domain() (cached)
alt headless domain present
Helper-->>Replacer: return domain
Replacer->>Replacer: parse URL, replace host
Replacer-->>Index: return replaced permalink
else no domain
Helper-->>Replacer: return false
Replacer-->>Index: return original permalink
end
Note over Settings,Helper: Admin preview/save flow
Settings->>Helper: validate/save domain (option or constant)
Settings-->>Settings: enqueue preview scripts/styles for Reading page
Note over WPCLI,Helper: CLI management
WPCLI->>Helper: read/validate domain, perform test/replace, read logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @hideokamoto, 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! This pull request significantly refactors the plugin's codebase and build process to enhance security, improve code quality, and modernize development workflows. Key changes include migrating from Grunt to native npm scripts for build tasks, patching a critical ReDoS vulnerability, and implementing various WordPress coding standards and best practices for better maintainability and user experience. These updates ensure a more robust, secure, and developer-friendly plugin. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![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 is a fantastic update that significantly modernizes the plugin's codebase and development workflow. The migration from Grunt to npm scripts, the security hardening (especially the fix for the ReDoS vulnerability), improved data validation and escaping, and adherence to WordPress best practices are all excellent changes. The code is now much cleaner, more secure, and easier to maintain.
I've found a couple of minor issues in the new readme-to-markdown.js script, which include some redundant code and an unused variable. My detailed comments are below.
Overall, great work on this comprehensive refactoring!
| .replace(/^= (.+) =$/gm, '### $1') | ||
| // Bold | ||
| .replace(/\*\*(.+?)\*\*/g, '**$1**') | ||
| // Code blocks | ||
| .replace(/`([^`]+)`/g, '`$1`') | ||
| // Links (basic support) | ||
| .replace(/\[([^\]]+)\]\(([^)]+)\)/g, '[$1]($2)'); |
There was a problem hiding this comment.
The regular expression replacements for bold, code blocks, and links are ineffective as they replace a pattern with itself (e.g., **text** is replaced with **text**). This appears to be redundant code that doesn't perform any transformation and can be confusing. These lines should be removed for clarity.
.replace(/^= (.+) =$/gm, '### $1');| const pluginInfo = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i]; | ||
| if (line.startsWith('Donate link:') || | ||
| line.startsWith('Tags:') || | ||
| line.startsWith('Requires at least:') || | ||
| line.startsWith('Tested up to:') || | ||
| line.startsWith('Requires PHP:') || | ||
| line.startsWith('Stable tag:') || | ||
| line.startsWith('License:')) { | ||
| pluginInfo.push(line); | ||
| } | ||
| } |
There was a problem hiding this comment.
The pluginInfo array is populated here but it is never used later in the script. This appears to be dead code. If there was an intention to use this information, that logic should be implemented. Otherwise, this block of code should be removed to avoid confusion and unnecessary processing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/readme-to-markdown.js (2)
29-33: Remove or fix no-op transformations.Lines 29, 31, and 33 are no-op transformations that replace patterns with themselves. If WordPress readme.txt uses different syntax for bold, code, or links, update these patterns; otherwise, remove them.
Apply this diff to remove the no-ops:
content = content // Headers .replace(/^=== (.+) ===$/gm, '# $1') .replace(/^== (.+) ==$/gm, '## $1') - .replace(/^= (.+) =$/gm, '### $1') - // Bold - .replace(/\*\*(.+?)\*\*/g, '**$1**') - // Code blocks - .replace(/`([^`]+)`/g, '`$1`') - // Links (basic support) - .replace(/\[([^\]]+)\]\(([^)]+)\)/g, '[$1]($2)'); + .replace(/^= (.+) =$/gm, '### $1');
52-53: Add error handling for file write operation.The
fs.writeFileSynccall should be wrapped in a try-catch block to handle potential write failures gracefully.Apply this diff:
- fs.writeFileSync(readmeMdPath, content, 'utf8'); - console.log('✓ Successfully converted readme.txt to README.md'); + try { + fs.writeFileSync(readmeMdPath, content, 'utf8'); + console.log('✓ Successfully converted readme.txt to README.md'); + } catch (error) { + console.error('Error writing README.md:', error.message); + process.exit(1); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Gruntfile.js(0 hunks)README.md(2 hunks)algolia-headless.php(1 hunks)package.json(1 hunks)scripts/readme-to-markdown.js(1 hunks)
💤 Files with no reviewable changes (1)
- Gruntfile.js
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # Search with Algolia Headless extention Donate link: https://www.amazon.jp/hz/wi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
README.md
2-2: Bare URL used
(MD034, no-bare-urls)
9-9: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (12)
scripts/readme-to-markdown.js (2)
1-12: LGTM!The shebang, imports, and path configuration are appropriate for a Node.js conversion script.
14-18: LGTM!Good defensive check for the source file existence.
README.md (1)
22-30: LGTM!The expanded installation steps provide clear, actionable guidance for users setting up the plugin with the Algolia headless configuration.
package.json (2)
5-10: Verify wp-cli availability in the build environment.The
i18n:make-potscript depends onwp-clibeing available globally. Ensure this is documented in the project README or CI/CD configuration.
11-13: LGTM!The addition of
@wordpress/envaligns with modern WordPress plugin development practices.algolia-headless.php (7)
15-18: LGTM!The ABSPATH check is a WordPress security best practice that prevents direct file access.
20-42: LGTM!The cached domain approach improves performance by avoiding repeated database queries. The PHPDoc comments follow WordPress standards.
44-54: LGTM!The lazy-loading pattern for the headless domain is efficient and correctly implemented.
56-94: Excellent security fix: ReDoS vulnerability eliminated.The switch from
preg_replacetostr_replaceaddresses the ReDoS vulnerability mentioned in the PR objectives. The comprehensive validation usingwp_parse_urland early returns ensures robust error handling.
96-120: LGTM!Both methods correctly validate array keys before replacement and delegate to the core replacement logic.
182-200: LGTM!Excellent improvements: proper output escaping with
esc_attrandesc_html_e, HTML5 URL input type, and helpful placeholder/description enhance both security and user experience.
203-204: LGTM!Both classes are correctly instantiated to activate the plugin functionality.
| // Add WordPress plugin badge information at the top | ||
| const lines = content.split('\n'); | ||
| const pluginInfo = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i]; | ||
| if (line.startsWith('Donate link:') || | ||
| line.startsWith('Tags:') || | ||
| line.startsWith('Requires at least:') || | ||
| line.startsWith('Tested up to:') || | ||
| line.startsWith('Requires PHP:') || | ||
| line.startsWith('Stable tag:') || | ||
| line.startsWith('License:')) { | ||
| pluginInfo.push(line); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove dead code: pluginInfo is never used.
The pluginInfo array is populated but never inserted into the output or used anywhere. Either remove this code or implement the functionality described in the comment.
Apply this diff to remove the unused code:
- // Add WordPress plugin badge information at the top
- const lines = content.split('\n');
- const pluginInfo = [];
-
- for (let i = 0; i < lines.length; i++) {
- const line = lines[i];
- if (line.startsWith('Donate link:') ||
- line.startsWith('Tags:') ||
- line.startsWith('Requires at least:') ||
- line.startsWith('Tested up to:') ||
- line.startsWith('Requires PHP:') ||
- line.startsWith('Stable tag:') ||
- line.startsWith('License:')) {
- pluginInfo.push(line);
- }
- }
-
fs.writeFileSync(readmeMdPath, content, 'utf8');📝 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.
| // Add WordPress plugin badge information at the top | |
| const lines = content.split('\n'); | |
| const pluginInfo = []; | |
| for (let i = 0; i < lines.length; i++) { | |
| const line = lines[i]; | |
| if (line.startsWith('Donate link:') || | |
| line.startsWith('Tags:') || | |
| line.startsWith('Requires at least:') || | |
| line.startsWith('Tested up to:') || | |
| line.startsWith('Requires PHP:') || | |
| line.startsWith('Stable tag:') || | |
| line.startsWith('License:')) { | |
| pluginInfo.push(line); | |
| } | |
| } | |
| fs.writeFileSync(readmeMdPath, content, 'utf8'); |
🤖 Prompt for AI Agents
In scripts/readme-to-markdown.js around lines 35 to 50, the pluginInfo array and
the for-loop that populates it are dead code (pluginInfo is never used); remove
the declaration const pluginInfo = []; the for-loop that collects lines into
pluginInfo, and the preceding comment about adding badge information at the top,
or alternatively implement the missing behavior by inserting pluginInfo into the
output where appropriate—preferably remove these unused variables and loop to
clean up the file unless you intend to actually use pluginInfo, in which case
add code to incorporate its contents into the generated markdown.
New Features: - URL Preview: Real-time URL replacement preview in settings page - WP-CLI Commands: Full CLI support (status, set-domain, test, logs, clear-logs) - Environment Variables: Support ALGOLIA_HEADLESS_DOMAIN constant in wp-config.php - Debug Logging: Secure database-stored logs (requires WP_DEBUG) - Admin Notices: Smart warnings for missing configuration - Site Health: Integration with WordPress Site Health dashboard Improvements: - Helper class for shared functionality - Domain caching for better performance - Comprehensive error logging - Settings page with live preview JavaScript - Better UX with inline previews and helpful messages Documentation: - Updated readme.txt with all new features - Added FAQ section - WP-CLI command documentation - Environment variable configuration guide - Version bump to 0.2.0 Technical: - All logs stored securely in database (transients) - No web-accessible log files - Proper WordPress coding standards - Complete PHPDoc coverage - Translatable strings
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)
algolia-headless.php (1)
3-3: Fix spelling: "extention" → "extension".The plugin name contains a typo.
Apply this diff:
- * Plugin Name: Search with Algolia Headless extention + * Plugin Name: Search with Algolia Headless extension
♻️ Duplicate comments (2)
README.md (1)
1-1: Fix spelling: "extention" → "extension".The header contains a typo.
Apply this diff:
-# Search with Algolia Headless extention +# Search with Algolia Headless extensionalgolia-headless.php (1)
342-342: Remove redundant register_option() call.The option is already registered via the
initaction hook in the constructor (line 225). This call causes duplicate registration.Apply this diff:
public function init_options() { - $this->register_option(); add_settings_section(
🧹 Nitpick comments (1)
includes/class-wp-cli.php (1)
199-199: Consider removing unused$argsparameter.The
$argsparameter is not used in the method body. WP-CLI command methods that only use associative arguments don't require the positional$argsparameter.Apply this diff:
- public function logs( $args, $assoc_args ) { + public function logs( $assoc_args ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)algolia-headless.php(1 hunks)includes/class-wp-cli.php(1 hunks)package.json(1 hunks)readme.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
includes/class-wp-cli.php (1)
algolia-headless.php (6)
Algolia_Headless_Helper(25-97)is_algolia_plugin_active(48-51)get_headless_domain(34-41)get_headless_domain(130-135)test_url_replacement(93-96)log(61-85)
algolia-headless.php (1)
includes/class-wp-cli.php (1)
logs(199-251)
🪛 LanguageTool
readme.txt
[style] ~67-~67: Consider using “inaccessible” to avoid wordiness.
Context: ...gs are stored securely in the database (not accessible via web). = Can I use WP-CLI to manage...
(NOT_ABLE_PREMIUM)
[style] ~75-~75: Consider using “inaccessible” to avoid wordiness.
Context: ...ase) when WP_DEBUG is enabled. They are not accessible from the web and expire after 7 days. ...
(NOT_ABLE_PREMIUM)
README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # Search with Algolia Headless extention Donate link: https://www.amazon.jp/hz/wi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~67-~67: Consider using “inaccessible” to avoid wordiness.
Context: ...gs are stored securely in the database (not accessible via web). ### Can I use WP-CLI to mana...
(NOT_ABLE_PREMIUM)
[style] ~75-~75: Consider using “inaccessible” to avoid wordiness.
Context: ...ase) when WP_DEBUG is enabled. They are not accessible from the web and expire after 7 days. ...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
README.md
2-2: Bare URL used
(MD034, no-bare-urls)
9-9: Bare URL used
(MD034, no-bare-urls)
47-47: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 PHPMD (2.15.0)
includes/class-wp-cli.php
199-199: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
- Fix typo: "extention" → "extension" in plugin name and readme - Remove duplicate register_option() call in init_options() - Remove unused $args parameter from WP-CLI logs() method - Regenerate README.md with spelling fix
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@algolia-headless.php`:
- Around line 470-487: The dismissible logic in show_notice() is incomplete
because nothing persists the dismissal; either remove the $dismissible parameter
and the get_user_meta() check from show_notice(), or implement persistence by
adding a JS handler that listens for the notice dismiss event and POSTs an AJAX
request (including a nonce and the notice id) to admin-ajax.php, plus a PHP AJAX
action handler (hooked via add_action('wp_ajax_...')) that verifies the nonce
and calls update_user_meta( get_current_user_id(),
"algolia_headless_dismissed_{$id}", true ) to save the dismissal; also ensure
the AJAX action name and nonce are enqueued/localized to the front-end so the JS
can call them.
- Add AJAX handler for notice dismissal (wp_ajax_algolia_headless_dismiss_notice) - Add JavaScript to handle dismiss button clicks and POST to admin-ajax.php - Include nonce verification for security - Persist dismissal state using update_user_meta() - Add enqueue_dismiss_script() method to load JavaScript - Fix incomplete dismissible logic as noted in code review
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@algolia-headless.php`:
- Around line 417-435: The click handler in get_dismiss_script binds to the
entire notice element which causes accidental dismissals; change the event
selector to target the dismiss button (use the '.notice-dismiss' inside the
notice) and when handling the click read the notice id from the closest
'.notice.is-dismissible[data-notice-id]' element (e.g.,
$(this).closest(...).data('notice-id')) before firing the AJAX post for action
'algolia_headless_dismiss_notice' with the existing nonce; update
get_dismiss_script to use delegated clicks on the dismiss button rather than the
whole notice element.
- Around line 646-675: The HTTPS check sets $result['status']='recommended' but
its description is then overwritten by the later domain-description logic;
modify the logic around $result and the domain description so the HTTPS warning
is preserved—e.g. when 'http' is detected, set $result['description'] to the
HTTPS warning and then, in the block that handles defined(
'ALGOLIA_HEADLESS_DOMAIN' ) / else, append the domain information to
$result['description'] (or only set the domain description when
$result['description'] is empty) instead of replacing it; reference $parsed,
$result, ALGOLIA_HEADLESS_DOMAIN and $headless_domain to locate the affected
code.
- Around line 172-182: The current use of str_replace on $url with
$replace_target risks replacing host strings in query/path; instead rebuild the
URL using $parsed_url parts and substitute only the host (and port if present)
with $replaced_domain: use $parsed_url['scheme'], ['user'], ['pass'], set
['host'] = $replaced_domain, preserve ['port'], ['path'], ['query'],
['fragment'] and then concatenate those parts to produce $result (referencing
$parsed_url, $replace_target, $replaced_domain, $url, $result); this ensures
only the authority/host portion is changed and avoids accidental replacements in
query strings or paths.
🧹 Nitpick comments (2)
algolia-headless.php (2)
61-85: Consider rate-limiting database writes in the log method.When
WP_DEBUGis enabled, every log call performs bothget_transient()andset_transient()database operations. In high-volume scenarios (e.g., bulk re-indexing), this could create unnecessary database load.Consider batching writes or using a static buffer that flushes on shutdown.
💡 Optional: Buffer logs and write on shutdown
+ /** + * Log buffer for batch writing + * + * `@var` array + */ + private static $log_buffer = array(); + public static function log( $message, $level = 'info' ) { if ( ! defined( 'WP_DEBUG' ) || ! WP_DEBUG ) { return; } // Log to PHP error log. error_log( sprintf( '[Algolia Headless][%s] %s', strtoupper( $level ), $message ) ); - // Store in database (transient, expires in 7 days). - $logs = get_transient( 'algolia_headless_debug_logs' ); - if ( ! is_array( $logs ) ) { - $logs = array(); - } - - $logs[] = array( + // Buffer log entry. + self::$log_buffer[] = array( 'timestamp' => current_time( 'mysql' ), 'level' => $level, 'message' => $message, ); + + // Register shutdown handler once. + static $registered = false; + if ( ! $registered ) { + add_action( 'shutdown', array( __CLASS__, 'flush_logs' ) ); + $registered = true; + } + } + + /** + * Flush buffered logs to database + */ + public static function flush_logs() { + if ( empty( self::$log_buffer ) ) { + return; + } + + $logs = get_transient( 'algolia_headless_debug_logs' ); + if ( ! is_array( $logs ) ) { + $logs = array(); + } - // Keep only last 100 logs. - $logs = array_slice( $logs, -100 ); + $logs = array_merge( $logs, self::$log_buffer ); + $logs = array_slice( $logs, -100 ); set_transient( 'algolia_headless_debug_logs', $logs, 7 * DAY_IN_SECONDS ); + self::$log_buffer = array(); }
688-692: Consider adding file existence check before require_once.If
includes/class-wp-cli.phpis missing or renamed, this will cause a fatal error. While this is unlikely in a controlled deployment, a defensive check improves robustness:💡 Optional: Add file existence check
// Register WP-CLI commands. if ( defined( 'WP_CLI' ) && WP_CLI ) { - require_once dirname( __FILE__ ) . '/includes/class-wp-cli.php'; - WP_CLI::add_command( 'algolia-headless', 'Algolia_Headless_CLI' ); + $cli_file = dirname( __FILE__ ) . '/includes/class-wp-cli.php'; + if ( file_exists( $cli_file ) ) { + require_once $cli_file; + WP_CLI::add_command( 'algolia-headless', 'Algolia_Headless_CLI' ); + } }
| $replace_target = $parsed_url['host']; | ||
| if ( isset( $parsed_url['port'] ) && $parsed_url['port'] ) { | ||
| $replace_target .= ':' . $parsed_url['port']; | ||
| } | ||
|
|
||
| // Use str_replace instead of preg_replace to avoid ReDoS vulnerability. | ||
| $result = str_replace( $replace_target, $replaced_domain, $url ); | ||
|
|
||
| Algolia_Headless_Helper::log( "URL replaced: {$url} -> {$result}", 'info' ); | ||
|
|
||
| return $result; |
There was a problem hiding this comment.
Potential over-replacement in URL string.
Using str_replace() on the full URL will replace all occurrences of the host, including any that appear in query parameters or path segments.
For example, https://wp.example.com/page?source=wp.example.com would incorrectly become https://headless.com/page?source=headless.com.
Consider a more targeted replacement that only affects the host portion:
🛠️ Proposed fix for targeted host replacement
- // Use str_replace instead of preg_replace to avoid ReDoS vulnerability.
- $result = str_replace( $replace_target, $replaced_domain, $url );
+ // Build the new URL by reconstructing with replaced host.
+ $scheme = isset( $parsed_url['scheme'] ) ? $parsed_url['scheme'] . '://' : '';
+ $new_host = $replaced_domain;
+ $path = isset( $parsed_url['path'] ) ? $parsed_url['path'] : '';
+ $query = isset( $parsed_url['query'] ) ? '?' . $parsed_url['query'] : '';
+ $fragment = isset( $parsed_url['fragment'] ) ? '#' . $parsed_url['fragment'] : '';
+
+ $result = $scheme . $new_host . $path . $query . $fragment;🤖 Prompt for AI Agents
In `@algolia-headless.php` around lines 172 - 182, The current use of str_replace
on $url with $replace_target risks replacing host strings in query/path; instead
rebuild the URL using $parsed_url parts and substitute only the host (and port
if present) with $replaced_domain: use $parsed_url['scheme'], ['user'],
['pass'], set ['host'] = $replaced_domain, preserve ['port'], ['path'],
['query'], ['fragment'] and then concatenate those parts to produce $result
(referencing $parsed_url, $replace_target, $replaced_domain, $url, $result);
this ensures only the authority/host portion is changed and avoids accidental
replacements in query strings or paths.
| private function get_dismiss_script() { | ||
| $ajax_url = admin_url( 'admin-ajax.php' ); | ||
| $nonce = wp_create_nonce( 'algolia_headless_dismiss_notice' ); | ||
|
|
||
| return " | ||
| jQuery(document).ready(function($) { | ||
| $(document).on('click', '.notice.is-dismissible[data-notice-id]', function() { | ||
| var noticeId = $(this).data('notice-id'); | ||
| if (!noticeId) return; | ||
|
|
||
| $.post('" . esc_js( $ajax_url ) . "', { | ||
| action: 'algolia_headless_dismiss_notice', | ||
| notice_id: noticeId, | ||
| nonce: '" . esc_js( $nonce ) . "' | ||
| }); | ||
| }); | ||
| }); | ||
| "; | ||
| } |
There was a problem hiding this comment.
Click event selector is too broad.
The click handler binds to the entire notice element (.notice.is-dismissible[data-notice-id]), meaning clicking anywhere on the notice—including the message text—will trigger the dismiss AJAX call. Users might accidentally dismiss notices when trying to interact with links inside them.
WordPress's dismissible notices have a .notice-dismiss button. Bind to that instead:
🛠️ Proposed fix to target the dismiss button
return "
jQuery(document).ready(function($) {
- $(document).on('click', '.notice.is-dismissible[data-notice-id]', function() {
- var noticeId = $(this).data('notice-id');
+ $(document).on('click', '.notice[data-notice-id] .notice-dismiss', function() {
+ var noticeId = $(this).closest('.notice').data('notice-id');
if (!noticeId) return;
$.post('" . esc_js( $ajax_url ) . "', {📝 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.
| private function get_dismiss_script() { | |
| $ajax_url = admin_url( 'admin-ajax.php' ); | |
| $nonce = wp_create_nonce( 'algolia_headless_dismiss_notice' ); | |
| return " | |
| jQuery(document).ready(function($) { | |
| $(document).on('click', '.notice.is-dismissible[data-notice-id]', function() { | |
| var noticeId = $(this).data('notice-id'); | |
| if (!noticeId) return; | |
| $.post('" . esc_js( $ajax_url ) . "', { | |
| action: 'algolia_headless_dismiss_notice', | |
| notice_id: noticeId, | |
| nonce: '" . esc_js( $nonce ) . "' | |
| }); | |
| }); | |
| }); | |
| "; | |
| } | |
| private function get_dismiss_script() { | |
| $ajax_url = admin_url( 'admin-ajax.php' ); | |
| $nonce = wp_create_nonce( 'algolia_headless_dismiss_notice' ); | |
| return " | |
| jQuery(document).ready(function($) { | |
| $(document).on('click', '.notice[data-notice-id] .notice-dismiss', function() { | |
| var noticeId = $(this).closest('.notice').data('notice-id'); | |
| if (!noticeId) return; | |
| $.post('" . esc_js( $ajax_url ) . "', { | |
| action: 'algolia_headless_dismiss_notice', | |
| notice_id: noticeId, | |
| nonce: '" . esc_js( $nonce ) . "' | |
| }); | |
| }); | |
| }); | |
| "; | |
| } |
🤖 Prompt for AI Agents
In `@algolia-headless.php` around lines 417 - 435, The click handler in
get_dismiss_script binds to the entire notice element which causes accidental
dismissals; change the event selector to target the dismiss button (use the
'.notice-dismiss' inside the notice) and when handling the click read the notice
id from the closest '.notice.is-dismissible[data-notice-id]' element (e.g.,
$(this).closest(...).data('notice-id')) before firing the AJAX post for action
'algolia_headless_dismiss_notice' with the existing nonce; update
get_dismiss_script to use delegated clicks on the dismiss button rather than the
whole notice element.
| // Check HTTPS. | ||
| if ( isset( $parsed['scheme'] ) && 'http' === $parsed['scheme'] ) { | ||
| $result['status'] = 'recommended'; | ||
| $result['label'] = __( 'Headless domain uses HTTP instead of HTTPS', 'algolia-headless' ); | ||
| $result['description'] = sprintf( | ||
| '<p>%s</p>', | ||
| __( 'For better security, consider using HTTPS for your headless domain.', 'algolia-headless' ) | ||
| ); | ||
| } | ||
|
|
||
| // Check if using constant. | ||
| if ( defined( 'ALGOLIA_HEADLESS_DOMAIN' ) && ALGOLIA_HEADLESS_DOMAIN ) { | ||
| $result['description'] = sprintf( | ||
| '<p>%s</p>', | ||
| sprintf( | ||
| /* translators: %s: Domain configured via constant */ | ||
| __( 'Domain is configured via ALGOLIA_HEADLESS_DOMAIN constant: <code>%s</code>', 'algolia-headless' ), | ||
| esc_html( ALGOLIA_HEADLESS_DOMAIN ) | ||
| ) | ||
| ); | ||
| } else { | ||
| $result['description'] = sprintf( | ||
| '<p>%s</p>', | ||
| sprintf( | ||
| /* translators: %s: Configured domain */ | ||
| __( 'Headless domain: <code>%s</code>', 'algolia-headless' ), | ||
| esc_html( $headless_domain ) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
HTTPS warning description gets overwritten.
When the headless domain uses HTTP (lines 646-654), the status is set to 'recommended' but the description is not set. The code then continues to lines 656-675, which overwrites any potential description with domain information, losing the context of why the status is 'recommended'.
The HTTPS warning should include a description, or the final description blocks should append rather than replace:
🛠️ Proposed fix to preserve HTTPS warning message
// Check HTTPS.
if ( isset( $parsed['scheme'] ) && 'http' === $parsed['scheme'] ) {
$result['status'] = 'recommended';
$result['label'] = __( 'Headless domain uses HTTP instead of HTTPS', 'algolia-headless' );
$result['description'] = sprintf(
'<p>%s</p>',
__( 'For better security, consider using HTTPS for your headless domain.', 'algolia-headless' )
);
+ return $result;
}Alternatively, concatenate the domain info to the existing description rather than replacing it.
📝 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.
| // Check HTTPS. | |
| if ( isset( $parsed['scheme'] ) && 'http' === $parsed['scheme'] ) { | |
| $result['status'] = 'recommended'; | |
| $result['label'] = __( 'Headless domain uses HTTP instead of HTTPS', 'algolia-headless' ); | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| __( 'For better security, consider using HTTPS for your headless domain.', 'algolia-headless' ) | |
| ); | |
| } | |
| // Check if using constant. | |
| if ( defined( 'ALGOLIA_HEADLESS_DOMAIN' ) && ALGOLIA_HEADLESS_DOMAIN ) { | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| sprintf( | |
| /* translators: %s: Domain configured via constant */ | |
| __( 'Domain is configured via ALGOLIA_HEADLESS_DOMAIN constant: <code>%s</code>', 'algolia-headless' ), | |
| esc_html( ALGOLIA_HEADLESS_DOMAIN ) | |
| ) | |
| ); | |
| } else { | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| sprintf( | |
| /* translators: %s: Configured domain */ | |
| __( 'Headless domain: <code>%s</code>', 'algolia-headless' ), | |
| esc_html( $headless_domain ) | |
| ) | |
| ); | |
| } | |
| // Check HTTPS. | |
| if ( isset( $parsed['scheme'] ) && 'http' === $parsed['scheme'] ) { | |
| $result['status'] = 'recommended'; | |
| $result['label'] = __( 'Headless domain uses HTTP instead of HTTPS', 'algolia-headless' ); | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| __( 'For better security, consider using HTTPS for your headless domain.', 'algolia-headless' ) | |
| ); | |
| return $result; | |
| } | |
| // Check if using constant. | |
| if ( defined( 'ALGOLIA_HEADLESS_DOMAIN' ) && ALGOLIA_HEADLESS_DOMAIN ) { | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| sprintf( | |
| /* translators: %s: Domain configured via constant */ | |
| __( 'Domain is configured via ALGOLIA_HEADLESS_DOMAIN constant: <code>%s</code>', 'algolia-headless' ), | |
| esc_html( ALGOLIA_HEADLESS_DOMAIN ) | |
| ) | |
| ); | |
| } else { | |
| $result['description'] = sprintf( | |
| '<p>%s</p>', | |
| sprintf( | |
| /* translators: %s: Configured domain */ | |
| __( 'Headless domain: <code>%s</code>', 'algolia-headless' ), | |
| esc_html( $headless_domain ) | |
| ) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@algolia-headless.php` around lines 646 - 675, The HTTPS check sets
$result['status']='recommended' but its description is then overwritten by the
later domain-description logic; modify the logic around $result and the domain
description so the HTTPS warning is preserved—e.g. when 'http' is detected, set
$result['description'] to the HTTPS warning and then, in the block that handles
defined( 'ALGOLIA_HEADLESS_DOMAIN' ) / else, append the domain information to
$result['description'] (or only set the domain description when
$result['description'] is empty) instead of replacing it; reference $parsed,
$result, ALGOLIA_HEADLESS_DOMAIN and $headless_domain to locate the affected
code.
2 participants
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.