-
Notifications
You must be signed in to change notification settings - Fork 33
PCI Engagement Boost: Allow Pages and CPTs #3532
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
Conversation
📝 Walkthrough""" WalkthroughThis change introduces a new REST API utility endpoint to retrieve the REST route for any post, supporting built-in and custom post types (CPTs), including pages. The frontend provider is updated to use this endpoint for fetching post data. The Engagement Boost UI is refactored to support all REST-enabled post types, replacing previous "Traffic Boost" terminology. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Row_Actions (UI)
participant Provider as BaseWordPressProvider (Frontend)
participant API as REST_API_Controller (Backend)
participant Utils as Utils_Controller / Endpoint_Post
participant WP as WordPress Core
UI->>Provider: User clicks "Engagement Boost" link (with post ID)
Provider->>Utils: GET /wp-parsely/v2/utils/post/{post_id}/rest-route
Utils->>WP: rest_get_route_for_post(post_id)
WP-->>Utils: Returns REST route string
Utils-->>Provider: { data: "/wp/v2/{post_type}/{post_id}" }
Provider->>API: GET {rest_route}?_embed&context=edit
API-->>Provider: Returns post data
Provider-->>UI: Hydrates and displays Engagement Boost UI
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.15)Note: Using configuration file /phpstan.neon. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
✨ 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
|
src/content-helper/common/providers/base-wordpress-provider.tsx
Dismissed
Show dismissed
Hide dismissed
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
♻️ Duplicate comments (1)
src/content-helper/common/providers/base-wordpress-provider.tsx (1)
353-358: Log the original error for debugging purposes.The static analysis hint is valid - the catch block should log the original error to aid in debugging while still throwing the user-friendly error.
} catch ( error ) { + // Log the original error for debugging purposes + console.error( 'Failed to fetch post REST route:', error ); throw new ContentHelperError( __( "The target post's REST route could not be fetched.", 'wp-parsely' ), ContentHelperErrorCode.UnknownError, ); }
🧹 Nitpick comments (1)
src/rest-api/utils/class-endpoint-post.php (1)
66-66: Add explicit return type declaration.The method signature should include the return type for better type safety and documentation.
- public function get_rest_route( WP_REST_Request $request ) { + public function get_rest_route( WP_REST_Request $request ): WP_REST_Response {
📜 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 (7)
src/UI/class-row-actions.php(6 hunks)src/content-helper/common/providers/base-wordpress-provider.tsx(2 hunks)src/rest-api/class-rest-api-controller.php(2 hunks)src/rest-api/utils/class-endpoint-post.php(1 hunks)src/rest-api/utils/class-utils-controller.php(1 hunks)tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php(1 hunks)tests/Integration/RestAPI/Utils/EndpointUtilsPostTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,php}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,jsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (4)
📓 Common learnings
Learnt from: acicovic
PR: Parsely/wp-parsely#0
File: :0-0
Timestamp: 2024-10-16T13:03:58.056Z
Learning: User: acicovic
URL: https://github.com/Parsely/wp-parsely/pull/2355
Timestamp: 2024-04-03T08:04:35.576Z
Learning: In the context of the `wp-parsely` project's documentation, bullet points are consistently capitalized. This standard should be respected in reviews and suggestions regarding document formatting.
Learnt from: acicovic
PR: Parsely/wp-parsely#0
File: :0-0
Timestamp: 2024-07-26T21:07:21.167Z
Learning: User: acicovic
URL: https://github.com/Parsely/wp-parsely/pull/2355
Timestamp: 2024-04-03T08:04:35.576Z
Learning: In the context of the `wp-parsely` project's documentation, bullet points are consistently capitalized. This standard should be respected in reviews and suggestions regarding document formatting.
Learnt from: vaurdan
PR: Parsely/wp-parsely#2507
File: src/RemoteAPI/class-referrers-post-detail-api.php:32-37
Timestamp: 2024-07-26T21:07:21.167Z
Learning: The `is_available_to_current_user` method in `class-referrers-post-detail-api.php` and potentially other similar methods now include an optional `$request` parameter to enhance flexibility while retaining backward compatibility.
Learnt from: vaurdan
PR: Parsely/wp-parsely#2507
File: src/RemoteAPI/class-referrers-post-detail-api.php:32-37
Timestamp: 2024-10-12T10:01:08.699Z
Learning: The `is_available_to_current_user` method in `class-referrers-post-detail-api.php` and potentially other similar methods now include an optional `$request` parameter to enhance flexibility while retaining backward compatibility.
Learnt from: acicovic
PR: Parsely/wp-parsely#2554
File: src/UI/class-settings-page.php:28-29
Timestamp: 2024-10-12T10:01:08.699Z
Learning: acicovic prefers to use the term "resolve" to indicate that an issue or request has been addressed satisfactorily.
Learnt from: acicovic
PR: Parsely/wp-parsely#2554
File: src/UI/class-settings-page.php:28-29
Timestamp: 2024-06-18T09:33:19.519Z
Learning: acicovic prefers to use the term "resolve" to indicate that an issue or request has been addressed satisfactorily.
src/rest-api/utils/class-endpoint-post.php (2)
Learnt from: vaurdan
PR: Parsely/wp-parsely#2544
File: src/Endpoints/content-helper/class-smart-linking-endpoint.php:92-103
Timestamp: 2024-07-26T21:07:21.167Z
Learning: The `private_api_request_validate_post_id` function in `class-smart-linking-endpoint.php` checks if the post exists to validate the `post_id`, but does not explicitly check for the `post_id` being a positive integer.
Learnt from: vaurdan
PR: Parsely/wp-parsely#2544
File: src/Endpoints/content-helper/class-smart-linking-endpoint.php:92-103
Timestamp: 2024-10-12T10:01:08.699Z
Learning: The `private_api_request_validate_post_id` function in `class-smart-linking-endpoint.php` checks if the post exists to validate the `post_id`, but does not explicitly check for the `post_id` being a positive integer.
src/UI/class-row-actions.php (2)
Learnt from: acicovic
PR: Parsely/wp-parsely#2811
File: src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx:282-285
Timestamp: 2024-10-09T07:42:35.719Z
Learning: When linking to official WordPress documentation in `src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx`, wrapping URLs with the translation function `__()` is appropriate to allow localization to non-English documentation pages.
Learnt from: acicovic
PR: Parsely/wp-parsely#2811
File: src/content-helper/editor-sidebar/excerpt-suggestions/component-panel.tsx:237-238
Timestamp: 2024-10-16T12:31:43.982Z
Learning: In this WordPress plugin project, prefer using `rel="noopener"` for external links in buttons that open new tabs, as used in the Performance Stats component.
src/content-helper/common/providers/base-wordpress-provider.tsx (6)
Learnt from: acicovic
PR: Parsely/wp-parsely#2811
File: src/content-helper/editor-sidebar/tabs/sidebar-tools-tab.tsx:5-5
Timestamp: 2024-10-09T07:53:49.520Z
Learning: `PostTypeSupportCheck` should be imported from `@wordpress/editor`, following the official WordPress documentation.
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.
Learnt from: acicovic
PR: Parsely/wp-parsely#3293
File: src/content-helper/dashboard-page/components/posts-table/components/post-details.tsx:24-36
Timestamp: 2025-05-08T11:26:10.195Z
Learning: The PostDetails component should use the `getSmartShortDate()` function from '../../../../common/utils/date' rather than the WordPress format function for formatting dates. This provides better localization support and intelligent formatting that adapts to whether the date is in the current year.
Learnt from: acicovic
PR: Parsely/wp-parsely#2554
File: src/class-parsely.php:463-483
Timestamp: 2024-10-12T10:01:08.699Z
Learning: The `get_nested_option_value` method in `src/class-parsely.php` is intentionally designed without error handling for non-existent keys to meet specific design criteria.
Learnt from: acicovic
PR: Parsely/wp-parsely#2554
File: src/class-parsely.php:463-483
Timestamp: 2024-06-18T09:26:44.334Z
Learning: The `get_nested_option_value` method in `src/class-parsely.php` is intentionally designed without error handling for non-existent keys to meet specific design criteria.
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.
🧬 Code Graph Analysis (3)
tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php (1)
tests/Integration/RestAPI/BaseEndpointTest.php (1)
BaseEndpointTest(28-447)
src/rest-api/class-rest-api-controller.php (3)
src/rest-api/utils/class-utils-controller.php (1)
Utils_Controller(23-47)wp-parsely.php (1)
get_parsely(71-77)src/rest-api/class-base-api-controller.php (1)
get_parsely(127-129)
src/content-helper/common/providers/base-wordpress-provider.tsx (1)
src/content-helper/common/content-helper-error.tsx (1)
ContentHelperError(52-223)
🪛 GitHub Check: SonarCloud
src/content-helper/common/providers/base-wordpress-provider.tsx
[notice] 353-358: Exceptions should not be ignored
Handle this exception or don't catch it at all.See more on SonarQube Cloud
🪛 GitHub Check: PHP 8.3
tests/Integration/RestAPI/Utils/EndpointUtilsPostTest.php
[warning] 436-436:
Using ${var} in strings is deprecated, use {$var} instead
🪛 GitHub Check: PHP 8.4
tests/Integration/RestAPI/Utils/EndpointUtilsPostTest.php
[warning] 436-436:
Using ${var} in strings is deprecated, use {$var} instead
[warning] 436-436:
Using ${var} in strings is deprecated, use {$var} instead
🪛 GitHub Check: PHP 8.2
tests/Integration/RestAPI/Utils/EndpointUtilsPostTest.php
[warning] 436-436:
Using ${var} in strings is deprecated, use {$var} instead
[warning] 436-436:
Using ${var} in strings is deprecated, use {$var} instead
🔇 Additional comments (15)
tests/Integration/RestAPI/Stats/EndpointStatsPostTest.php (1)
26-26: LGTM: Class name change improves clarity.The renaming from
EndpointPostTesttoEndpointStatsPostTestmakes it clear that this test is specifically for the Stats API endpoint, helping to distinguish it from the new Utils API endpoint tests.src/rest-api/class-rest-api-controller.php (2)
16-16: LGTM: Import follows proper namespace conventions.The import statement correctly brings in the new
Utils_Controllerclass.
68-68: LGTM: Controller registration follows established pattern.The
Utils_Controlleris properly instantiated and added to the controllers array, following the same pattern as other controllers in the system.src/rest-api/utils/class-utils-controller.php (2)
23-33: LGTM: Controller follows established architecture patterns.The
Utils_Controllerclass correctly extendsREST_API_Controllerand implements the requiredget_route_prefix()method, following the same pattern as other controllers in the system.
40-46: LGTM: Endpoint registration follows standard pattern.The
init()method properly instantiates and registers endpoints using the establishedregister_endpoints()method.src/content-helper/common/providers/base-wordpress-provider.tsx (2)
106-113: LGTM: Type definition is well-structured.The
PostRestRouteResponsetype definition correctly represents the expected API response structure and follows TypeScript conventions.
346-375: LGTM: Dynamic REST route fetching improves flexibility.The implementation successfully replaces the hardcoded REST path with dynamic fetching, enabling support for pages and custom post types. The error handling appropriately catches failures and provides user-friendly error messages.
src/UI/class-row-actions.php (5)
22-22: Documentation update looks good.The @SInCE tag correctly documents the addition of the Engagement Boost link feature.
55-59: Filter hooks are properly implemented.The filters are correctly added for both post and page row actions, maintaining consistency with the existing Parse.ly Stats link implementation.
152-157: REST API support check is correctly implemented.The check properly validates that the post type supports the REST API, which is essential for the Engagement Boost feature to retrieve post data. The null check is also appropriately handled.
164-187: Method implementation is well-structured and secure.The renamed method properly:
- Escapes all output (URL, attributes, HTML text)
- Reuses the existing ARIA label generation for consistency
- Constructs the admin URL correctly with the post ID parameter
138-142: Keep internal feature identifier ‘traffic_boost’ in permission checks.The Permissions class and all feature mappings still use the key
traffic_boost. Renaming this identifier toengagement_boostwould break existing permission checks. No change is needed here—only the UI label has been updated to “Engagement Boost.”Likely an incorrect or invalid review comment.
tests/Integration/RestAPI/Utils/EndpointUtilsPostTest.php (3)
99-113: Route registration test is comprehensive.The test properly verifies that all endpoint routes are registered with the REST server and associated with the GET method.
145-223: Access control tests provide good security coverage.Both tests properly verify:
- API secret requirement with appropriate 403 error
- User capability restrictions for contributors
- Correct error codes and messages
261-417: Excellent test coverage for various post type scenarios.The tests comprehensively cover:
- Built-in post types (post, page)
- Custom post types with different REST API configurations
- Private vs public post types
- Custom REST base handling
- Filter hook functionality
Proper cleanup with
unregister_post_type()calls ensures test isolation.
…gement-boost" (988b5a1)
Description
Although we were showing "Engagement Boost" links for Pages and CPTs, the implementation only supported properly the built-in
Poststype. With this PR,Pageand custom post types are now also supported.It's also worth noting that Engagement Boost is not supported on post types that have their
show_in_restproperty set to false, as it uses the REST API to get the target post data.Motivation and context
How has this been tested?
Summary by CodeRabbit