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

Current user capabilities #3991

Merged

Conversation

oguzkocer
Copy link
Contributor

Related #3901. This PR adds a capabilities field to the Blog model which is saved as a json string from the network request. That property is also used to check list_users capability to decide whether to show the People page to the current user for a specific site or not. This is the very first PR that uses the capabilities, there will be others soon which will use at least edit_users & promote_users capabilities.

To test:

  • Go into my site page for a .com blog where you have the list_users capability (any site where you're an admin will do) and make sure the People row is visible
  • Go into another .com site where you don't have the list_users capability and make sure the People row is NOT visible. P.S: I've added my 2nd account as an Author to a test site to test this out.
  • Go into a self-hosted site and make sure the People row is NOT visible.

/cc @maxme: Whenever I add something to Blog I feel like I missed something, because it's scattered around. Do you mind taking a quick look at the PR to make sure that's not the case? Thanks!
/cc @astralbodies

Needs review: @hypest

@hypest hypest self-assigned this Apr 20, 2016
@@ -150,6 +152,10 @@ public static boolean addOrUpdateBlog(String blogName, String xmlRpcUrl, String
blog.setPlanShortName(planShortName);
blogUpdated = true;
}
if (!blog.getCapabilities().equals(capabilities)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The rest of the code allows for null and so the blog.getCapabilities() can return null. Better to null check. Cheers!

@hypest
Copy link
Contributor

hypest commented Apr 20, 2016

@oguzkocer , check out https://github.com/wordpress-mobile/WordPress-Android/blob/feature/current-user-capabilities/WordPress/src/main/java/org/wordpress/android/ui/main/MySiteFragment.java#L364. Up to now, the header was only dependant to the Settings view be visible but it will now need to take into account the People view as well. Cheers!

@hypest
Copy link
Contributor

hypest commented Apr 20, 2016

Thanks for the PR @oguzkocer. I finished with my review pass. Let me know if it needs any additional review. Cheers!

@hypest hypest removed their assignment Apr 20, 2016
@oguzkocer oguzkocer force-pushed the feature/current-user-capabilities branch from 179ad0e to 6d0f21a Compare April 20, 2016 16:43
@oguzkocer
Copy link
Contributor Author

Thanks for the review @hypest. Should be ready for another round!

@hypest
Copy link
Contributor

hypest commented Apr 21, 2016

Done with my 2nd pass @oguzkocer , thanks for the extra effort! No blockers. Ping me if more review is needed or to merge. Cheers!

@oguzkocer
Copy link
Contributor Author

I have replied to your latest comments @hypest and made the change for the toString method. Let's wait for @maxme before merging it in.

@maxme It's not that big of a deal if you don't have time for the review right now, we're going to ping you guys once this is ready to be merged into develop. So if you prefer, you can take a look at then. However you prefer is fine by me :)

@maxme
Copy link
Contributor

maxme commented Apr 25, 2016

Minor note #3991 (comment) - PR looks good, :shipit:

@maxme maxme merged commit 5bdfe9a into feature/people-management-sync Apr 25, 2016
@maxme maxme deleted the feature/current-user-capabilities branch April 25, 2016 12:24
@oguzkocer oguzkocer mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants