As described at https://www.drupal.org/project/prlp/issues/2572237 (that one is for D7), there are some cases when certain user roles are allowed to access/manage some site administration sections. But that's not the case for some "more sensitive" places, like this one.

Said that, I think it's better to have separated permissions. That way we can to decide if mentioned roles are able to access settings or not.

Patch in the way...

CommentFileSizeAuthor
#5 prlp-split_permissions-3126080-5.patch1.24 KBloze
#2 prlp-split_permissions-3126080-2.patch707 byteswaspper

Issue fork prlp-3126080

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

waspper created an issue. See original summary.

waspper’s picture

waspper commented
StatusFileSize
new prlp-split_permissions-3126080-2.patch707 bytes

Attaching patch...

waspper’s picture

waspper commented
Status: Active » Needs review
jitesh doshi’s picture

jitesh doshi commented
Status: Needs review » Postponed (maintainer needs more info)

Making this change will break existing installations. Can you please include a migration that assigns this new permission to all existing users that have the "administer users" permission?

loze’s picture

loze commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new prlp-split_permissions-3126080-5.patch1.24 KB

Here's a patch that adds an update hook.
Thanks.

qzmenko’s picture

qzmenko
Primary language Russian
Location Novosibirsk
commented
Status: Needs review » Reviewed & tested by the community

Thanks, new permission works.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented

Making this change will break existing installations. Can you please include a migration that assigns this new permission to all existing users that have the "administer users" permission?

Not sure patch #2 will break anything. Anyway, site administrators still get access to PRLP settings.

And in my case, this patch is useful because I don't want users with the permission "administer users" to be able to manage PRLP Settings. And in my case, when i give the permission "adminsiter users" I didn't want to give them the permision to manage PRLP settings. So In my mind patch #2 should be committed, not #5 which ensure that an unwanted situation is still present.

Otherwise RTBC++ (#2 or #5)

elc made their first commit to this issue’s fork.

elc opened merge request !20

elc’s picture

elc commented
Version: 8.x-1.x-dev » 2.0.x-dev

Turned into MR against 2.0.x branch.

Fixed up tests to use new permission. Cleanup on tests to avoid doing extra work which isn't tested or needed.

Tests are green all round.

anybody made their first commit to this issue’s fork.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Nice work, confirming RTBC. This is surely kind of a good standard now and makes sense!

  • anybody committed 7e064359 on 2.0.x authored by loze 
    [#3126080] by loze,waspper: Add permissions to administer PRLP.
ccrawford91’s picture

ccrawford91 commented
Status: Reviewed & tested by the community » Fixed

The PR has been merged and will be included in the next release.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.