Refactor: Subscription from Vue to React#1824
Open
sapayth wants to merge 47 commits intoweDevsOfficial:developfrom
Open
Refactor: Subscription from Vue to React#1824sapayth wants to merge 47 commits intoweDevsOfficial:developfrom
sapayth wants to merge 47 commits intoweDevsOfficial:developfrom
Conversation
…into refactor/subscription_react
Changed the font size utility from wpuf-text-md to wpuf-text-base for the active account navigation item to ensure consistent typography.
Added 'account' CSS to the registered styles in Assets.php and updated Frontend.php to enqueue it by handle instead of direct path. This improves consistency and leverages the style registration system.
Refined styles in account.css and updated dashboard templates for edit-profile and subscription. Improved layout and appearance for user account and subscription management pages.
…into refactor/subscription_react
WalkthroughComprehensive migration of subscriptions UI from Vue.js with Pinia stores to React.js with WordPress data stores. Removes all Vue components and stores, replaces with React components, Redux-style stores, custom hooks, and updates build pipeline from Vite to wp-scripts/webpack with feature flag support. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/Admin/Menu.php (1)
47-53:⚠️ Potential issue | 🟠 MajorFeature flag rollout is bypassed for subscriptions routing
Line [47], Line [190], and Line [201] hardcode the React route/hook/view, so
WPUF_USE_REACT_SUBSCRIPTIONSno longer controls which UI is loaded from the menu.♻️ Suggested fix
- 'wpuf_subscriptions', + ( defined( 'WPUF_USE_REACT_SUBSCRIPTIONS' ) && WPUF_USE_REACT_SUBSCRIPTIONS ) ? 'wpuf_subscriptions' : 'wpuf_subscription', [ $this, 'subscription_menu_page' ] ); @@ public function subscription_menu_action() { @@ - do_action( 'wpuf_load_subscriptions_react_page' ); + if ( defined( 'WPUF_USE_REACT_SUBSCRIPTIONS' ) && WPUF_USE_REACT_SUBSCRIPTIONS ) { + do_action( 'wpuf_load_subscriptions_react_page' ); + return; + } + + do_action( 'wpuf_load_subscription_page' ); } @@ public function subscription_menu_page() { - $page = WPUF_INCLUDES . '/Admin/views/subscriptions-react.php'; + $page = ( defined( 'WPUF_USE_REACT_SUBSCRIPTIONS' ) && WPUF_USE_REACT_SUBSCRIPTIONS ) + ? WPUF_INCLUDES . '/Admin/views/subscriptions-react.php' + : WPUF_INCLUDES . '/Admin/views/subscriptions.php'; wpuf_require_once( $page ); }Also applies to: 190-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/Admin/Menu.php` around lines 47 - 53, The code currently hardcodes the React route/hook 'wpuf_subscriptions' and always registers subscription_hook and load-<hook> action, bypassing the WPUF_USE_REACT_SUBSCRIPTIONS flag; update the registration so that adding the menu/page and assigning $this->all_submenu_hooks['subscription_hook'] = $subscription_hook plus add_action('load-' . $subscription_hook, [ $this, 'subscription_menu_action' ]) only occurs when WPUF_USE_REACT_SUBSCRIPTIONS is true (or conversely register the legacy route when false), and inside subscription_menu_page and subscription_menu_action ensure they choose the React view or legacy view based on WPUF_USE_REACT_SUBSCRIPTIONS to restore proper feature-flagged routing.
🟠 Major comments (31)
src/css/subscriptions.css-47-49 (1)
47-49:⚠️ Potential issue | 🟠 MajorAdd keyboard/focus trigger for tooltip visibility.
Tooltip only appears on
:hover, so keyboard users won’t get it. Add:focus-visible(and/or:focus-within) parity.♿ Proposed fix
.wpuf-tooltip[data-tip]:hover::before, -.wpuf-tooltip[data-tip]:hover::after { +.wpuf-tooltip[data-tip]:hover::after, +.wpuf-tooltip[data-tip]:focus-visible::before, +.wpuf-tooltip[data-tip]:focus-visible::after, +.wpuf-tooltip[data-tip]:focus-within::before, +.wpuf-tooltip[data-tip]:focus-within::after { opacity: 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/subscriptions.css` around lines 47 - 49, The tooltip only becomes visible on hover via the selectors .wpuf-tooltip[data-tip]:hover::before and :hover::after, which excludes keyboard users; update the rule set to also target keyboard focus by adding matching selectors for focus states such as .wpuf-tooltip[data-tip]:focus-visible::before, .wpuf-tooltip[data-tip]:focus-visible::after and optionally .wpuf-tooltip[data-tip]:focus-within::before, .wpuf-tooltip[data-tip]:focus-within::after so the same opacity change applies when the trigger receives keyboard focus; ensure the interactive trigger inside .wpuf-tooltip is programmatically focusable (e.g., has tabindex or is a focusable element) so focus-visible can activate.src/css/subscriptions.css-2-4 (1)
2-4:⚠️ Potential issue | 🟠 MajorConfigure CSS linters to ignore Tailwind at-rules and prevent CI failures.
The current
.stylelintrc.jsonandbiome.jsondo not ignore@tailwindat-rules. Both linters will flag lines 2-4 as errors and fail the pipeline. Add the following configurations:🔧 Suggested config update
# .stylelintrc.json { "extends": [ "stylelint-config-standard-scss" ], "rules": { + "scss/at-rule-no-unknown": [true, { + "ignoreAtRules": ["tailwind", "apply", "layer", "config", "variants", "responsive", "screen"] + }], "selector-id-pattern": null, ... } }# biome.json { "css": { "linter": { "enabled": true } } }Note: Biome's CSS linter does not yet support Tailwind at-rule exceptions in the same way as Stylelint. Consider suppressing or disabling CSS linting for Tailwind-specific files, or configure Biome to treat these as warnings rather than errors if full suppression isn't available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/subscriptions.css` around lines 2 - 4, The CI failures are caused by Stylelint and Biome flagging the Tailwind at-rules (`@tailwind` base, `@tailwind` components, `@tailwind` utilities) in subscriptions.css; update .stylelintrc.json to allow these at-rules (add them to "at-rule-no-unknown" exceptions or add "ignoreAtRules": ["tailwind", "apply", "variants", "responsive"]) and update biome.json to either disable CSS linting for Tailwind-specific files (e.g., exclude subscriptions.css or a tailwind/*.css glob) or lower the severity for unknown at-rules so `@tailwind` lines don’t error; ensure the configs reference the `@tailwind` token so lines 2–4 are ignored in CI.templates/dashboard/subscription.php-191-191 (1)
191-191:⚠️ Potential issue | 🟠 MajorHardcoded
/yearcan display incorrect billing periods.
Line 191 and Line 252 force yearly labeling even when the pack isn’t yearly. This can misrepresent pricing details in the UI.Proposed fix
- <span class="wpuf-price-period">/year</span> + <?php if ( ! empty( $recurring_des ) ) : ?> + <span class="wpuf-price-period"><?php echo esc_html( $recurring_des ); ?></span> + <?php endif; ?> ... - <?php if ( ! empty( $recurring_des ) ) : ?> - <span class="wpuf-price-period"><?php echo esc_html( $recurring_des ); ?></span> - <?php else : ?> - <span class="wpuf-price-period">/year</span> - <?php endif; ?> + <?php if ( ! empty( $recurring_des ) ) : ?> + <span class="wpuf-price-period"><?php echo esc_html( $recurring_des ); ?></span> + <?php endif; ?>Also applies to: 251-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/dashboard/subscription.php` at line 191, The span with class wpuf-price-period currently contains a hardcoded "/year" which mislabels non-yearly packs; update both occurrences to output the pack's actual billing period (e.g., use the pack/packaging variable like $pack->billing_period or $package->period or your existing pricing object) instead of the literal string, ensuring you format it (prepend "/" if needed), escape/output it safely, and wrap with translation/fallback logic so monthly/weekly/custom intervals render correctly for all packs.src/js/components/Header.jsx-8-11 (1)
8-11:⚠️ Potential issue | 🟠 MajorGuard against missing
window.wpuf_admin_scriptto avoid runtime crashes.
Line 8 assumes the global always exists; if it doesn’t, Lines 9–11 throw and break rendering.Proposed fix
- const wpuf = window.wpuf_admin_script; - const logoUrl = wpuf.asset_url + '/images/wpuf-icon-circle.svg'; - const upgradeUrl = wpuf.upgradeUrl + '?utm_source=' + utm + '&utm_medium=wpuf-header'; - const supportUrl = wpuf.support_url; + const wpuf = window.wpuf_admin_script || {}; + const logoUrl = ( wpuf.asset_url || '' ) + '/images/wpuf-icon-circle.svg'; + const upgradeBase = wpuf.upgradeUrl || '#'; + const upgradeUrl = wpuf.upgradeUrl + ? `${ upgradeBase }${ upgradeBase.includes( '?' ) ? '&' : '?' }utm_source=${ encodeURIComponent( utm ) }&utm_medium=wpuf-header` + : '#'; + const supportUrl = wpuf.support_url || '#';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/Header.jsx` around lines 8 - 11, Guard against missing window.wpuf_admin_script by first using a safe fallback (e.g., const wpuf = window.wpuf_admin_script || {}) before computing logoUrl, upgradeUrl, and supportUrl; ensure you build logoUrl and supportUrl only when asset_url/support_url exist (or use sensible defaults), and construct upgradeUrl by concatenating utm only if utm is defined to avoid "undefined" in the string. Update the references in Header.jsx where logoUrl, upgradeUrl, and supportUrl are defined so they use the safe wpuf fallback and conditional concatenation rather than assuming window.wpuf_admin_script always exists.src/js/components/subscriptions/SidebarMenu.jsx-30-37 (1)
30-37:⚠️ Potential issue | 🟠 MajorUse a semantic interactive element for menu actions (keyboard accessibility).
Clickable
<li>items are not keyboard-focusable/actionable by default. This blocks keyboard navigation.♿ Suggested fix
- <li - key={ item.key } - onClick={ () => onStatusClick && onStatusClick( item.key ) } - className={ - 'wpuf-justify-between wpuf-text-gray-700 hover:wpuf-text-primary hover:wpuf-bg-gray-50 group wpuf-flex wpuf-gap-x-3 wpuf-rounded-md wpuf-py-2 wpuf-px-[20px] wpuf-text-sm wpuf-leading-6 hover:wpuf-cursor-pointer' + - ( isActive ? ' wpuf-bg-gray-50 wpuf-text-primary' : '' ) - } - > + <li key={ item.key }> + <button + type="button" + onClick={ () => onStatusClick && onStatusClick( item.key ) } + className={ + 'wpuf-w-full wpuf-justify-between wpuf-text-gray-700 hover:wpuf-text-primary hover:wpuf-bg-gray-50 group wpuf-flex wpuf-gap-x-3 wpuf-rounded-md wpuf-py-2 wpuf-px-[20px] wpuf-text-sm wpuf-leading-6' + + ( isActive ? ' wpuf-bg-gray-50 wpuf-text-primary' : '' ) + } + > { item.label } { count > 0 && ( <span @@ > { count } </span> ) } - </li> + </button> + </li>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SidebarMenu.jsx` around lines 30 - 37, The list items in SidebarMenu.jsx use a non-interactive <li> with onClick (see the element using onStatusClick and isActive), which prevents keyboard focus/activation; change each interactive item to a semantic interactive control—either render a <button> inside the <li> or replace the <li> with a <button> element—move the onClick={ () => onStatusClick && onStatusClick(item.key) } and className to that button, ensure it receives focus, add an accessible state attribute (e.g., aria-pressed or aria-current when isActive), and remove any duplicate click handling so keyboard (Enter/Space) will activate the menu item.tailwind.config.js-17-23 (1)
17-23:⚠️ Potential issue | 🟠 MajorUpdate Tailwind content globs to match actual React subscription file locations.
The current globs point to non-existent paths and omit
.jsxfiles:
./src/**/*.{js,css}omits.jsxextension./src/js/components-react/**/*.{js,jsx}targets wrong directory (actual:./src/js/components/)./src/js/subscriptions-react.jsxtargets wrong filename (actual:./src/js/subscriptions.jsx)This causes Tailwind CSS to purge styles from the actual React subscription components (19+ .jsx files under
src/js/components/subscriptions/, hooks, stores, etc.) during production builds.✅ Proposed fix
- './src/**/*.{js,css}', - './assets/js/components-react/**/*.{js,jsx}', - './assets/js/subscriptions-react.jsx', - './src/js/components-react/**/*.{js,jsx}', - './src/js/subscriptions-react.jsx', + './src/**/*.{js,jsx,css}', + './src/js/components/subscriptions/**/*.{js,jsx}', + './src/js/subscriptions.jsx',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tailwind.config.js` around lines 17 - 23, Update the Tailwind "content" array in tailwind.config.js to point at the real React files: include .jsx in the general src glob (replace the existing './src/**/*.{js,css}' entry with a glob that includes .jsx), change the './src/js/components-react/**/*.{js,jsx}' entry to target the actual components directory (components not components-react), replace './src/js/subscriptions-react.jsx' with the actual subscriptions filename, and add a glob for the subscriptions component folder (the directory containing the 19+ .jsx files) so Tailwind will scan those files; make these edits in the content array entries to ensure .jsx files under the subscriptions/components/hooks/stores paths are picked up.src/js/components/subscriptions/QuickEdit.jsx-90-104 (1)
90-104:⚠️ Potential issue | 🟠 MajorRejected
updateItem()promises are not handled.Line 90 only handles resolved results. If
updateItem()rejects (network/API failure), the UI gets no controlled error path.Add a rejection path
updateItem().then( ( result ) => { if ( result.success ) { addNotice( { content: result.message || __( 'Subscription updated successfully', 'wp-user-frontend' ), type: 'success', } ); setQuickEditStatus( false ); // Refresh the list after a short delay setTimeout( () => { window.location.reload(); }, 1000 ); } else { setError( 'fetch', result.message || __( 'An error occurred while updating', 'wp-user-frontend' ) ); } - } ); + } ).catch( ( error ) => { + setError( 'fetch', error?.message || __( 'An error occurred while updating', 'wp-user-frontend' ) ); + addNotice( { + content: __( 'Failed to update subscription', 'wp-user-frontend' ), + type: 'error', + } ); + } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/QuickEdit.jsx` around lines 90 - 104, The call to updateItem() only handles resolved results and misses rejected promises; add a rejection path (e.g., append .catch or convert to async/await with try/catch) to handle network/API failures: when updateItem() rejects, call setError('fetch', err.message || __( 'An error occurred while updating', 'wp-user-frontend' )), optionally addNotice with a failure message/type, and ensure UI state is cleaned up (e.g., setQuickEditStatus(false)) and any loading state is cleared so the user sees a controlled error path instead of silent failure.src/js/stores-react/router/index.js-120-125 (1)
120-125:⚠️ Potential issue | 🟠 MajorEmpty-string query params are never cleared.
At Line 121–Line 125,
''is excluded fromset, but not included indelete, so clearing a param with''leaves stale URL state.Fix param cleanup logic
- Object.entries(action.params).forEach(([key, value]) => { - if (value !== null && value !== undefined && value !== '') { - url.searchParams.set(key, value); - } else if (value === null || value === undefined) { - url.searchParams.delete(key); - } - }); + Object.entries(action.params).forEach(([key, value]) => { + if (value === null || value === undefined || value === '') { + url.searchParams.delete(key); + } else { + url.searchParams.set(key, value); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/router/index.js` around lines 120 - 125, The loop that syncs action.params to url.searchParams currently skips setting when value === '' but only deletes when value === null or undefined, leaving empty-string params stale; update the param cleanup logic in the Object.entries(action.params) handling so that values that are null, undefined, or '' all trigger url.searchParams.delete(key), and only non-empty-string values are passed to url.searchParams.set(key, value), modifying the block that references Object.entries(action.params), action.params and url.searchParams accordingly.src/js/components/subscriptions/UpdateButton.jsx-15-62 (1)
15-62:⚠️ Potential issue | 🟠 MajorHover-only dropdown blocks reliable access to “Save as Draft”.
Line 41 only reveals the menu on hover, while Line 21 publishes on click. Keyboard and touch users can’t reliably open the action menu before triggering publish.
Suggested direction (state-driven menu visibility)
+import { useState } from '@wordpress/element'; const UpdateButton = ( { ... } ) => { + const [ isMenuOpen, setIsMenuOpen ] = useState( false ); return ( <div className="wpuf-relative"> <button type="button" - onClick={ onPublish } + aria-haspopup="menu" + aria-expanded={ isMenuOpen } + onClick={ () => setIsMenuOpen( ( open ) => ! open ) } > { buttonText } </button> - <div className="wpuf-hidden hover:wpuf-block peer-hover:wpuf-block ..."> + <div className={ `${ isMenuOpen ? 'wpuf-block' : 'wpuf-hidden' } ...` } role="menu"> <button type="button" - onClick={ onPublish } + onClick={ () => { + onPublish?.(); + setIsMenuOpen( false ); + } } > { __( 'Publish', 'wp-user-frontend' ) } </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/UpdateButton.jsx` around lines 15 - 62, The dropdown menu is only shown on hover which prevents keyboard and touch users from reliably accessing "Save as Draft"; update the UpdateButton.jsx component to make the menu visibility state-driven: add a boolean state (e.g., isMenuOpen) and toggle it from the main button click (and via keyboard handlers like Enter/Escape/ArrowDown), replace the CSS hover-only logic on the menu container with conditional rendering/class based on isMenuOpen, ensure onPublish still triggers from the primary action (or open the menu when appropriate) and wire aria-expanded on the main button and proper focus management so onSaveDraft and onPublish are reachable for keyboard/touch users while respecting isUpdating.src/js/components/subscriptions/QuickEdit.jsx-133-218 (1)
133-218:⚠️ Potential issue | 🟠 MajorModal lacks core dialog accessibility semantics.
Line 133 renders a modal container without
role="dialog",aria-modal, and a keyboard escape path, which weakens keyboard/screen-reader navigation for this flow.Minimal accessibility baseline
- <div className="wpuf-fixed wpuf-inset-0 wpuf-z-50 wpuf-flex wpuf-items-center wpuf-justify-center wpuf-p-4"> - <div className="wpuf-mx-auto wpuf-w-full wpuf-max-w-lg wpuf-rounded-lg wpuf-bg-white wpuf-shadow-xl wpuf-p-6"> + <div + className="wpuf-fixed wpuf-inset-0 wpuf-z-50 wpuf-flex wpuf-items-center wpuf-justify-center wpuf-p-4" + onKeyDown={ ( e ) => { + if ( e.key === 'Escape' && ! isUpdating ) { + handleCancel(); + } + } } + > + <div + className="wpuf-mx-auto wpuf-w-full wpuf-max-w-lg wpuf-rounded-lg wpuf-bg-white wpuf-shadow-xl wpuf-p-6" + role="dialog" + aria-modal="true" + aria-label={ __( 'Quick Edit Subscription', 'wp-user-frontend' ) } + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/QuickEdit.jsx` around lines 133 - 218, The modal container is missing dialog semantics and an Escape keyboard path; update the top-level modal wrapper (the div with className starting "wpuf-fixed wpuf-inset-0...") to include role="dialog" and aria-modal="true" and set aria-labelledby to a new id (e.g., "quick-edit-title") — add a visually-hidden heading element with that id (or reuse an existing visible heading) so screen readers announce the dialog; also make the container focusable (tabIndex={-1}) and implement an Escape key handler that calls the existing handleCancel (attach via an onKeyDown on the container or via a useEffect keydown listener) so pressing Esc closes the modal. Ensure these changes reference the existing handleCancel and the top-level modal wrapper so they’re easy to locate.src/js/components/subscriptions/Preferences.jsx-63-65 (1)
63-65:⚠️ Potential issue | 🟠 Major
Clearshould remove the override, not set a hardcoded color.At Line 64,
Clearsets#079669, so the saved value is still explicit and cannot return to true “default” behavior described in Line 159.Proposed fix
const handleClearColor = useCallback(() => { - setButtonColor('#079669'); + setButtonColor(''); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/Preferences.jsx` around lines 63 - 65, handleClearColor currently sets a hardcoded color which prevents reverting to the true default; update the handleClearColor function to remove the override by calling setButtonColor(null) (or undefined) instead of setting '#079669', and ensure any persistence/save logic that writes the button color (and any loader that applies defaults) treats null/undefined as "no override" and either removes the saved key or falls back to the default behavior used elsewhere.src/js/stores-react/subscription/selectors.js-63-67 (1)
63-67:⚠️ Potential issue | 🟠 MajorGuard
wpufSubscriptions.fieldsbefore iteration to prevent TypeError.At lines 64 and 86,
wpufSubscriptions.fieldscan beundefined. When passed tofor...in, this throws a TypeError and breaks the selectors. Other code in this file (e.g., line 159) already guards against missing fields, confirming this pattern is needed.Proposed fix
export function getFieldNames(state) { const wpufSubscriptions = getWpufSubscriptions(); - const sections = wpufSubscriptions.fields; + const sections = wpufSubscriptions.fields || {}; const names = []; @@ export function getFields(state) { const wpufSubscriptions = getWpufSubscriptions(); - const sections = wpufSubscriptions.fields; + const sections = wpufSubscriptions.fields || {}; const fields = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/subscription/selectors.js` around lines 63 - 67, The selector assumes wpufSubscriptions.fields exists before iterating and can throw a TypeError; update the code that uses getWpufSubscriptions() (references: wpufSubscriptions, getWpufSubscriptions, sections variable and the for...in loop) to guard against a missing fields object—e.g., if wpufSubscriptions is falsy or wpufSubscriptions.fields is undefined, set sections to an empty object/array or return an empty result so the for...in loop and later usage (including the other loop at the second occurrence) safely noop instead of throwing.src/js/stores-react/subscription/selectors.js-191-203 (1)
191-203:⚠️ Potential issue | 🟠 MajorUse nullish coalescing (
??) instead of logical OR (||) to preserve falsy meta values.At lines 191, 201, and 203, the
||operator incorrectly converts legitimate falsy values like0andfalseinto empty strings. Additionally, the guard clause at line 196 (!item.meta_value[key]) has the same issue and will return''early when the key exists with a falsy value.Replace
||with??(nullish coalescing) to only treatnullandundefinedas missing values:Proposed fix
export function getMetaValue(state, key) { const item = state.item; if (!item || !item.meta_value) { return ''; } - return item.meta_value[key] || ''; + return item.meta_value[key] ?? ''; } export function getSerializedMetaValue(state, key, serializeKey) { const item = state.item; - if (!item || !item.meta_value || !item.meta_value[key]) { + if (!item || !item.meta_value || item.meta_value[key] === null || item.meta_value[key] === undefined) { return ''; } const serializedData = item.meta_value[key]; if (typeof serializedData === 'object' && serializeKey) { - return serializedData[serializeKey] || ''; + return serializedData[serializeKey] ?? ''; } - return serializedData || ''; + return serializedData ?? ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/subscription/selectors.js` around lines 191 - 203, In getSerializedMetaValue, replace uses of || that convert falsy but valid values to '' by using nullish checks: change the guard from checking !item.meta_value[key] to a null/undefined check (e.g., item.meta_value[serializeKey] !== undefined/ item.meta_value[key] == null or use !(key in item.meta_value)), and replace the return fallbacks item.meta_value[key] || '' and serializedData[serializeKey] || '' with nullish coalescing (item.meta_value[key] ?? '' and serializedData[serializeKey] ?? ''), ensuring falsy values like 0 or false are preserved; the same change applies to the initial one-line helper that returns item.meta_value[key] || '' so it becomes item.meta_value[key] ?? ''.wpuf.php-379-383 (1)
379-383:⚠️ Potential issue | 🟠 MajorAdd
rel="noopener noreferrer"to external links opened in new tabs.
These links usetarget="_blank"withoutrel, which is a security hardening gap.Proposed fix
- $links[] = '<a href="https://wedevs.com/docs/wp-user-frontend-pro/getting-started/how-to-install/" target="_blank"> ' . esc_html( 'Docs' ) . '</a>'; + $links[] = '<a href="https://wedevs.com/docs/wp-user-frontend-pro/getting-started/how-to-install/" target="_blank" rel="noopener noreferrer"> ' . esc_html( 'Docs' ) . '</a>'; - $links[] = '<a href="https://wedevs.com/wp-user-frontend-pro/pricing/?utm_source=installed_plugins" target="_blank" style="color: `#64C273`;"> ' . esc_html( 'Upgrade to Pro' ) . '</a>'; - $links[] = '<a href="https://wedevs.com/coupons/?utm_source=installed_plugins" target="_blank" style="color: `#5368FF`;">' . esc_html( 'Check Discounts' ) . '</a>'; + $links[] = '<a href="https://wedevs.com/wp-user-frontend-pro/pricing/?utm_source=installed_plugins" target="_blank" rel="noopener noreferrer" style="color: `#64C273`;"> ' . esc_html( 'Upgrade to Pro' ) . '</a>'; + $links[] = '<a href="https://wedevs.com/coupons/?utm_source=installed_plugins" target="_blank" rel="noopener noreferrer" style="color: `#5368FF`;">' . esc_html( 'Check Discounts' ) . '</a>';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wpuf.php` around lines 379 - 383, The external anchor tags being pushed into the $links array (the 'Docs', 'Upgrade to Pro', and 'Check Discounts' links inside the block guarded by !$this->is_pro()) use target="_blank" without rel attributes; update the string literals that build these anchors (the $links[] entries for those three links) to include rel="noopener noreferrer" alongside target="_blank" while keeping the existing esc_html() calls and attributes intact.wpuf.php-254-259 (1)
254-259:⚠️ Potential issue | 🟠 MajorFix authorization gate in
plugin_upgrades(currently too permissive).
Line 254 should return when either condition fails. With&&, users withoutmanage_optionscan still reach upgrades logic in admin context.Proposed fix
- if ( !is_admin() && !current_user_can( 'manage_options' ) ) { + if ( !is_admin() || !current_user_can( 'manage_options' ) ) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wpuf.php` around lines 254 - 259, The authorization gate in the plugin_upgrades block is using && and therefore only returns when both is_admin() is false AND current_user_can('manage_options') is false; change the condition to return when either check fails by using || so that the block only continues for admin users who have manage_options; update the condition that precedes assigning $this->container['upgrades'] = new WeDevs\Wpuf\Admin\Upgrades() to use is_admin() || current_user_can('manage_options') in the correct logical sense (i.e., return if not is_admin() OR not current_user_can('manage_options')) so only authorized admin users reach the upgrades initialization.src/js/components/subscriptions/SubscriptionField.jsx-138-138 (1)
138-138:⚠️ Potential issue | 🟠 MajorAvoid injecting unsanitized HTML into label content.
Line 138 uses
dangerouslySetInnerHTMLwith runtime-providedfield.label; this is a stored-XSS risk unless strictly sanitized upstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionField.jsx` at line 138, The label currently injects raw HTML via dangerouslySetInnerHTML using field.label (label element with htmlFor={field.name}), which is an XSS risk; change this to render plain text (e.g., use the label as children: <label htmlFor={field.name}>{field.label}</label>) or, if HTML is required, run field.label through a trusted sanitizer (e.g., DOMPurify) before using dangerouslySetInnerHTML so only sanitized content is inserted; ensure any sanitization is applied where field.label is produced or immediately before rendering in SubscriptionField so the label output is never raw, unsanitized HTML.src/js/components/subscriptions/SubscriptionForm.jsx-38-55 (1)
38-55:⚠️ Potential issue | 🟠 MajorGuard against stale fetch responses in edit mode.
If mode/ID changes quickly, a late response can still call
setItem/setItemCopyand overwrite the current form state.Proposed fix
useEffect(() => { + let isActive = true; if (mode === 'edit' && subscriptionId) { // Fetch single subscription by ID fetchSubscription(subscriptionId) .then((data) => { + if (!isActive) { + return; + } if (data.success && data.subscription) { setItem(data.subscription); setItemCopy(JSON.parse(JSON.stringify(data.subscription))); populateTaxonomyRestrictionData(data.subscription); doAction( 'wpuf.subscription.formMounted', data.subscription, mode ); } else { setError(data.message || __('Subscription not found', 'wp-user-frontend')); } }) .catch((err) => { + if (!isActive) { + return; + } setError(err.message || __('Failed to load subscription', 'wp-user-frontend')); }); } else if (mode === 'add-new') { // Initialize blank item for new subscription setBlankItem(); doAction( 'wpuf.subscription.formMounted', null, mode ); } return () => { + isActive = false; doAction( 'wpuf.subscription.formUnmounted' ); }; }, [mode, subscriptionId, setItem, setItemCopy, setBlankItem]);Also applies to: 61-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionForm.jsx` around lines 38 - 55, The fetch in the edit branch can race and apply stale responses to the form; update the logic around fetchSubscription to ignore late responses by introducing a request identifier or AbortController: before calling fetchSubscription, capture the current mode and subscriptionId (or create an abort signal/requestId), and after the promise resolves check that mode === 'edit' and subscriptionId matches the captured id (or that the requestId matches and not aborted) before calling setItem, setItemCopy, populateTaxonomyRestrictionData, or doAction; apply the same guard to the other fetch block that sets state (the code around lines 61-64) so late responses cannot overwrite newer form state.src/js/components/subscriptions/SubscriptionSubsection.jsx-102-102 (1)
102-102:⚠️ Potential issue | 🟠 MajorAvoid raw HTML injection in subsection notice.
Line 102 uses
dangerouslySetInnerHTMLwith dynamic notice text, which is an XSS risk unless strictly sanitized before it reaches the client.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionSubsection.jsx` at line 102, The subsection currently injects raw HTML via dangerouslySetInnerHTML for subSection.notice.message in the SubscriptionSubsection component, which is an XSS risk; change this to render the notice as plain text (e.g., a normal <p>{subSection.notice.message}</p> render) or, if HTML is required, sanitize subSection.notice.message first using a vetted sanitizer (e.g., DOMPurify or sanitize-html) before passing it into dangerouslySetInnerHTML so only safe markup is rendered.src/js/stores-react/subscription/reducer.js-95-103 (1)
95-103:⚠️ Potential issue | 🟠 MajorGuard
MODIFY_ITEMwhenstate.itemis not ready.Line 101 assumes
newItemis an object. Ifstate.itemis null/invalid, this path can throw at runtime.Proposed fix
case ACTION_TYPES.MODIFY_ITEM: // Deep clone to avoid mutation issues - const newItem = JSON.parse(JSON.stringify(state.item)); + if (!state.item || typeof state.item !== 'object') { + return state; + } + const newItem = JSON.parse(JSON.stringify(state.item)); const { key, value, serializeKey } = action;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/subscription/reducer.js` around lines 95 - 103, In the reducer case handling ACTION_TYPES.MODIFY_ITEM, guard against state.item being null/undefined before attempting JSON.parse(JSON.stringify(state.item)); either return the current state early if state.item is not an object, or initialize newItem to an empty object when state.item is falsy so subsequent property checks (newItem.hasOwnProperty(key)) and assignments are safe; update the MODIFY_ITEM branch to perform that null check and proceed only when newItem is a valid object.src/js/stores-react/subscription/reducer.js-115-131 (1)
115-131:⚠️ Potential issue | 🟠 MajorInitialize serialized meta objects generically, not just one key.
In Line 115–Line 131, first writes to serialized keys other than
additional_cpt_optionscan be ignored because the target object is never initialized.Proposed fix
- if (!newItem.meta_value.hasOwnProperty(key)) { - // If key doesn't exist in meta_value, initialize it - if (key === 'additional_cpt_options') { - newItem.meta_value[key] = {}; - } else { - // Default behavior for other keys if needed, or just return - } - } - - // Handle the specific case for additional_cpt_options where it might be a string - if (typeof newItem.meta_value[key] === 'string' && key === 'additional_cpt_options') { + if ( + !newItem.meta_value.hasOwnProperty(key) || + typeof newItem.meta_value[key] !== 'object' || + newItem.meta_value[key] === null + ) { newItem.meta_value[key] = {}; } if (typeof newItem.meta_value[key] === 'object') { newItem.meta_value[key][serializeKey] = value; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/subscription/reducer.js` around lines 115 - 131, The reducer currently only initializes the nested object for the specific key 'additional_cpt_options', so writes for other keys (newItem.meta_value[key][serializeKey] = value) can fail when that key is missing or a string; update the initialization logic in the block handling newItem.meta_value so that for any key (not just 'additional_cpt_options') you ensure newItem.meta_value[key] is an object before assigning: if the key is missing or its type is 'string' or not 'object', set newItem.meta_value[key] = {}; then proceed to set newItem.meta_value[key][serializeKey] = value; keep special-case handling for 'additional_cpt_options' only if needed for semantics but do the generic initialization for all keys.src/js/components/subscriptions/SubscriptionField.jsx-227-227 (1)
227-227:⚠️ Potential issue | 🟠 MajorFix switcher background class conflict.
Line 227 always includes
wpuf-bg-gray-200, which can override the active style and make the switch appear off.Proposed fix
- className={ `${ isSwitcherOn ? 'wpuf-bg-primary' : 'wpuf-bg-gray-200' } placeholder:wpuf-text-gray-400 wpuf-bg-gray-200 wpuf-relative wpuf-inline-flex wpuf-h-6 wpuf-w-11 wpuf-flex-shrink-0 wpuf-cursor-pointer wpuf-rounded-full wpuf-border-2 wpuf-border-transparent wpuf-transition-colors wpuf-duration-200 wpuf-ease-in-out` } + className={ `${ isSwitcherOn ? 'wpuf-bg-primary' : 'wpuf-bg-gray-200' } placeholder:wpuf-text-gray-400 wpuf-relative wpuf-inline-flex wpuf-h-6 wpuf-w-11 wpuf-flex-shrink-0 wpuf-cursor-pointer wpuf-rounded-full wpuf-border-2 wpuf-border-transparent wpuf-transition-colors wpuf-duration-200 wpuf-ease-in-out` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionField.jsx` at line 227, The className for the switch wrapper in SubscriptionField.jsx currently always includes "wpuf-bg-gray-200", which overrides the active state; remove the unconditional "wpuf-bg-gray-200" from that class list and let the ternary expression `${ isSwitcherOn ? 'wpuf-bg-primary' : 'wpuf-bg-gray-200' }` control the background entirely (keep the rest of the classes intact) so the switch shows the correct active/inactive styles.src/js/components/subscriptions/Pagination.jsx-19-21 (1)
19-21:⚠️ Potential issue | 🟠 MajorFix off-by-one in last-page window calculation.
Line 20 should include the current last page in the button window. Current math can hide the current page button.
Proposed fix
if ( currentPg === totalPages ) { - return totalPages - maxVisibleButtons; + return Math.max( 1, totalPages - maxVisibleButtons + 1 ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/Pagination.jsx` around lines 19 - 21, The last-page window calculation in Pagination.jsx currently returns totalPages - maxVisibleButtons when currentPg === totalPages, which omits the current last page from the visible button window; change that return to totalPages - maxVisibleButtons + 1 so the window includes the final page button (ensure the calculation still floors at 1 if you have such bounds elsewhere). Use the existing variables currentPg, totalPages, and maxVisibleButtons to make the change.src/js/components/subscriptions/SubscriptionField.jsx-72-87 (1)
72-87:⚠️ Potential issue | 🟠 MajorPreserve falsy values in
getFieldValue.Line 74, Line 78, and Line 83 use
||, so valid values like0/falseare replaced by defaults.Proposed fix
switch ( field.db_type ) { case 'meta': - return subscription.meta_value?.[ field.db_key ] || field.default || ''; + return subscription.meta_value?.[ field.db_key ] ?? field.default ?? ''; case 'meta_serialized': if ( subscription.meta_value?.[ field.db_key ] ) { - return subscription.meta_value[ field.db_key ][ field.serialize_key ] || field.default || ''; + return subscription.meta_value[ field.db_key ][ field.serialize_key ] ?? field.default ?? ''; } - return field.default || ''; + return field.default ?? ''; case 'post': - return subscription[ field.db_key ] || field.default || ''; + return subscription[ field.db_key ] ?? field.default ?? ''; default: - return field.default || ''; + return field.default ?? ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionField.jsx` around lines 72 - 87, The getFieldValue logic in SubscriptionField.jsx is using || which overwrites valid falsy values (0, false); replace the fallbacks in the 'meta', 'meta_serialized', and 'post' cases to use the nullish coalescing operator (??) instead of || so only null/undefined trigger the default, and adjust the nested serialized check to use optional chaining with ?? (e.g., subscription.meta_value?.[field.db_key]?.[field.serialize_key] ?? field.default ?? '') so existing falsy values are preserved while still falling back to field.default or '' when truly missing.src/js/components/subscriptions/Pagination.jsx-5-8 (1)
5-8:⚠️ Potential issue | 🟠 MajorSync local
currentPgwhencurrentPageprop changes.
currentPgis initialized from props once (Line 8) but never updated afterward, so controlled page changes from parent can show stale UI.Proposed fix
-import { useState, useMemo } from '@wordpress/element'; +import { useState, useMemo, useEffect } from '@wordpress/element'; @@ const [ currentPg, setCurrentPg ] = useState( currentPage ); + useEffect( () => { + setCurrentPg( currentPage ); + }, [ currentPage ] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/Pagination.jsx` around lines 5 - 8, The Pagination component initializes local state currentPg from the currentPage prop but never updates it when the parent changes currentPage; update the component to synchronize local state by adding an effect that listens to the currentPage prop and calls setCurrentPg(currentPage) (only when different) so the UI reflects controlled updates from the parent; modify the Pagination component (the currentPg state logic and lifecycle) to include this synchronization.src/js/components/subscriptions/SubscriptionField.jsx-381-384 (1)
381-384:⚠️ Potential issue | 🟠 MajorHandle empty
datetime-localvalues correctly.Line 383 turns an empty input into
:00, which is invalid and can corrupt stored values.Proposed fix
onChange={ ( e ) => { // Convert datetime-local format (YYYY-MM-DDTHH:mm) to MySQL format (YYYY-MM-DD HH:mm:ss) - const newVal = e.target.value.replace( 'T', ' ' ) + ':00'; - handleChange( newVal ); + if ( ! e.target.value ) { + handleChange( '' ); + return; + } + const newVal = `${ e.target.value.replace( 'T', ' ' ) }:00`; + handleChange( newVal ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionField.jsx` around lines 381 - 384, The onChange handler in SubscriptionField.jsx currently converts e.target.value blindly with replace('T',' ') + ':00', which turns an empty datetime-local into ':00'; update the handler used by the input (the inline onChange that calls handleChange) to first check for a falsy/empty e.target.value and call handleChange with null or an empty string (whichever the rest of the code expects) instead of appending ':00', otherwise perform the existing conversion (replace 'T' with ' ' and append ':00') before calling handleChange.src/js/subscriptions.jsx-87-94 (1)
87-94:⚠️ Potential issue | 🟠 MajorDiscard flow misses form-cancel navigation.
When the modal is opened from
SubscriptionFormcancel (without a sidebar status click),pendingStatusis null. Line 90 blocks navigation, so “Discard Changes” only closes the modal.Proposed fix
const handleDiscardChanges = useCallback(() => { setIsDirty(false); setIsUnsavedPopupOpen(false); if (pendingStatus) { navigate({ action: null, id: null, post_status: pendingStatus === 'all' ? null : pendingStatus, p: null }); setPendingStatus(null); + return; + } + + if (action === 'edit' || action === 'new') { + navigate({ action: null, id: null, post_status: status === 'all' ? null : status, p: null }); } - }, [pendingStatus, navigate, setIsDirty, setIsUnsavedPopupOpen]); + }, [pendingStatus, action, status, navigate, setIsDirty, setIsUnsavedPopupOpen]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/subscriptions.jsx` around lines 87 - 94, handleDiscardChanges currently skips navigation when pendingStatus is null, so cancelling from SubscriptionForm only closes the modal; always trigger the cancel navigation instead. In handleDiscardChanges (function name), remove the conditional around navigate and always call navigate({ action: null, id: null, post_status: pendingStatus === 'all' ? null : pendingStatus, p: null }); then call setPendingStatus(null) afterward; keep the setIsDirty(false) and setIsUnsavedPopupOpen(false) behavior the same so the discard flow works whether pendingStatus is null or set.src/js/stores-react/subscription/actions.js-282-289 (1)
282-289:⚠️ Potential issue | 🟠 Major
updateItem/deleteItemcan resolveundefined, breaking callers expectingresult.successLine [288] and Line [300] swallow errors without returning a result. Also, Line [284] clears dirty state before checking whether the API call actually succeeded.
🐛 Suggested fix
export function updateItem() { @@ try { const response = await updateSubscription(updatedItem); - - dispatch.setIsDirty(false); - doAction( 'wpuf.subscription.itemSaved', response, updatedItem ); + if (response?.success) { + dispatch.setIsDirty(false); + doAction( 'wpuf.subscription.itemSaved', response, updatedItem ); + } else { + dispatch.setError('fetch', response?.message || 'Failed to update subscription.'); + } return response; } catch (error) { dispatch.setError('fetch', 'An error occurred while updating the subscription.'); + return { success: false, error }; } finally { dispatch.setIsUpdating(false); } @@ export function deleteItem(id) { return async () => { try { const response = await deleteSubscription(id); return response; } catch (error) { console.error(error); + return { success: false, error }; } }; }Also applies to: 295-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/stores-react/subscription/actions.js` around lines 282 - 289, The updateItem and deleteItem flows currently clear dirty state before verifying success and swallow errors (e.g., updateSubscription call) returning undefined; move dispatch.setIsDirty(false) to after a successful API response (after the await updateSubscription(updatedItem) and before dispatching wpuf.subscription.itemSaved), and in the catch block return a consistent failure result (e.g., return { success: false, error }) or rethrow the error so callers relying on result.success don’t receive undefined; apply the same changes to the delete flow (the deleteSubscription call and its catch) so both updateItem and deleteItem always return a well-formed result object on error or success.src/js/components/subscriptions/SubscriptionBox.jsx-95-96 (1)
95-96:⚠️ Potential issue | 🟠 MajorPublish/Draft toggle writes the wrong status for non-draft states
Line [95] sets
newStatustodraftfor every non-draftvalue. Forpending/private, Line [175] shows “Publish” but the saved status becomesdraft.🐛 Suggested fix
- const newStatus = subscription.post_status === 'draft' ? 'publish' : 'draft'; + const newStatus = subscription.post_status === 'publish' ? 'draft' : 'publish';Also applies to: 174-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionBox.jsx` around lines 95 - 96, The toggle logic currently sets newStatus = subscription.post_status === 'draft' ? 'publish' : 'draft', which flips every non-draft state to 'draft' (so "Publish" shown for pending/private ends up saving draft); change the condition to check for 'publish' instead (e.g., newStatus = subscription.post_status === 'publish' ? 'draft' : 'publish') so non-publish states become 'publish' and publish becomes 'draft'; update the same usage in the updatedSubscription object (post_status) so the saved status matches the UI toggle.src/js/components/subscriptions/SubscriptionList.jsx-31-35 (1)
31-35:⚠️ Potential issue | 🟠 MajorPagination changes URL but does not fetch paged results
Line [74] navigates to a new page, but Line [60] fetches only by status and never passes an offset. Also, Line [31] does not reset to page 1 when
pis cleared.🐛 Suggested fix
- const currentPageFromUrl = params.p ? parseInt(params.p, 10) : 1; + const currentPageFromUrl = params.p ? parseInt(params.p, 10) : 1; + // eslint-disable-next-line no-undef + const wpufSubscriptions = window.wpufSubscriptions || {}; + const perPage = parseInt(wpufSubscriptions.perPage || 10, 10); @@ - useEffect(() => { - if (params.p) { - setCurrentPage(currentPageFromUrl); - } - }, [currentPageFromUrl, params.p]); + useEffect(() => { + setCurrentPage(currentPageFromUrl || 1); + }, [currentPageFromUrl]); @@ useEffect(() => { const fetchData = async () => { - await fetchItems(currentSubscriptionStatus || 'all'); + const offset = Math.max(0, (currentPageFromUrl - 1) * perPage); + await fetchItems(currentSubscriptionStatus || 'all', offset); await fetchCounts(); }; fetchData(); - }, [currentSubscriptionStatus]); + }, [currentSubscriptionStatus, currentPageFromUrl, perPage, fetchItems, fetchCounts]); @@ - // eslint-disable-next-line no-undef - const wpufSubscriptions = window.wpufSubscriptions || {}; - const perPage = parseInt(wpufSubscriptions.perPage || 10); + // perPage moved near URL/page calculationsAlso applies to: 60-66, 72-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/components/subscriptions/SubscriptionList.jsx` around lines 31 - 35, The pagination updates the URL via params.p but the data fetch (e.g., fetchSubscriptions or fetchSubscriptionsByStatus) never receives an offset/limit and the effect that reads currentPageFromUrl (useEffect using setCurrentPage and params.p) doesn't reset to page 1 when p is removed; update the data-fetch call (the function that currently only accepts status) to accept page/offset and limit and include those parameters when calling it from the page-change handler (e.g., onPageChange), and modify the useEffect that watches currentPageFromUrl and params.p so that when params.p is undefined or empty it sets setCurrentPage(1) (and when defined sets setCurrentPage(currentPageFromUrl)) and triggers a fetch with the appropriate offset/limit based on currentPage.includes/Admin/Admin_Subscription.php-316-319 (1)
316-319:⚠️ Potential issue | 🟠 MajorRemove response output from profile-save handler
Line [318] prints output during update flow. This can break redirects and produce malformed admin responses.
🧹 Suggested fix
- echo esc_html( $user_pack[ '_post_expiration_time' ] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/Admin/Admin_Subscription.php` around lines 316 - 319, The handler is printing output during the profile-save flow by calling echo esc_html(...), which breaks redirects and admin responses; remove the echo and stop emitting any direct output—assign the value to $user_pack['_post_expiration_time'] as already done and do not echo it (remove the echo esc_html( $user_pack['_post_expiration_time'] ) line), or if a debug/log is needed use error_log or process-safe logging instead of echo; ensure no other direct echo/print calls remain in the same save handler (look for Admin_Subscription class and the profile-save method that sets $user_pack).includes/Admin/Admin_Subscription.php-114-116 (1)
114-116:⚠️ Potential issue | 🟠 MajorUse generated asset manifest for script dependencies and version
Lines 114 and 171 hardcode dependencies that diverge from the generated
subscriptions.min.asset.phpmanifest. The hardcoded array is missing'react'and'wp-url'which are present in the manifest, and usesWPUF_VERSION(4.2.9) instead of the generated version hash for proper cache-busting.Replace both occurrences with:
Suggested fix
+ $asset = include WPUF_ROOT . '/assets/js/subscriptions.min.asset.php'; wp_enqueue_script( 'wpuf-admin-subscriptions-react', WPUF_ASSET_URI . '/js/subscriptions.min.js', - [ 'wp-element', 'wp-data', 'wp-api-fetch', 'wp-i18n', 'wp-hooks', 'wp-components', 'wp-primitives' ], - WPUF_VERSION, + $asset['dependencies'], + $asset['version'], true );Also applies to: 171-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/Admin/Admin_Subscription.php` around lines 114 - 116, The script/style enqueues currently hardcode the dependency array and use WPUF_VERSION; instead, load the generated asset manifest (subscriptions.min.asset.php) and use its 'dependencies' and 'version' values when calling wp_enqueue_script and wp_enqueue_style (replace the hardcoded array and WPUF_VERSION), updating both occurrences where wp_enqueue_script, wp_enqueue_style and $script_handle are set so the enqueues include 'react' and 'wp-url' and use the manifest's hash for cache-busting (keep using WPUF_ASSET_URI for the file paths).
Comment on lines
+271
to
273
| if ( !is_admin() && !current_user_can( 'edit_users' ) ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Authorization guard is too permissive
Line [271] uses &&, which allows users without edit_users capability to continue whenever is_admin() is true.
🔒 Suggested fix
- if ( !is_admin() && !current_user_can( 'edit_users' ) ) {
+ if ( ! current_user_can( 'edit_users' ) ) {
return;
}📝 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.
Suggested change
| if ( !is_admin() && !current_user_can( 'edit_users' ) ) { | |
| return; | |
| } | |
| if ( ! current_user_can( 'edit_users' ) ) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/Admin/Admin_Subscription.php` around lines 271 - 273, The
authorization check in Admin_Subscription.php uses && between !is_admin() and
!current_user_can('edit_users'), which lets users who are is_admin() pass even
without the 'edit_users' capability; change the guard to require both checks
(replace the && logic with || so the method returns when either not an admin OR
lacks edit_users) so both is_admin() and current_user_can('edit_users') must be
true before proceeding.
Comment on lines
+46
to
+48
| "tailwindcss-scoped-preflight": "^3.4.12", | ||
| "vitawind": "^2.3.0", | ||
| "vite": "^5.1.4", | ||
| "tailwindcss-scoped-preflight": "^3.4.12" | ||
| "vite": "^5.1.4" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Direct tailwindcss dependency =="
jq -r '.dependencies.tailwindcss // "missing", .devDependencies.tailwindcss // "missing"' package.json
echo "== build:css script =="
jq -r '.scripts["build:css"]' package.json
echo "== Tailwind usage in config files =="
fd 'Gruntfile*' -x sh -c 'echo "--- {}"; rg -n "tailwind|tailwindcss|daisyui" "{}" || true'
fd 'postcss.config*' -x sh -c 'echo "--- {}"; rg -n "tailwind|tailwindcss|daisyui" "{}" || true'
fd 'tailwind.config*' -x sh -c 'echo "--- {}"; rg -n "tailwind|tailwindcss|daisyui" "{}" || true'Repository: weDevsOfficial/wp-user-frontend
Length of output: 1932
Add tailwindcss as a direct dependency or the build:css pipeline will fail at build time.
tailwindcss is missing from both dependencies and devDependencies, but the build system explicitly requires it:
Gruntfile.jsrunsnpx tailwindcsscommands directlypostcss.config.jsandpostcss.config.react.jsinclude thetailwindcssplugintailwind.config.jsimports from thetailwindcsspackage
Relying on transitive dependencies is fragile and will cause build failures when tailwindcss is not available through other packages. Add tailwindcss as a direct devDependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 46 - 48, The package.json is missing the
tailwindcss package which the build pipeline expects (Gruntfile.js runs npx
tailwindcss, postcss.config*.js references tailwindcss, tailwind.config.js
imports it); add "tailwindcss" as a direct devDependency in package.json (pin to
a compatible version with tailwindcss-scoped-preflight, e.g., a 3.x release) and
run npm install/yarn install to update lockfile so the build:css pipeline and
postcss/tailwind imports resolve reliably.
Comment on lines
+16
to
+22
| export function getItems( state, status, offset ) { | ||
| // Only fetch if items are empty (resolver won't trigger if data exists) | ||
| if ( state.items && state.items.length > 0 ) { | ||
| return; | ||
| } | ||
| return fetchItems( status || 'all', offset || 0 ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress @wordpress/data resolvers signature how do resolvers receive arguments
💡 Result:
In @wordpress/data, a resolver is tied to a selector name, and it’s called with the same arguments you passed to the selector—except the state argument. So if your selector is:
selectors: {
getThing: ( state, a, b ) => { ... }
}…the resolver signature is:
resolvers: {
getThing: ( a, b ) => { ... } // same args as selector, minus `state`
}What actually gets called (how args flow)
When you do:
select( store ).getPrice( 'hammer' );WordPress will invoke the resolver (first time / when not yet resolved) like:
resolvers.getPrice( 'hammer' )Per the docs, resolvers “receive the same arguments passed to the selector, excluding the state argument.” [1]
Resolver return shape (common pattern)
A very common resolver pattern is a function returning a thunk that receives { dispatch, select, registry } (you often only use dispatch):
resolvers: {
getPrice: ( item ) => async ( { dispatch } ) => {
// fetch...
dispatch.setPrice( item, price );
},
}This exact pattern is shown in the official package docs. [1]
Important: resolver caching is keyed by the selector args
Resolution status is cached using the argument list you call the selector with (e.g. [123] vs ['123'] are different keys). If you need consistent caching, you can add __unstableNormalizeArgs on the selector to coerce args. [2]
Sources:
[1] @wordpress/data package docs (Resolvers section + example) https://developer.wordpress.org/block-editor/reference-guides/packages/packages-data/
[2] @wordpress/data package docs (Normalizing Selector Arguments) https://developer.wordpress.org/block-editor/reference-guides/packages/packages-data/
🏁 Script executed:
cat -n src/js/stores-react/subscription/resolvers.js | head -40Repository: weDevsOfficial/wp-user-frontend
Length of output: 1703
Fix incorrect resolver signatures — resolvers must not include the state parameter.
WordPress data resolvers in @wordpress/data receive the same arguments as their corresponding selector, excluding the state argument. The current code has incorrect signatures:
getItems(state, status, offset)should begetItems(status, offset)getCounts(state)should begetCounts()(per JSDoc, status is unused and should not be received)
Because state is not actually passed to resolvers, the checks state.items and state.counts will fail at runtime—state will receive the first selector argument instead (e.g., a status string).
Update both resolver signatures to match their selectors' arguments (minus state), and use the dispatch/actions pattern to update state as shown in the @wordpress/data documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/js/stores-react/subscription/resolvers.js` around lines 16 - 22, Update
the resolver function signatures so they do not accept the `state` parameter:
change getItems(state, status, offset) to getItems(status, offset) and change
getCounts(state) to getCounts(), then remove any usage of `state` inside those
functions (e.g., checks like state.items or state.counts) and instead use the
`@wordpress/data` dispatch/actions pattern to fetch and store data (call
fetchItems(status, offset) and dispatch the appropriate action to set
items/counts) so the resolver arguments match their selectors and state updates
happen via dispatch.
Comment on lines
+32
to
+37
| export function getCounts( state ) { | ||
| // Only fetch if counts are empty | ||
| if ( state.counts && Object.keys( state.counts ).length > 0 ) { | ||
| return; | ||
| } | ||
| return fetchCounts(); |
There was a problem hiding this comment.
Same resolver signature issue applies to getCounts.
The getCounts resolver also incorrectly expects state as its first argument. Since the getCounts(state) selector has no additional arguments beyond state, the resolver should be a zero-argument function.
🐛 Suggested fix for both resolvers
-export function getItems( state, status, offset ) {
- // Only fetch if items are empty (resolver won't trigger if data exists)
- if ( state.items && state.items.length > 0 ) {
- return;
- }
- return fetchItems( status || 'all', offset || 0 );
+export function getItems() {
+ // Resolver auto-fetches on first call; `@wordpress/data` handles caching
+ return fetchItems( 'all', 0 );
}
-export function getCounts( state ) {
- // Only fetch if counts are empty
- if ( state.counts && Object.keys( state.counts ).length > 0 ) {
- return;
- }
+export function getCounts() {
return fetchCounts();
}📝 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.
Suggested change
| export function getCounts( state ) { | |
| // Only fetch if counts are empty | |
| if ( state.counts && Object.keys( state.counts ).length > 0 ) { | |
| return; | |
| } | |
| return fetchCounts(); | |
| export function getCounts() { | |
| return fetchCounts(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/js/stores-react/subscription/resolvers.js` around lines 32 - 37, The
getCounts resolver must be a zero-argument function (not getCounts(state))
because the selector only receives state; change the function signature to
getCounts() and inside the resolver read the current store state via the
resolver environment (e.g., using the store selector/registry or a getState
helper available in this module) instead of the removed state parameter, then
keep the existing logic (check state.counts and
Object.keys(state.counts).length, return early or call fetchCounts()) and still
call fetchCounts() when counts are empty.
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
2 participants
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
Refactors the Subscriptions admin page from Vue.js to React, aligning it with the rest of the plugin's modern frontend stack. The functionality remains the same — managing subscription packs, quick-editing, preferences, and pagination — but the implementation is now cleaner, better organized, and easier to maintain going forward.
Technical Notes
src/js/components-react/subscriptions/) removed and replaced with a new React component tree undersrc/js/components/subscriptions/src/js/hooks/for subscription data fetching, navigation, and actions (e.g.,useSubscriptionData,useSubscriptionActions,useRouterParams)src/js/api/subscription), decoupling them from the Redux store actionssrc/js/stores-react/router/) added to manage list/create/edit navigation within the admin pagewpuf.subscription.blankItemfilter is now available to modify the default blank subscription item shape;wpuf.subscription.itemsLoadedaction fires after subscriptions are loadedAdmin_Subscription.phpupdated to enqueue the new React bundle (subscriptions.min.js) and its stylesheet (subscriptions.css) — the Vue bundle path and handle are no longer used when React mode is activeWPUF_USE_REACT_SUBSCRIPTIONS) continues to control which version is served, allowing a safe rolloutSecurity Considerations
Summary by CodeRabbit
New Features
Styling