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

Updating and reorganizing CAP tweaks #3238

Closed
wants to merge 8 commits into from
Closed

Updating and reorganizing CAP tweaks #3238

wants to merge 8 commits into from

Conversation

leogermani
Copy link
Contributor

@leogermani leogermani commented Jul 11, 2024

All Submissions:

Changes proposed in this Pull Request:

Improves our implementation of an alternative to CAP Guest Authors.

How to test the changes in this Pull Request:

  1. Checkout this branch
  2. Add define( 'NEWSPACK_DISABLE_CAP_GUEST_AUTHORS', true ); to wp-config
  3. Visit Users > Guest Authors and see the replacement page
  4. Click Add new Guest Author and confirm you see the regular form to add a new user but with some modifications (no username, no password)
  5. Create a user and confirm you can assign it as coauthor
  6. Confirm you can not login as this user
  7. Confirm the edit page for this user does not have a bunch of fields (username, password, application password, woo fields, profile colors, etc)
  8. @adekbadek confirm that this transitions well from a site using the previous approach. (the user role has the same slug and the capability is no longer checked)

-- test author cap ---

Add the edit_cap_posts capability to a subscribe user and confirm you can assign them as coauthors. Remove the capability and make sure you no longer can.

Make sure all other roles that can edit posts still can be assigned as coauthors.

-- test user account deletion --

Login as subscriber with the special cap and try to delete its account. Confirm you get a message saying you can't

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@leogermani leogermani marked this pull request as ready for review July 12, 2024 00:21
@leogermani leogermani requested a review from a team as a code owner July 12, 2024 00:21
@leogermani leogermani self-assigned this Jul 12, 2024
@leogermani leogermani added the [Status] Needs Review The issue or pull request needs to be reviewed label Jul 12, 2024
@leogermani leogermani changed the title Exploring cap Updating and reorganizing CAP tweaks Jul 12, 2024
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

I like how this improves what's proposed in #3233 and the blocking of the account deletion request. The login blocking and role name changes are not needed here.


\add_role( // phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.custom_role_add_role
self::CONTRIBUTOR_NO_EDIT_ROLE_NAME,
__( 'Guest Author', 'newspack-plugin' ),
Copy link
Member

Choose a reason for hiding this comment

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

This role is called "Non-Editing Contributor" now, it was already communicated as such to our publishers. It's intentionally different than "Guest Authors" to underline that it's something else than CoAuthors-Plus' Guest Authors. Someone used to creating Guest Authors should be directed to use this new feature instead, not confused by the familiar label cropping up in a different place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that, and the change here was also intentional.

The new overridden Guest Authors page point the user to use the Guest Authors role instead, so this change makes it consistent.

We are actually moving a feature the user knows from one place to another. Guest Authors used to live here, and now it lives there, but it's the same thing.

What could/should be called "Non-Editing Contributors" are users that have another "non editor" role but are cherry picked to be able to assigned as co authors (get the custom capability). Maybe we could add a section in their user profile to allow admins to grand this capability without the need of third-party plugins.

But these are just names. If you want we can keep the name and discuss it further. We just need to also change it in the new Guest Authors page replacement

}

if ( in_array( self::CONTRIBUTOR_NO_EDIT_ROLE_NAME, $user->roles, true ) ) {
return new WP_Error( 'guest_authors_cannot_login', __( 'Guest authors cannot login.', 'newspack-plugin' ) );
Copy link
Member

Choose a reason for hiding this comment

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

Why shouldn't these users be allowed to log in? The goal of the original implementation, in line with the requirements, was to allow any user to become assignable to posts. Many sites have users who are subscribers (should be able to manage their subscriptions), but they've contributed to an article and are listed as authors. They should still be able to log in and manage their subscriptions. The only thing they should not be able to do is editing posts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are blocking the users with this role from logging in, just as we are not event allowing a password to be set.

User with the custom capability that allows them to be assigned as co authors can still login. They are not affected.

Copy link
Member

Choose a reason for hiding this comment

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

User with the custom capability that allows them to be assigned as co authors can still login.

But users with multiple roles (e.g. subscriber,contributor_no_edit) won't be able to log in, and should.

Co-authored-by: Adam Cassis <adam.cassis@automattic.com>
@adekbadek
Copy link
Member

Should have asked earlier – the testing instructions state:

Add the edit_cap_posts capability to a subscribe user

How to do it in WP Admin UI, as a "non-technical user"?

@leogermani
Copy link
Contributor Author

Should have asked earlier – the testing instructions state:

Add the edit_cap_posts capability to a subscribe user

How to do it in WP Admin UI, as a "non-technical user"?

Using a plugin that manages roles and capabilities.

But as I said I think we should add an option in the edit user screen.

@adekbadek
Copy link
Member

adekbadek commented Jul 17, 2024

Using a plugin that manages roles and capabilities.

I'm using capability-manager-enhanced for role-related stuff. Can't see any option to add a capability to a single user. Capabilities can only be added to roles with this plugin. Can you recommend a plugin to achieve that goal?

Another issue with this approach is that it makes it impossible for the publisher to see which users are assignable to posts in the WP Admin Users view. It can be filtered by roles, but not by capabilities.

@leogermani
Copy link
Contributor Author

closed in favor of #3277 See pemhSX-Sz-p2

@leogermani leogermani closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review The issue or pull request needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants