Fix update metadata endpoint calling issues#3092
Conversation
acicovic
added
the
Changelog: Fixed
📝 WalkthroughWalkthroughThis pull request introduces modifications to the Parse.ly WordPress plugin's core functionality, focusing on metadata endpoint handling and settings configuration. The changes primarily involve updating the Changes
Sequence DiagramsequenceDiagram
participant Plugin as Parsely Plugin
participant Endpoint as Update Metadata Endpoint
Plugin->>Plugin: Validate Credentials
Plugin->>Plugin: Check Post Type
Plugin->>Plugin: Verify Post Status
alt Valid Conditions
Plugin->>Endpoint: Call Update Metadata
Endpoint-->>Plugin: Return Success
else Invalid Conditions
Plugin-->>Plugin: Return False
end
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
acicovic
added
the
Maintenance & Fixes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/class-parsely.php (2)
285-286: Consider addressing the PHPStan warning.The
@phpstan-ignore return.voidcomment suggests there might be a type system inconsistency that could be resolved.
513-531: Consider adding timeout to the API request.The current implementation doesn't specify a timeout for the non-blocking POST request. Consider adding a reasonable timeout value to prevent potential issues.
$request_options = array( 'method' => 'POST', 'headers' => $headers, 'blocking' => false, + 'timeout' => 5.0, 'body' => $body, 'data_format' => 'body', );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/UI/class-settings-page.php(0 hunks)src/class-parsely.php(5 hunks)tests/Integration/CallUpdateMetadataEndpointTest.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/UI/class-settings-page.php
🧰 Additional context used
📓 Path-based instructions (2)
src/class-parsely.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/CallUpdateMetadataEndpointTest.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 (13)
tests/Integration/CallUpdateMetadataEndpointTest.php (8)
1-14: LGTM! Well-structured test class setup.The namespace, imports, and class documentation are clear and follow WordPress coding standards.
64-71: LGTM! Comprehensive credential validation test.The test method and its data provider effectively cover all credential validation scenarios:
- No Site ID and Metadata Secret
- Missing Site ID only
- Missing Metadata Secret only
Also applies to: 80-101
131-164: LGTM! Well-structured post type validation test.The test method and its data provider effectively validate endpoint behavior for different post types:
- Trackable: posts and pages
- Non-trackable: attachments and revisions
188-190: LGTM! Important error case validation.The test ensures the endpoint correctly handles non-existent post IDs.
215-234: LGTM! Thorough custom post type validation.The test effectively validates that:
- Custom post types are not tracked by default
- Custom post types can be tracked when added to track_post_types
- Proper cleanup is performed by unregistering the test post type
260-275: LGTM! Good post status validation.The test effectively validates that:
- Draft posts are not tracked
- Published posts are tracked after status update
301-321: LGTM! Comprehensive custom status validation.The test effectively validates that:
- Custom statuses are not tracked by default
- Custom statuses can be tracked when added via filter
- Proper cleanup by removing the filter
332-336: LGTM! Clear helper method implementation.The method has a clear purpose and follows WordPress coding standards.
src/class-parsely.php (5)
417-418: LGTM! Simplified status check logic.Removing the caching mechanism eliminates potential edge cases where post status might change during request execution.
440-451: LGTM! Good credential validation.The method properly validates required credentials before proceeding.
453-466: LGTM! Thorough post type validation.The code effectively validates that:
- Post type exists
- Post type is in the list of tracked types
468-476: LGTM! Proper post validation.The code validates both:
- Post status is trackable
- Post exists
478-481: LGTM! Good test environment handling.The code properly handles integration test scenarios without making actual API calls.
vaurdan
left a comment
vaurdan
left a comment
There was a problem hiding this comment.
Thank you so much for working on this. Looks good to me!
I have replied to your comments, so feel free to take a look before merging :)
2 participants
Description
Our logic for calling Parse.ly's update metadata endpoint had multiple flaws. For example, the endpoint would be called for untrackable post types (e.g. revisions) and post statuses (e.g drafts).
This PR makes a bit of a cleanup and updates the function to work as expected, which is the endpoint getting called only for post types and statuses which are trackable. We're also stopping the calls from happening while integration tests are running.
Motivation and context
Eliminate needless calls to infrastructure and prevent invalid data from being posted to the endpoint.
How has this been tested?
A few integration tests were added.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Refactor
update_metadata_endpointmethod tocall_update_metadata_endpointDocumentation