Upgrade 2.0.0#286
Open
krugazul wants to merge 23 commits into
Open
Conversation
Removes the old Gulp-based build pipeline and introduces @wordpress/scripts as the build tool, enabling modern Gutenberg block development. Adds webpack.config.js targeting the new currency-switcher block source files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes UIX framework, Customizer, and legacy option-key dependencies. Settings are now read from a single flat `lsx_currencies_settings` option. All inputs are sanitized with sanitize_key() / sanitize_text_field(); API URL is built with esc_url_raw(). Migration helpers removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion Replaces the old UIX-framework and Customizer-based admin with a clean settings page registered as a submenu under Tour Operator (falls back to Settings when TO is not active). All inputs sanitized and protected by a dedicated nonce. Settings saved to `lsx_currencies_settings` option. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Registers lsx-currencies/currency-switcher as a standalone server-rendered block. Appearance controlled via inspector controls (flags, symbol position, layout, collapsed/expanded). Frontend currency switching uses vanilla JS (no jQuery) — reads/writes a cookie, converts prices via money.js, and updates .amount.lsx-currencies spans. Includes block.json (apiVersion 3), index.js editor component, inspector.js controls, style.scss, view.js, and render.php. class-block.php handles PHP registration and JS param injection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n file class-frontend.php: removes jQuery menu injection (replaced by block), switches to wp_safe_remote_get(), dynamic data-price allowlist, fully escaped output. class-woocommerce.php: fixes cart_subtotal dead code, escapes data attribute injection. template-tags.php: removes [lsx_currency_value] shortcode; rewrites lsx_currencies_get_price_html() with sanitize_key() and proper escaping. lsx-currencies.php: bumps version to 2.0.0, drops deprecated class require, sets Requires at least: 7.0 / Requires PHP: 8.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deletes the old UIX-era shortcode JS (lsx-currencies.js, lsx-currencies-admin.js), the deprecated PHP class wrapper (class-lsx-currencies.php), and the old settings partial templates (settings/general.php, display.php, api.php). All replaced by the new block architecture and Tour Operator settings integration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds compiled block output to build/blocks/currency-switcher/ (index.js, view.js, style-index.css, block.json, render.php, asset manifests). Updates changelog.md with a full 2.0.0 entry documenting all added, changed, removed, and security improvements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ngs page Replaces the standalone Currencies submenu page with proper integration into the Tour Operator settings page, using the same hook pattern as class-settings.php: - `lsx_to_settings_fields` filter adds our fields to the save loop so they are persisted in lsx_to_settings alongside TO's own settings. - `lsx_to_framework_dashboard_tab_content` (priority 20) renders additional- currency checkboxes in the 'currency' section and the OpenExchangeRates API key text field in the 'api' section. - Additional-currencies multi-checkbox uses a hidden input + inline JS to POST a comma-separated string that sanitize_text_field() handles cleanly. - class-currencies.php set_defaults() now reads from lsx_to_settings: base currency from existing 'currency' field; our fields from lsx_currencies_* keys. Fixes LSX_BASE_CURRENCY constant to use global namespace prefix (\LSX_BASE_CURRENCY). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vigation-submenu output - block.json: add "parent": ["core/navigation"], category "design", full usesContext array (openSubmenusOnClick, showSubmenuIcon, colour, fontSize), simplified attributes (displayFlags, flagPosition, showSymbol) - render.php: rewrite to output core/navigation-submenu HTML structure — <li.wp-block-navigation-item.has-child> with current currency as top-level <a>/<button> and non-selected currencies in <ul.wp-block-navigation__submenu-container>; respects openSubmenusOnClick and showSubmenuIcon context; embeds data-flag-relations for JS DOM updates - index.js: editor preview mirrors navigation-submenu markup with static USD/EUR/GBP preview; remove layout/showCurrentOnly attributes - inspector.js: remove layout and showCurrentOnly controls; keep displayFlags, flagPosition, showSymbol - view.js: rewrite click handlers for navigation-submenu structure; dynamically swaps top-level label and rebuilds submenu on currency change (no page reload) - style.scss: strip standalone block styles; keep flag-icon sizing and editor-note only (navigation block owns all structural CSS) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…actly render.php now produces HTML identical to core/navigation-submenu: - <li> carries data-wp-interactive="core/navigation" so the navigation store owns hover/click/focus open-close behaviour without extra JS - data-wp-context JSON sets submenuOpenedBy/type/modal/previousFocus - data-wp-on--pointerenter/leave, focusout, keydown, data-wp-watch, tabindex="-1" — all matching the example output verbatim - Toggle <button> gets data-wp-bind--aria-expanded="state.isMenuOpen" and data-wp-on--click="actions.toggleMenuOnClick" - <ul> gets data-wp-on--focus="actions.openMenuOnFocus" plus overlay colour classes from nav context (overlayTextColor / overlayBackgroundColor) - SVG caret has no role="presentation" (matches core output) - open-on-hover-click class only when hover AND showSubmenuIcon (open_on_hover_and_click logic inlined from core) - submenuVisibility enum handled with backward-compat for deprecated openSubmenusOnClick boolean, matching core migration logic Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- block.json: remove displayFlags and flagPosition attributes; remove style reference (no stylesheet) - inspector.js: remove Show Flags and Flag Position controls - index.js: remove CurrencyLabel flag props and PREVIEW flag fields; remove style.scss import - render.php: remove $display_flags, $flag_position, $flag_relations variables; remove flag span from $render_label; remove data-display-flags, data-flag-position, data-flag-relations from wrapper attributes - view.js: remove buildLabelHTML flag params; remove dataset.displayFlags, flagPosition, flagRelations reads from updateSwitcher - class-currencies.php: remove $flag_relations property, get_flag_relations() method, get_currency_flag() method, and set_defaults() assignment - style.scss: deleted (no block-level CSS needed) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eload render.php embeds data-show-symbol on the <li> wrapper so view.js knows whether the block has symbols enabled. buildLabelHTML now accepts a showSymbol param and reads params.symbols[code] to append the <span class="lsx-currency-symbol"> when switching currencies, matching the server-rendered output exactly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sanitize_key() lowercases the stored currency code (e.g. 'zar') but
money.js rates keys are uppercase ('ZAR'), causing fx().from('zar') to
throw 'fx error' on every conversion. Added strtoupper() wrapping all
base_currency assignments in set_defaults() so the code is consistently
uppercase from PHP through to the JS params object.
Also add console.log instrumentation to view.js to aid debugging.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request replaces the legacy currency switcher with a server-rendered block, adds a new frontend switcher runtime, and rewires the plugin bootstrap, admin settings, and build configuration for the new flow. ChangesCurrency Switcher Block Rewrite
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the LSX Currencies plugin to version 2.0.0, introducing a new block-based Currency Switcher integrated with the WordPress Interactivity API, migrating settings storage to the Tour Operator options, and rewriting the frontend switching logic in vanilla JavaScript. Feedback on the changes highlights two critical bugs in classes/class-frontend.php: first, the API response decoding lacks validation for the rates property, which could lead to PHP warnings and caching of null values; second, retrieving the additional_prices meta with $single = false returns a nested array, causing the loop to skip additional prices.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+84
to
+96
| $body = wp_remote_retrieve_body( $response ); | ||
| $decoded = json_decode( $body ); | ||
|
|
||
| if ( ! empty( $decoded->error ) ) { | ||
| $this->rates_message = sanitize_text_field( $decoded->description ); | ||
| } elseif ( empty( $body ) ) { | ||
| $this->rates_message = esc_html__( 'Error: API response is empty.', 'lsx-currencies' ); | ||
| } else { | ||
| $this->rates = $decoded->rates; | ||
| $this->rates_message = esc_html__( 'Success (new request).', 'lsx-currencies' ); | ||
| set_transient( 'lsx_currencies_rates', $this->rates, 12 * HOUR_IN_SECONDS ); | ||
| do_action( 'lsx_currencies_rates_refreshed' ); | ||
| } |
There was a problem hiding this comment.
If the API response is not valid JSON or is missing the rates property, $decoded will be null or won't have the rates property. Accessing $decoded->rates directly will trigger a PHP warning/error, and set_transient will cache null for 12 hours, causing subsequent requests to fail silently without retrying.
We should validate that $decoded is an object and contains the rates property before using it.
$body = wp_remote_retrieve_body( $response );
$decoded = json_decode( $body );
if ( ! empty( $decoded->error ) ) {
$this->rates_message = sanitize_text_field( $decoded->description );
} elseif ( empty( $body ) ) {
$this->rates_message = esc_html__( 'Error: API response is empty.', 'lsx-currencies' );
} elseif ( is_object( $decoded ) && isset( $decoded->rates ) ) {
$this->rates = $decoded->rates;
$this->rates_message = esc_html__( 'Success (new request).', 'lsx-currencies' );
set_transient( 'lsx_currencies_rates', $this->rates, 12 * HOUR_IN_SECONDS );
do_action( 'lsx_currencies_rates_refreshed' );
} else {
$this->rates_message = esc_html__( 'Error: Invalid API response format.', 'lsx-currencies' );
}
Comment on lines
+173
to
+175
| $additional_prices = get_post_meta( get_the_ID(), 'additional_prices', false ); | ||
|
|
||
| if ( ! empty( lsx_currencies()->display_flags ) && 'right' === lsx_currencies()->flag_position ) { | ||
| $sub_items .= lsx_currencies()->get_currency_flag( $key ); | ||
| if ( lsx_currencies()->multi_prices && ! empty( $additional_prices ) ) { |
There was a problem hiding this comment.
Since additional_prices is registered as a repeatable group field, it is stored in the database as a single serialized array. Retrieving it with $single = false (the third argument of get_post_meta) returns a nested array like [ [ ... ] ].
When iterating over $additional_prices, $a_price will be the inner array of prices, meaning $a_price['currency'] and $a_price['amount'] will be undefined, causing the loop to skip all additional prices.
Retrieving it with $single = true will return the array of prices directly, allowing the loop to function correctly.
$additional_prices = get_post_meta( get_the_ID(), 'additional_prices', true );
if ( lsx_currencies()->multi_prices && is_array( $additional_prices ) && ! empty( $additional_prices ) ) {![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
classes/class-frontend.php (1)
125-132: 💤 Low valueUnused
$labelvariable in foreach loop.The
$labelvariable is extracted but never used. You can simplify this:- foreach ( $additional_currencies as $code => $label ) { + foreach ( array_keys( $additional_currencies ) as $code ) { $code = strtoupper( sanitize_key( $code ) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/class-frontend.php` around lines 125 - 132, The foreach loop in classes/class-frontend.php extracts $label but never uses it; update the loop to iterate keys only (so $label is not created) — for example, iterate over array_keys($additional_currencies) or use a blank/underscore placeholder for the value — and keep the existing logic that sanitizes $code and assigns $symbols[$code] = $all_symbols[$code] when present; change references to $additional_currencies, $code, $label, $all_symbols, and $symbols accordingly.classes/class-admin.php (1)
157-163: 💤 Low valueConsider using WordPress's
checked()helper for consistency.The current approach is safe since
$checkedis a hardcoded string, but using the built-inchecked()function would be more idiomatic WordPress and slightly cleaner:- $checked = in_array( $code, $saved_codes, true ) ? 'checked="checked"' : ''; ... - <input type="checkbox" - class="lsx-currency-check" - data-code="<?php echo esc_attr( strtoupper( $code ) ); ?>" - <?php echo $checked; ?>> + <input type="checkbox" + class="lsx-currency-check" + data-code="<?php echo esc_attr( strtoupper( $code ) ); ?>" + <?php checked( in_array( $code, $saved_codes, true ) ); ?>>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/class-admin.php` around lines 157 - 163, Replace the manual $checked string logic with WordPress's checked() helper: instead of setting $checked = in_array( $code, $saved_codes, true ) ? 'checked="checked"' : '' and echoing $checked inside the input, call checked() inline using the same in_array() condition (preserving strict true) so the checkbox attribute is rendered idiomatically; keep the existing data-code attribute (esc_attr( strtoupper( $code ) )) and the input class 'lsx-currency-check' unchanged.classes/class-block.php (1)
64-89: 💤 Low valueConsider using an empty array instead of
new \stdClass()for rates.On line 83, when no rates are available, you're passing
new \stdClass()to the frontend. Sinceview.jsaccesses rates as an object (e.g.,fx.rates = params.rates || {}), passing an empty object literal or array might be more conventional. WhilestdClasswill work, it's slightly unusual in a JavaScript context.♻️ Optional refactor to use array (which JSON-encodes to object)
$params = apply_filters( 'lsx_currencies_js_params', array( 'base' => $base_currency, 'current' => $current_currency, - 'rates' => $frontend->rates ?: new \stdClass(), + 'rates' => $frontend->rates ?: array(), 'symbols' => $frontend->get_available_symbols(), 'removeDecimals' => lsx_currencies()->remove_decimals, 'convertToSingle' => lsx_currencies()->convert_to_single, 'ratesMessage' => $frontend->rates_message, ) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/class-block.php` around lines 64 - 89, In pass_params_to_view(), change the fallback for 'rates' in the params array from new \stdClass() to an empty array (e.g., [] or array()) so JSON encoding produces an empty object in JS; update the 'rates' entry in the params array (used by view.js as fx.rates) to use the empty array instead of \stdClass to be more conventional for front-end consumption.src/blocks/currency-switcher/view.js (1)
232-236: Confirm base currency handling for Open Exchange Rates
Open Exchange Rates’ free tier always uses USD as the base currency (the ability to set a differentbaseis restricted to paid plans). Sofx.base = 'USD'is consistent for a free-tier integration. If the plugin later supports paid-plan/base-param behaviour whereparams.basecan vary, thenfx.baseshould trackparams.baseinstead of staying hardcoded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/blocks/currency-switcher/view.js` around lines 232 - 236, The init function currently hardcodes fx.base = 'USD', which is correct for Open Exchange Rates free tier but will break when supporting paid-plan base changes; update init (function init) to set fx.base from params.base when provided (e.g., fx.base = params.base || 'USD') and keep assigning fx.rates = params.rates || {} so the base follows params.base if present while preserving USD as the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@classes/class-block.php`:
- Around line 92-93: Enqueued vendor scripts money.min.js and accounting.min.js
are added via wp_enqueue_script using LSX_CURRENCIES_URL without checking the
files exist; update the enqueue logic (where wp_enqueue_script is called) to
first verify the physical files exist (e.g., using file_exists() against the
corresponding filesystem path derived from LSX_CURRENCIES_URL or a
LSX_CURRENCIES_DIR constant), only call wp_enqueue_script('lsx-moneyjs', ...)
and wp_enqueue_script('lsx-accountingjs', ...) if the files are present, and
otherwise skip enqueuing and emit a developer-facing notice (error_log or
trigger_error when WP_DEBUG) so missing build artifacts are detectable.
In `@classes/class-frontend.php`:
- Around line 258-265: In filter_post_meta, wp_cache_get( $object_id,
'post_meta' ) can return false on cache miss causing a PHP 8 "array offset on
bool" notice; before checking $meta_cache[ $meta_key ] ensure $meta_cache is an
array (e.g. use is_array($meta_cache) or !== false) and only then call
isset($meta_cache[$meta_key]) or compare to '' so the early-return '0' branch
runs safely when convert_to_single is true and meta_key === 'price' without
touching a boolean.
- Around line 101-112: Validate the sanitized cookie against the plugin's
allowed currency list before assigning it to $this->current_currency: after
reading and sanitizing $_COOKIE['lsx_currencies_choice'], check that the
uppercased value exists in the available currencies returned by lsx_currencies
(e.g. the currencies array or a get/list method) and only then set
$this->current_currency to that value; otherwise fall back to
lsx_currencies()->base_currency; keep the existing override for
lsx_currencies()->convert_to_single intact.
In `@src/blocks/currency-switcher/index.js`:
- Around line 20-32: The Caret SVG component is missing a stroke attribute so
the <path> won't inherit text color; update the Caret component (the Caret arrow
SVG and its <path> element) to include stroke="currentColor" on the <path> so it
inherits the navigation item color and remains visible across backgrounds.
In `@src/blocks/currency-switcher/view.js`:
- Around line 123-130: The buildLabelHTML function inserts params.symbols[code]
into HTML unsafely; fix by escaping or avoiding innerHTML concatenation for the
symbol. Update buildLabelHTML to either (a) use a safe escape helper (e.g.,
escapeHTML(symbol)) when concatenating so the symbol is HTML-encoded before
appending, or (b) build the symbol span as a DOM element (createElement('span'),
set textContent = params.symbols[code], add class 'lsx-currency-symbol', then
append its outerHTML) and use that safe string in the returned HTML. Ensure the
change targets buildLabelHTML and covers the conditional branch that checks
params.symbols[code].
---
Nitpick comments:
In `@classes/class-admin.php`:
- Around line 157-163: Replace the manual $checked string logic with WordPress's
checked() helper: instead of setting $checked = in_array( $code, $saved_codes,
true ) ? 'checked="checked"' : '' and echoing $checked inside the input, call
checked() inline using the same in_array() condition (preserving strict true) so
the checkbox attribute is rendered idiomatically; keep the existing data-code
attribute (esc_attr( strtoupper( $code ) )) and the input class
'lsx-currency-check' unchanged.
In `@classes/class-block.php`:
- Around line 64-89: In pass_params_to_view(), change the fallback for 'rates'
in the params array from new \stdClass() to an empty array (e.g., [] or array())
so JSON encoding produces an empty object in JS; update the 'rates' entry in the
params array (used by view.js as fx.rates) to use the empty array instead of
\stdClass to be more conventional for front-end consumption.
In `@classes/class-frontend.php`:
- Around line 125-132: The foreach loop in classes/class-frontend.php extracts
$label but never uses it; update the loop to iterate keys only (so $label is not
created) — for example, iterate over array_keys($additional_currencies) or use a
blank/underscore placeholder for the value — and keep the existing logic that
sanitizes $code and assigns $symbols[$code] = $all_symbols[$code] when present;
change references to $additional_currencies, $code, $label, $all_symbols, and
$symbols accordingly.
In `@src/blocks/currency-switcher/view.js`:
- Around line 232-236: The init function currently hardcodes fx.base = 'USD',
which is correct for Open Exchange Rates free tier but will break when
supporting paid-plan base changes; update init (function init) to set fx.base
from params.base when provided (e.g., fx.base = params.base || 'USD') and keep
assigning fx.rates = params.rates || {} so the base follows params.base if
present while preserving USD as the default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8fdfb076-314e-4191-b8f1-9ae1faf2b03e
📒 Files selected for processing (27)
assets/js/src/lsx-currencies-admin.jsassets/js/src/lsx-currencies.jsbuild/blocks/currency-switcher/block.jsonbuild/blocks/currency-switcher/index.asset.phpbuild/blocks/currency-switcher/index.jsbuild/blocks/currency-switcher/render.phpbuild/blocks/currency-switcher/view.asset.phpbuild/blocks/currency-switcher/view.jschangelog.mdclasses/class-admin.phpclasses/class-block.phpclasses/class-currencies.phpclasses/class-frontend.phpclasses/class-woocommerce.phpclasses/deprecated/class-lsx-currencies.phpincludes/settings/api.phpincludes/settings/display.phpincludes/settings/general.phpincludes/template-tags.phplsx-currencies.phppackage.jsonsrc/blocks/currency-switcher/block.jsonsrc/blocks/currency-switcher/index.jssrc/blocks/currency-switcher/inspector.jssrc/blocks/currency-switcher/render.phpsrc/blocks/currency-switcher/view.jswebpack.config.js
💤 Files with no reviewable changes (6)
- includes/settings/display.php
- assets/js/src/lsx-currencies.js
- classes/deprecated/class-lsx-currencies.php
- includes/settings/general.php
- includes/settings/api.php
- assets/js/src/lsx-currencies-admin.js
- class-block.php: guard vendor script enqueue with file_exists(); emit error_log notice under WP_DEBUG when money.min.js or accounting.min.js is missing so absent build artifacts are detectable - index.js (Caret): add stroke="currentColor" on <path> so the SVG chevron inherits the navigation item text colour across all backgrounds - view.js: buildLabelHTML already uses createElement/textContent for the symbol span (linter-applied); include compiled output Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/blocks/currency-switcher/view.js`:
- Line 1: The cookie reader in the startup path can throw if
`lsx_currencies_choice` contains a malformed encoded value, which prevents the
switcher setup from running. Update the `e()` helper (the cookie lookup used by
`i` and `a()`) to wrap `decodeURIComponent(...)` in a `try/catch` and return
`null` on failure so bad cookie values are ignored safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b77aa82-48d1-401d-bc80-33263e686e4a
📒 Files selected for processing (4)
build/blocks/currency-switcher/index.jsbuild/blocks/currency-switcher/view.jsclasses/class-block.phpsrc/blocks/currency-switcher/index.js
✅ Files skipped from review due to trivial changes (1)
- build/blocks/currency-switcher/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/blocks/currency-switcher/index.js
- classes/class-block.php
| @@ -0,0 +1 @@ | |||
| !function(){"use strict";function e(e){const t=("; "+document.cookie).split("; "+e+"=");return 2===t.length?decodeURIComponent(t.pop().split(";").shift()):null}function t(e){return String(e).replace(/[^A-Za-z]/g,"").toUpperCase().slice(0,3)}const n="undefined"!=typeof lsx_currencies_params?lsx_currencies_params:null;if(!n)return;const c=n.removeDecimals?0:2;function o(e){"undefined"!=typeof fx&&"undefined"!=typeof accounting&&document.querySelectorAll(".amount.lsx-currencies").forEach(function(t){const o=t.querySelector(".value");if(!o)return;const r="data-price-"+e,i=t.getAttribute(r)||o.getAttribute(r);let s;if(null!==i&&""!==i)s=parseFloat(i);else{const c="data-price-"+n.base,r=parseFloat(t.getAttribute(c)||o.getAttribute(c)||0);try{s=fx(r).from(n.base).to(e)}catch(e){s=r}}o.textContent=accounting.formatNumber(s,c);const a=t.querySelector(".currency-icon");a&&(a.className="currency-icon "+e.toLowerCase(),a.textContent=e)})}function r(e,t){let c='<span class="wp-block-navigation-item__label">'+e;if(t&&n.symbols&&n.symbols[e]){const t=document.createElement("span");t.className="lsx-currency-symbol",t.setAttribute("aria-hidden","true"),t.textContent=n.symbols[e],c+=" "+t.outerHTML}return c+="</span>",c}let i=t(e("lsx_currencies_choice")||n.current||n.base);function s(e,n){n.addEventListener("click",function(c){c.preventDefault();const a=t(n.dataset.lsxCurrency||"");if(a){(function(e){const n=t(e);if(!n||n===i)return;const c=i;i=n,function(e,t){const n=new Date;n.setTime(n.getTime()+2592e6),document.cookie="lsx_currencies_choice="+encodeURIComponent(t)+";expires="+n.toUTCString()+";path=/;SameSite=Lax"}(0,n),o(n),document.querySelectorAll(".wp-block-lsx-currencies-currency-switcher").forEach(function(e){!function(e,t,n){const c="1"===e.dataset.showSymbol,o=e.querySelector(":scope > .wp-block-navigation-item__content");o&&(o.innerHTML=r(t,c),o.setAttribute("href","#"+t.toLowerCase()));const i=e.querySelector(":scope > .wp-block-navigation__submenu-container");if(!i)return;const a=i.querySelector('a[data-lsx-currency="'+t+'"]');if(a&&a.closest("li")?.remove(),!i.querySelector('a[data-lsx-currency="'+n+'"]')){const t=document.createElement("li");t.className="wp-block-navigation-item wp-block-navigation-link";const o=document.createElement("a");o.className="wp-block-navigation-item__content",o.href="#"+n.toLowerCase(),o.dataset.lsxCurrency=n,o.innerHTML=r(n,c),t.appendChild(o),i.appendChild(t),s(e,o)}}(e,n,c)})})(a),e.classList.remove("is-menu-open");const n=e.querySelector(".wp-block-navigation-submenu__toggle");n&&"true"===n.getAttribute("aria-expanded")&&n.click()}})}function a(){"undefined"!=typeof fx&&(fx.base="USD",fx.rates=n.rates||{});const c=e("lsx_currencies_choice");c&&(i=t(c)),"undefined"!=typeof fx&&"undefined"!=typeof accounting&&o(i),document.querySelectorAll(".wp-block-lsx-currencies-currency-switcher").forEach(function(e){var t;(t=e).querySelectorAll("a[data-lsx-currency]").forEach(function(e){s(t,e)})})}"loading"===document.readyState?document.addEventListener("DOMContentLoaded",a):a()}(); No newline at end of file | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the cookie decode path is guarded in both source and built assets.
fd -i '^view\.js$' . | while read -r file; do
case "$file" in
*currency-switcher/view.js)
printf '\n--- %s ---\n' "$file"
rg -n -C2 'decodeURIComponent|lsx_currencies_choice|try\s*\{' "$file" || true
;;
esac
doneRepository: lightspeedwp/lsx-currencies
Length of output: 4115
🏁 Script executed:
#!/bin/bash
# Inspect the source implementation around the cookie helper and params setup.
file="src/blocks/currency-switcher/view.js"
printf '\n--- %s (lines 1-80) ---\n' "$file"
sed -n '1,80p' "$file" | cat -n
printf '\n--- %s (lines 160-220) ---\n' "$file"
sed -n '160,220p' "$file" | cat -nRepository: lightspeedwp/lsx-currencies
Length of output: 6169
🏁 Script executed:
#!/bin/bash
# Inspect the source implementation around the cookie helper and params setup.
file="src/blocks/currency-switcher/view.js"
printf '\n--- %s (lines 1-80) ---\n' "$file"
sed -n '1,80p' "$file" | cat -n
printf '\n--- %s (lines 160-220) ---\n' "$file"
sed -n '160,220p' "$file" | cat -nRepository: lightspeedwp/lsx-currencies
Length of output: 6169
Guard getCookie() against malformed values. decodeURIComponent(...) runs during startup via lsx_currencies_choice, so a bad cookie can throw a URIError and stop the switcher from wiring up. Wrap the decode in try/catch and fall back to null.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: o.innerHTML=r(t,c)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: o.innerHTML=r(n,c)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build/blocks/currency-switcher/view.js` at line 1, The cookie reader in the
startup path can throw if `lsx_currencies_choice` contains a malformed encoded
value, which prevents the switcher setup from running. Update the `e()` helper
(the cookie lookup used by `i` and `a()`) to wrap `decodeURIComponent(...)` in a
`try/catch` and return `null` on failure so bad cookie values are ignored
safely.
- class-admin.php: replace manual checked string with WordPress checked()
helper for currency checkboxes
- class-block.php: use array() instead of new \stdClass() as rates fallback
so JSON encodes as {} consistently without stdClass quirks
- class-frontend.php: iterate array_keys() in get_available_symbols() to
avoid creating an unused \$label variable per iteration
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
core/navigation— mimicscore/navigation-submenuHTML and Interactivity API output exactly (hover/click/focus open-close handled by the navigation store, no extra JS)lsx_to_settings_fieldsfilter +lsx_to_framework_dashboard_tab_contentaction; settings stored in the sharedlsx_to_settingsoptionapiVersion: 3,Requires at least: 7.0,Requires PHP: 8.0,@wordpress/scriptsbuild tooling replacing Gulpbase_currencynow stored uppercase so money.js rate lookups succeed; currency symbol updates immediately on switch without page reloadWhat changed
lsx-currencies/currency-switcherblock, parent-restricted tocore/navigation, server-side rendered viarender.phpcore/navigation-submenuHTML + Interactivity API attributes (data-wp-interactive,data-wp-context,data-wp-on--*,data-wp-watch)lsx_to_settingsoption alongside Tour Operator's own settingsesc_attr()/esc_html()throughout,sanitize_key()+ uppercase for all currency codes,esc_url_raw()for API URLTest plan
submenuVisibilitysettinglsx_to_settingsnpm run buildcompiles cleanly🤖 Generated with Claude Code