Traffic Boost: Fix "Invalid page template" error#3294
Traffic Boost: Fix "Invalid page template" error#3294alecgeatches merged 1 commit intoadd/traffic-boostfrom
Conversation
…ed page templates
📝 WalkthroughWalkthroughThis update introduces a comprehensive Traffic Boost feature to the Parse.ly WordPress plugin, affecting both backend and frontend. Major changes include new PHP models and REST API endpoints for managing inbound smart links and traffic boost suggestions, enhanced smart link data structures, and canonical URL handling. The frontend gains a React-based dashboard with Traffic Boost pages, a sidebar, preview components, and extensive UI for managing and previewing link suggestions. Supporting infrastructure includes new settings, permissions, validation classes, and enriched caching and status management for smart links. Additionally, the update modifies existing smart linking logic to handle ITM parameters and context, and extends settings and menu integration across the plugin. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WP_Admin
participant DashboardPage (PHP)
participant ReactApp
participant REST_API
participant SuggestionsAPI
participant SmartLinkModel
User->>WP_Admin: Navigates to Parse.ly Dashboard/Traffic Boost
WP_Admin->>DashboardPage: Loads dashboard page
DashboardPage->>ReactApp: Enqueues scripts, injects settings/permissions
ReactApp->>REST_API: Fetches posts, settings, permissions
ReactApp->>REST_API: Requests Traffic Boost suggestions/inbound links
REST_API->>SuggestionsAPI: Calls suggest-inbound-links or suggest-inbound-link-positions
SuggestionsAPI->>REST_API: Returns suggested links
REST_API->>SmartLinkModel: Creates/updates Inbound_Smart_Link(s)
SmartLinkModel->>REST_API: Applies/updates/removes smart links as needed
REST_API->>ReactApp: Returns updated smart link data
ReactApp->>User: Renders suggestions, inbound links, preview, and actions
User->>ReactApp: Accepts, discards, or updates a suggestion
ReactApp->>REST_API: Applies change (accept/discard/update)
REST_API->>SmartLinkModel: Updates smart link status/content
SmartLinkModel->>REST_API: Confirms operation
REST_API->>ReactApp: Returns result
ReactApp->>User: Updates UI and feedback
Possibly related PRs
Suggested labels
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (51)
package.json (1)
81-83: 💡 Verification agent
🧩 Analysis chain
Duplicate dependency and version consistency issues
There are two issues here:
lodash.debounceis already defined independencies(line 42)react-routerv7.1.3 is used, but@types/react-routerv5.1.20 is used - these versions might be incompatible- "lodash.debounce": "^4.0.8",
🌐 Web query:
Are @types/react-router v5.1.20 and react-router v7.1.3 compatible?💡 Result:
Compatibility of
@types/react-routerv5.1.20 withreact-routerv7.1.3Summary:
@types/react-routerv5.1.20 is not compatible withreact-routerv7.1.3.
Detailed Explanation
Type Packages and Major Versions
- The
@types/react-routerpackage provides TypeScript type definitions for React Router versions prior to v6, specifically for v4 and v5 APIs[1][3].- As of React Router v6, and continuing in v7, the library is written in TypeScript and ships its own type definitions as part of the package. Therefore, no separate
@types/react-routerpackage is needed or maintained for v6 or later[1][2][3].Breaking API Changes
- React Router v7 introduces significant breaking changes compared to v5, including removal and redesign of many APIs[2][10]. As a result, the types in
@types/react-routerv5.1.20 do not match the new APIs and internal structure of v7.- Attempting to use
@types/react-routerv5.1.20 withreact-routerv7.1.3 will lead to mismatches, type errors, and potentially missing or incorrect types for new/changed APIs.Type Management in React Router v7
- For v7+, type definitions are included natively in the
react-routerpackage. No separate type installation is needed[2][10].- Installing or referencing
@types/react-routerwith React Router v7 can cause conflicts or type errors due to overlaps or outdated expectations of the API surface.
Correct Approach for React Router v7.1.3 and TypeScript
- Only install
react-routerv7.1.3; do not install@types/react-router.- Use the built-in type support that comes with
react-routerv7. This ensures you have accurate, up-to-date type coverage for all APIs.
Conclusion
@types/react-router v5.1.20 is only for React Router v4/v5, and is not compatible with react-router v7.1.3. For v7, rely on the built-in types and remove any separate
@types/react-routerdependency from your project[1][2][10].Citations:
- 1: https://www.npmjs.com/package/@types/react-router
- 2: https://reactrouter.com/changelog
- 3: https://www.npmjs.com/package/@types/react-router-dom
- 4: DefinitelyTyped/DefinitelyTyped#65766
- 5: DefinitelyTyped/DefinitelyTyped#13299
- 6: https://www.youtube.com/watch?v=XjdIv_Yyh8I
- 7: https://www.elastic.co/docs/reference/apm/agents/rum-js/react-integration
- 8: https://classic.yarnpkg.com/en/package/react-router
- 9: https://www.npmjs.com/search?q=keywords%3Areact+router+types
- 10: https://reactrouter.com
Remove duplicate lodash.debounce and outdated @types/react-router
To keep your dependencies clean and avoid type conflicts with React Router v7, please:
- Delete the duplicate
lodash.debounceentry from devDependencies (it’s already declared underdependenciesat line 42).- Remove the
@types/react-routerv5.1.20 dependency—React Router v7.1.3 includes its own TypeScript definitions.Suggested diff:
--- package.json @@ devDependencies - "lodash.debounce": "^4.0.8", - "@types/react-router": "5.1.20",Committable suggestion skipped: line range outside the PR's diff.
src/css/admin-settings.scss (1)
5-5: 🛠️ Refactor suggestion
Convert pixel values to rem units.
Per project guidelines, replace
--padding-default: 15px;with--padding-default: to_rem(15px);for scalable spacing.src/js/admin-settings.ts (1)
72-73: 💡 Verification agent
🧩 Analysis chain
Successfully added Traffic Boost to the feature checkboxes.
The Traffic Boost checkbox has been correctly integrated into the existing pattern of feature checkbox selectors, which will ensure it benefits from the same UI toggling behavior as other features.
🏁 Script executed:
#!/bin/bash # Verify that the Traffic Boost checkbox HTML element exists in the settings page # Look for the Traffic Boost checkbox ID in PHP files echo "Checking for Traffic Boost checkbox definition in PHP files:" rg -A 2 "content_helper_traffic_boost_enabled" --type phpLength of output: 188
🏁 Script executed:
#!/bin/bash # Verify presence of Traffic Boost checkbox ID across all files in the repo echo "Global search for 'content_helper_traffic_boost_enabled':" rg -n content_helper_traffic_boost_enabled || echo "No occurrences found." echo echo "Global search for 'traffic_boost':" rg -n traffic_boost || echo "No occurrences found."Length of output: 7774
Add the Traffic Boost checkbox to the settings page markup
The JS now targets
input#content_helper_traffic_boost_enabled, but there’s no matching HTML input in the settings form. Please add the checkbox for Traffic Boost in the PHP settings class.• File:
src/UI/class-settings-page.php
– Around where other content_helper feature fields are rendered (e.g. lines ~120–130 and ~510–520), insert:<tr> <th scope="row"> <label for="content_helper_traffic_boost_enabled"> <?php esc_html_e( 'Traffic Boost', 'wp-parsely' ); ?> </label> </th> <td> <input type="checkbox" id="content_helper_traffic_boost_enabled" name="parsely_settings[content_helper][traffic_boost][enabled]" value="1" <?php checked( $options['content_helper']['traffic_boost']['enabled'], true ); ?> /> </td> </tr>• Verify that the new input’s ID matches your JS selector and that its
name/option keys align with the existing settings schema.
• Rerun the settings page tests to ensure the element is detected by your UI toggling code.src/Telemetry/telemetry-init.php (1)
55-55: 💡 Verification agent
🧩 Analysis chain
Updated action hook name to match new admin page structure.
The telemetry hook has been correctly updated to use the new admin page slug pattern ('parse-ly_page_parsely-settings'). This ensures telemetry events will continue to work with the updated admin menu structure.
🏁 Script executed:
#!/bin/bash # Verify that the new hook string is consistent with admin page registration # Look for the admin page registration to confirm the hook should match echo "Checking for admin page registration:" rg -A 2 "add_submenu_page|register_setting" --type php | grep -i "parsely-settings" # Check if there are any remaining references to the old hook echo "Checking for references to old hook 'load-settings_page_parsely':" rg "load-settings_page_parsely" --type phpLength of output: 358
🏁 Script executed:
#!/bin/bash # Search for menu page registration related to Parse.ly settings echo "=== add_submenu_page calls ===" rg -n "add_submenu_page" -C3 --type php echo "=== add_options_page calls ===" rg -n "add_options_page" -C3 --type php echo "=== register_setting for Parsely ===" rg -n "register_setting" -C3 --type php echo "=== All 'parsely' mentions ===" rg -i -n "parsely" -C3 --type phpLength of output: 68827
🏁 Script executed:
#!/bin/bash echo "=== Telemetry-init: action_hook assignment ===" rg -n "action_hook" -C2 src/Telemetry/telemetry-init.php echo "=== Telemetry-init: add_action calls ===" rg -n "add_action" -C2 src/Telemetry/telemetry-init.php echo "=== Settings Page registration in class-settings-page.php ===" sed -n '190,210p' src/UI/class-settings-page.phpLength of output: 1870
Fix telemetry action hook for settings page
The hard-coded hook in Telemetry initialization doesn’t match the actual submenu hook suffix (
parsely-dashboard-page_page_parsely-settings). Update it so telemetry fires when the settings page loads.• File:
src/Telemetry/telemetry-init.php(around line 55)
Change from:'action_hook' => 'load-parse-ly_page_parsely-settings',To:
'action_hook' => 'load-parsely-dashboard-page_page_parsely-settings',src/content-helper/dashboard-page/components/typography-components.tsx (2)
12-15: 🛠️ Refactor suggestion
Props type definition doesn't match implementation.
The type definition has a
props?: HeadingPropsproperty, but in the implementation (line 27), you're spreading all remaining props with...props. This mismatch could cause TypeScript errors or unexpected behavior.-export type DashboardHeadingProps = { - children: React.ReactNode; - props?: HeadingProps -}; +export type DashboardHeadingProps = HeadingProps & { + children: React.ReactNode; +};
27-35:
⚠️ Potential issueComponent implementation needs to match the type definition.
The component implementation doesn't align with the type definition. Either update the type definition as suggested above or modify the implementation to use the nested props property.
If keeping the current type definition, update the implementation:
-export const DashboardHeading = ( { children, ...props }: DashboardHeadingProps ) => { +export const DashboardHeading = ( { children, props = {} }: DashboardHeadingProps ) => { return ( <Heading className="parsely-dashboard-header" - { ...props }> + { ...props }> { children } </Heading> ); };📝 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.export const DashboardHeading = ( { children, props = {} }: DashboardHeadingProps ) => { return ( <Heading className="parsely-dashboard-header" { ...props }> { children } </Heading> ); };src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/post-details.tsx (1)
44-44:
⚠️ Potential issueSecurity concern: Using dangerouslySetInnerHTML without sanitization.
The use of
dangerouslySetInnerHTMLto render HTML content could expose users to cross-site scripting (XSS) attacks if the post title contains malicious code.Consider sanitizing the HTML content before rendering it or using a safer alternative. If WordPress already sanitizes
post?.title.rendered, add a code comment explaining this:-<div className="post-title" dangerouslySetInnerHTML={ { __html: post?.title.rendered ?? __( '(no title)', 'wp-parsely' ) } } /> +{/* WordPress Core sanitizes post.title.rendered, making it safe to use with dangerouslySetInnerHTML */} +<div className="post-title" dangerouslySetInnerHTML={ { __html: post?.title.rendered ?? __( '(no title)', 'wp-parsely' ) } } />🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/content-helper/dashboard-page/pages/traffic-boost/preview/preview.scss (2)
6-6: 🛠️ Refactor suggestion
Convert pixel values to rem units using variables.
According to the coding guidelines, dimensions greater than or equal to 3px should be converted to rem units using the to_rem function, and variables for sizes should be used from the variables.scss file instead of hardcoding values.
While you're already using the to_rem function correctly, consider using size variables defined in variables.scss instead of hardcoded pixel values:
- border-radius: to_rem(8px); + border-radius: var(--border-radius);If
--border-radiusdoesn't exist in the variables.scss file, you should add it there first.Committable suggestion skipped: line range outside the PR's diff.
189-196: 🛠️ Refactor suggestion
Avoid repeated width values for the popover.
The same width value is used in both the popover and its content, which could lead to maintenance issues if one value changes but not the other.
Define the width once in a variable and reuse it:
+/* Import or add to variables.scss */ +/* --popover-width: to_rem(275px); */ .wp-parsely-traffic-boost-preview-settings-popover { - width: to_rem(275px); + width: var(--popover-width); margin-top: var(--grid-unit-20); &-content { - width: to_rem(275px); + width: var(--popover-width); padding: var(--grid-unit-20); - min-width: to_rem(220px); + min-width: var(--popover-min-width); } }Committable suggestion skipped: line range outside the PR's diff.
src/rest-api/settings/class-endpoint-traffic-boost-settings.php (1)
52-55:
⚠️ Potential issueReplace placeholder settings with actual implementation.
The current implementation includes a placeholder setting 'Setting1' with a default value of 'Hello World!', which appears to be development code rather than a production-ready implementation.
Replace the placeholder with actual Traffic Boost settings:
return array( - 'Setting1' => array( - 'values' => array(), - 'default' => 'Hello World!', + 'enabled' => array( + 'values' => array( true, false ), + 'default' => true, ), + // Add other required Traffic Boost settings here );📝 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.return array( 'enabled' => array( 'values' => array( true, false ), 'default' => true, ), // Add other required Traffic Boost settings here );src/content-helper/dashboard-page/pages/traffic-boost/preview/preview-post.php (1)
30-31:
⚠️ Potential issueFix incorrect CSS property.
The property
min-height: noneis invalid. Useautoor0instead.- min-height: none; + min-height: 0;src/UI/class-row-actions.php (1)
125-154:
⚠️ Potential issueIncorrect ARIA label for Traffic Boost link.
The method uses the same ARIA label generation function for both Parse.ly Stats and Traffic Boost links, which results in an incorrect ARIA label for the Traffic Boost feature. This could confuse screen reader users.
Modify the code to use a Traffic Boost specific ARIA label:
- esc_attr( $this->generate_aria_label_for_post( $post ) ), + esc_attr( sprintf( + /* translators: Post title */ + __( 'Go to Traffic Boost for "%s"', 'wp-parsely' ), + $post->post_title + ) ),Committable suggestion skipped: line range outside the PR's diff.
src/content-helper/dashboard-page/pages/dashboard/page-component.tsx (3)
30-33: 🛠️ Refactor suggestion
The default settings object appears to be a placeholder.
The
defaultSettingsobject currently contains only a placeholder valueSetting1: 'Hello World!'. Consider replacing this with actual production-ready default settings that align with the Traffic Boost feature's requirements.Would you like me to help define more appropriate default settings for the Traffic Boost feature?
52-54: 🛠️ Refactor suggestion
Add validation for all settings properties.
Currently, only
Setting1is being validated. As more settings are added, ensure you implement proper validation for each setting property to maintain robustness.
64-90: 💡 Verification agent
🧩 Analysis chain
Consider adding error state handling in the VerifyCredentials component.
The dashboard page renders content within the
VerifyCredentialscomponent, but there's no visible handling for credential verification failures. Consider adding error state handling to provide a better user experience when credentials are invalid.
🏁 Script executed:
#!/bin/bash # Check if VerifyCredentials component has error handling ast-grep --pattern $'<VerifyCredentials> $$$ { $$$ isLoading $$$ } $$$ </VerifyCredentials>' # Or find the VerifyCredentials component implementation fd --type f --extension tsx --extension ts "VerifyCredentials" srcLength of output: 180
🏁 Script executed:
#!/bin/bash # Show the import for VerifyCredentials in the page component sed -n '1,20p' src/content-helper/dashboard-page/pages/dashboard/page-component.tsx # Find all references to VerifyCredentials to locate its implementation rg -n "VerifyCredentials" src # Case-insensitive search for a file name containing verifycredentials fd -i --type f --extension tsx --extension ts "verifycredentials" srcLength of output: 3466
🏁 Script executed:
#!/bin/bash # Display the implementation of VerifyCredentials to check for error handling sed -n '1,200p' src/content-helper/common/verify-credentials.tsxLength of output: 1085
Add loading and robust error states to VerifyCredentials
The current VerifyCredentials component only checks for a missing‐credentials flag on window and otherwise renders its children. It doesn’t account for asynchronous credential checks (e.g. network failures or invalid API keys) or provide a loading indicator. Please enhance it to:
• Show a loading spinner or placeholder while verifying credentials.
• Render a clear error message (distinct from “empty credentials”) when the verification request fails or credentials are invalid.
• Expose props or callbacks so parent components can customize loading and error UIs.Files to update:
- src/content-helper/common/verify-credentials.tsx (add loading/error state and UI)
- Any consumers (e.g. dashboard/page-component.tsx) to handle or style the new states accordingly.
src/content-helper/dashboard-page/pages/dashboard/dashboard.scss (1)
10-12: 💡 Verification agent
🧩 Analysis chain
Use variables instead of hardcoded colors.
Replace hardcoded colors with variables for better maintainability and theming support.
.dashboard-header-background { - background: linear-gradient(303deg, #000 30.51%, #487006 133.77%); + background: linear-gradient(303deg, var(--sidebar-black) 30.51%, var(--parsely-green) 133.77%); }
🏁 Script executed:
#!/bin/bash # Check available color variables cat src/content-helper/common/css/variables.scss | grep -E "color|--sidebar|--parsely"Length of output: 466
Action Required: Introduce matching CSS variables for exact colors
The existing theme variables don’t match the original values you’re replacing:
--sidebar-blackis#1e1e1e(vs.#000)--parsely-greenishsl(107, 42%, 46%)(vs.#487006)To maintain exact color fidelity and enable theming:
- Define two new variables in
src/content-helper/common/css/variables.scss::root { --dashboard-header-black: #000; --dashboard-header-olive: #487006; }- Update your SCSS:
.dashboard-header-background { - background: linear-gradient(303deg, #000 30.51%, #487006 133.77%); + background: linear-gradient( + 303deg, + var(--dashboard-header-black) 30.51%, + var(--dashboard-header-olive) 133.77% + ); }If you’d rather use the existing variables (
--sidebar-black,--parsely-green), please confirm with the design team that the slight hue/contrast shifts are acceptable.src/content-helper/dashboard-page/pages/dashboard/header-component.tsx (2)
19-32: 🛠️ Refactor suggestion
HeaderSummary component contains hardcoded text.
The component contains hardcoded performance metrics that should be dynamic or localized using WordPress translation functions.
- <div className="summary-title">Today is an exceptional day.</div> - <div className="summary-text">75% more traffic than last week</div> - <div className="summary-text">Yesterday was the 33rd best Tuesday, 214th overall.</div> + <div className="summary-title">{ __( 'Today is an exceptional day.', 'wp-parsely' ) }</div> + <div className="summary-text">{ sprintf( + /* translators: %s: percentage increase */ + __( '%s more traffic than last week', 'wp-parsely' ), + '75%' + ) }</div> + <div className="summary-text">{ sprintf( + /* translators: %1$s: day rank, %2$s: overall rank */ + __( 'Yesterday was the %1$s best Tuesday, %2$s overall.', 'wp-parsely' ), + '33rd', + '214th' + ) }</div>Don't forget to import the WordPress i18n functions:
+import { __, sprintf } from '@wordpress/i18n';📝 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.import { __, sprintf } from '@wordpress/i18n'; const HeaderSummary = (): React.JSX.Element => { return ( <div className="dashboard-header-summary"> <div className="summary-info"> <div className="summary-title">{ __( 'Today is an exceptional day.', 'wp-parsely' ) }</div> <div className="summary-text">{ sprintf( /* translators: %s: percentage increase */ __( '%s more traffic than last week', 'wp-parsely' ), '75%' ) }</div> <div className="summary-text">{ sprintf( /* translators: %1$s: day rank, %2$s: overall rank */ __( 'Yesterday was the %1$s best Tuesday, %2$s overall.', 'wp-parsely' ), '33rd', '214th' ) }</div> </div> <div className="summary-button"> <Button variant="secondary">View more in Parse.ly</Button> </div> </div> ); };
85-110: 🛠️ Refactor suggestion
DashboardHeader component contains hardcoded metrics.
The component contains hardcoded statistics that should ideally be fetched from an API or passed as props.
Consider refactoring to accept dynamic data:
-export const DashboardHeader = (): React.JSX.Element => { +type DashboardHeaderProps = { + summary: { + title: string; + trafficIncrease: string; + rankSummary: string; + }; + topMetrics: { + trafficBoost: string; + smartLinkClicks: string; + }; + metrics: Array<HeaderCardProps>; +}; +export const DashboardHeader = ({ + summary, + topMetrics, + metrics +}: Readonly<DashboardHeaderProps>): React.JSX.Element => {Then update the component to use these props instead of hardcoded values.
📝 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.type DashboardHeaderProps = { summary: { title: string; trafficIncrease: string; rankSummary: string; }; topMetrics: { trafficBoost: string; smartLinkClicks: string; }; metrics: Array<HeaderCardProps>; }; -export const DashboardHeader = (): React.JSX.Element => { +export const DashboardHeader = ({ + summary, + topMetrics, + metrics +}: Readonly<DashboardHeaderProps>): React.JSX.Element => { return ( <div className="dashboard-header-background"> <PageHeader className="dashboard-header"> <HeaderSummary /> <div className="dashboard-header-stats"> <div className="stats-top"> <StatCard className="stat-intro" title="Parse.ly Working For You" /> <StatCard title="Traffic Boost" value="14%" icon={ link } /> <StatCard title="Smart Link Clicks" value="784" icon={ link } /> </div> {[ { title: 'Page Views', value: '4.6K', change: '24%' }, { title: 'Visitors', value: '1.5K', change: '25%' }, { title: 'Minutes', value: '32', change: '40%' }, { title: 'Avg. Time', value: '32', change: '40%', down: true }, { title: 'Soc. Interactions', value: '32', change: '40%' }, { title: 'New Posts', value: '2', change: '40%' }, ].map((metric, index) => ( <StatCard key={index} {...metric} /> ))} </div> </PageHeader> </div> ); };src/content-helper/dashboard-page/pages/traffic-boost/preview/components/preview-footer.tsx (2)
83-88:
⚠️ Potential issueAdd accessible labels to icon-only navigation buttons.
<Button>components that render only an icon should provide either thelabelprop oraria-labelto ensure screen-reader users understand the action. Without it, WP accessibility guidelines (and WPCS) flag the control as non-compliant.-<Button - variant="tertiary" - onClick={ onPrevious } - icon={ arrowLeft } -/> +<Button + variant="tertiary" + onClick={ onPrevious } + icon={ arrowLeft } + label={ __( 'Previous suggestion', 'wp-parsely' ) } +/>Apply the same fix to the “next” button (lines 197-201).
Also applies to: 197-201
65-73: 🛠️ Refactor suggestion
Separate loading flags for “Accept” vs “Update” actions.
isAcceptingis reused for the Update Link button’sisBusy/disabledstate. If the store distinguishes between “accept” and “update”, a long-running accept request will disable the update button (and vice-versa), confusing users.Consider introducing an
isUpdatingselector:const { isAccepting, isRemoving, + isUpdating, isGenerating, } = useSelect( ( select ) => ( { isAccepting: activeLink ? select( TrafficBoostStore ).isAccepting( activeLink ) : false, isRemoving: activeLink ? select( TrafficBoostStore ).isRemoving( activeLink ) : false, + isUpdating: activeLink ? select( TrafficBoostStore ).isUpdating( activeLink ) : false, isGenerating: activeLink ? select( TrafficBoostStore ).isGenerating( activeLink ) : false, } ), [ activeLink ] );and wire
isUpdatingto the Update Link button.Also applies to: 125-128
src/content-helper/dashboard-page/pages/traffic-boost/preview/components/text-selection-tooltip.tsx (1)
25-28:
⚠️ Potential issueHard-coded core CSS path breaks on non-standard installations.
/wp-includes/css/dist/components/style.cssassumes WordPress is installed at the web-root andwp-includesis publicly accessible.
• Sub-directory installs (/wordpress/),
• customWP_CONTENT_DIR, or
• reverse proxies
will return 404, leaving the tooltip unstyled.Use the asset URL provided by
wp_default_styles, or dynamically read it fromwindow.wp.componentsStyleif exposed by the admin, e.g.:- wpComponentsLink.href = '/wp-includes/css/dist/components/style.css'; + // Prefer the URL registered by WordPress core. + wpComponentsLink.href = + ( window?.wp?.componentsStyleUrl ) || + `${ window.location.origin }${ window?.wp?.url?.get( 'wp-includes/css/dist/components/style.css' ) }`;Committable suggestion skipped: line range outside the PR's diff.
src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/links-list/links-list.tsx (1)
366-375:
⚠️ Potential issueAdd accessible labels to pagination icon buttons.
Similar to the preview footer, the previous/next buttons here are icon-only. Supply
labeloraria-labelfor screen-reader compliance.<Button icon={ previous } onClick={ handlePrevious } disabled={ currentPage <= 1 } + label={ __( 'Previous page', 'wp-parsely' ) } />Apply to the “next” button as well.
📝 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.<Button icon={ previous } onClick={ handlePrevious } disabled={ currentPage <= 1 } label={ __( 'Previous page', 'wp-parsely' ) } /> <Button icon={ next } onClick={ handleNext } disabled={ currentPage >= totalPages } label={ __( 'Next page', 'wp-parsely' ) } />src/content-helper/common/base-wordpress-provider.tsx (1)
224-230: 🛠️ Refactor suggestion
thumbnailmay becomeundefinedwhile typed asstring.When the
thumbnailsize is missing you fall back tofeaturedMedia?.source_url ?? undefined;, sothumbnailcan beundefined, violating its declaredstringtype and risking runtime surprises.-const featuredMedia = post._embedded[ 'wp:featuredmedia' ]?.[ 0 ]; -thumbnail = featuredMedia?.media_details?.sizes?.thumbnail?.source_url; - -if ( ! thumbnail ) { - thumbnail = featuredMedia?.source_url ?? undefined; -} +const featuredMedia = post._embedded?.[ 'wp:featuredmedia' ]?.[ 0 ]; +thumbnail = + featuredMedia?.media_details?.sizes?.thumbnail?.source_url ?? + featuredMedia?.source_url ?? + '';This always returns a string, keeping typings honest.
📝 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.const featuredMedia = post._embedded?.['wp:featuredmedia']?.[0]; thumbnail = featuredMedia?.media_details?.sizes?.thumbnail?.source_url ?? featuredMedia?.source_url ?? '';src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/tabs/suggestions-tab.tsx (1)
162-176: 🛠️ Refactor suggestion
await-ing Redux dispatches is misleading.
addSuggestion()andsetIsGenerating()are synchronous action creators; returningvoidyet youawaitthem. This gives readers the false impression that they are async and delays the following code by a micro-tick. Remove theawaitkeywords (or convert the actions to return promises).- await addSuggestion( trafficBoostLink ); - await setIsGenerating( trafficBoostLink, true ); + addSuggestion( trafficBoostLink ); + setIsGenerating( trafficBoostLink, true );📝 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.const trafficBoostLink = trafficBoostProvider.createSuggestion( post ); addSuggestion( trafficBoostLink ); setIsGenerating( trafficBoostLink, true ); try { const updatedLink = await trafficBoostProvider.generateSuggestionForPost( currentPost, post, trafficBoostLink ); await updateSuggestion( updatedLink, trafficBoostLink.uid ); setIsGenerating( trafficBoostLink, false ); } catch ( err: unknown ) { let errorMessage = __( 'Failed to find a link placement.', 'wp-parsely' );src/content-helper/editor-sidebar/smart-linking/utils.ts (2)
601-603: 🛠️ Refactor suggestion
URL matching may yield false positives
Using
a.href.includes( missingSmartLink.href.raw )can accidentally match unrelated URLs that merely contain the raw URL as a substring (e.g., a proxy, redirect, or analytics wrapper). To reduce the risk of restoring the wrong anchor, use strict, normalised comparison:- a.href.includes( missingSmartLink.href.raw ) && + removeITMParamsFromURL( a.href ) === missingSmartLink.href.raw &&This keeps behaviour consistent with your earlier normalisation logic.
📝 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.removeITMParamsFromURL( a.href ) === missingSmartLink.href.raw && ! a.hasAttribute( 'data-smartlink' );
436-439:
⚠️ Potential issuePossible ITM-parameter duplication
addSmartLinkITMParamsToURL()is invoked with the originallink.href, which may already contain ITM parameters. Because the precedingrawfield is explicitly stripped of those parameters, calling the ITM helper on the un-sanitised URL can lead to duplicated query-string keys (itm_campaign,itm_source, …) whenever the DOM already holds a decorated link.- const href = { - raw: removeITMParamsFromURL( link.href ), - itm: addSmartLinkITMParamsToURL( link.href, uid ), + const rawHref = removeITMParamsFromURL( link.href ); + const href = { + raw: rawHref, + itm: addSmartLinkITMParamsToURL( rawHref, uid ), };This one-liner prevents query-string bloat and avoids potentially breaking canonicalisation / matching logic elsewhere.
📝 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.const rawHref = removeITMParamsFromURL( link.href ); const href = { raw: rawHref, itm: addSmartLinkITMParamsToURL( rawHref, uid ), };src/content-helper/dashboard-page/pages/traffic-boost/single-post-component.tsx (2)
385-466:
⚠️ Potential issueStale-closure risk in suggestions
useEffectThis effect intentionally suppresses exhaustive-deps linting, yet it references
inboundLinks,settings,setSelectedLink, andsetSuggestionswhich are not stable across renders. If any of these change after the first run, the effect will still hold the old snapshot, causing:• Suggestions generation to omit new inbound links.
•settings.maxItemschanges to be ignored until a full reload.Consider either:
a) moving helper functions outside the component and passing current values as parameters, or
b) including the mutable variables in the dependency array and restructuring to avoid an infinite loop.Failing to address this will produce hard-to-debug state inconsistencies.
84-138: 🛠️ Refactor suggestion
ResizeObserver cleanup & cross-browser safety
Excellent job detaching the observer, but a couple of hardening tweaks will improve robustness:
ResizeObserveris undefined in some older browsers / test environments. Wrap the instantiation in a feature-detection guard to avoid runtime crashes.- You call
resizeObserver.disconnect()insidereturn, yet you don’t guard for the case where the constructor failed or was unavailable.- const resizeObserver = new ResizeObserver( () => { + const resizeObserver = window.ResizeObserver + ? new ResizeObserver( () => { updateSidebarWidth(); - } ); + } ) + : null; ... - resizeObserver.observe( adminMenuBack ); + if ( resizeObserver ) { + resizeObserver.observe( adminMenuBack ); + } ... - resizeObserver.disconnect(); + resizeObserver?.disconnect();📝 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.useEffect( () => { // Set the background color of the page container to the background color of the admin menu. const adminMenuBack = document.getElementById( 'adminmenuback' ); if ( ! adminMenuBack ) { return; } const computedStyle = window.getComputedStyle( adminMenuBack ); setBackgroundColor( computedStyle.backgroundColor ); // Set the background color of #wpwrap to the background color of the admin menu. const wpWrap = document.querySelector( '#wpwrap' ); if ( wpWrap ) { ( wpWrap as HTMLElement ).style.backgroundColor = computedStyle.backgroundColor; } /** * Updates the sidebar width. * * @since 3.18.0 */ const updateSidebarWidth = () => { const width = adminMenuBack.getBoundingClientRect().width; setSidebarWidth( width ); }; // Grab the sidebar width so it can be used to calculate the container width. updateSidebarWidth(); // Use a ResizeObserver to watch for width changes. const resizeObserver = window.ResizeObserver ? new ResizeObserver( () => { updateSidebarWidth(); } ) : null; if ( resizeObserver ) { resizeObserver.observe( adminMenuBack ); } return () => { // When the component unmounts, make sure to cancel all the provider requests. TrafficBoostProvider.getInstance().cancelAll(); // Disconnect the resize observer. resizeObserver?.disconnect(); // Remove the background color of #wpwrap. ( wpWrap as HTMLElement ).style.backgroundColor = ''; // Clean up the store state. setIsGeneratingSuggestions( false ); setLoading( false ); setError( null ); setInboundLinks( [] ); setSuggestions( [] ); setCurrentPost( null ); setSelectedLink( null ); setSelectedTab( TrafficBoostSidebarTabs.SUGGESTIONS ); }; }, [] ); // eslint-disable-line react-hooks/exhaustive-depssrc/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-link-positions.php (1)
113-128:
⚠️ Potential issueAnchor-text sanitation too permissive
wp_kses_post()will allow inline HTML (e.g.,<em>), but anchor text should be plain text. Usingsanitize_text_field()prevents accidental markup or script execution leaking into the editor.- wp_kses_post( $anchor_text_suggestion['text'] ), + sanitize_text_field( $anchor_text_suggestion['text'] ),src/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-links.php (1)
92-98: 🛠️ Refactor suggestion
Cast
offsetto int & usesanitize_text_fieldfor the title.
esc_attr()is intended for HTML attributes, not plain strings, and the offset arrives as mixed data from the API – explicitly casting helps avoid type juggling bugs.- esc_attr( $link['title'] ), + sanitize_text_field( $link['title'] ), ... - $anchor_text_suggestion['offset'] + (int) $anchor_text_suggestion['offset']src/content-helper/dashboard-page/pages/traffic-boost/store.ts (1)
401-405: 🛠️ Refactor suggestion
Avoid mutating the action object inside the reducer.
Redux actions should be immutable; mutating
action.loadingTypebreaks time-travel debugging and violates functional principles. Prefer a local variable:- if ( ! action.loadingType ) { - action.loadingType = 'default'; - } + const loadingType = action.loadingType ?? 'default';and use
loadingTypebelow.📝 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.case 'SET_LOADING': const loadingType = action.loadingType ?? 'default';src/class-parsely.php (3)
1042-1045: 🛠️ Refactor suggestion
Sanitize the stored canonical URL.
update_post_meta()should receive a cleaned value to avoid storing invalid / unsafe data.- return false !== update_post_meta( $post_id, self::PARSELY_CANONICAL_URL_META_KEY, $url ); + return false !== update_post_meta( + $post_id, + self::PARSELY_CANONICAL_URL_META_KEY, + esc_url_raw( $url ) +);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.public static function set_canonical_url( $post, string $url ): bool { $post_id = is_int( $post ) ? $post : $post->ID; - return false !== update_post_meta( $post_id, self::PARSELY_CANONICAL_URL_META_KEY, $url ); + return false !== update_post_meta( + $post_id, + self::PARSELY_CANONICAL_URL_META_KEY, + esc_url_raw( $url ) + ); }
984-999:
⚠️ Potential issueReturn an empty string or
WP_Error, not the literal'no permalink'.Returning a sentinel string leaks into downstream consumers and can surface in canonical URLs. Prefer an early
''or aWP_Error.- if ( false === $permalink ) { - return 'no permalink'; - } - - return self::get_canonical_url( $permalink ); + if ( false === $permalink ) { + return ''; + } + return self::get_canonical_url( $permalink );📝 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.public static function get_canonical_url_from_post( $post ): string { $post_id = is_int( $post ) ? $post : $post->ID; $canonical_url = get_post_meta( $post_id, self::PARSELY_CANONICAL_URL_META_KEY, true ); if ( null !== $canonical_url && is_string( $canonical_url ) && '' !== $canonical_url ) { return $canonical_url; } $permalink = get_permalink( $post ); if ( false === $permalink ) { return ''; } return self::get_canonical_url( $permalink ); }
1013-1031: 🛠️ Refactor suggestion
Host replacement logic fails for sub-domains & mixed protocols.
str_replace()on the full URL may miss cases such aswww.prefixes or alternate schemes. Consider usingwp_parse_url()to swap only the host:- // Replace the current domain with the Parse.ly site ID. - return str_replace( $home_url, $site_id, $url ); + $parts = wp_parse_url( $url ); + if ( ! $parts || empty( $parts['host'] ) ) { + return $url; + } + $parts['host'] = $site_id; + return untrailingslashit( ( $parts['scheme'] ?? 'https' ) . '://' . $parts['host'] . ( $parts['path'] ?? '' ) . ( $parts['query'] ? '?' . $parts['query'] : '' ) );📝 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.// Strip the protocol from the home URL. $home_url = preg_replace( '/^https?:\/\//', '', $home_url ); if ( null === $home_url ) { return $url; } $parts = wp_parse_url( $url ); if ( ! $parts || empty( $parts['host'] ) ) { return $url; } $parts['host'] = $site_id; return untrailingslashit( ( $parts['scheme'] ?? 'https' ) . '://' . $parts['host'] . ( $parts['path'] ?? '' ) . ( ! empty( $parts['query'] ) ? '?' . $parts['query'] : '' ) ); }src/content-helper/dashboard-page/components/posts-table/component.tsx (2)
210-230: 🛠️ Refactor suggestion
queryobject in the effect dependency causes unnecessary re-fetches.Because objects are compared by reference, passing an inline literal from a parent will trigger
fetchPostsevery render. Either:
- Memoize the
queryprop in the parent (useMemo), or- Depend on a stable serialisation:
-}, [ currentPage, itemsPerPage, query ] ); +}, [ currentPage, itemsPerPage, JSON.stringify( query ) ] );Be aware that large queries will incur a serialisation cost; option #1 is preferable.
198-204: 🛠️ Refactor suggestion
itemsPerPagenever updates whenquery.per_pagechanges.Because
itemsPerPageis initialised fromqueryonly once (useState), subsequent prop updates will be ignored. Either compute it inside the effect or derive it withuseMemo/useEffect.-const [ itemsPerPage ] = useState<number>( query.per_page ?? 10 ); +const itemsPerPage = useMemo( + () => query.per_page ?? 10, + [ query.per_page ] +);Remove
itemsPerPagefrom the state array, and update the dependencies accordingly.📝 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.const [ currentPage, setCurrentPage ] = useState<number>( 1 ); const [ totalPages, setTotalPages ] = useState<number>( 1 ); - const [ itemsPerPage ] = useState<number>( query.per_page ?? 10 ); + const itemsPerPage = useMemo( + () => query.per_page ?? 10, + [ query.per_page ] + ); const [ isLoading, setIsLoading ] = useState<boolean>( true ); const didFirstSearch = useRef( false );src/content-helper/dashboard-page/pages/traffic-boost/preview/preview.tsx (2)
494-499: 🛠️ Refactor suggestion
activeLink.smartLink = undefinedmutates state and may create race conditions.Create a fresh object instead of mutating the existing one. This guarantees Redux notice of the change and avoids side effects if another component still holds a reference.
-const oldSmartLink = activeLink.smartLink; -activeLink.smartLink = undefined; -// Update the active link. -updateSuggestion( activeLink ); +const oldSmartLink = activeLink.smartLink; +const updated = { ...activeLink, smartLink: undefined }; +updateSuggestion( updated );📝 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.// Remove the smart link from the active link. const oldSmartLink = activeLink.smartLink; const updated = { ...activeLink, smartLink: undefined }; updateSuggestion( updated );
292-299:
⚠️ Potential issueAvoid mutating Redux-managed objects in place.
handleAcceptflipslink.isSuggestionby direct mutation, breaking the expectation of immutability and potentially bypassing change detection in selectors.-// Flag isSuggestion to false. -link.isSuggestion = false; -// Add the link to the inbound links list. -addInboundLink( link ); +const inboundCopy = { ...link, isSuggestion: false }; +addInboundLink( inboundCopy );Apply the same pattern wherever the code mutates
activeLinkor its nested properties (e.g. regeneration handlers).📝 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.removeSuggestion( link ); const inboundCopy = { ...link, isSuggestion: false }; addInboundLink( inboundCopy );src/UI/class-dashboard-page.php (2)
111-113: 🛠️ Refactor suggestion
Use
plugin_dir_path()for includes to avoid path-resolution issues.
include_once __DIR__ . '/../content-helper/...';can break when the plugin directory is symlinked.
Prefer:- include_once __DIR__ . '/../content-helper/dashboard-page/pages/traffic-boost/preview/preview-post.php'; + include_once plugin_dir_path( PARSELY_FILE ) . 'src/content-helper/dashboard-page/pages/traffic-boost/preview/preview-post.php';This is WordPress-safe and follows coding standards.
📝 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.// Output the preview template. include_once plugin_dir_path( PARSELY_FILE ) . 'src/content-helper/dashboard-page/pages/traffic-boost/preview/preview-post.php'; exit;
144-157:
⚠️ Potential issueInternationalise all UI strings passed to
add_menu_page/add_submenu_page.WordPress requires that page titles and menu labels are wrapped in translation functions so that they can be localised.
'Parse.ly Dashboard Page','Parse.ly','Dashboard', and'Traffic Boost'are currently hard-coded.- 'Parse.ly Dashboard Page', - 'Parse.ly', + __( 'Parse.ly Dashboard Page', 'wp-parsely' ), + __( 'Parse.ly', 'wp-parsely' ),…and similarly for the other labels.
📝 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.public function add_dashboard_page_to_menu(): void { // Base64-encoded version of leaf-icon.tsx with size of 20 and fill of #1d2327. // <svg width="20" height="20" viewBox="0 0 60 65" xmlns="http://www.w3.org/2000/svg">…</svg>. $icon = 'data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMjAiIGhlaWdodD0iMjAiIHZpZXdCb3g9IjAgMCA2MCA2NSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cGF0aCBmaWxsPSIjMWQyMzI3IiBkPSJNMjMuNzIsNTEuNTNjMC0uMTgsMC0uMzQtLjA2LS41MmExMy4xMSwxMy4xMSwwLDAsMC0yLjEtNS41M0ExNC43NCwxNC43NCwwLDAsMCwxOS4xMiw0M2MtLjI3LS4yMS0uNS0uMTEtLjUxLjIybC0uMjQsMy40MmMwLC4zMy0uMzguMzUtLjQ5LDBsLTEuNS00LjhhMS40LDEuNCwwLDAsMC0uNzctLjc4LDIzLjkxLDIzLjkxLDAsMCwwLTMuMS0uODRjLTEuMzgtLjI0LTMuMzktLjM5LTMuMzktLjM5LS4zNCwwLS40NS4yMS0uMjUuNDlsMi4wNiwzLjc2Yy4yLjI3LDAsLjU0LS4yOS4zM2wtNC41MS0zLjZhMy42OCwzLjY4LDAsMCwwLTIuODYtLjQ4Yy0xLC4xNi0yLjQ0LjQ2LTIuNDQuNDZhLjY4LjY4LDAsMCwwLS4zOS4yNS43My43MywwLDAsMC0uMTQuNDVTLjQxLDQzLC41NCw0NGEzLjYzLDMuNjMsMCwwLDAsMS4yNSwyLjYyTDYuNDgsNTBjLjI4LjIuMDkuNDktLjIzLjM3bC00LjE4LS45NGMtLjMyLS4xMi0uNSwwLS40LjM3LDAsMCwuNjksMS44OSwxLjMxLDMuMTZhMjQsMjQsMCwwLDAsMS42NiwyLjc0LDEuMzQsMS4zNCwwLDAsMCwxLC41Mmw1LC4xM2MuMzMsMCwuNDEuMzguMS40OEw3LjUxLDU4Yy0uMzEuMS0uMzQuMzUtLjA3LjU1YTE0LjI5LDE0LjI5LDAsMCwwLDMuMDUsMS42NiwxMy4wOSwxMy4wOSwwLDAsMCw1LjkuNSwyNS4xMywyNS4xMywwLDAsMCw0LjM0LTEsOS41NSw5LjU1LDAsMCwxLS4wOC0xLjIsOS4zMiw5LjMyLDAsMCwxLDMuMDctNi45MSI+PC9wYXRoPjxwYXRoIGZpbGw9IiMxZDIzMjciIGQ9Ik01OS43LDQxLjUzYS43My43MywwLDAsMC0uMTQtLjQ1LjY4LjY4LDAsMCwwLS4zOS0uMjVzLTEuNDMtLjMtMi40NC0uNDZhMy42NCwzLjY0LDAsMCwwLTIuODYuNDhsLTQuNTEsMy42Yy0uMjYuMjEtLjQ5LS4wNi0uMjktLjMzbDIuMDYtMy43NmMuMi0uMjguMDktLjQ5LS4yNS0uNDksMCwwLTIsLjE1LTMuMzkuMzlhMjMuOTEsMjMuOTEsMCwwLDAtMy4xLjg0LDEuNCwxLjQsMCwwLDAtLjc3Ljc4bC0xLjUsNC44Yy0uMTEuMzItLjQ4LjMtLjQ5LDBsLS4yNC0zLjQyYzAtLjMzLS4yNC0uNDMtLjUxLS4yMmExNC43NCwxNC43NCwwLDAsMC0yLjQ0LDIuNDdBMTMuMTEsMTMuMTEsMCwwLDAsMzYuMzQsNTFjMCwuMTgsMCwuMzQtLjA2LjUyYTkuMjYsOS4yNiwwLDAsMSwzLDguMSwyNC4xLDI0LjEsMCwwLDAsNC4zNCwxLDEzLjA5LDEzLjA5LDAsMCwwLDUuOS0uNSwxNC4yOSwxNC4yOSwwLDAsMCwzLjA1LTEuNjZjLjI3LS4yLjI0LS40NS0uMDctLjU1bC0zLjIyLTEuMTdjLS4zMS0uMS0uMjMtLjQ3LjEtLjQ4bDUtLjEzYTEuMzgsMS4zOCwwLDAsMCwxLS41MkEyNC42LDI0LjYsMCwwLDAsNTcsNTIuOTJjLjYxLTEuMjcsMS4zMS0zLjE2LDEuMzEtMy4xNi4xLS4zMy0uMDgtLjQ5LS40LS4zN2wtNC4xOC45NGMtLjMyLjEyLS41MS0uMTctLjIzLS4zN2w0LjY5LTMuMzRBMy42MywzLjYzLDAsMCwwLDU5LjQ2LDQ0Yy4xMy0xLC4yNC0yLjQ3LjI0LTIuNDciPjwvcGF0aD48cGF0aCBmaWxsPSIjMWQyMzI3IiBkPSJNNDYuNSwyNS42MWMwLS41My0uMzUtLjcyLS44LS40M2wtNC44NiwyLjY2Yy0uNDUuMjgtLjU2LS4yNy0uMjMtLjY5bDQuNjYtNi4yM2EyLDIsMCwwLDAsLjI4LTEuNjgsMzYuNTEsMzYuNTEsMCwwLDAtMi4xOS00Ljg5LDM0LDM0LDAsMCwwLTIuODEtMy45NGMtLjMzLS40MS0uNzQtLjM1LS45MS4xNmwtMi4yOCw1LjY4Yy0uMTYuNS0uNi40OC0uNTktLjA1bC4yOC04LjkzYTIuNTQsMi41NCwwLDAsMC0uNjYtMS42NFMzNSw0LjI3LDMzLjg4LDMuMjcsMzAuNzguNjksMzAuNzguNjlhMS4yOSwxLjI5LDAsMCwwLTEuNTQsMHMtMS44OCwxLjQ5LTMuMTIsMi41OS0yLjQ4LDIuMzUtMi40OCwyLjM1QTIuNSwyLjUsMCwwLDAsMjMsNy4yN2wuMjcsOC45M2MwLC41My0uNDEuNTUtLjU4LjA1bC0yLjI5LTUuNjljLS4xNy0uNS0uNTctLjU2LS45MS0uMTRhMzUuNzcsMzUuNzcsMCwwLDAtMyw0LjIsMzUuNTUsMzUuNTUsMCwwLDAtMiw0LjYyLDIsMiwwLDAsMCwuMjcsMS42N2w0LjY3LDYuMjRjLjMzLjQyLjIzLDEtLjIyLjY5bC00Ljg3LTIuNjZjLS40NS0uMjktLjgyLS4xLS44Mi40M2ExOC42LDE4LjYsMCwwLDAsLjgzLDUuMDcsMjAuMTYsMjAuMTYsMCwwLDAsNS4zNyw3Ljc3YzMuMTksMyw1LjkzLDcuOCw3LjQ1LDExLjA4QTkuNiw5LjYsMCwwLDEsMzAsNDkuMDlhOS4zMSw5LjMxLDAsMCwxLDIuODYuNDVjMS41Mi0zLjI4LDQuMjYtOC4xMSw3LjQ0LTExLjA5YTIwLjQ2LDIwLjQ2LDAsMCwwLDUuMDktNywxOSwxOSwwLDAsMCwxLjExLTUuODIiPjwvcGF0aD48cGF0aCBmaWxsPSIjMWQyMzI3IiBkPSJNMzYuMTIsNTguNDRBNi4xMiw2LjEyLDAsMSwxLDMwLDUyLjMyYTYuMTEsNi4xMSwwLDAsMSw2LjEyLDYuMTIiPjwvcGF0aD48L3N2Zz4='; add_menu_page( __( 'Parse.ly Dashboard Page', 'wp-parsely' ), __( 'Parse.ly', 'wp-parsely' ), Parsely::CAPABILITY, // phpcs:ignore WordPress.WP.Capabilities.Undetermined 'parsely-dashboard-page', array( $this, 'add_dashboard_page_placeholder' ), $icon, 30 ); }src/content-helper/dashboard-page/pages/traffic-boost/preview/hooks/use-iframe-highlight.ts (2)
594-603:
⚠️ Potential issue
siteUrldetection can mis-classify links; compare hostnames instead ofincludes().A URL may contain the site domain as a query string or path segment, leading to false “internal” positives/negatives.
- links = links.filter( ( link ) => ! link.href.includes( siteUrl ) ); + links = links.filter( ( link ) => { + try { + return new URL( link.href ).hostname !== siteUrl; + } catch { + return true; // treat malformed URLs as external. + } + } );Apply the inverse logic for the
'internal'branch.📝 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.switch ( selectedLinkType ) { case 'external': links = links.filter( ( link ) => { try { return new URL( link.href ).hostname !== siteUrl; } catch { return true; // treat malformed URLs as external. } } ); break; case 'internal': links = links.filter( ( link ) => link.href.includes( siteUrl ) ); break; case 'smart': links = links.filter( ( link ) => link.hasAttribute( 'data-smartlink' ) ); break; }
52-67: 🛠️ Refactor suggestion
Avoid injecting duplicate
<style>tags on repeated calls.
injectHighlightStylesadds a new<style>element every time it runs, which can bloat the DOM if the hook re-initialises.+ if ( iframeDocument.getElementById( 'parsely-highlight-styles' ) ) { + return; // already injected. + } const style = iframeDocument.createElement( 'style' ); + style.id = 'parsely-highlight-styles';📝 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.const injectHighlightStyles = useCallback( ( iframe: HTMLIFrameElement ) => { const iframeDocument = iframe.contentDocument ?? iframe.contentWindow?.document; if ( ! iframeDocument ) { return; } if ( iframeDocument.getElementById( 'parsely-highlight-styles' ) ) { return; // already injected. } const style = iframeDocument.createElement( 'style' ); style.id = 'parsely-highlight-styles'; style.textContent = ` /** Smart link highlight styles. */ .smart-link-highlight { outline: 2px solid #3858E9; border-radius: 2px; background-color: #3858E9; color: #ffffff; } `; // ...append the style to the iframe document }, [] );src/rest-api/content-helper/class-endpoint-smart-linking.php (1)
104-110:
⚠️ Potential issueValidation callbacks rely on a namespaced class that is not imported.
Validations\Validate_Url_Exclusion_Listis referenced but theusestatement is missing, leading to fatal errors in PHP < 8.0 where automatic class loading is case-sensitive.+ use Parsely\REST_API\Validations;(or fully-qualified class strings).
src/content-helper/dashboard-page/pages/traffic-boost/provider.ts (2)
596-612: 🛠️ Refactor suggestion
getPostscall lacks error handling – bubble up or default-guard.If the REST
getPostsrequest fails (network error or non-2xx status) the
provider will throw, crashing the whole batch.
Consider wrapping every promise with a try/catch and marking failures so the UI
can still render partial results.
252-258:
⚠️ Potential issuePotential
undefinedpollution in the URL exclusion list.
parsely_canonical_urlmay beundefined; concatenating it intoexcludedUrls
will inject empty-string entries, artificially inflating the list and causing
includes()look-ups to succeed incorrectly.- excludedUrls = [ ...excludedUrls, ...filteredGeneratedSuggestions.map( ( suggestion ) => suggestion.smartLink?.post_data?.parsely_canonical_url ?? '' ) ]; + excludedUrls = [ + ...excludedUrls, + ...filteredGeneratedSuggestions + .map( ( s ) => s.smartLink?.post_data?.parsely_canonical_url ) + .filter( ( url ): url is string => typeof url === 'string' && url.length ) + ];📝 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.const filteredGeneratedSuggestions = newGeneratedSuggestions.filter( ( suggestion ) => ! excludedUrls.includes( suggestion.smartLink?.post_data?.parsely_canonical_url ?? '' ) ); // Update the excluded URLs. excludedUrls = [ ...excludedUrls, ...filteredGeneratedSuggestions .map( ( s ) => s.smartLink?.post_data?.parsely_canonical_url ) .filter( ( url ): url is string => typeof url === 'string' && url.length ) ];src/rest-api/content-helper/class-endpoint-traffic-boost.php (1)
88-128:
⚠️ Potential issue
permission_callbackis not declared for the generated-suggestion route.Without an explicit
permission_callback, the route falls back to the default
one defined inBase_Endpointonly if it overridesregister_rest_route.
When registering routes manually, it is safer to state the capability
requirement up-front:'permission_callback' => array( $this, 'check_read_access' ),This prevents unauthenticated users from POST-ing to
/generate.src/Models/class-inbound-smart-link.php (2)
172-180:
⚠️ Potential issueDuplicate-link check may false-positive on substring matches.
strpos( $post->post_content, $this->href )triggers even when$this->href
appears as a query-string portion of a different URL.
Parse the DOM and search for exact<a href>equality instead to avoid
incorrectly blocking valid placements.
1213-1221:
⚠️ Potential issue
find_line_with_text()relies on newline-delimited HTML.Minified or REST-stripped post content is often single-line, causing the search
to return''and later trigger a fatalstr_replace()failure.
Split on/>boundaries or use DOM + XPath instead for reliable matching.src/Models/class-smart-link.php (3)
900-904: 🛠️ Refactor suggestion
Cache lookup unnecessarily bypassed when result is an empty array
The additional
count( $smart_links ) > 0condition causes a DB query every time the cached value is an empty array (no smart links). Return as soon as the cache hit is non-false to avoid redundant queries.- if ( false !== $smart_links && count( $smart_links ) > 0 ) { + if ( false !== $smart_links ) {
1042-1044:
⚠️ Potential issueInvalid
orderbyvalue passed toWP_Query
'date modified'is not a supportedorderbyvalue. WordPress accepts'modified'(for modified date) or'date'(for published date). The current value will be ignored and the query will default topost_date DESC, altering the intended sort order.- 'orderby' => 'date modified', + 'orderby' => 'modified',📝 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.'orderby' => 'modified', 'order' => 'ASC', ),
1092-1095:
⚠️ Potential issueFix static call to non-static method in
flush_cache()
get_cache_group()is an instance method, but it’s being called statically.
PHP 8 throwsError: Cannot make static call to non-static method, so this will fatally break cache flushes (and anything that triggers them).- wp_cache_delete( 'smart-link-uid-map-' . $this->uid . '-' . $this->source_post_id, self::get_cache_group() ); + wp_cache_delete( + 'smart-link-uid-map-' . $this->uid . '-' . $this->source_post_id, + $this->get_cache_group() + );📝 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.protected function flush_cache(): void { // Delete the cache for the smart link UID to post ID association. wp_cache_delete( 'smart-link-uid-map-' . $this->uid . '-' . $this->source_post_id, $this->get_cache_group() ); }
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
Thanks for debugging and working on this!
acicovic
added
Maintenance & Fixes
acicovic
changed the title
2 participants
Description
In testing, we encountered some "Invalid page template." errors followed by "Failed to accept suggestion. The current link is already linked to this smart link." when attempting to insert links in Traffic Boost:
This was caused by our posts and pages using an out-of-date page template. The error works like this:
wp_insert_page()checks the page_template value saved with the post.page_templatefor the page we're inserting to iswp-custom-template-single-post-page.$wp_error = true, the page template mismatch is returned as an error.wp_insert_page()to change the content with the proper link.In short, I think the root cause is data that uses posts with templates that no longer exist. When we attempt to update these pages, we get an error, and due to the way
wp_insert_page()works, we also successfully insert the link at the same time.To fix this, the PR specifically ignores the
invalid_page_templateerror. The content is already updated, and there's nothing we can do about it at the point we receive the error anyhow.How has this been tested?
I've tested locally with the original data and confirmed that updates to a post with an invalid page template now work correctly.
If you'd like to try testing manually with other data:
add/traffic-boostbranch (without this fix), generate Traffic Boost links for a post. Do not insert yet._wp_page_templatepost meta value to something invalid, e.g.not-a-valid-page-template.fix/invalid-page-template-error._wp_page_templateto an invalid value.