Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 1 file.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Agents Manager feature to use a new dedicated /wpcom/v2/agents-manager/state API endpoint instead of the general /me/preferences endpoint. The change follows the principle of least privilege by limiting access to only the specific Agents Manager preferences rather than all Calypso preferences.
Changes:
- Switched REST API proxy endpoint from
/me/preferencesto/agents-manager/statefor persisted state management - Removed
calypso_preferenceswrapper from request/response handling in favor of flat key-value format with astateparameter - Extracted a
DEFAULTSconstant to reduce duplication and centralize default values
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| class-wp-rest-agents-manager-persisted-open-state.php | Updated to use new /agents-manager/state endpoint, removed calypso_preferences wrapper, added DEFAULTS constant, improved error handling |
| class-agents-manager.php | Updated fetch_unified_experience_preference() to call /agents-manager/state?key=unified_ai_chat with updated comments |
| Agents_Manager_Test.php | Updated test mocks to check for new endpoint URL /agents-manager/state |
| changelog entry | Added changelog entry documenting the endpoint migration |
|
|
||
| // The response is the value of the preference directly when using preference_key. | ||
| // The response is { "unified_ai_chat": true/false } when using key param. | ||
| $result = ! empty( $decoded_body ); |
There was a problem hiding this comment.
The response parsing logic appears incorrect. According to the comment, the response should be { "unified_ai_chat": true/false } when using the key param, but the code uses ! empty( $decoded_body ) which will return true for any non-empty response object, regardless of the actual value of unified_ai_chat.
For example, if the API returns { "unified_ai_chat": false }, this code will incorrectly evaluate to true because the array is not empty. The code should explicitly check the value: $result = ! empty( $decoded_body['unified_ai_chat'] );
| $result = ! empty( $decoded_body ); | |
| $result = is_array( $decoded_body ) && ! empty( $decoded_body['unified_ai_chat'] ); |
Fixes AI-850
Paired with 202452-ghe-Automattic/wpcom
Proposed changes:
WP_REST_Agents_Manager_Persisted_Open_State) from the general/me/preferencesendpoint to the new dedicated/wpcom/v2/agents-manager/stateendpoint.calypso_preferenceswrapper from both request and response handling — the new endpoint uses flat key/value format.DEFAULTSconstant for fallback values, reducing duplication betweenget_stateandset_state.class-agents-manager.phpto fetch theunified_ai_chatpreference from/agents-manager/stateinstead of/me/preferences.Why are these changes being made?
/me/preferencesendpoint exposes all calypso preferences, which is unnecessarily broad for Agents Manager.Other information:
Does this pull request change what data or activity we track or use?
No
Testing instructions:
See Automattic/wp-calypso#108636 for testing instructions