Fix: Support multiple shortcode instances on the same page and add filtering capabilities - #2
Conversation
…multiple to be on the same page Possible breaking changes The following IDs no longer exist in the rendered HTML and any CSS or JavaScript targeting them will stop working: - #edmm-meeting-minutes-table - #edmm-pagination - #pagination-info These have been replaced with classes of the same names so CSS can be migrated easily: - .edmm-meeting-minutes-table-wrapper - .edmm-pagination - .edmm-pagination-info
|
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:
WalkthroughAdds a new per-meeting taxonomy Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Shortcode UI
participant Server as REST Endpoint
participant WP as WordPress DB
User->>Browser: Click category filter (UID-scoped)
Browser->>Server: GET /wp-json/.../meeting-minutes?category_filter=slug&tags=...
Server->>WP: Build WP_Query (tax_query for edmm_meeting_category + tags)
WP-->>Server: Query results with category data attached
Server-->>Browser: JSON response including categories per meeting
Browser->>Browser: Render table/pagination using UID-scoped IDs
Browser-->>User: Updated meeting list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Summary of ChangesHello, 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 addresses a critical issue where the [edmm_meeting_minutes] shortcode failed to render correctly when used multiple times on the same page. By introducing a unique identifier for each shortcode instance and updating the associated JavaScript to scope its DOM manipulations to these unique IDs, the shortcode now functions independently and reliably across all instances on a page. This significantly improves the flexibility and usability of the shortcode for content creators. Highlights
Changelog
Activity
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
The pull request modifies the meeting_minutes_shortcode function to support multiple instances on a single page by generating unique IDs for the HTML elements (table, pagination, and info divs) and updating the corresponding JavaScript to reference these dynamic IDs, thereby preventing ID collisions. The reviewer suggests an improvement to replace an inline style used for visually hiding an element with the standard WordPress .screen-reader-text utility class for better maintainability and consistency.
| <div id="pagination-info" aria-live="polite" aria-atomic="true" style="position: absolute; left: -9999px;"></div> | ||
| <div id="<?php echo esc_attr( $uid ); ?>-table" class="edmm-meeting-minutes-table-wrapper"></div> | ||
| <div id="<?php echo esc_attr( $uid ); ?>-pagination" class="edmm-pagination"></div> | ||
| <div id="<?php echo esc_attr( $uid ); ?>-info" class="edmm-pagination-info" aria-live="polite" aria-atomic="true" style="position: absolute; left: -9999px;"></div> |
There was a problem hiding this comment.
For better maintainability and adherence to WordPress standards, it's recommended to use a utility class for visually hiding elements instead of an inline style. WordPress core provides the .screen-reader-text class for this purpose, which you're already using elsewhere. Using this class centralizes the styling logic and makes the code cleaner.
<div id="<?php echo esc_attr( $uid ); ?>-info" class="edmm-pagination-info screen-reader-text" aria-live="polite" aria-atomic="true"></div>There was a problem hiding this comment.
Pull request overview
Updates the [edmm_meeting_minutes] shortcode output so multiple instances can coexist on the same page without clobbering each other’s DOM, by generating per-instance unique element IDs and scoping DOM lookups accordingly.
Changes:
- Generate a unique
$uidper shortcode render and use it to suffix container element IDs. - Update the inline JavaScript to target instance-specific IDs for table rendering, pagination rendering, aria-live updates, and focus management.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pattonwebz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
equalize-digital-meeting-minutes.php (1)
420-420:⚠️ Potential issue | 🟡 MinorMissing space before
aria-currentattribute produces invalid HTML.The string concatenation creates
aria-label="Page 1"aria-current="true"without a space between attributes, which is invalid HTML and may break screen reader interpretation.🐛 Proposed fix
- pagination += '<button type="button" class="edmm-pagination-button' + (slot === data.current_page ? ' current' : '') + '" data-page="' + slot + '" aria-label="Page ' + slot + '"'+ (slot === data.current_page ? 'aria-current="true"' : '') +'>' + slot + '</button>'; + pagination += '<button type="button" class="edmm-pagination-button' + (slot === data.current_page ? ' current' : '') + '" data-page="' + slot + '" aria-label="Page ' + slot + '"' + (slot === data.current_page ? ' aria-current="true"' : '') + '>' + slot + '</button>';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` at line 420, The pagination button string concatenation misses a space before the aria-current attribute, producing invalid HTML; update the concatenation that builds the pagination string (the line that appends to the pagination variable using slot and data.current_page) so the ternary that outputs aria-current includes a leading space when true (e.g., change the ternary to return ' aria-current="true"' instead of 'aria-current="true"'), ensuring no extra space is added when false.
🧹 Nitpick comments (3)
equalize-digital-meeting-minutes.php (3)
210-230: Consider documenting the distinction betweentagsandtags_filterparameters.Both parameters filter by the same taxonomy with an AND relation, which could confuse maintainers. From the shortcode usage:
tags_filteris the permanent shortcode-level filter whiletagsis the dynamic user-selected filter. A brief inline comment would clarify this intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 210 - 230, Add a brief inline comment above the $tax_query handling explaining the difference between the two parameters: $tags_filter is the permanent shortcode-level filter and $tags is the dynamic user-selected filter, both targeting the 'edmm_meeting_tag' taxonomy and combined with an AND relation; place this comment close to where $tags_filter and $tags are checked (the blocks that append to $tax_query and the final count check that sets $args['tax_query']) so future maintainers understand the intent.
379-388: Consider escaping tag data when building HTML strings.While taxonomy term names come from WordPress admin input (trusted), building HTML via string concatenation without escaping is a fragile pattern. If term names ever contain special characters like
<or", the markup could break.♻️ Defensive escaping approach
Consider using a simple escape helper or the DOM API:
// Helper at the top of the script const escapeHtml = (str) => str.replace(/[&<>"']/g, c => ({'&':'&','<':'<','>':'>','"':'"',"'":'''})[c]); // Then in the loop: tagButtons += '<button type="button" class="edmm-tag-filter' + (isActive ? ' active' : '') + '" data-slug="' + escapeHtml(tag.slug) + '" aria-pressed="' + isActive + '">' + escapeHtml(tag.name) + '</button>';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 379 - 388, The current string-concatenation that builds tagButtons using meeting.tags (inside the block that checks '<?php echo esc_js( $atts['hide_tags'] ); ?>') is not escaping tag.slug or tag.name and can break markup if they contain special characters; add a small escape helper (e.g., escapeHtml) near the top of the script and use it when inserting tag.slug and tag.name into the button string (keep using activeTagFilter and the existing isActive logic), or build the button via DOM APIs to avoid direct string injection.
333-608: Consider extracting inline scripts/styles to enqueued assets for larger deployments.The current inline approach works correctly and maintains instance isolation via closures. However, if this plugin scales to pages with many shortcode instances, the duplicated CSS and common JavaScript functions (like
calculatePaginationSlots) could be extracted to an enqueued file with instance-specific data passed viawp_localize_script(). This is a future optimization rather than a current issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 333 - 608, The inline JS and CSS (including functions like calculatePaginationSlots, renderTable, fetchMeetings and constants maxSlots, postsPerPage, apiUrl, uid, and the CSS for .edmm-meeting-minutes-table/.edmm-pagination-button/.edmm-tag-filter) should be extracted into enqueued assets; move common JS to a single bundled script and common CSS to a stylesheet, then pass instance-specific values (uid, apiUrl, postsPerPage, activeTagFilter, any att flags) into that script via wp_localize_script or wp_add_inline_script so the bundled calculatePaginationSlots and event handlers operate per-instance using the localized uid/selectors and avoid duplicating styles and functions across multiple shortcode instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@equalize-digital-meeting-minutes.php`:
- Line 420: The pagination button string concatenation misses a space before the
aria-current attribute, producing invalid HTML; update the concatenation that
builds the pagination string (the line that appends to the pagination variable
using slot and data.current_page) so the ternary that outputs aria-current
includes a leading space when true (e.g., change the ternary to return '
aria-current="true"' instead of 'aria-current="true"'), ensuring no extra space
is added when false.
---
Nitpick comments:
In `@equalize-digital-meeting-minutes.php`:
- Around line 210-230: Add a brief inline comment above the $tax_query handling
explaining the difference between the two parameters: $tags_filter is the
permanent shortcode-level filter and $tags is the dynamic user-selected filter,
both targeting the 'edmm_meeting_tag' taxonomy and combined with an AND
relation; place this comment close to where $tags_filter and $tags are checked
(the blocks that append to $tax_query and the final count check that sets
$args['tax_query']) so future maintainers understand the intent.
- Around line 379-388: The current string-concatenation that builds tagButtons
using meeting.tags (inside the block that checks '<?php echo esc_js(
$atts['hide_tags'] ); ?>') is not escaping tag.slug or tag.name and can break
markup if they contain special characters; add a small escape helper (e.g.,
escapeHtml) near the top of the script and use it when inserting tag.slug and
tag.name into the button string (keep using activeTagFilter and the existing
isActive logic), or build the button via DOM APIs to avoid direct string
injection.
- Around line 333-608: The inline JS and CSS (including functions like
calculatePaginationSlots, renderTable, fetchMeetings and constants maxSlots,
postsPerPage, apiUrl, uid, and the CSS for
.edmm-meeting-minutes-table/.edmm-pagination-button/.edmm-tag-filter) should be
extracted into enqueued assets; move common JS to a single bundled script and
common CSS to a stylesheet, then pass instance-specific values (uid, apiUrl,
postsPerPage, activeTagFilter, any att flags) into that script via
wp_localize_script or wp_add_inline_script so the bundled
calculatePaginationSlots and event handlers operate per-instance using the
localized uid/selectors and avoid duplicating styles and functions across
multiple shortcode instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2e79c88-ae09-4b21-a524-40eedab843e6
📒 Files selected for processing (2)
README.mdequalize-digital-meeting-minutes.php
✅ Files skipped from review due to trivial changes (1)
- README.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@equalize-digital-meeting-minutes.php`:
- Around line 262-267: The category values (term->name and term->slug) stored
into $categories_data are later injected into the DOM via innerHTML and must be
escaped to prevent stored XSS; update the code that populates or outputs
$categories_data so the values are safely encoded (e.g., escape when building
the PHP array with esc_html/esc_attr or better, output data via wp_json_encode
and in the JS assign into the DOM using textContent/createTextNode or properly
escape before setting innerHTML). Ensure you update the handling around
$category_terms / $categories_data and any JS that uses category.name or
category.slug to use the safe-encoding approach rather than concatenating raw
values into innerHTML.
- Around line 336-337: activeCategoryFilter is initialized from
esc_js($atts['tags']) while later interactive fetches append a 'tags' query
param; unify both to use 'category_filter' on the client side. Change the
initialization of activeCategoryFilter to read the server-provided
category_filter value (falling back to tags only on server-side PHP if needed)
and update the interactive fetch code that currently appends 'tags' to instead
append 'category_filter', leaving any server-side handling of 'tags' as a
backward-compatible alias.
- Around line 309-311: The default user-facing string assigned to
'categories_label' is not localized; update the value for 'categories_label' to
use a translation function with the plugin text domain (e.g., __('Categories',
'your-plugin-text-domain') or esc_html__('Categories',
'your-plugin-text-domain')) so the default label can be translated; locate the
array entry for 'categories_label' and replace the raw "Categories" string
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13570afb-d23b-452f-8248-ea83c4616035
📒 Files selected for processing (1)
equalize-digital-meeting-minutes.php
| $category_terms = get_the_terms( get_the_ID(), 'edmm_meeting_category' ); | ||
| $categories_data = array(); | ||
| if ( $category_terms && ! is_wp_error( $category_terms ) ) { | ||
| foreach ( $category_terms as $term ) { | ||
| $categories_data[] = array( 'name' => $term->name, 'slug' => $term->slug, 'parent' => $term->parent ); | ||
| } |
There was a problem hiding this comment.
Escape category values before injecting into innerHTML.
On Line 384 and Line 387, category.slug/category.name are concatenated directly into HTML. These values should be escaped before insertion to avoid stored-XSS risk via taxonomy data.
🔒 Suggested hardening
const renderTable = (data, refocus = false) => {
refocus = refocus || false;
+ const escapeHtml = (value) =>
+ String(value).replace(/[&<>"']/g, (ch) => ({
+ '&': '&',
+ '<': '<',
+ '>': '>',
+ '"': '"',
+ "'": ''',
+ }[ch]));
let tableClass = '<?php echo esc_js( $atts['class'] ); ?>';
@@
if (meeting.categories && meeting.categories.length) {
meeting.categories.forEach(category => {
- const isActive = activeCategoryFilter === category.slug;
- categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + category.slug + '" aria-pressed="' + isActive + '">' + category.name + '</button>';
+ const rawSlug = String(category.slug || '');
+ const isActive = activeCategoryFilter === rawSlug;
+ categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + escapeHtml(rawSlug) + '" aria-pressed="' + isActive + '">' + escapeHtml(category.name || '') + '</button>';
});
}Also applies to: 379-388
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@equalize-digital-meeting-minutes.php` around lines 262 - 267, The category
values (term->name and term->slug) stored into $categories_data are later
injected into the DOM via innerHTML and must be escaped to prevent stored XSS;
update the code that populates or outputs $categories_data so the values are
safely encoded (e.g., escape when building the PHP array with esc_html/esc_attr
or better, output data via wp_json_encode and in the JS assign into the DOM
using textContent/createTextNode or properly escape before setting innerHTML).
Ensure you update the handling around $category_terms / $categories_data and any
JS that uses category.name or category.slug to use the safe-encoding approach
rather than concatenating raw values into innerHTML.
| 'hide_categories' => 'true', | ||
| 'categories_label' => 'Categories', // only used if hide_categories is false. | ||
| 'category_filter' => '', // outputs items only with this category slug. |
There was a problem hiding this comment.
Localize the new default categories label.
Line 310 introduces a user-facing default string ('Categories') without translation wrapping. Use the plugin text domain so this default can be localized.
🌐 Suggested change
- 'categories_label' => 'Categories', // only used if hide_categories is false.
+ 'categories_label' => __( 'Categories', 'edmm' ), // only used if hide_categories is false.📝 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.
| 'hide_categories' => 'true', | |
| 'categories_label' => 'Categories', // only used if hide_categories is false. | |
| 'category_filter' => '', // outputs items only with this category slug. | |
| 'hide_categories' => 'true', | |
| 'categories_label' => __( 'Categories', 'edmm' ), // only used if hide_categories is false. | |
| 'category_filter' => '', // outputs items only with this category slug. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@equalize-digital-meeting-minutes.php` around lines 309 - 311, The default
user-facing string assigned to 'categories_label' is not localized; update the
value for 'categories_label' to use a translation function with the plugin text
domain (e.g., __('Categories', 'your-plugin-text-domain') or
esc_html__('Categories', 'your-plugin-text-domain')) so the default label can be
translated; locate the array entry for 'categories_label' and replace the raw
"Categories" string accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
equalize-digital-meeting-minutes.php (3)
169-173:⚠️ Potential issue | 🟠 Major
tagsalias handling is inconsistent and can break expected filtering.Line 304 still exposes
tags, but the shortcode bootstrap now only seedscategory_filter(Line 336) and the REST layer can combine both viaAND(Line 210-230). This can break legacy shortcode usage and produce overly restrictive results when both are present.🔧 Suggested alignment (single effective filter, `tags` as fallback alias)
function get_meeting_minutes( $request ) { @@ - $tags = isset($params['tags']) ? sanitize_text_field($params['tags']) : ''; - $category_filter = isset($params['category_filter']) ? sanitize_text_field($params['category_filter']) : ''; + $category_filter = isset($params['category_filter']) ? sanitize_text_field($params['category_filter']) : ''; + if ( '' === $category_filter && isset( $params['tags'] ) ) { + $category_filter = sanitize_text_field( $params['tags'] ); + } @@ - if ( ! empty( $tags ) ) { - $tax_query[] = array( - 'taxonomy' => 'edmm_meeting_category', - 'field' => 'slug', - 'terms' => array_map( 'trim', explode( ',', $tags ) ), - ); - } - if ( count( $tax_query ) > 1 ) { $args['tax_query'] = $tax_query; } } @@ function meeting_minutes_shortcode( $atts ) { $atts = shortcode_atts( array( @@ ), $atts, 'edmm_meeting_minutes' ); + $initial_category_filter = ! empty( $atts['category_filter'] ) ? $atts['category_filter'] : $atts['tags']; $api_url = rest_url( 'edmm/v1/meeting-minutes/' ) . '?included_years=' . urlencode( $atts['included_years'] ) @@ - . ( ! empty( $atts['category_filter'] ) ? '&category_filter=' . urlencode( $atts['category_filter'] ) : '' ); + . ( ! empty( $initial_category_filter ) ? '&category_filter=' . urlencode( $initial_category_filter ) : '' ); @@ - let activeCategoryFilter = '<?php echo esc_js( $atts['category_filter'] ); ?>'; + let activeCategoryFilter = '<?php echo esc_js( $initial_category_filter ); ?>';Also applies to: 210-230, 304-304, 318-323, 336-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 169 - 173, The handling of the tags parameter is inconsistent and can make filtering overly restrictive; change the bootstrap so tags is treated strictly as a fallback alias for category_filter: after sanitizing both $tags and $category_filter in the shortcode/bootstrap (variables $tags and $category_filter), if $category_filter is empty and $tags is not, assign $category_filter = $tags and thereafter only use $category_filter for filtering and REST requests; remove or stop exposing $tags downstream (e.g., where results are seeded or passed to REST) so the REST layer and shortcode always use a single effective filter (category_filter) and legacy shortcodes still work.
310-310:⚠️ Potential issue | 🟡 MinorLocalize the default categories label.
Line 310 introduces a user-facing default string without i18n wrapping.
🌐 Suggested fix
- 'categories_label' => 'Categories', // only used if hide_categories is false. + 'categories_label' => __( 'Categories', 'edmm' ), // only used if hide_categories is false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` at line 310, The default user-facing string assigned to the 'categories_label' setting is not internationalized; replace the raw 'Categories' literal with a localization call (e.g. __('Categories', 'equalize-digital-meeting-minutes') or esc_html__('Categories', 'equalize-digital-meeting-minutes')) so the label is translatable—update the value for the 'categories_label' key where default settings are defined to use the chosen i18n function and the plugin text domain.
262-267:⚠️ Potential issue | 🟠 MajorEscape category values before building HTML strings.
Line 266 passes raw term data through the API, and Line 384 injects it into
innerHTMLwithout escaping. This keeps a stored-XSS surface in the category filter UI.🔒 Suggested hardening
const renderTable = (data, refocus = false) => { refocus = refocus || false; + const escapeHtml = (value) => + String(value).replace(/[&<>"']/g, (ch) => ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + }[ch])); @@ if (meeting.categories && meeting.categories.length) { meeting.categories.forEach(category => { - const isActive = activeCategoryFilter === category.slug; - categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + category.slug + '" aria-pressed="' + isActive + '">' + category.name + '</button>'; + const rawSlug = String(category.slug || ''); + const isActive = activeCategoryFilter === rawSlug; + categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + escapeHtml(rawSlug) + '" aria-pressed="' + isActive + '">' + escapeHtml(category.name || '') + '</button>'; }); }Also applies to: 379-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 262 - 267, The category term values are added raw into $categories_data (variable built from $category_terms) and later injected into the page via innerHTML (around the code that renders the category filter), creating a stored-XSS risk; sanitize all term fields before output by escaping when building the HTML/JS: use esc_html() or esc_attr() for name/slug/parent when embedding into HTML attributes or content, or use wp_json_encode()/esc_js() when passing the array into inline JS, and ensure the code that uses $categories_data for innerHTML uses the escaped/encoded values instead of raw $term->name/$term->slug.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@equalize-digital-meeting-minutes.php`:
- Around line 547-561: The category filter styles (.edmm-category-filter and
.edmm-category-filter.active) use `#007bff` and white which fail WCAG AA contrast
for normal-size text; replace those colors with combinations that meet 4.5:1
contrast (for example use a darker blue like `#0056b3` for borders/text and white
for active text, or keep `#007bff` but change text to `#111827` for non-active) and
ensure border-color, color and background-color in both .edmm-category-filter
and .edmm-category-filter.active are updated consistently so non-active text and
active text/background meet >=4.5:1 contrast against their backgrounds.
---
Duplicate comments:
In `@equalize-digital-meeting-minutes.php`:
- Around line 169-173: The handling of the tags parameter is inconsistent and
can make filtering overly restrictive; change the bootstrap so tags is treated
strictly as a fallback alias for category_filter: after sanitizing both $tags
and $category_filter in the shortcode/bootstrap (variables $tags and
$category_filter), if $category_filter is empty and $tags is not, assign
$category_filter = $tags and thereafter only use $category_filter for filtering
and REST requests; remove or stop exposing $tags downstream (e.g., where results
are seeded or passed to REST) so the REST layer and shortcode always use a
single effective filter (category_filter) and legacy shortcodes still work.
- Line 310: The default user-facing string assigned to the 'categories_label'
setting is not internationalized; replace the raw 'Categories' literal with a
localization call (e.g. __('Categories', 'equalize-digital-meeting-minutes') or
esc_html__('Categories', 'equalize-digital-meeting-minutes')) so the label is
translatable—update the value for the 'categories_label' key where default
settings are defined to use the chosen i18n function and the plugin text domain.
- Around line 262-267: The category term values are added raw into
$categories_data (variable built from $category_terms) and later injected into
the page via innerHTML (around the code that renders the category filter),
creating a stored-XSS risk; sanitize all term fields before output by escaping
when building the HTML/JS: use esc_html() or esc_attr() for name/slug/parent
when embedding into HTML attributes or content, or use wp_json_encode()/esc_js()
when passing the array into inline JS, and ensure the code that uses
$categories_data for innerHTML uses the escaped/encoded values instead of raw
$term->name/$term->slug.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4311d0fa-fd7b-4993-815a-658b6b208e8c
📒 Files selected for processing (1)
equalize-digital-meeting-minutes.php
There was a problem hiding this comment.
♻️ Duplicate comments (3)
equalize-digital-meeting-minutes.php (3)
310-310:⚠️ Potential issue | 🟡 MinorLocalize the default
categories_labelstring.Line 310 uses a raw user-facing string, so it cannot be translated by default.
🌐 Suggested fix
- 'categories_label' => 'Categories', // only used if hide_categories is false. + 'categories_label' => __( 'Categories', 'edmm' ), // only used if hide_categories is false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` at line 310, Replace the raw user-facing string for the array key 'categories_label' with a translatable call (e.g. __('Categories', 'your-textdomain') or _x(...) as appropriate) so the default label can be localized; update the value for 'categories_label' where it is defined to use the translation function and use the plugin/theme text domain used elsewhere in the codebase.
262-267:⚠️ Potential issue | 🟠 MajorEscape category values before concatenating HTML for
innerHTML.Line 266 and Line 384 still pass raw taxonomy data into HTML strings. A crafted term name/slug can inject markup/script when rendered.
🔒 Suggested hardening
const renderTable = (data, refocus = false) => { refocus = refocus || false; + const escapeHtml = (value) => + String(value).replace(/[&<>"']/g, (ch) => ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + }[ch])); @@ if ('<?php echo esc_js( $atts['hide_categories'] ); ?>' !== 'true') { let categoryButtons = ''; if (meeting.categories && meeting.categories.length) { meeting.categories.forEach(category => { - const isActive = activeCategoryFilter === category.slug; - categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + category.slug + '" aria-pressed="' + isActive + '">' + category.name + '</button>'; + const rawSlug = String(category.slug || ''); + const rawName = String(category.name || ''); + const isActive = activeCategoryFilter === rawSlug; + categoryButtons += '<button type="button" class="edmm-category-filter' + (isActive ? ' active' : '') + '" data-slug="' + escapeHtml(rawSlug) + '" aria-pressed="' + isActive + '">' + escapeHtml(rawName) + '</button>'; }); } table += '<td data-label="<?php echo esc_js( $atts['categories_label'] ); ?>">' + categoryButtons + '</td>'; }Also applies to: 379-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 262 - 267, The code currently pushes raw term values from $category_terms into $categories_data and later concatenates them into innerHTML (term->name, term->slug), allowing XSS; sanitize/encode these values before output: either sanitize when building $categories_data (e.g., use esc_html( $term->name ) / esc_attr( $term->slug ) or use wp_json_encode() / esc_js() when passing data into JS) and ensure any string concatenation assigned to innerHTML uses those escaped values; update the code paths that build $categories_data and where innerHTML is set (references: $category_terms, $categories_data, $term->name, $term->slug, innerHTML) so all taxonomy values are escaped for the correct context.
171-173:⚠️ Potential issue | 🟠 Major
tagsshortcode compatibility is broken and alias handling can over-filter.
tagsis still a declared shortcode attribute (Line 304), but the client only initializes/sendscategory_filter(Line 322, Line 336). Also, server logic adds both params intotax_querywithAND(Line 220-Line 230), which can unintentionally narrow results when both are present. Treat them as aliases and resolve one effective filter.🧭 Suggested alignment
function get_meeting_minutes( $request ) { @@ - $tags = isset($params['tags']) ? sanitize_text_field($params['tags']) : ''; - $category_filter = isset($params['category_filter']) ? sanitize_text_field($params['category_filter']) : ''; + $tags = isset($params['tags']) ? sanitize_text_field($params['tags']) : ''; + $category_filter = isset($params['category_filter']) ? sanitize_text_field($params['category_filter']) : ''; + $effective_filter = $category_filter ?: $tags; @@ - if ( ! empty( $category_filter ) ) { - $tax_query[] = array( - 'taxonomy' => 'edmm_meeting_category', - 'field' => 'slug', - 'terms' => array_map( 'trim', explode( ',', $category_filter ) ), - ); - } - - if ( ! empty( $tags ) ) { + if ( ! empty( $effective_filter ) ) { $tax_query[] = array( 'taxonomy' => 'edmm_meeting_category', 'field' => 'slug', - 'terms' => array_map( 'trim', explode( ',', $tags ) ), + 'terms' => array_map( 'trim', explode( ',', $effective_filter ) ), ); } @@ function meeting_minutes_shortcode( $atts ) { @@ + $initial_filter = ! empty( $atts['category_filter'] ) ? $atts['category_filter'] : $atts['tags']; + $api_url = rest_url( 'edmm/v1/meeting-minutes/' ) . '?included_years=' . urlencode( $atts['included_years'] ) @@ - . ( ! empty( $atts['category_filter'] ) ? '&category_filter=' . urlencode( $atts['category_filter'] ) : '' ); + . ( ! empty( $initial_filter ) ? '&category_filter=' . urlencode( $initial_filter ) : '' ); @@ - let activeCategoryFilter = '<?php echo esc_js( $atts['category_filter'] ); ?>'; + let activeCategoryFilter = '<?php echo esc_js( $initial_filter ); ?>';Also applies to: 220-230, 304-305, 318-323, 336-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@equalize-digital-meeting-minutes.php` around lines 171 - 173, The shortcode/server mismatch and over-filtering is caused by treating both $tags and $category_filter as separate tax_query clauses; change the code that reads shortcode attributes ($tags and $category_filter via sanitize_text_field($params['tags'] / $params['category_filter'])) to resolve them as aliases into a single effective filter variable (e.g., $effective_category_filter = $category_filter ?: $tags), ensure the client sends that single field (so the client-side should initialize/send category_filter when tags is used), and update the tax_query construction (where tax_query entries are built) to add only one taxonomy clause using the resolved $effective_category_filter instead of adding both entries with 'relation' => 'AND' which over-narrows results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@equalize-digital-meeting-minutes.php`:
- Line 310: Replace the raw user-facing string for the array key
'categories_label' with a translatable call (e.g. __('Categories',
'your-textdomain') or _x(...) as appropriate) so the default label can be
localized; update the value for 'categories_label' where it is defined to use
the translation function and use the plugin/theme text domain used elsewhere in
the codebase.
- Around line 262-267: The code currently pushes raw term values from
$category_terms into $categories_data and later concatenates them into innerHTML
(term->name, term->slug), allowing XSS; sanitize/encode these values before
output: either sanitize when building $categories_data (e.g., use esc_html(
$term->name ) / esc_attr( $term->slug ) or use wp_json_encode() / esc_js() when
passing data into JS) and ensure any string concatenation assigned to innerHTML
uses those escaped values; update the code paths that build $categories_data and
where innerHTML is set (references: $category_terms, $categories_data,
$term->name, $term->slug, innerHTML) so all taxonomy values are escaped for the
correct context.
- Around line 171-173: The shortcode/server mismatch and over-filtering is
caused by treating both $tags and $category_filter as separate tax_query
clauses; change the code that reads shortcode attributes ($tags and
$category_filter via sanitize_text_field($params['tags'] /
$params['category_filter'])) to resolve them as aliases into a single effective
filter variable (e.g., $effective_category_filter = $category_filter ?: $tags),
ensure the client sends that single field (so the client-side should
initialize/send category_filter when tags is used), and update the tax_query
construction (where tax_query entries are built) to add only one taxonomy clause
using the resolved $effective_category_filter instead of adding both entries
with 'relation' => 'AND' which over-narrows results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 382c4805-bc10-4077-b952-b85b89987b67
📒 Files selected for processing (1)
equalize-digital-meeting-minutes.php
|
@pattonwebz, does this need reviewed or is it waiting for something else? |
|
@SteveJonesDev we talked about this in the weekly meeting and it needs to be tweaked to prevent loss of focus when the filtering runs. We decided it was ok to make that change next week and Amber updated the due date on the basecamp ticket. |
|
This work was done for a client and is not compatible with the rewrite - but the rewrite does already solve for this exact same set of problems. |
Adds tags to the meeting minutes cpt, adds some tag filtering capabilities to the shortcode as well as having optional output of said tags (also with clickable filters).
Additionally it was made to be able to have multiple on a page at once. Details about that breaking change is below:
Previously, the [edmm_meeting_minutes] shortcode used hard-coded element IDs (edmm-meeting-minutes-table, edmm-pagination, pagination-info), meaning only the first instance on a page would render correctly. Additional instances would silently update the first instance's DOM instead of their own.
This change generates a unique ID per shortcode instance (e.g., edmm-instance-1-table) and scopes all JavaScript DOM queries to that instance.
Possible breaking changes
The following IDs no longer exist in the rendered HTML and any CSS or JavaScript targeting them will stop working:
These have been replaced with classes of the same names so CSS can be migrated easily:
Anywhere that was styling this by the ids will need to be updated for this.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation