Skip to content
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

REST API: Fix user connection data without owner #18363

Merged
merged 2 commits into from Jan 15, 2021

Conversation

fgiannar
Copy link
Contributor

This PR fixes a PHP notice when trying to fetch user connection data while a connection owner does not exist (user-less).

Changes proposed in this Pull Request:

  • Updates the get_user_connection_data method in Jetpack_Core_Json_Api_Endpoints to properly handle cases where the connection owner does not exist.
  • Adds a corresponding unit test

Jetpack product discussion

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Manual

Before applying this fix:

  • Start with a fresh Jetpack site
  • Enable user-less connection mode by setting JETPACK_NO_USER_TEST_MODE ( in wp-config.php or via Settings -> Jetpack Constants)
  • Set up Jetpack in user-less mode (aka skip user authentication)
  • Go to Jetpack Dashboard and notice the following notice when fetching: jetpack/v4/connection/data:
    The notice will be visible either in debug.log or in your browser console, Network tab -> XHR -> GET wp-json/jetpack/v4/connection/data:

PHP Notice: Trying to get property 'display_name' of non-object in /usr/local/src/jetpack-monorepo/projects/plugins/jetpack/_inc/lib/class.core-rest-api-endpoints.php on line 1598

  • Switch to this branch and confirm that the Notice is gone.

Automated

phpunit --filter=test_get_user_connection_data_without_master_user

Proposed changelog entry for your changes:

  • REST API: fix PHP notice when fetching user connection data without a connection owner.

@fgiannar fgiannar added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Feature] WP REST API labels Jan 15, 2021
@fgiannar fgiannar added this to the 9.4 milestone Jan 15, 2021
@fgiannar fgiannar self-assigned this Jan 15, 2021
@jetpackbot
Copy link

jetpackbot commented Jan 15, 2021

Scheduled Jetpack release: February 2, 2021.
Scheduled code freeze: January 25, 2021

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 05b975a

@sergeymitr sergeymitr self-requested a review January 15, 2021 16:43
Copy link
Contributor

@sergeymitr sergeymitr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sergeymitr sergeymitr added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Jan 15, 2021
@kraftbj kraftbj merged commit aa967fe into master Jan 15, 2021
@kraftbj kraftbj deleted the fix/user-connection-data-no-owner branch January 15, 2021 21:56
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 15, 2021
jeherve added a commit that referenced this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] WP REST API [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants