Smart Linking: Add post details to the "Review" modal #3176
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the logic for retrieving post IDs by removing the private method from Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Editor Sidebar
participant SP as SmartLinkingProvider
participant REST as REST API Endpoint
participant Utils as Utils Class
UI->>SP: generateSmartLinks(content, maxLinks, exclusionList)
SP->>SP: Parse content for smart links
SP->>REST: Concurrently fetch post metadata & performance data for URLs
REST-->>SP: Return respective metadata and stats
SP->>Utils: Invoke Utils::get_post_id_by_url(url) for post ID resolution
SP->>UI: Return enriched OutboundSmartLinks array
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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
|
|
Hey @vaurdan! I noticed you recently introduced a thumbnail component, so I'll attempt to port that commit into this PR. That should allow you to keep the credit, and also prevent us from duplicating things. This should probably do it, but I wanted to bring your attention to this in case there's anything else I need to do. |
|
@LauraKalnicky, we're currently hardcoding the stats to be for the last 30 days. Let me know if you think we should change the period to something else. We can eventually offer a setting for this at some point. |
|
@dabowman: Regarding visuals, the final piece is adding the thumbnail, as well as making any possible CSS micro-adjustments needed. Let me know if you see anything that obviously needs fixing in the screenshot. If you want to take a look at the files, these would be the relevant ones: |
|
@dabowman, related to my comment to Laura, should we be actually showing the number of days somewhere? |
@acicovic 30 days is in alignment with our longest option for Top Posts in PCH performance data on the WP admin dash. If we are only starting with one option I think having the most data works, although I envision that some of the posts will not have collected 30 days of data yet. In that case, would we just show all the data we have for them? I'll also answer your question to @dabowmon - we should definitely indicate the number of days so it's clear to the user. |
|
@LauraKalnicky, if there's no data for the last 30 days (or whichever timeframe we choose), the metrics in question will show as @dabowman, let's see where we can put the number of days. |
|
@acicovic I was thinking that if a post was too new to have 30 days of data would we then show whatever amount of data we've collected during that time? |
|
@acicovic @LauraKalnicky i would probably put the time period in a tooltip for each stat. So "visitors over last 30 days" or something like that. I'm not sure how folks are going to be using these metrics here but I don't think the period is super super relevant. Good to know, but probably not as important as the stats themselves. I imagine folks would be looking at these numbers to confirm trends and high-level performance not compare performance across posts or do any kind of analysis. I think less is more in those situations, that's why I opted for icons instead of writing stat names. But we should definitely make the period clear if we don't let them configure it. I think a tooltip should do it, maybe a little info icon, I'll try a couple things. In any case I think it's just enhancing what you've already built with a little extra progressive disclosure. |
|
@dabowman, another scenario I can think of is adding the period next to the metrics. This should work as we limit number length (eg 100k instead of 100000). |
There was a problem hiding this comment.
The following files/code are cherry-picked from @vaurdan's work in commits 1e50213 and d04ecb9:
src/content-helper/common/base-provider.tsx
src/content-helper/common/base-wordpress-provider.tsx
src/content-helper/common/components/thumbnail/component.tsx
src/content-helper/common/components/thumbnail/index.ts
src/content-helper/common/components/thumbnail/style.scss
src/content-helper/common/css/common.scss
src/content-helper/common/css/variables.scss
As such, credit for that code is kept to him, and the files don't need review since they are already merged work. 🎉
src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/Utils/class-utils.php (1)
410-435: Clean implementation of the URL to post ID utility method.This method effectively retrieves a post ID from a URL with proper caching implementation. The function follows good practices by:
- Using cache to avoid redundant processing
- Checking for VIP environment functions
- Including appropriate documentation with
@sincetagsFor optimal handling, consider adding caching for the VIP implementation as well.
if ( function_exists( 'wpcom_vip_url_to_postid' ) ) { $post_id = wpcom_vip_url_to_postid( $url ); + wp_cache_set( $url, $post_id, 'wp_parsely_smart_link_url_to_postid' ); } else { // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.url_to_postid_url_to_postid $post_id = url_to_postid( $url ); wp_cache_set( $url, $post_id, 'wp_parsely_smart_link_url_to_postid' ); }src/content-helper/common/components/thumbnail/component.tsx (2)
1-24: Ensure JSDoc comments end with periods.The component interface and imports look good, but JSDoc comments should end with periods as per WordPress coding standards.
/** * Defines the props structure for Thumbnail. * - * @since 3.18.0 + * @since 3.18.0 */
25-58: Well-structured component with clear rendering logic.The Thumbnail component is well-implemented with proper handling of optional props and conditional rendering. For clarity, consider adding units to size values in the style object and ensuring all JSDoc comments end with periods.
/** * Thumbnail component that displays either a post's thumbnail, a custom image, or a fallback icon. * - * @since 3.18.0 + * @since 3.18.0. * - * @param {ThumbnailProps} props Component props. + * @param {ThumbnailProps} props Component props. */ export const Thumbnail = ( { post, imageUrl, icon = page, size = 100, className = '', }: Readonly<ThumbnailProps> ): React.JSX.Element => { const thumbnailUrl = post?.thumbnail ?? imageUrl; const altText = post?.title.rendered ?? ''; return ( - <div className={ `parsely-thumbnail ${ className }` } style={ { width: size, height: size } }> + <div className={ `parsely-thumbnail ${ className }` } style={ { width: `${size}px`, height: `${size}px` } }> { thumbnailUrl ? ( <img src={ thumbnailUrl } alt={ altText } width={ size } height={ size } /> ) : ( <div className="parsely-thumbnail-icon-container"> <Icon icon={ icon } size={ size } /> </div> ) } </div> ); };src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx (1)
110-121: Update JSDoc to match the new function signature and finalize doc style.You've changed the component to accept
OutboundSmartLinkbut the doc is still marked with@since 3.16.0. Additionally, the@paramline doesn’t end with a period. Suggested fix:- * @since 3.16.0 - * @param {{link: OutboundSmartLink}} props The component props + * @since 3.18.0 + * @param {{link: OutboundSmartLink}} props The component props.src/content-helper/editor-sidebar/smart-linking/provider.ts (1)
167-239: generateSmartLinks updates are solid, but consider chunking large batches.Using
Promise.allfor parallel fetching of stats and post meta is good. For large sets of links, you might consider queueing or pagination to avoid performance bottlenecks. Otherwise, the concurrency approach looks fine.src/content-helper/common/base-wordpress-provider.tsx (2)
167-182: Consider unifying the@sinceversion tags.Lines 101-108 consistently use
@since 3.18.0, whereas lines 167-182 reference@since 3.15.0. If this code was truly introduced in 3.15.0, that’s fine. Otherwise, updating these references to match a single version would improve clarity.
215-217: Use optional chaining to simplify the conditional checks.Instead of verifying
post._embedded && post._embedded[...], consider using optional chaining for better readability.Apply this diff to adopt optional chaining:
-if ( post._embedded && post._embedded['wp:term'] ) { +if ( post._embedded?.['wp:term'] ) { [ categories, tags ] = post._embedded['wp:term']; } -if ( post._embedded && post._embedded['wp:featuredmedia'] ) { +if ( post._embedded?.['wp:featuredmedia'] ) { const featuredMedia = post._embedded['wp:featuredmedia'][ 0 ]; thumbnail = featuredMedia.media_details.sizes.thumbnail.source_url; }Also applies to: 220-223
🧰 Tools
🪛 Biome (1.9.4)
[error] 215-215: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
build/admin-settings-rtl.cssis excluded by!build/**build/admin-settings.asset.phpis excluded by!build/**build/admin-settings.cssis excluded by!build/**build/content-helper/dashboard-widget-rtl.cssis excluded by!build/**build/content-helper/dashboard-widget.asset.phpis excluded by!build/**build/content-helper/dashboard-widget.cssis excluded by!build/**build/content-helper/editor-sidebar-rtl.cssis excluded by!build/**build/content-helper/editor-sidebar.asset.phpis excluded by!build/**build/content-helper/editor-sidebar.cssis excluded by!build/**build/content-helper/editor-sidebar.jsis excluded by!build/**
📒 Files selected for processing (16)
src/Models/class-smart-link.php(3 hunks)src/Utils/class-utils.php(1 hunks)src/content-helper/common/base-provider.tsx(2 hunks)src/content-helper/common/base-wordpress-provider.tsx(1 hunks)src/content-helper/common/components/thumbnail/component.tsx(1 hunks)src/content-helper/common/components/thumbnail/index.ts(1 hunks)src/content-helper/common/components/thumbnail/style.scss(1 hunks)src/content-helper/common/css/common.scss(1 hunks)src/content-helper/common/css/variables.scss(1 hunks)src/content-helper/editor-sidebar/smart-linking/provider.ts(5 hunks)src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx(2 hunks)src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx(3 hunks)src/content-helper/editor-sidebar/smart-linking/smart-linking.scss(2 hunks)src/rest-api/content-helper/class-endpoint-smart-linking.php(3 hunks)src/rest-api/stats/class-endpoint-posts.php(3 hunks)src/services/content-api/endpoints/class-endpoint-analytics-posts.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the pr...
**/*.{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/components/thumbnail/index.tssrc/content-helper/common/base-provider.tsxsrc/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsxsrc/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsxsrc/content-helper/editor-sidebar/smart-linking/provider.tssrc/content-helper/common/components/thumbnail/component.tsxsrc/content-helper/common/base-wordpress-provider.tsx
`**/*.{css,scss}`: "Perform a detailed review of the provide...
**/*.{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/common/css/variables.scsssrc/content-helper/common/components/thumbnail/style.scsssrc/content-helper/common/css/common.scsssrc/content-helper/editor-sidebar/smart-linking/smart-linking.scss
`**/*.{html,php}`: "Perform a detailed review of the provide...
**/*.{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/rest-api/stats/class-endpoint-posts.phpsrc/Utils/class-utils.phpsrc/Models/class-smart-link.phpsrc/services/content-api/endpoints/class-endpoint-analytics-posts.phpsrc/rest-api/content-helper/class-endpoint-smart-linking.php
🪛 Biome (1.9.4)
src/content-helper/common/base-wordpress-provider.tsx
[error] 215-215: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 220-220: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (30)
src/content-helper/common/css/variables.scss (1)
45-45: LGTM! Clean addition of the new gray color variable.The new variable follows the existing naming convention and is placed in the appropriate location within the grayscale sequence.
src/content-helper/common/css/common.scss (1)
10-10: LGTM! Properly added import for the thumbnail component style.The import statement is correctly positioned with other component style imports, maintaining consistent organization.
src/services/content-api/endpoints/class-endpoint-analytics-posts.php (3)
112-114: LGTM! Well-documented variable definition.The
$urlsvariable is properly documented with a PHPDoc comment and type annotation, following the established pattern in the file.
119-119: LGTM! Clean removal of processed parameter.The code properly unsets the
urlsparameter after storing it in a variable, consistent with how other parameters are handled.
128-128: LGTM! Properly implemented URL parameter handling.The code correctly appends URL parameters to the endpoint using the existing helper method, ensuring consistent parameter handling and proper URL encoding.
src/rest-api/stats/class-endpoint-posts.php (3)
177-182: Well-structured parameter definition.The new
urlsparameter is properly defined with appropriate description, type, sanitization callback, and required flag.
205-215: Secure implementation of URL sanitization.The
sanitize_urlsmethod provides a clean implementation for sanitizing an array of URLs, properly leveraging WordPress's built-insanitize_urlfunction. The documentation is clear and includes the appropriate@sincetag.
272-272: Parameter properly passed to API.The
urlsparameter is correctly passed to the Content API service, maintaining consistency with other parameters.src/Models/class-smart-link.php (3)
15-15: Added necessary import for Utils class.The addition of this import statement is required to support the refactoring of the
get_post_id_by_urlmethod to the Utils class.
246-246: Replaced private method with centralized utility method.The code now uses
Utils::get_post_id_by_urlinstead of a private method implementation, which is good for code maintainability and reuse.
438-438: Consistent use of utility method.Using the same
Utils::get_post_id_by_urlmethod here ensures consistency throughout the codebase.src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx (2)
16-16: Updated import to include OutboundSmartLink type.This change supports the enhancement of the Smart Linking "Review" modal with additional post details as mentioned in the PR objectives.
86-86: Updated selectedLink state type to use OutboundSmartLink.The state now uses
OutboundSmartLinkinstead of the genericSmartLink, which properly reflects the enhanced data structure needed for displaying additional post details in the modal.src/content-helper/common/base-provider.tsx (2)
48-48: Changed access modifier from private to protected.This change allows subclasses to access the
abortControllersmap directly, which is necessary for the newBaseWordPressProviderclass to manage API request cancellations.
111-111: Changed access modifier from private to protected.Making
getOrCreateControllerprotected allows subclasses to utilize this method for managing abort controllers, enhancing the reusability of this functionality.src/content-helper/editor-sidebar/smart-linking/smart-linking.scss (2)
325-325: Cleaned up selector formatting.This is a minor formatting change that improves code readability.
430-493: Enhanced styling for link suggestion details.The complete overhaul of the
.wp-parsely-link-suggestion-link-detailsclass adds support for the new post details (thumbnail, title, metadata) mentioned in the PR objectives. The updated styling uses a clean, flexible layout with proper spacing and overflow handling.I particularly like:
- The use of flexbox for responsive layout
- Proper handling of text overflow with ellipsis
- The well-organized nested structure for different components
- Consistent use of WordPress design variables
This implementation effectively supports the UI enhancements requested in the PR.
src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx (7)
8-13: New imports for the suggestion component look good.These newly introduced imports (Dashicon, KeyboardShortcuts, Tooltip, selectFn) are aligned with WordPress standards.
15-28: Icons import is consistent with WordPress best practices.Importing the icon set from
@wordpress/iconsis appropriate. No concerns identified.
34-37: Imports from common utilities and new type references look fine.The new imports (Thumbnail, getSmartShortDate, formatToImpreciseNumber, and OutboundSmartLink) integrate well with existing code.
124-145: Thumbnail and URL button block appear correct.Utilizing
<Tooltip>and addingrel="noopener"for external links meets security standards.
146-169: Data presentation layout looks good.Displaying date, author, post type, views, and visitors is coherent.
181-181: Type change to OutboundSmartLink is valid.Switching
linktoOutboundSmartLinkaligns with the new approach. Ensure any dependent usage is also updated.
343-343: Param type updated to OutboundSmartLink for ReviewSuggestion.Making sure the
linkprop references the richerOutboundSmartLinktype is consistent with the rest of the changes.src/rest-api/content-helper/class-endpoint-smart-linking.php (3)
18-18: New import from Parsely\Utils\Utils is fine.Including
Utilsis consistent with the newly introduced functionality.
187-201: New REST route for retrieving post meta looks good.The registration of
POST /smart-linking/get-post-meta-for-urlsis well documented. Make sure to confirm all relevant permissions and validations if you anticipate invalid input.
486-519: New method get_post_meta_for_urls appears logically correct.Fetching a post by URL and returning associated metadata follows the intended design. Consider verifying URLs via
filter_varor a validation callback if stricter handling of invalid URLs is required.Would you like a script to search for existing references to confirm error handling for invalid URLs?
src/content-helper/editor-sidebar/smart-linking/provider.ts (2)
12-14: Additional imports for API period parameters, Metric, Period, and PerformanceData are appropriate.These added imports align with existing usage patterns for stats retrieval.
343-360: New getPostMetaForURLs function is well-structured.This method aligns with the new
get-post-meta-for-urlsendpoint. The doc block references@since 3.18.0correctly, and the return type is well-defined.src/content-helper/common/base-wordpress-provider.tsx (1)
1-316: Well-structured implementation and thorough documentation.All functions and types are documented with
@sincetags, adhere to WordPress coding standards, and handle localization with@wordpress/i18n. The usage ofBaseProviderfor request cancellation is a sensible architectural decision that promotes maintainability. Great work!🧰 Tools
🪛 Biome (1.9.4)
[error] 215-215: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 220-220: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
…smart-linking-review-modal Smart Linking: Add post details to the "Review" modal" (d995f57)
Description
Following user feedback, we're introducing additional post details in the Smart Linking "Review" modal. The following data points have been added:
In addition, it's now possible to open a preview of the suggested post, using the icon located next to the post title.
Motivation and context
How has this been tested?
Manual testing, existing tests pass.
Screenshots
Suggestion with thumbnail
Suggestion with long post type name (shrinks author and post type)
Shrinkable Elements (post title, author, post type) show a tooltip when hovered
Summary by CodeRabbit
New Features
Style
Refactor