Add traffic boost post list stats#3293
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis change introduces enhanced smart link and traffic boost metadata to REST API responses and frontend data structures, adds campaign metrics support to stats endpoints, and refactors caching strategies for smart link and post ID lookups. The UI is updated with new components for post stats, link overviews, and suggestion bubbles, and the posts table is modularized. Pagination and query context handling are improved throughout. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant REST API
participant SmartLinkModel
participant InboundSmartLinkModel
Frontend->>REST API: GET /posts/{id}
REST API->>SmartLinkModel: get_link_counts(post_id)
SmartLinkModel->>SmartLinkModel: Query & cache inbound/outbound counts
REST API->>InboundSmartLinkModel: get_suggestions_count(post_id)
InboundSmartLinkModel->>InboundSmartLinkModel: Query & cache suggestion count
REST API-->>Frontend: Respond with post data + canonical_url, smart_links, traffic_boost_suggestions_count
Frontend->>REST API: GET /stats/posts?campaign_id=...&use_wp_permalink=...
REST API->>REST API: If use_wp_permalink, expand URLs with permalinks
REST API->>REST API: If campaign params, fetch campaign metrics
REST API-->>Frontend: Respond with post stats + campaign_metrics
Possibly related PRs
Suggested labels
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
alecgeatches
force-pushed
the
add/traffic-boost-post-list-stats
branch
from
9701c47 to
88c7fb1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (2)
src/Models/class-smart-link.php (1)
1042-1044:⚠️ Potential issueInvalid
orderbyvalue – WordPress will ignore it.
WP_Queryaccepts'modified'(or'date') but not'date modified'. The current value silently falls back to default ordering.- 'orderby' => 'date modified', + 'orderby' => 'modified',src/rest-api/stats/class-endpoint-posts.php (1)
246-248: 🛠️ Refactor suggestion
sanitize_url()is not a WordPress core function.Using undefined helpers breaks installs that do not ship custom global functions.
Replace withesc_url_raw()which is the sanctioned equivalent.-return array_map( 'sanitize_url', $urls ); +return array_map( 'esc_url_raw', $urls );Also update the
@sincetag to 3.19.0 if this helper is newly introduced.
♻️ Duplicate comments (1)
src/Models/class-inbound-smart-link.php (1)
1338-1342: Keep cache-group consistency when deleting.After adopting a named cache group above, make sure the deletion mirrors the group:
- wp_cache_delete( 'traffic_boost_suggestions_count_' . $post_id ); + wp_cache_delete( 'traffic_boost_suggestions_count_' . $post_id, 'parsely' );
🧹 Nitpick comments (11)
src/content-helper/dashboard-page/components/posts-table/components/suggestion-bubble.tsx (1)
1-56: Well-structured new component for displaying suggestion counts.This component is well-designed with clear prop types, proper JSDoc comments, and separation of concerns. It effectively leverages React Router for navigation and WordPress components for UI elements.
However, there are a few potential improvements:
- Consider adding ARIA attributes for better accessibility
- The click handler could be memoized with useCallback to prevent unnecessary re-renders
Here's an improved implementation with these enhancements:
/** * External dependencies */ import { useNavigate } from 'react-router'; +import { useCallback } from '@wordpress/element'; /** * WordPress dependencies */ import { Tooltip } from '@wordpress/components'; /** * Type definition for the SuggestionBubble component. * * @since 3.18.0 */ type SuggestionBubbleProps = { postId: number; numberOfSuggestions: number; }; /** * SuggestionBubble component. * * Used to display the number of pending suggestions for a post. * * @since 3.18.0 * * @param {SuggestionBubbleProps} props The props for the SuggestionBubble component. */ export const SuggestionBubble = ( { postId, numberOfSuggestions }: SuggestionBubbleProps ) => { const navigate = useNavigate(); /** * Handles the click event on the suggestion bubble. * * @since 3.18.0 */ - const handleClick = () => { + const handleClick = useCallback(() => { navigate( `/traffic-boost/${ postId }` ); - }; + }, [navigate, postId]); return ( <Tooltip text={ `${ numberOfSuggestions } pending suggestions` } className="suggestion-bubble" > <button className="suggestion-bubble" onClick={ handleClick } + aria-label={`View ${numberOfSuggestions} pending suggestions for post ID ${postId}`} + role="link" > <span className="suggestion-bubble-number">{ numberOfSuggestions }</span> </button> </Tooltip> ); };src/content-helper/dashboard-page/components/posts-table/components/links-overview.tsx (3)
25-39: Simplify conditional rendering logic for inbound links.The code applies a redundant conditional class. Since you're already using conditional rendering with the ternary operator, you don't need to apply the
hiddenclass in the className attribute.- { inboundLinks > 0 ? ( - <Tooltip text={ __( 'Inbound Links', 'wp-parsely' ) } > - <div className={ `boost-inbound ${ inboundLinks > 0 ? '' : 'hidden' }` }> - <Icon size={ 16 } icon={ external } className="boost-link-status-icon-inbound" /> - { inboundLinks } - </div> - </Tooltip> - ) : ( - <div className={ `boost-inbound hidden` }> - <Icon size={ 16 } icon={ external } className="boost-link-status-icon-inbound" /> - </div> - ) } + { inboundLinks > 0 ? ( + <Tooltip text={ __( 'Inbound Links', 'wp-parsely' ) } > + <div className="boost-inbound"> + <Icon size={ 16 } icon={ external } className="boost-link-status-icon-inbound" /> + { inboundLinks } + </div> + </Tooltip> + ) : ( + <div className="boost-inbound hidden"> + <Icon size={ 16 } icon={ external } className="boost-link-status-icon-inbound" /> + </div> + ) }
41-52: Simplify conditional rendering logic for outbound links.Similar to the inbound links section, the code applies a redundant conditional class. Since you're already using conditional rendering with the ternary operator, you don't need to apply the
hiddenclass in the className attribute.- { outboundLinks > 0 ? ( - <Tooltip text={ __( 'Outbound Links', 'wp-parsely' ) } > - <div className={ `boost-outbound ${ outboundLinks > 0 ? '' : 'hidden' }` }> - <Icon size={ 16 } icon={ external } className="boost-link-status-icon-outbound" /> - { outboundLinks } - </div> - </Tooltip> - ) : ( - <div className={ `boost-outbound hidden` }> - <Icon size={ 16 } icon={ external } className="boost-link-status-icon-outbound" /> - </div> - ) } + { outboundLinks > 0 ? ( + <Tooltip text={ __( 'Outbound Links', 'wp-parsely' ) } > + <div className="boost-outbound"> + <Icon size={ 16 } icon={ external } className="boost-link-status-icon-outbound" /> + { outboundLinks } + </div> + </Tooltip> + ) : ( + <div className="boost-outbound hidden"> + <Icon size={ 16 } icon={ external } className="boost-link-status-icon-outbound" /> + </div> + ) }
25-55: Consider enhancing component with accessibility improvements.To improve accessibility, consider adding ARIA attributes to convey the meaning of these icons to screen readers. Also, since these are informational elements rather than interactive controls, ensure they don't receive keyboard focus unnecessarily.
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx (1)
37-45: Improve title truncation logic.The current title truncation doesn't consider word boundaries, which might result in words being cut in the middle. Consider using a utility function that truncates at word boundaries and adds an ellipsis.
- // If the title is longer than 80 characters, truncate it and add an ellipsis. - if ( postTitle !== '' ) { - if ( postTitle.length > 80 ) { - postTitle = postTitle.substring( 0, 80 ) + '…'; - } - } + // If the title is longer than 80 characters, truncate it at word boundary and add an ellipsis. + if ( postTitle !== '' && postTitle.length > 80 ) { + // Find the last space within the 80 character limit + const lastSpace = postTitle.substring( 0, 80 ).lastIndexOf( ' ' ); + // If no space found, just cut at 80, otherwise cut at the last space + const truncateAt = lastSpace > 0 ? lastSpace : 80; + postTitle = postTitle.substring( 0, truncateAt ) + '…'; + }src/content-helper/dashboard-page/components/posts-table/style.scss (2)
79-82: Replace hard-coded red with a design-system variable.
#900000bypasses the shared colour palette and risks breaking dark-mode / theme overrides.
Use an existing variable (e.g.var(--parsely-red)if available) or add one tovariables.scss.- color: #900000; + color: var(--parsely-red);
172-178: Convert fixedpxpadding toto_rem()as per guidelines.The SCSS rule mandates converting dimensions ≥ 3 px:
- padding: 3px to_rem(6px); + padding: to_rem(3px) to_rem(6px);This keeps typography scalable and consistent.
src/content-helper/dashboard-page/components/posts-table/component.tsx (1)
29-31: Remove stray import inside JSDoc – it renders in docs and confuses readers.The extra
import { SinglePostRow … }line is inside the comment block, not code, but ships in generated docs and violates the “each line comment ends with a period.” rule.
Delete it to keep docs clean.src/content-helper/common/providers/stats-provider.tsx (1)
89-94: Prefer explicit class reference in static methods.Using
thisinside a static context can be confusing becausethisis bound to the constructor function rather than an instance. Replacing it with the class name makes the intent crystal-clear and satisfies the Biome rulenoThisInStatic.- if ( ! this.instance ) { - this.instance = new StatsProvider(); + if ( ! StatsProvider.instance ) { + StatsProvider.instance = new StatsProvider(); } - return this.instance; + return StatsProvider.instance;🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 91-91: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 93-93: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
src/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsx (1)
165-171: Skip boost label when percentage is zero.Right now
trafficBoostPercentagemight be0(orNaNafter the fix). Showing “0.0% BOOSTED” provides no value and may confuse users.Add a simple guard:
-{ ! isLoadingStats && trafficBoostPercentage > 0 && ( +{ ! isLoadingStats && trafficBoostPercentage > 0 && Number.isFinite( trafficBoostPercentage ) && (src/content-helper/common/providers/base-wordpress-provider.tsx (1)
274-278: Avoid leakingrest_endpointto the REST API query string.
rest_endpointis an internal flag yet it is spread into theaddQueryArgs()parameter list, resulting in a query paramrest_endpoint=/wp/v2/poststhat WordPress will silently ignore but clutters logs and can break caching keys.-const posts = await this.apiFetch<Post[]>( { - path: addQueryArgs( restEndpoint, { ...queryParams, _embed: true, context } ), +const { rest_endpoint, ...apiParams } = queryParams; +const posts = await this.apiFetch<Post[]>( { + path: addQueryArgs( restEndpoint, { ...apiParams, _embed: true, context } ),Doing the same for
getPages()keeps behaviour consistent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
build/content-helper/dashboard-page-rtl.cssis excluded by!build/**build/content-helper/dashboard-page.asset.phpis excluded by!build/**build/content-helper/dashboard-page.cssis excluded by!build/**build/content-helper/dashboard-page.jsis excluded by!build/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
package.json(1 hunks)src/Endpoints/class-rest-metadata.php(3 hunks)src/Models/class-inbound-smart-link.php(6 hunks)src/Models/class-smart-link.php(7 hunks)src/content-helper/common/components/thumbnail/component.tsx(1 hunks)src/content-helper/common/providers/base-provider.tsx(1 hunks)src/content-helper/common/providers/base-wordpress-provider.tsx(8 hunks)src/content-helper/common/providers/stats-provider.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/component.tsx(6 hunks)src/content-helper/dashboard-page/components/posts-table/components/links-overview.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/components/suggestion-bubble.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/style.scss(5 hunks)src/content-helper/dashboard-page/pages/dashboard/page-component.tsx(3 hunks)src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx(3 hunks)src/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-counter.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-options-panel.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/preview/components/preview-header.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/preview/preview.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/provider.ts(3 hunks)src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/add-new-link-button.tsx(2 hunks)src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/header.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/post-details.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/tabs/suggestions-tab.tsx(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/store.ts(1 hunks)src/content-helper/dashboard-page/provider.tsx(1 hunks)src/content-helper/dashboard-widget/provider.ts(1 hunks)src/content-helper/editor-sidebar/excerpt-suggestions/provider.ts(1 hunks)src/content-helper/editor-sidebar/performance-stats/provider.ts(1 hunks)src/content-helper/editor-sidebar/related-posts/provider.ts(1 hunks)src/content-helper/editor-sidebar/smart-linking/provider.ts(1 hunks)src/content-helper/editor-sidebar/title-suggestions/provider.ts(1 hunks)src/rest-api/content-helper/class-endpoint-smart-linking.php(4 hunks)src/rest-api/content-helper/class-endpoint-traffic-boost.php(1 hunks)src/rest-api/stats/class-endpoint-posts.php(3 hunks)src/rest-api/stats/trait-post-data.php(2 hunks)src/services/content-api/class-content-api-service.php(1 hunks)tests/Integration/Endpoints/RestMetadataTest.php(6 hunks)tests/Integration/RestAPI/Stats/EndpointPostsTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the code to ensure it is well-structured and adheres to best ...
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/editor-sidebar/excerpt-suggestions/provider.tssrc/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-options-panel.tsxsrc/content-helper/editor-sidebar/performance-stats/provider.tssrc/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/tabs/suggestions-tab.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/preview/components/preview-header.tsxsrc/content-helper/common/components/thumbnail/component.tsxsrc/content-helper/editor-sidebar/smart-linking/provider.tssrc/content-helper/editor-sidebar/related-posts/provider.tssrc/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/header.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/post-details.tsxsrc/content-helper/editor-sidebar/title-suggestions/provider.tssrc/content-helper/common/providers/base-provider.tsxsrc/content-helper/dashboard-widget/provider.tssrc/content-helper/dashboard-page/pages/traffic-boost/store.tssrc/content-helper/dashboard-page/pages/traffic-boost/preview/preview.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-counter.tsxsrc/content-helper/dashboard-page/pages/dashboard/page-component.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/add-new-link-button.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/page-component.tsxsrc/content-helper/dashboard-page/provider.tsxsrc/content-helper/dashboard-page/components/posts-table/components/suggestion-bubble.tsxsrc/content-helper/dashboard-page/components/posts-table/components/post-details.tsxsrc/content-helper/dashboard-page/pages/traffic-boost/provider.tssrc/content-helper/dashboard-page/components/posts-table/components/links-overview.tsxsrc/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsxsrc/content-helper/common/providers/stats-provider.tsxsrc/content-helper/dashboard-page/components/posts-table/component.tsxsrc/content-helper/common/providers/base-wordpress-provider.tsx
`**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the HTML and PHP code to ensure it is well-structured and adheres ...
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/services/content-api/class-content-api-service.phpsrc/rest-api/content-helper/class-endpoint-traffic-boost.phptests/Integration/RestAPI/Stats/EndpointPostsTest.phpsrc/rest-api/content-helper/class-endpoint-smart-linking.phpsrc/rest-api/stats/trait-post-data.phpsrc/Endpoints/class-rest-metadata.phpsrc/Models/class-smart-link.phpsrc/Models/class-inbound-smart-link.phpsrc/rest-api/stats/class-endpoint-posts.phptests/Integration/Endpoints/RestMetadataTest.php
`**/*.{css,scss}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the SCSS code to ensure it is well-structured and adheres to best ...
**/*.{css,scss}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the SCSS code to ensure it is well-structured and adheres to best practices.
- Convert dimensions greater than or equal to 3px to rem units using the to_rem function.
- Utilize variables for sizes and colors defined in src/content-helper/common/css/variables.scss instead of hardcoding values."
src/content-helper/dashboard-page/components/posts-table/style.scss
🧬 Code Graph Analysis (8)
src/rest-api/content-helper/class-endpoint-traffic-boost.php (1)
src/Models/class-smart-link.php (1)
flush_all_cache(1193-1201)
src/rest-api/content-helper/class-endpoint-smart-linking.php (1)
src/Models/class-smart-link.php (1)
flush_all_cache(1193-1201)
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx (3)
src/content-helper/common/providers/base-wordpress-provider.tsx (1)
HydratedPost(79-85)src/content-helper/common/components/thumbnail/component.tsx (1)
Thumbnail(32-60)src/content-helper/dashboard-page/components/posts-table/components/suggestion-bubble.tsx (1)
SuggestionBubble(30-55)
src/content-helper/dashboard-page/pages/traffic-boost/provider.ts (1)
src/content-helper/common/content-helper-error.tsx (1)
ContentHelperError(56-228)
src/content-helper/common/providers/stats-provider.tsx (2)
src/content-helper/common/content-helper-error.tsx (1)
ContentHelperError(56-228)src/content-helper/common/providers/base-wordpress-provider.tsx (1)
HydratedPost(79-85)
src/Models/class-smart-link.php (3)
src/Models/class-smart-link-status.php (3)
Smart_Link_Status(19-46)get_all_statuses(43-45)is_valid_status(32-34)src/rest-api/content-helper/class-endpoint-smart-linking.php (1)
get_smart_links(273-290)src/Models/class-inbound-smart-link.php (1)
flush_cache_by_post_id(1338-1342)
src/Models/class-inbound-smart-link.php (2)
src/Models/class-smart-link.php (2)
flush_all_cache(1193-1201)flush_cache_by_post_id(1222-1239)src/Models/class-smart-link-status.php (1)
Smart_Link_Status(19-46)
tests/Integration/Endpoints/RestMetadataTest.php (3)
tests/Integration/TestCase.php (3)
get_permalink(315-323)get_post_in_array(275-283)get_post(249-264)src/Endpoints/class-rest-metadata.php (1)
get_callback(74-130)src/class-parsely.php (2)
Parsely(75-1152)get_canonical_url(1012-1031)
🪛 Biome (1.9.4)
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx
[error] 56-56: 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/common/providers/stats-provider.tsx
[error] 90-90: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 91-91: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 93-93: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (54)
src/content-helper/editor-sidebar/performance-stats/provider.ts (1)
15-15: Approve import path alignment.
TheBaseProviderimport has been updated to the newprovidersdirectory, reflecting the reorganized module structure. This aligns with similar updates across other provider modules.src/content-helper/editor-sidebar/related-posts/provider.ts (1)
9-9: Approve import path update.
TheBaseProviderimport path has been correctly updated to../../common/providers/base-provider, matching the repository’s new layout for provider classes.src/content-helper/common/components/thumbnail/component.tsx (1)
10-10: Approve updated type import.
TheHydratedPosttype import path has been updated to theprovidersdirectory, aligning with the refactored module organization.src/content-helper/editor-sidebar/smart-linking/provider.ts (1)
11-11: Approve provider import adjustment.
TheBaseProviderimport path now points to../../common/providers/base-provider, ensuring consistency with the codebase-wide refactoring of provider modules.src/content-helper/editor-sidebar/excerpt-suggestions/provider.ts (1)
9-9: Import path correctly updated for BaseProvider.The import has been adjusted to use the new
common/providersdirectory. Ensure that the filesrc/content-helper/common/providers/base-provider.tsexists and exportsBaseProvider, matching other provider imports.src/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-options-panel.tsx (1)
16-16: Align HydratedPost import to new providers structure.The
HydratedPosttype import now points to thecommon/providersfolder. Confirm thatbase-wordpress-provider.tsexportsHydratedPostand the relative path is correct.src/content-helper/dashboard-page/provider.tsx (1)
4-4: Update BaseWordPressProvider import path.The import of
BaseWordPressProviderhas been redirected tocommon/providersto match the reorganized module layout. Verify that this module path is valid and consistent across dashboard components.src/content-helper/dashboard-page/pages/traffic-boost/preview/components/preview-header.tsx (1)
13-13: Correct HydratedPost import location.The
HydratedPostimport now references theproviderssubdirectory. Ensure this aligns with other traffic-boost components and that the provider export is intact.src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/post-details.tsx (1)
11-11: Adjust HydratedPost import to new path.The import path for
HydratedPosthas been updated to thecommon/providersdirectory. Confirm consistency with similar refactors in related UI components.src/content-helper/dashboard-page/pages/traffic-boost/preview/components/link-counter.tsx (1)
12-12: Consistent import path corrected.
The path forHydratedPostnow aligns with the newprovidersdirectory structure and matches other Traffic Boost components. No functional changes introduced.src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/header.tsx (1)
11-11: Updated import source forHydratedPost.
Importing from theproviderssubdirectory standardizes the module path. No changes to component logic.src/content-helper/common/providers/base-provider.tsx (1)
11-11: Adjusted import path for error classes.
Switched to a parent‐directory import forContentHelperErrorandContentHelperErrorCode, reflecting the updated file hierarchy. Behaviour is unchanged.src/content-helper/dashboard-page/pages/traffic-boost/store.ts (2)
10-10: StandardizeHydratedPostimport path.
Now references theprovidersdirectory, in line with other refactors.
12-12: ReorderedTrafficBoostLinkimport.
Moving this import belowLinkTypegroups related imports logically. No functional impact.src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/tabs/suggestions-tab.tsx (1)
14-14: Corrected provider import path forHydratedPost.
Aligns with the reorganizedcommon/providersmodule layout; logic remains intact.src/content-helper/dashboard-widget/provider.ts (1)
14-15: Import paths reorganized correctly.The import paths have been updated to reflect the new directory structure, moving from direct imports to the new modular organization under the 'providers' directory. This change maintains consistency with other files in the codebase.
src/content-helper/editor-sidebar/title-suggestions/provider.ts (1)
10-11: Import paths properly restructured.The import paths have been updated to reflect the new directory structure, moving BaseProvider to the 'providers' subdirectory. The reordering of imports also improves code organization by grouping related imports together.
src/rest-api/content-helper/class-endpoint-traffic-boost.php (1)
827-829: Good addition for cache invalidation.Adding the cache flush after successfully applying a smart link ensures that subsequent API calls will reflect the latest state of the link, which is critical for consistent data across the application. This is especially important since the link is being changed in the system.
src/content-helper/dashboard-page/pages/traffic-boost/preview/preview.tsx (1)
15-15: Import path correctly updated.The import path for HydratedPost has been updated to reflect the new directory structure, maintaining consistency with other provider-related imports across the codebase.
tests/Integration/RestAPI/Stats/EndpointPostsTest.php (1)
325-334: New parameter validation for posts endpoint.This change updates the test to validate that the new
use_wp_permalinkparameter is correctly included in the API response with its default value set tofalse. This parameter allows controlling whether URLs are returned as WordPress permalinks or canonical URLs.src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/add-new-link-button.tsx (2)
14-14: Import path correction.The import path has been updated to reflect the new file structure, using the provider from the dedicated providers directory.
98-112: Updated PostsTable component with external pagination and context.The component has been updated with three important additions:
- External pagination controls (
currentPageandsetCurrentPage)- Setting
context: 'edit'in the query parameters- Adding
hideSuggestionBubbleto control UI element visibilityThis aligns with the broader refactoring that externalizes pagination state management.
src/content-helper/dashboard-page/pages/dashboard/page-component.tsx (4)
9-9: Added useState import for pagination management.The component now imports the useState hook from WordPress element to manage pagination state.
15-15: Updated import path for DashboardHeader component.The import path has been simplified, removing unnecessary relative path segments.
66-66: Added external pagination state management.The component now maintains pagination state using useState, initializing
currentPageto 1. This state will be passed to the PostsTable component.
83-90: Updated PostsTable component with pagination props and explicit query parameters.The PostsTable component now receives external pagination control via the
currentPageandsetCurrentPageprops, along with explicit query parameters for fetching published posts with a limit of 5 per page.This change aligns with the broader refactoring to externalize pagination state management across the application.
src/rest-api/content-helper/class-endpoint-smart-linking.php (4)
346-348: Added cache invalidation after adding a smart link.Cache flushing has been added immediately after saving a smart link to ensure data consistency. This maintains the integrity of count-based metrics and ensures that newly added links appear in statistics immediately.
405-406: Added cache invalidation in bulk smart link addition.Cache flushing is now performed for each smart link after it's successfully saved in the bulk addition process. This maintains data consistency and ensures metrics are up-to-date.
478-479: Added cache invalidation after removing a smart link.Cache flushing has been added after deleting a smart link to ensure data consistency. This maintains the integrity of count-based metrics and ensures that removed links are immediately reflected in statistics.
500-501: Added cache invalidation for each smart link in set operation.Cache flushing is now performed for each smart link after it's successfully saved in the set_smart_links operation. This ensures that any cached data is properly invalidated after modifications.
src/content-helper/dashboard-page/pages/traffic-boost/provider.ts (3)
6-6: Import path updated to reflect new module structure.The import path has been updated to use the new providers directory structure, which improves code organization by grouping related provider classes together.
267-274: Improved error handling for abort errors.The error handling has been enhanced to specifically identify and propagate abort errors, which is crucial for proper cancellation handling in asynchronous operations. This change correctly distinguishes between abort-related errors (which should be re-thrown) and other errors (which are logged and lead to retry decrements).
This implementation is robust as it handles both DOM
AbortErrorexceptions andContentHelperErrorinstances with the specific code.
613-613: Added 'edit' context to request options.Adding the
context: 'edit'parameter to thegetPostsrequest ensures the API returns enriched metadata, which is necessary for the traffic boost feature to properly display information like suggestion counts.src/services/content-api/class-content-api-service.php (1)
191-191: Updated PHPDoc parameter type to support nested array structures.The type annotation for the
$paramsparameter has been refined to explicitly allow for nested arrays, which correctly documents the more complex parameter structures now being passed to the content API service, such as campaign parameters.This change improves code documentation accuracy without modifying the actual implementation logic.
src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx (4)
22-25: Enhanced search functionality to reset pagination.The debounced search function now appropriately resets the current page to 1 when a search is performed, which provides a better user experience by showing the first page of search results.
27-27: Added state management for pagination.Adding state management for the current page allows the component to control pagination externally and pass it down to the PostsTable component, which is a good pattern for state management.
45-47: Lifted pagination state to parent component.Passing the pagination state and setter to the PostsTable component enables external control of pagination, which is a good React pattern for lifting state up when needed by parent components.
51-51: Enhanced search functionality with targeted column search.Adding the
search_columnsparameter to target both post titles and excerpts provides more comprehensive search results for users, improving the overall search experience.src/content-helper/dashboard-page/components/posts-table/components/links-overview.tsx (2)
1-16: Type definition looks good.The type definition for
LinksOverviewPropsis well-structured with proper JSDoc comments and an appropriate@sincetag indicating the version this component will be introduced in.
18-24: Good documentation of the component purpose.The JSDoc for the component itself provides a clear description of its purpose and properly references the type definition using the
@paramtag.src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx (3)
1-23: Props interface is well-defined.The PostDetailsProps type definition is clear and includes appropriate JSDoc comments with the correct
@sincetag. The use of an optional boolean prop with a default value is a good practice.
59-61: Good conditional rendering of SuggestionBubble.The component correctly renders the SuggestionBubble only when there are suggestions and when the showSuggestionBubble prop is true. This creates a clean and flexible API for the component.
55-57:⚠️ Potential issueAddress potential XSS vulnerability with dangerouslySetInnerHTML.
Using
dangerouslySetInnerHTMLwith post content can expose users to cross-site scripting (XSS) attacks if the content contains malicious code. Consider sanitizing the HTML content or using a safer method to render the title.- { postTitle !== '' - ? <span title={ post.title.rendered } dangerouslySetInnerHTML={ { __html: postTitle } } /> - : __( '(no title)', 'wp-parsely' ) - } + { postTitle !== '' + ? <span title={ post.title.rendered }>{ wp.element.RawHTML ? + <wp.element.RawHTML>{ postTitle }</wp.element.RawHTML> : + postTitle.replace(/…/g, '...') } + </span> + : __( '(no title)', 'wp-parsely' ) + }⛔ Skipped due to learnings
Learnt from: vaurdan PR: Parsely/wp-parsely#3149 File: src/content-helper/dashboard-page/pages/traffic-boost/preview/components/preview-header.tsx:152-152 Timestamp: 2025-03-06T10:24:43.274Z Learning: The WordPress REST API sanitizes HTML content in post titles before it's returned, so using dangerouslySetInnerHTML with title.rendered is considered acceptable within this codebase despite typical security concerns.Learnt from: vaurdan PR: Parsely/wp-parsely#3149 File: src/content-helper/dashboard-page/pages/traffic-boost/sidebar/components/links-list/single-link.tsx:76-79 Timestamp: 2025-03-06T10:24:30.310Z Learning: Properties with the `.rendered` suffix from WordPress REST API (like `post.title.rendered`, `post.content.rendered`, etc.) are pre-sanitized by WordPress and safe to use with `dangerouslySetInnerHTML` in React components.🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
src/rest-api/stats/trait-post-data.php (3)
104-106: Good implementation of new recirculation_rate metric.The new code correctly extracts and formats the recirculation_rate metric from the metrics array, following the same pattern as the existing metrics.
123-142: Well-structured campaign metrics extraction.The code for handling campaign metrics follows the established pattern for regular metrics, making it consistent and maintainable. The extraction and formatting of each metric is clear and properly documented.
144-153: Improved URL handling with canonical URL update.The replacement of the direct call to
url_to_postidwithUtils::get_post_id_by_urlis a good improvement. The code now also updates the canonical URL when a valid post ID is found, which helps maintain consistency in the data.However, consider adding error logging for cases where a valid post ID can't be found:
// If we have a post ID, update the post canonical URL. if ( 0 !== $post_id ) { Parsely::set_canonical_url( $post_id, $post_url ); + } else { + // Log that we couldn't find a post ID for this URL if debugging is enabled + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( sprintf( 'Parse.ly: Could not find post ID for URL: %s', $item['url'] ) ); + } }tests/Integration/Endpoints/RestMetadataTest.php (5)
197-216: Good test updates for canonical_url and smart links.The test correctly validates the presence of the new fields in the REST API response, including
canonical_url,smart_linkswith inbound and outbound counts, andtraffic_boost_suggestions_count. The use of explicit type casting for the permalink is also a good practice.
264-277: Consistent test implementation for filtered responses.The test for filtered responses follows the same pattern as the main test, ensuring that the new fields are properly included even when certain parts of the response are filtered out.
326-343: Well-structured test for URL filtering.The test for URL filtering validates that the new fields are still present when URL tracking is disabled, maintaining consistency with the rest of the test suite.
365-372: Good handling of non-existent post case.The test for non-existent posts correctly verifies that the new fields are present with appropriate default values when the post doesn't exist, ensuring robust error handling.
420-424: Improved type documentation for post IDs.The addition of explicit type annotations for the post ID variables enhances code clarity and helps maintain type safety throughout the codebase.
Also applies to: 488-492
src/Endpoints/class-rest-metadata.php (1)
123-128: Gracefully handle possible WP_Error returns from helper methods.
Smart_Link::get_link_counts()andInbound_Smart_Link::get_suggestions_count()currently return scalars, but nothing in their contract guarantees they will never surface aWP_Error.
Guarding against this prevents a fatal JSON-encoding failure:-$response['smart_links'] = Smart_Link::get_link_counts( $post_id, Smart_Link_Status::APPLIED ); +$counts = Smart_Link::get_link_counts( $post_id, Smart_Link_Status::APPLIED ); +if ( ! is_wp_error( $counts ) ) { + $response['smart_links'] = $counts; +}The same pattern applies to the suggestions count.
src/content-helper/dashboard-page/components/posts-table/component.tsx (1)
198-216: Potential duplicate-stat entries when retrying with permalinks.
onErrorLoadingStatspushes the new stat unconditionally; if the first call later succeeds you’ll have two entries for the same post ID.
Guard before pushing:-setStats( ( prev ) => [ ...prev, fetchedStats[ 0 ] ] ); +setStats( ( prev ) => + prev.some( ( s ) => s.postId === post.id ) + ? prev + : [ ...prev, fetchedStats[ 0 ] ] +);src/rest-api/stats/class-endpoint-posts.php (1)
190-216: Add validation / sanitization callbacks for new campaign parameters.Unlike
author,section, etc., the five newcampaign_*parameters lacksanitize_callbackandvalidate_callback.
At minimum these should be:'sanitize_callback' => 'sanitize_text_field', 'validate_callback' => '__return_true',This prevents arbitrary HTML injection via the REST API.
Would you like me to open a follow-up PR that adds proper callbacks to all campaign parameters?
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx
Show resolved
Hide resolved
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx
Show resolved
Hide resolved
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
Hey @alecgeatches, thank you so much for picking this up and continuing the work.
I'm halfway through my review and leaving a couple of comments. I'll also be pushing a nitpicking PR (spacing/comments) very shortly.
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx
Show resolved
Hide resolved
src/content-helper/dashboard-page/components/posts-table/components/suggestion-bubble.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/content-helper/common/providers/stats-provider.tsx (1)
133-143:urlsmay be unintentionally overridden – spread order matters.By spreading
argsafter the{ urls: postURLs }object, a caller could pass its ownurlsarray and silently shadow the URLs derived fromposts, defeating the method's purpose.- return this.getStats( { urls: postURLs, ...args } ); + return this.getStats( { ...args, urls: postURLs } );This tiny re-ordering guarantees that the computed
postURLsalways win while still allowing other parameters to be overridden.
🧹 Nitpick comments (2)
src/content-helper/common/providers/stats-provider.tsx (2)
79-95: Address staticthisusage in the singleton implementation.The static analysis tool flagged the use of
thisin a static context which can lead to confusion.public static getInstance(): StatsProvider { - if ( ! this.instance ) { - this.instance = new StatsProvider(); - } - return this.instance; + if ( ! StatsProvider.instance ) { + StatsProvider.instance = new StatsProvider(); + } + return StatsProvider.instance; }🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 91-91: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 94-94: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
133-140: Consider adding null/empty array check for posts parameter.There's no validation to ensure that the posts array is not empty. If an empty array is passed, the method will make an API call with an empty URLs array, which may not be intended behavior.
public async getStatsForPosts( posts: HydratedPost[], args: StatsRequestParams ): Promise<PostStats[]> { + if (!posts || posts.length === 0) { + throw new ContentHelperError( + __( 'No posts provided to get stats for.', 'wp-parsely' ), + ContentHelperErrorCode.InvalidInput + ); + } const postURLs = posts.map( ( post ) => { if ( args.use_wp_permalink ) { return post.link; } return post.parsely?.canonical_url ?? post.link; } ); return this.getStats( { urls: postURLs, ...args } ); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Models/class-inbound-smart-link.php(6 hunks)src/content-helper/common/providers/stats-provider.tsx(1 hunks)src/content-helper/dashboard-page/components/posts-table/components/links-overview.tsx(1 hunks)src/content-helper/dashboard-page/pages/dashboard/page-component.tsx(4 hunks)src/content-helper/dashboard-page/pages/traffic-boost/provider.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/content-helper/dashboard-page/pages/traffic-boost/provider.ts
- src/content-helper/dashboard-page/pages/dashboard/page-component.tsx
- src/content-helper/dashboard-page/components/posts-table/components/links-overview.tsx
- src/Models/class-inbound-smart-link.php
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the code to ensure it is well-structured and adheres to best ...
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/common/providers/stats-provider.tsx
🪛 Biome (1.9.4)
src/content-helper/common/providers/stats-provider.tsx
[error] 90-90: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 91-91: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 94-94: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: E2E against WordPress latest
- GitHub Check: build
🔇 Additional comments (6)
src/content-helper/common/providers/stats-provider.tsx (6)
1-13: Import structure looks good.The file maintains clean separation between WordPress and internal dependencies with proper JSDoc blocks. This follows WordPress coding standards.
14-24: Well-defined PostMetrics interface with proper documentation.The interface provides a clear type definition for metrics data with appropriate JSDoc including the required @SInCE tag.
26-42: PostStats interface looks good with campaign metrics support.The interface properly extends PostMetrics and includes the new campaign field to support traffic boost tracking. All fields are properly typed and documented.
44-70: StatsRequestParams interface includes necessary campaign tracking parameters.The interface correctly implements all the required parameters for campaign tracking (campaign_id, campaign_medium, campaign_source, campaign_content, campaign_term, itm_source) which aligns with the PR objective to track traffic boost links.
106-121: The getStats implementation looks good.The method properly handles error cases by throwing a ContentHelperError when no data is returned. The WordPress internationalization function is correctly used for the error message.
134-140: Good fallback mechanism for URLs.The code correctly handles cases where Parse.ly canonical URLs might not be available by falling back to the post link, ensuring robust behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/content-helper/dashboard-page/components/posts-table/component.tsx (2)
155-166: Consider extracting campaign parameters as constants.The campaign tracking parameters like 'wp-parsely', 'smart-link' appear to be hard-coded values that align with the PR objectives. For better maintainability, consider extracting these as constants.
+// Campaign tracking parameters +const CAMPAIGN_ID = 'wp-parsely'; +const CAMPAIGN_MEDIUM = 'smart-link'; +const PERIOD_START = '30d'; + const [ isLoadingStats, setIsLoadingStats ] = useState<boolean>( true ); const didFirstSearch = useRef( false ); /** * Fetches posts from the API, using the query and pagination. * * @since 3.18.0 */ useEffect( () => { const fetchPosts = async () => { try { const fetchedPosts = await DashboardProvider.getInstance().getPosts( { context: 'embed', ...query, per_page: itemsPerPage, page: currentPage, } ); if ( ! hideStats ) { // Fetch the stats for the posts. const fetchedStats = await StatsProvider.getInstance().getStatsForPosts( fetchedPosts.data, { limit: fetchedPosts.data.length, - period_start: '30d', + period_start: PERIOD_START, page: 1, - campaign_id: 'wp-parsely', - campaign_medium: 'smart-link', + campaign_id: CAMPAIGN_ID, + campaign_medium: CAMPAIGN_MEDIUM, } );
200-207: Duplicate campaign parameters should be extracted as constants.Similar to the previous suggestion, these campaign parameters should use the same constants to ensure consistency.
try { // Try to fetch the stats again for this URL, but instead try with the WordPress permalink. // The API will then try to convert the permalink to a canonical URL, and use // both the permalink and the canonical URL to fetch the stats. const fetchedStats = await StatsProvider.getInstance().getStatsForPosts( [ post ], { limit: 1, - period_start: '30d', + period_start: PERIOD_START, page: 1, - campaign_id: 'wp-parsely', - campaign_medium: 'smart-link', + campaign_id: CAMPAIGN_ID, + campaign_medium: CAMPAIGN_MEDIUM, use_wp_permalink: true, } );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/content-helper/dashboard-page/components/posts-table/component.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the code to ensure it is well-structured and adheres to best ...
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/dashboard-page/components/posts-table/component.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: E2E against WordPress latest
🔇 Additional comments (10)
src/content-helper/dashboard-page/components/posts-table/component.tsx (10)
9-9: Code looks good, but could add JSDoc comments for imported hooks.The updated import now includes
useCallback,useEffect,useRef, anduseStatehooks, which is appropriate for the component's enhanced functionality.
19-22: Imports look good, properly organized for new functionality.The added imports for
HydratedPost,QueryParams,PostStats, andStatsProvidersupport the new stats tracking functionality, and theSinglePostRowimport facilitates the component refactoring.
93-94: Good architectural change moving pagination control to parent component.Lifting state up by making
currentPageandsetCurrentPagerequired props enables better control from parent components and improves component reusability.
98-99: Well-structured configuration flags for UI customization.Adding
hideStatsandhideActionsas configurable props improves the component's flexibility and allows for different use cases.
131-131: Good separation of loading states for posts and stats.The separate state variables for posts and stats loading provides better granularity for UI feedback.
Also applies to: 137-137
184-184: Ensure dependency array is correct for the useEffect hook.The addition of
hideStatsto the dependency array is correct since the effect's behavior changes based on this prop.
195-213: Well-implemented error recovery for stats loading.The
onErrorLoadingStatscallback function is a good implementation of graceful degradation. It attempts to fetch stats using WordPress permalink when the initial attempt fails, providing better user experience.
241-243: Good use of the useCallback hook for optimization.The
getStatsForPosthelper function is correctly memoized withuseCallbackto prevent unnecessary re-renders when passed to child components.
290-295: Table headers correctly updated to reflect the new stats functionality.The table header is now appropriately labeled as "VIEWS (30 days)" to match the stats being displayed.
302-313: Good refactoring to use the SinglePostRow component.Replacing inline JSX with a dedicated component improves code organization and maintainability. All necessary props are correctly passed to the component.
src/content-helper/dashboard-page/components/posts-table/component.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsx
Outdated
Show resolved
Hide resolved
acicovic
dismissed
their stale review
I hit the "Approve" button by mistake, I meant to hit "Comment".
|
Also, a few notes/observations about caching:
I don't think that addressing all the above questions is within the scope of this PR, but addressing what we can in the new code in order not to further increase any inconsistencies would definitely be desirable. I'd be interested to hear your thoughts on all of these. |
…y/wp-parsely into add/traffic-boost-post-list-stats
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/content-helper/dashboard-page/components/posts-table/component.tsx (1)
290-296: 🛠️ Refactor suggestionMake the view period string translatable.
The "(30 days)" string should be translatable to ensure proper localization.
- <span className="views-header-period">{ __( '(30 days)', 'wp-parsely' ) }</span> + <span className="views-header-period"> + { /* translators: %s: number of days */ } + { __( '(%s days)', 'wp-parsely' ).replace( '%s', '30' ) } + </span>
🧹 Nitpick comments (1)
src/content-helper/dashboard-page/components/posts-table/component.tsx (1)
186-213: Robust error handling for stats loading.The onErrorLoadingStats callback provides a good fallback mechanism when initial stats loading fails by using WordPress permalinks. This aligns with the PR objective of tracking link performance reliably.
Consider adding user-facing error messaging instead of just logging to console, especially for production environments.
} catch ( error ) { console.error( error ); // eslint-disable-line no-console + // Consider adding user-facing error notification here }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
build/content-helper/dashboard-page.asset.phpis excluded by!build/**build/content-helper/dashboard-page.jsis excluded by!build/**
📒 Files selected for processing (2)
src/content-helper/dashboard-page/components/posts-table/component.tsx(6 hunks)src/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/content-helper/dashboard-page/components/posts-table/components/single-post-row.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the code to ensure it is well-structured and adheres to best ...
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/dashboard-page/components/posts-table/component.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: E2E against WordPress latest
🔇 Additional comments (11)
src/content-helper/dashboard-page/components/posts-table/component.tsx (11)
9-9: Import organization looks good.The imports are now properly organized with all necessary hooks from WordPress element.
19-22: Properly structured imports for new statistics functionality.The imports for new types and components are well-organized. The SinglePostRow component is now correctly imported once at the top of the file.
93-94: Good refactoring to external pagination control.Making the pagination state externally controlled improves component reusability and allows parent components to manage the state appropriately.
98-99: Appropriate addition of toggle props for UI elements.The new hideStats and hideActions props provide good flexibility for controlling what UI elements are displayed.
117-118: Props correctly destructured from new interface.The props are properly destructured to match the updated PostsTableType interface.
Also applies to: 122-123
131-132: Well-structured state management for stats functionality.The new state variables for stats and loading state are properly initialized.
Also applies to: 137-138
155-169: Stats fetching logic looks well implemented.The conditional stats fetching based on the hideStats prop is appropriately implemented. The campaign parameters align with PR objectives for tracking Traffic Boost links.
176-177: Loading state management is properly handled.Both isLoading and isLoadingStats states are correctly reset and updated during the fetch process, and hideStats is correctly included in the dependencies array.
Also applies to: 181-184
233-243: Well-implemented stats helper function.The getStatsForPost helper function is properly memoized with useCallback and efficiently finds stats for a specific post based on its ID.
302-313: Good refactoring to use SinglePostRow component.The refactoring to use the SinglePostRow component improves code modularity and maintainability. All necessary props are passed down correctly, including the stats data and loading states.
149-153:Details
❓ Verification inconclusive
Changed context parameter to 'embed'.
Using 'embed' context can improve performance by requesting less data. Make sure this context provides all the necessary fields needed for rendering.
🏁 Script executed:
#!/bin/bash # Check what fields are included in the 'embed' context versus other contexts rg -A 5 -B 5 "register_rest_field.*post" --glob "*.php"Length of output: 57
Confirm required fields are present when using ‘embed’ context
Changing
contextto'embed'reduces payload size by returning only embedded fields. Before merging, double-check that all properties the PostsTable component relies on are still included in the REST response, for example:
iddate(ordate_gmt)title.renderedexcerpt.renderedsluglink_embedded.author[0].name_embedded['wp:featuredmedia'][0].source_urlInspect the API response in your browser’s network tab or via a curl request to confirm.
|
Good questions and comments, @acicovic!
I have now replied to that comment, the area of concern is actually cached.
It's possible to rely on side-effects to clear cache (e.g. If we update the other cache uses, I think
This is very true. I'm going to have a go at standardizing these uses.
I think this, along with a reasonable upper bound on cache expiration, is the correct way to do things. |
|
Hey @alecgeatches, thank you for your comments. Everything makes sense! |
…roup and key structure.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
src/Metadata/class-metadata-builder.php (1)
413-420:⚠️ Potential issueGuard against
$parentnot being aWP_Term.After tightening the loop condition, the function can still exit with
$parent === false(e.g. the original term has been deleted).
Accessing$parent->nameon line 419 would then trigger a fatal error.- return $parent->name ?? false; + // Bail out early if the term chain could not be resolved. + if ( ! ( $parent instanceof WP_Term ) ) { + return false; + } + + return $parent->name;src/services/class-cached-service-endpoint.php (1)
88-100:⚠️ Potential issuePass resolved cache-group instead of a possibly undefined constant.
Same undefined-constant issue surfaces at every
wp_cache_get/setcall.- $cache = wp_cache_get( $cache_key, PARSELY_CACHE_GROUP ); + $group = defined( 'Parsely\\PARSELY_CACHE_GROUP' ) ? PARSELY_CACHE_GROUP : 'wp-parsely'; + $cache = wp_cache_get( $cache_key, $group ); … - wp_cache_set( $cache_key, $response, PARSELY_CACHE_GROUP, $this->cache_ttl ); + wp_cache_set( $cache_key, $response, $group, $this->cache_ttl );🧰 Tools
🪛 GitHub Actions: Integration Tests
[error] 89-89: Undefined constant "Parsely\PARSELY_CACHE_GROUP" error in multiple RestAPI Stats endpoint tests.
src/Utils/class-utils.php (1)
430-444: 🛠️ Refactor suggestionCache path via
wpcom_vip_url_to_postid()is not cached.When the VIP helper exists, the result (success or miss) isn’t stored, causing repeated look-ups every call.
- $post_id = wpcom_vip_url_to_postid( $url ); + $post_id = wpcom_vip_url_to_postid( $url ); + // Cache VIP result for one week to match the other branches. + wp_cache_set( $cache_key, $post_id, PARSELY_CACHE_GROUP, WEEK_IN_SECONDS );🧰 Tools
🪛 GitHub Actions: Integration Tests
[error] 431-431: Undefined constant "Parsely\PARSELY_CACHE_GROUP" error in EndpointSmartLinking and EndpointSuggestLinkedReference tests.
src/Models/class-smart-link.php (1)
1043-1049: 🛠️ Refactor suggestionInvalid
orderbyvalue will be ignored by WP_Query.
'date modified'is not a recognisedorderbyparameter.
Use'modified'(modified date) or'modified_gmt'.- 'orderby' => 'date modified', + 'orderby' => 'modified',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Endpoints/class-graphql-metadata.php(1 hunks)src/Metadata/class-metadata-builder.php(2 hunks)src/Models/class-inbound-smart-link.php(8 hunks)src/Models/class-smart-link.php(14 hunks)src/Utils/class-utils.php(4 hunks)src/services/class-cached-service-endpoint.php(4 hunks)wp-parsely.php(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- wp-parsely.php
- src/Endpoints/class-graphql-metadata.php
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Models/class-inbound-smart-link.php
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the HTML and PHP code to ensure it is well-structured and adheres ...
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/services/class-cached-service-endpoint.phpsrc/Utils/class-utils.phpsrc/Metadata/class-metadata-builder.phpsrc/Models/class-smart-link.php
🧠 Learnings (1)
src/Models/class-smart-link.php (2)
Learnt from: alecgeatches
PR: Parsely/wp-parsely#3293
File: src/Models/class-smart-link.php:1099-1105
Timestamp: 2025-05-08T18:42:19.757Z
Learning: When using WP_Query to count posts rather than retrieve them, setting 'posts_per_page' => 0 is a performance optimization. This instructs WordPress to calculate found_posts without retrieving any actual post objects, which is more efficient than using -1 when only needing a count.
Learnt from: alecgeatches
PR: Parsely/wp-parsely#3293
File: src/Models/class-inbound-smart-link.php:1067-1081
Timestamp: 2025-05-08T18:20:50.947Z
Learning: When using WP_Query only to get the count of posts via $query->found_posts, setting posts_per_page to 0 is more efficient than -1 as it minimizes data retrieval while still providing an accurate count.
🪛 GitHub Actions: Integration Tests
src/services/class-cached-service-endpoint.php
[error] 89-89: Undefined constant "Parsely\PARSELY_CACHE_GROUP" error in multiple RestAPI Stats endpoint tests.
src/Utils/class-utils.php
[error] 431-431: Undefined constant "Parsely\PARSELY_CACHE_GROUP" error in EndpointSmartLinking and EndpointSuggestLinkedReference tests.
src/Models/class-smart-link.php
[error] 1250-1250: Undefined constant "Parsely\PARSELY_CACHE_GROUP" used causing errors in multiple tests.
[error] 1097-1097: Undefined constant "Parsely\PARSELY_CACHE_GROUP" referenced in call stack.
[error] 738-738: Undefined constant "Parsely\PARSELY_CACHE_GROUP" error in EndpointSmartLinking and EndpointSuggestLinkedReference tests.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: E2E against WordPress latest
🔇 Additional comments (2)
src/Metadata/class-metadata-builder.php (1)
16-16: Type import is correct – nice improvement.Explicitly importing
WP_Termmakes the new type-check below self-documenting and prevents accidental symbol clashes.src/services/class-cached-service-endpoint.php (1)
71-74: 👍 Cache-key algorithm simplified and collision-resistant.Using a single SHA-1 over a formatted string is both shorter and avoids repeated
wp_hashcalls. Nice clean-up.
This reverts commit 73d4020.
|
@acicovic I've made some updates to
Note: Changed hashing to One more thing, I just noticed that |
Addressed in 32e9869. The problem was that we were calling into
This is a bit unusual, and as you can see from the screenshot also includes inner objects like The code has been modified to instead cache the smart link IDs. Although this will incur a performance cost to re-hydrate objects, it'll still cache the heavy taxonomy query and avoid potential object serialization and stale data issues. |
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
I think we're ready to merge, left a couple of minor comments.
alecgeatches
force-pushed
the
add/traffic-boost-post-list-stats
branch
from
6cd339a to
eb92cb3
Compare
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
Thank you so much for working on this bringing it to its best version!
acicovic
added
the
Feature: Engagement Boost
3 participants
Description
Adds campaign tracking for inserted links in Traffic Boost, and displays those results in the UI:
Testing
Smart Link Insertion
Verify links inserted by Traffic Boost have the correct campaign information:
Go to WordPress Admin -> Parse.ly menu -> Traffic Boost.
Click the "Boost Traffic" button next to a post.
Wait for analysis and see a link suggestion. Note the page's title, then click "Accept" on a link suggestion.
After accepting the suggestion, open the "Inbound Links" page and select the accepted insertion page. Next, click the "..." symbol on the top right and open the post with the inserted link:
In the modified post, find the inserted link and view the URL. It should look something like this:
Ensure
itm_campaign=wp-parsely,itm_medium=smart-link,itm_source=traffic-boost, anditm_termhas a value.Traffic Boost Results
"Results" here refer to the new UI displaying impact on a boosted page:
This data is provided by Parse.ly directly, so it's difficult to test locally. However, we have a couple of methods we can use to display real data from Parse.ly:
Test recently-inserted link
For testing purposes, a smart link was inserted into real post data from "How to Ensure Good Governance for WordPress Block Themes" to "Using a Design System with the WordPress Block Editor: Block Types & Styles".
The real data is shown in the screenshot above. Using the same local data, you should be able to find these results by searching "theme.json" in the Traffic Boost search bar as shown above.
You can also increment the number by clicking the "Block Editor" smart link inserted in "How to Ensure Good Governance for WordPress Block Themes":
Next, wait for about 5 minutes for the local cache to clear and re-search "theme.json". You should see the counter and overall impact increase.
Historical Traffic Boost Results
The WPVIP story "Pros and Cons of Using GraphQL on WordPress", a recent post, has some historical testing data available with a small change:
This data is over 30 days old, so in order to view it we need to modify $request_params in [
src/rest-api/stats/class-endpoint-posts.phpbefore making the analytics request:// Fill the URLs with the campaign params. /** @var array<string, array<string, mixed>> $request_params_with_campaign */ $request_params_with_campaign['urls'] = $post_urls; + $request_params_with_campaign['period_start'] = '60d'; /** * The raw analytics data, received by the API. * * @var array<array<string, mixed>>|WP_Error $campaign_request */ $campaign_request = $this->content_api->get_posts( $request_params_with_campaign );After making the above change, you should be able to see some click data for "Pros and Cons of Using GraphQL on WordPress", as displayed above.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Style
Chores