Fix SQL logic bug and Admin UI hydration - #1
google-labs-jules[bot] wants to merge 3 commits into
Conversation
- In `includes/frontend.php`, changed the SQL query for mapping values from `LIKE` to `=` to prevent partial matches (e.g. Post ID 1 matching Post ID 11). - In `includes/admin.php`, added hydration logic to fetch post titles and term names for the `fqj_assoc_data` JSON payload, ensuring Select2 inputs are correctly pre-filled on page load.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
- Fix PSR-12 coding standard violations across all files (line length, spacing, indentation). - Rename `FQJ_CLI` class to `FQJ\FqjCli` and methods to camelCase to comply with PSR-1. - Fix SQL logic bug in `includes/frontend.php` (changed `LIKE` to `=`) to prevent incorrect mapping matches. - Fix Admin UI bug in `includes/admin.php` by hydrating Select2 data to display titles instead of IDs. - Add `squizlabs/php_codesniffer` to composer dev dependencies.
…l bugs
- Rename `includes/wpcli.php` to `includes/class-fqjcli.php` and use `snake_case` methods.
- Update `composer.json` to include `wp-coding-standards/wpcs` and allow installer plugin.
- Fix thousands of formatting issues (spacing, tabs, Yoda conditions, arrays) using `phpcbf`.
- Manually fix logic bugs:
- Hydrate Admin UI Select2 fields with titles in `includes/admin.php`.
- Fix SQL logic in `includes/frontend.php` (changed `LIKE` to `=`) and implement secure prepared statements.
- Add proper escaping (`esc_html`, `esc_attr`, `intval`) and sanitization (`sanitize_text_field`, `wp_unslash`) throughout.
- Add missing docblocks and `@package` tags.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e3c05b31a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'post_type' => 'any', | ||
| 'posts_per_page' => -1, |
There was a problem hiding this comment.
When an associated post is later moved to draft, private, or trash, get_posts() omits it because its default post_status is publish. The hydrated select therefore no longer contains that ID, and the next ordinary save rewrites fqj_assoc_data_json and the mapping index without the association. Query the relevant statuses, subject to the editor's permissions, or preserve IDs that cannot be hydrated.
Useful? React with 👍 / 👎.
| // Load stored JSON payload for this FAQ. | ||
| $data_json = get_post_meta( $post->ID, 'fqj_assoc_data_json', true ); | ||
| $data_json = $data_json ? $data_json : '{}'; | ||
| $data = json_decode( $data_json, true ); |
There was a problem hiding this comment.
If existing association metadata contains malformed JSON or the valid JSON value null, json_decode() returns null, which is then encoded into the hidden field as null. The admin script successfully parses that value but immediately dereferences properties such as data.urls, causing the association editor to fail instead of using its previous malformed-data fallback. Normalize a non-array decode result to an empty array before hydration.
Useful? React with 👍 / 👎.
| // Use exact match for all types to prevent partial matches (e.g. post ID 1 matching 10, 11, etc.) | ||
| // Since we store scalar values directly (or serialized which resolves to string), = is safer. | ||
| // For 'url', if we wanted partial matches we'd need a different logic, but here we expect exact path match. | ||
| $where_clauses[] = '(mapping_type = %s AND mapping_value = %s)'; |
There was a problem hiding this comment.
For URL-only mappings where url_to_postid() cannot add the redundant post-ID mapping, exact equality now makes common saved URLs ending in / fail to match because the lookup candidate is explicitly stripped of its trailing slash earlier in this function. Query-string mappings have the same mismatch because the candidate also drops its query. Normalize URLs consistently when indexing and looking them up before switching this predicate to =.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
There are concrete regressions/bugs in the changed admin markup and exact-match URL/term lookup logic that can break existing associations and UI state.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR fixes the FAQ-to-post lookup accuracy by switching mapping table comparisons from partial LIKE matches to exact = matches, and addresses Select2 “empty on reload” behavior by hydrating stored IDs into {id,text} objects server-side before initializing the admin UI.
Changes:
- Changed frontend mapping table lookup to use exact comparisons to prevent ID substring collisions (e.g.,
1matching10/11/100). - Hydrated stored “By Posts” / “By Terms” association data with display labels so Select2 can render selections after reload.
- Refactored/standardized multiple files toward WordPress Coding Standards and moved WP-CLI commands into a namespaced class file.
File summaries
| File | Description |
|---|---|
| includes/wpcli.php | Removed legacy WP-CLI command definitions file. |
| includes/class-fqjcli.php | Added namespaced WP-CLI command class and command registration. |
| includes/frontend.php | Updated mapping lookup logic to use exact matches and refreshed code style. |
| includes/admin.php | Added server-side hydration of Select2 initial data; updated meta save handler sanitization/style. |
| includes/indexer.php | Style/sanitization updates around mapping/index rebuild and invalidation enqueueing. |
| includes/queue.php | Style/doc updates for invalidation queue functions and cron worker. |
| includes/health.php | Style/sanitization updates for Health page and AJAX handlers. |
| includes/settings.php | Improved sanitization and WPCS-style formatting for settings + tools UI. |
| faq-jsonld.php | Updated includes and reformatted plugin header/bootstrapping. |
| composer.json | Enabled PHPCS installer plugin and added WPCS as a dev dependency. |
| composer.lock | Updated lockfile to include newly added dev dependencies. |
Review details
Suppressed comments (3)
includes/admin.php:142
selected()returns an attribute fragment likeselected='selected'. Escaping it withesc_attr()HTML-encodes the quotes, producing invalid markup and preventing the correct option from being selected.
foreach ( $assoc_types as $k => $label ) {
$sel = selected( $assoc_type, $k, false );
echo "<option value='" . esc_attr( $k ) . "' " . esc_attr( $sel ) . '>' . esc_html( $label ) . '</option>';
}
includes/admin.php:192
- With the frontend lookup now using exact
=comparisons for URL mappings, storing URLs without normalization will cause regressions (e.g., trailing slash or query string differences will no longer match). Normalize URLs on save to the same shape used by the frontend (strip query string, trim trailing slash).
foreach ( $lines as $l ) {
$l = trim( $l );
if ( empty( $l ) ) {
continue;
}
$urls[] = esc_url_raw( $l );
}
$payload['urls'] = array_values( array_unique( $urls ) );
includes/frontend.php:84
- After switching URL mappings to exact
=comparisons, the lookup will no longer match mappings saved with a trailing slash (previously they matched viaLIKE). Adding both trailing-slash variants keeps exact matching while remaining backward compatible with existing stored URL values.
$permalink = get_permalink( $current_id );
if ( $permalink ) {
$candidate_values[] = array(
'type' => 'url',
'value' => rtrim( strtok( $permalink, '?' ), '/' ),
);
}
- Files reviewed: 10/11 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $data_json = get_post_meta( $post->ID, 'fqj_assoc_data_json', true ); | ||
| $data_json = $data_json ? $data_json : '{}'; | ||
| $data = json_decode( $data_json, true ); | ||
|
|
| // Terms. | ||
| $terms = wp_get_post_terms( $current_id ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $t ) { | ||
| $candidate_values[] = array( | ||
| 'type' => 'term', | ||
| 'value' => (string) $t->term_id, | ||
| ); | ||
| } | ||
| } |
This PR addresses two main issues:
LIKEcomparisons for IDs. This meant that mapping an FAQ to Post ID 1 would also cause it to appear on Post IDs 11, 10, 100, etc. This has been fixed to use exact=comparison.PR created automatically by Jules for task 9901016997777217283 started by @renderbit-technologies