-
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
Inject post ID into LD-JSON metadata #2510
Conversation
WalkthroughThe changes introduce a new method Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Client
participant PostBuilder
participant Metadata
Client->>PostBuilder: get_metadata()
PostBuilder->>Metadata: build_post_ids(post)
Metadata-->>PostBuilder: metadata with postID
PostBuilder-->>Client: metadata array
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- src/Metadata/class-metadata-builder.php (1 hunks)
- src/Metadata/class-post-builder.php (1 hunks)
- tests/Integration/Endpoints/RestMetadataTest.php (1 hunks)
Additional context used
Path-based instructions (3)
src/Metadata/class-post-builder.php (1)
Pattern
**/*.{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."
tests/Integration/Endpoints/RestMetadataTest.php (1)
Pattern
**/*.{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/Metadata/class-metadata-builder.php (1)
Pattern
**/*.{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."
Additional comments not posted (3)
src/Metadata/class-post-builder.php (1)
63-63
: The integration ofbuild_post_ids
method inget_metadata
is correctly placed and aligns with the PR's objectives.Verification successful
The
build_post_ids
method inclass-metadata-builder.php
is implemented as follows:protected function build_post_ids( WP_Post $post ): void { $this->metadata['postID'] = $post->ID; }This method correctly assigns the post ID to the
metadata
array, aligning with the intended functionality. The integration of this method inget_metadata
withinclass-post-builder.php
is appropriate and does not introduce any errors.
class-metadata-builder.php
protected function build_post_ids( WP_Post $post ): void { $this->metadata['postID'] = $post->ID; }
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php 'build_post_ids' | xargs grep -i 'error'Length of output: 656
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php 'build_post_ids'Length of output: 206
Script:
#!/bin/bash # Description: Extract and review the implementation of `build_post_ids` in `class-metadata-builder.php`. # Extract the implementation of the `build_post_ids` method. ast-grep --lang php --pattern 'protected function build_post_ids(WP_Post $post): void { $$$ }'Length of output: 203
Script:
#!/bin/bash # Description: Extract and review the implementation of `build_post_ids` in `class-metadata-builder.php`. # Search for the implementation of the `build_post_ids` method. rg --type php 'protected function build_post_ids'Length of output: 151
Script:
#!/bin/bash # Description: Extract the full implementation of `build_post_ids` in `class-metadata-builder.php`. # Extract the full implementation of the `build_post_ids` method. rg --type php -A 10 'protected function build_post_ids'Length of output: 323
tests/Integration/Endpoints/RestMetadataTest.php (1)
407-407
: The addition of"postID": $post_id
in the expected JSON LD script correctly tests the new functionality.src/Metadata/class-metadata-builder.php (1)
691-701
: The implementation ofbuild_post_ids
method is correct and effectively adds the post ID to the metadata.Verification successful
The
build_post_ids
method is correctly integrated and does not introduce any errors in the related files.
src/Metadata/class-post-builder.php
: No errors found.src/Metadata/class-metadata-builder.php
: No errors related tobuild_post_ids
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php 'build_post_ids' | xargs grep -i 'error'Length of output: 656
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php 'build_post_ids' | xargs -I {} sh -c 'echo "File: {}"; grep -i "error" {}'Length of output: 162
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php 'build_post_ids' | while read -r file; do echo "File: $file" grep -i "error" "$file" doneLength of output: 923
Script:
#!/bin/bash # Description: Verify that the `build_post_ids` method is correctly integrated and does not affect other metadata properties. # Test: Search for the method usage and check for any related issues in the log. rg --type php --files-with-matches 'build_post_ids' | while read -r file; do echo "File: $file" grep -i "error" "$file" doneLength of output: 766
@@ -60,6 +60,7 @@ public function get_metadata(): array { | |||
$this->build_publisher(); | |||
$this->build_keywords( $this->post ); | |||
$this->build_metadata_post_times( $this->post ); | |||
$this->build_post_ids( $this->post ); |
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.
Is it possible to pass in the post ID directly?
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.
Thanks for the suggestion! I get where you're coming from, but I think it'll be better to stick with the current pattern of passing the WP_Post
object around. Here's why:
- All our existing methods use WP_Post, so changing to post ID would break that pattern.
- Using WP_Post keeps all the post-related data and logic neatly contained.
- If we ever need to pass related post IDs or additional post data, we'd add more methods, increasing the risk of breaking changes.
So it sounds like maintaining the current approach of working with WP_Post
objects makes the codebase cleaner and easier to maintain. What do you think?
Closing in favor of #2511 |
Description
Injecting post IDs will let Parsely link back to the edit form of the post.
Motivation and context
How has this been tested?
Screenshots (if appropriate)
Summary by CodeRabbit
New Features
Bug Fixes
Tests