-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Only extract author info on a new Post / Page if the user can query for authors. #16870
Conversation
You can trigger an installable build for these changes by visiting CircleCI here. |
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
7be31c4
to
d2b017f
Compare
@@ -54,8 +54,8 @@ - (Post *)createPostForBlog:(Blog *)blog { | |||
post.postType = Post.typeDefaultIdentifier; | |||
|
|||
BlogAuthor *author = [blog getAuthorWithId:blog.userID]; | |||
post.authorID = author.userID ?: blog.account.userID; | |||
post.author = author.displayName ?: blog.account.displayName; | |||
post.authorID = author.userID; |
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.
Originally we fell back on the blog.account.userID
if we had no hit with getAuthorWithId
, but I don't believe this was necessary. Tracking the author ID and display name is only needed to have for the UI to switch authors.
Therefore if we don't have authors to choose from, we won't be doing any switching. When authorID
is nil
here, it won't be passed to the API and the update will be done "as the current user".
Testing d2b017f: Non-Jetpack self-hosted sitesAdministrator role
Author role
Jetpack self-hosted sitesAdministrator roleAdmin user:
Author roleAuthor user:
WordPress.com Simple sitesSite: paulvonschrottky.wordpress.com Administrator roleAdmin user:
Author roleAuthor user:
WordPress.com Atomic sitesSite: pschrottkyunsupportedblockatomic.wpcomstaging.com Administrator roleAdmin user:
Author roleAuthor user:
|
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.
Tested thoroughly and looks good! Great to see a fix involving deleted code! ❤️
Thank you @guarani! |
@twstokes this has been bundled as part of 17.8 beta 2 (17.8.0.2). Thanks for your work 🙌 |
Fixes #16858
This change addresses a bug where Atomic site users who had limited permissions (e.g. Authors, Contributors) couldn't create new Posts or Pages. This occurred because we were always passing the
blog.account.userID
to the API as theauthor
value on new Post and Page API requests. We learned that for Atomic sites this ID - which is their wp.com user ID - is not accepted by the backend. We should have instead been sending their ID that is local to the Atomic site.This fix drops sending the author's user ID unless the user has permissions to query for other authors on the site, and can populate
blog.authors
. In theblog.authors
array, the user ID is always the correct ID for the site, e.g. the ID for a user on the Atomic site and not necessarily always their wp.com user ID.To test:
Note: If the app's logged in user's role is downgraded, leftover state (e.g. previously downloaded authors) may remain, resulting in failures of the tests below. It's recommended to test user role changes by going from lowest up, or by wiping the app state clean each time. This should be considered a bug and an issue will be created.
Prerequisites for all test sites
Non-Jetpack self-hosted sites
There is a discrepancy for the Editor role. On web, an Editor can choose new authors. On WPiOS, the client cannot query for the list of users, and thus can't populate the authors, so the user will not have the Author option. In other words, for self-hosted sites the user role must be Administrator to use this feature.
Administrator role
Author role
Jetpack self-hosted sites
There is a discrepancy for the Editor role. On web, an Editor can choose new authors. On WPiOS, the client cannot query for the list of users, and thus can't populate the authors, so the user will not have the Author option. In other words, for self-hosted sites the user role must be Administrator to use this feature.
Administrator role
Author role
WordPress.com Simple sites
Administrator role
Author role
WordPress.com Atomic sites
Administrator role
Author role
Creating with author change
Updating with author change
Creating without author change
Updating without author change
Regression Notes
What I did to test those areas of impact (or what existing automated tests I relied on)
Worked with @guarani to complete the test cases provided in this PR.
What automated tests I added (or what prevented me from doing so)
No new tests were added for this change.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.