Skip to content

Conversation

@jeherve
Copy link
Member

@jeherve jeherve commented Aug 1, 2019

Changes proposed in this Pull Request:

current_user_can only accepts object IDs as the optional second param; passing anything else (like an instance of Jetpack_Post like we did) will cause issues.

This was reported in 2184839-zen by a site owner using Polylang Pro.

Testing instructions:

I could not reproduce the Fatal myself, but something like this may help:

  • Connect your test site to WordPress.com.
  • Install the Polylang plugin.
  • Add the site to your mobile WordPress app.
  • Try to access the Posts screen in the app.
  • Check your site's error logs and make sure you do not see any errors like those:
PHP Catchable fatal error: Object of class Jetpack_Post could not be converted to string in /wp-content/plugins/polylang-pro/include/filters.php on line 305 

Proposed changelog entry for your changes:

  • WordPress.com API: avoid Fatal Errors when fetching posts from the mobile apps.

current_user_can only accepts object IDs as the optional second param; passing anything else (like an instance of Jetpack_Post like we did) will cause issues.
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] WPCOM API [Status] Needs Review This PR is ready for review. labels Aug 1, 2019
@jeherve jeherve added this to the 7.7 milestone Aug 1, 2019
@jeherve jeherve requested review from a team and zinigor August 1, 2019 13:52
@jeherve jeherve self-assigned this Aug 1, 2019
@matticbot matticbot added Customer Report [LEGACY] Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" Touches WP.com Files labels Aug 1, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello jeherve! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D31068-code before merging this PR. Thank you!

@jeherve jeherve modified the milestones: 7.7, 7.6 Aug 1, 2019
@jetpackbot
Copy link
Collaborator

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.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against aad988e

@Chouby
Copy link

Chouby commented Aug 1, 2019

@jeherve Thank you for the PR!
Some precision if you need to reproduce: You'll need to setup a privacy policy page to pass the first part of the test in https://github.com/polylang/polylang/blob/2.6.2/include/filters.php#L305.
FYI, the goal of the function is to disallow non-admins ( 'manage_privacy_options' capability ) to edit / delete the translations of the privacy policy page as WP does for the original page.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

Copy link
Contributor

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

I have been able to reproduce this problem using the plugin code from GitHub and a request to the v1.1 post update endpoint to the privacy page via WordPress.com REST API. LGTM!

@zinigor zinigor merged commit 391e015 into master Aug 2, 2019
@zinigor zinigor deleted the fix/fatal-current-user-can-sal branch August 2, 2019 22:16
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Aug 2, 2019
jeherve added a commit that referenced this pull request Aug 5, 2019
…r_can (#13166)

current_user_can only accepts object IDs as the optional second param; passing anything else (like an instance of Jetpack_Post like we did) will cause issues.
@jeherve
Copy link
Member Author

jeherve commented Aug 5, 2019

Cherry-picked to branch-7.6 in 6e2e2a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Customer Report [LEGACY] Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] WPCOM API Touches WP.com Files [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.

8 participants