-
Notifications
You must be signed in to change notification settings - Fork 381
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
Always hide AMP admin menu item and compatibility tool menu items for non-admins role #3005
Conversation
…AMP settings screen
@@ -994,6 +975,9 @@ public function test_add_admin_notices() { | |||
* @covers \AMP_Validation_Error_Taxonomy::filter_tag_row_actions() | |||
*/ | |||
public function test_filter_tag_row_actions() { | |||
wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); | |||
global $pagenow; | |||
$pagenow = 'edit-tags.php'; |
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.
I'm confused as to why this is needed now. Perhaps global state had set it before.
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.
Yes, this seems to be the case.
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.
Welcome change! Works as expected.
…p-bind-syntax * 'develop' of github.com:ampproject/amp-wp: RTLCSS all the things (#2977) Fix AMP Story editor compatibility with code editor (#3007) Update dependency core-js to v3.2.1 (#3011) Update amphtml validator spec to v1907301630320 (#3003) Improve handling of unlisted Vimeo videos (#2986) Always hide AMP admin menu item and compatibility tool menu ite… (#3005) Update dependency dom-scroll-into-view to v2.0.1 (#3008) Hide tooltips that should be hidden (#2988)
See https://wordpress.org/support/topic/hide-amp-in-side-bar-for-specific-roles-authors/
When a user is not an administrator, they still have access to the AMP settings screen but all of the fields on the screen are disabled and they can't modify anything. This was allowed because the Validated URLs and Validation Error screens are are admin submenu items under the top-level AMP menu page. Thus the top-level page was given
edit_posts
capability, though all of the settings requiredmanage_options
to change.The thinking here was to allow the users to access the admin screens the compatibility tool even if they cannot
manage_options
. In reality, this is just noise and non-admins should not be concerned with site-level validation errors. See #2316 (comment) and #2673.Nevertheless, a case can be made to continue allowing a user to access the Validated URL screen individually for posts that they can edit. This is what this PR does. Non-admin users never see the top-level AMP admin menu item, and they never see the admin menu items for Validated URLs and Validation Errors. The only way they can get to these screens is by causing a validation error, at which point they will see the warning notice in Gutenberg, allowing them to access the screen via the “Review Issues” link.
This PR also hides the Validated URLs from the “At a Glance” dashboard widget, if the user is not an administrator.
Before
After
Build for testing: amp.zip - 1.2.1-beta1-20190809T205628Z-303e81ea
Fixes #2702.