-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PCH Smart Linking: Add Inbound Smart Links support #2553
Conversation
…rt-linking-backend # Conflicts: # src/Models/class-smart-link.php # src/content-helper/editor-sidebar/smart-linking/provider.ts
…rt-linking-backend # Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js # src/content-helper/editor-sidebar/smart-linking/component.tsx
…rt-linking-backend
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js # src/Endpoints/class-base-endpoint.php # src/Endpoints/content-helper/class-smart-linking-endpoint.php # src/Models/class-smart-link.php # src/content-helper/editor-sidebar/smart-linking/component.tsx # src/content-helper/editor-sidebar/smart-linking/hooks.ts # src/content-helper/editor-sidebar/smart-linking/provider.ts # src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # src/content-helper/editor-sidebar/smart-linking/review-modal/component-suggestion.tsx # src/content-helper/editor-sidebar/smart-linking/utils.ts
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js
# Conflicts: # src/Models/class-smart-link.php # src/content-helper/editor-sidebar/smart-linking/component.tsx # src/content-helper/editor-sidebar/smart-linking/provider.ts
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js # src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js
src/RemoteAPI/content-suggestions/class-suggest-linked-reference-api.php
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/smart-linking.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
Outdated
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-block-preview.tsx
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Cicovic <23142906+acicovic@users.noreply.github.com>
# Conflicts: # build/content-helper/editor-sidebar.asset.php # build/content-helper/editor-sidebar.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (3)
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx (3)
Line range hint
85-96
: The initial state forselectedLink
should handle the case wheresmartLinks
is empty to prevent runtime errors.- const [ selectedLink, setSelectedLink ] = useState<SmartLink | InboundSmartLink>( smartLinks[ 0 ] ); + const [ selectedLink, setSelectedLink ] = useState<SmartLink | InboundSmartLink>( smartLinks.length > 0 ? smartLinks[ 0 ] : null );
Line range hint
188-200
: This function could benefit from separating concerns: one function to handle the display of the confirmation dialog and another to handle the closure logic. This would improve readability and maintainability.
[!TIP]
Codebase VerificationThe extracted
useEffect
hook is:useEffect( () => { // When the modal is open, disable autosave if ( isModalOpen ) { dispatchCoreEditor.lockPostAutosaving( 'smart-linking-review-modal' ); return; } // If the modal is open, but there are no more smart links, close the modal. if ( isModalOpen && allSmartLinks.length === 0 ) { onCloseHandler(); } }, [ isModalOpen, onClose, allSmartLinks, onCloseHandler ] );Analysis of Dependencies:
- isModalOpen: This is necessary as it determines whether the modal is open, affecting both the autosave lock and the modal close logic.
- onClose: This dependency is not directly used within the
useEffect
hook and could be unnecessary.- allSmartLinks: This is necessary to check if there are no more smart links, triggering the modal close.
- onCloseHandler: This is necessary as it is called to close the modal when there are no more smart links.
Conclusion:
The dependency
onClose
appears to be unnecessary in thisuseEffect
hook and could be removed to prevent unintended re-renders.
- Location: src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx, lines 417-426
Analysis chain
Line range hint
408-423
: Check the dependencies of thisuseEffect
hook. It seems to include more dependencies than necessary, which could lead to unintended re-renders or side effects.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the necessity of each dependency in this useEffect hook. echo "Analyze the dependencies of the useEffect hook at lines 408-423 to ensure they are all necessary."Length of output: 204
Script:
#!/bin/bash # Extract relevant parts of the codebase to understand the context and usage of each dependency in the useEffect hook. # Extract the implementation of the useEffect hook echo "Extracting the useEffect hook implementation..." rg -A 15 'useEffect\(\s*\(\)\s*=>\s*{\s*// When the modal is open, disable autosave' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of isModalOpen echo "Extracting the implementation of isModalOpen..." rg 'const isModalOpen' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onClose echo "Extracting the implementation of onClose..." rg 'const onClose' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of allSmartLinks echo "Extracting the implementation of allSmartLinks..." rg 'const allSmartLinks' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onCloseHandler echo "Extracting the implementation of onCloseHandler..." rg 'const onCloseHandler' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsxLength of output: 1457
Script:
#!/bin/bash # Extract relevant parts of the codebase to understand the context and usage of each dependency in the useEffect hook. # Extract the implementation of the useEffect hook echo "Extracting the useEffect hook implementation..." rg -A 15 'useEffect\(\s*\(\)\s*=>\s*{\s*\/\/ When the modal is open, disable autosave' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of isModalOpen echo "Extracting the implementation of isModalOpen..." rg 'const isModalOpen' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onClose echo "Extracting the implementation of onClose..." rg 'const onClose' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of allSmartLinks echo "Extracting the implementation of allSmartLinks..." rg 'const allSmartLinks' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onCloseHandler echo "Extracting the implementation of onCloseHandler..." rg 'const onCloseHandler' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsxLength of output: 1461
Script:
#!/bin/bash # Extract relevant parts of the codebase to understand the context and usage of each dependency in the useEffect hook. # Extract the implementation of the useEffect hook echo "Extracting the useEffect hook implementation..." rg -A 15 'useEffect\(' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of isModalOpen echo "Extracting the implementation of isModalOpen..." rg 'const isModalOpen' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onClose echo "Extracting the implementation of onClose..." rg 'const onClose' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of allSmartLinks echo "Extracting the implementation of allSmartLinks..." rg 'const allSmartLinks' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx # Extract the implementation of onCloseHandler echo "Extracting the implementation of onCloseHandler..." rg 'const onCloseHandler' src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsxLength of output: 2102
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- src/Models/class-inbound-smart-link.php (1 hunks)
- src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx (14 hunks)
- src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx (5 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/Models/class-inbound-smart-link.php
- src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
Additional context used
Path-based instructions (1)
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx (1)
Pattern
**/*.{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."
Additional comments not posted (4)
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx (4)
399-399
: This function's implementation is generally sufficient for selecting smart links, but consider enhancing its robustness by ensuringblockContent
is anHTMLElement
before proceeding.
446-472
: Ensure that the componentsReviewModalSidebar
,InboundLinkDetails
, andReviewSuggestion
are optimized for re-renders, perhaps by memoizing them if they are not already.
[PERFORMANCE]
54-80
: Ensure theuseSelect
hook's dependencies are correctly specified. Currently, it appears to have an empty dependency array, which might not be appropriate if theSmartLinkingStore
or its methods are expected to change during the component's lifecycle.
295-295
: Ensure thatgetSuggestedLinks
is always expected to return an array to avoid potential runtime errors here.Verification successful
The
getSuggestedLinks
method insrc/content-helper/editor-sidebar/smart-linking/store.ts
is implemented as follows:getSuggestedLinks( state: SmartLinkingState ): SmartLink[] { return state.smartLinks.outbound.filter( ( link ) => ! link.applied ); }This implementation ensures that
getSuggestedLinks
always returns an array, specifically an array filtered fromstate.smartLinks.outbound
.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check that getSuggestedLinks always returns an array, even if it might be empty. echo "Review the implementation of getSuggestedLinks to ensure it returns an array."Length of output: 164
Script:
#!/bin/bash # Locate the definition of the getSuggestedLinks function in the codebase. ast-grep --lang typescript --pattern 'function getSuggestedLinks($_) { $$$ }'Length of output: 79
Script:
#!/bin/bash # Use ripgrep to search for all instances of getSuggestedLinks in the codebase to find its definition and references. rg 'getSuggestedLinks'Length of output: 73547
Script:
#!/bin/bash # Extract the implementation of getSuggestedLinks from the identified file. ast-grep --lang typescript --pattern 'function getSuggestedLinks($_) { $$$ }' src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 136
Script:
#!/bin/bash # Use ripgrep to search for all instances of getSuggestedLinks in the identified file. rg 'getSuggestedLinks' src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 142
Script:
#!/bin/bash # Extract the implementation of the getSuggestedLinks method from the identified file. sed -n '/getSuggestedLinks( state: SmartLinkingState ): SmartLink[] {/,/}/p' src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 194
Script:
#!/bin/bash # Correct the sed command to extract the implementation of the getSuggestedLinks method from the identified file. sed -n '/getSuggestedLinks( state: SmartLinkingState ): SmartLink[] {/,/^}/p' src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 195
Script:
#!/bin/bash # Use ripgrep to search for the method signature and display the surrounding lines to capture the full implementation. rg 'getSuggestedLinks\( state: SmartLinkingState \): SmartLink\[] {' -A 20 src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 323
Script:
#!/bin/bash # Use ripgrep to search for the method signature and display the surrounding lines to capture the full implementation, with properly escaped characters. rg 'getSuggestedLinks\( state: SmartLinkingState \): SmartLink\[\] {' -A 20 src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 326
Script:
#!/bin/bash # Use ripgrep to search for the method name and display a larger number of surrounding lines to capture the full implementation. rg 'getSuggestedLinks' -A 50 src/content-helper/editor-sidebar/smart-linking/store.tsLength of output: 1160
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx
Show resolved
Hide resolved
src/content-helper/editor-sidebar/smart-linking/review-modal/component-modal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
build/content-helper/editor-sidebar.asset.php
is excluded by!build/**
build/content-helper/editor-sidebar.js
is excluded by!build/**
Files selected for processing (5)
- src/Models/class-inbound-smart-link.php (1 hunks)
- src/content-helper/editor-sidebar/smart-linking/component.tsx (8 hunks)
- src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx (5 hunks)
- src/content-helper/editor-sidebar/smart-linking/smart-linking.tsx (3 hunks)
- src/content-helper/editor-sidebar/smart-linking/utils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/Models/class-inbound-smart-link.php
- src/content-helper/editor-sidebar/smart-linking/component.tsx
- src/content-helper/editor-sidebar/smart-linking/review-modal/component-sidebar.tsx
- src/content-helper/editor-sidebar/smart-linking/smart-linking.tsx
Additional context used
Path-based instructions (1)
src/content-helper/editor-sidebar/smart-linking/utils.ts (1)
Pattern
**/*.{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."
Biome
src/content-helper/editor-sidebar/smart-linking/utils.ts
[error] 62-62: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 124-124: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 181-181: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 349-349: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
Additional comments not posted (1)
src/content-helper/editor-sidebar/smart-linking/utils.ts (1)
13-13
: Import statements are correctly placed and organized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this!
Description
This PR is an extension of the work on #2544 and #2507. This PR adds a new "Inbound" view to the Smart Linking modal, where it's possible to visualize a list of all the smart links that are linking to the post currently being edited.
A new model,
Inbound_Smart_Link
has been added, that extends from theSmart_Link
and includes the relevant post data from the post where that smart link is inserted.There is a small deviation from the original design proposition, since I have introduced tabs to the modal sidebar, so it has a section for Outbound smart links, and another section for Inbound.
Motivation and context
Improve the usability of the Smart Linking modal, by giving the ability to review which posts are currently linking to the Smart Link.
How has this been tested?
Tested locally. I have added a new WordPress filter that helps with testing this feature:
wp_parsely_suggest_linked_reference_link
. This filter allows to change properties of each suggested link before its returned by the API. With this snippet, I was able to replace the URL of each Smart Link to a random published post on my testing environment.Screenshots
Summary by CodeRabbit
New Features
InboundSmartLink
model with methods for handling smart links.Bug Fixes
Enhancements
Dependencies
composer.json
to includeext-dom
andext-libxml
extensions.