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

make user.editGroups depending on viewHiddenGroups #2880

Merged
merged 1 commit into from
Aug 10, 2021
Merged

make user.editGroups depending on viewHiddenGroups #2880

merged 1 commit into from
Aug 10, 2021

Conversation

Ornanovitch
Copy link
Contributor

@Ornanovitch Ornanovitch commented May 20, 2021

Fixes flarum/issue-archive#149

Changes proposed in this pull request:

make user.editGroups depending on viewHiddenGroups

Reviewers should focus on:

is it that simple?

Confirmed

  • Frontend changes: tested on a local Flarum installation.

@SychO9 SychO9 self-requested a review June 5, 2021 18:40
Copy link
Member

@SychO9 SychO9 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I was unsure if we did anything else on the backend for permissions relying on others, but it seems to just be a helper on the frontend 🤔

@SychO9 SychO9 merged commit 13d302b into flarum:master Aug 10, 2021
@luceos
Copy link
Member

luceos commented Aug 13, 2021

Wouldn't it have been more logical to make the backend responsible for looping over the groups that are not visible and locking them, while updating those that are visible and have been changed?

Because now we require a permission for another permission to work. I can imagine situations where groups can be hidden (like authorative users) but mods should still be allowed to upgrade simple member groups. @jordanjay29 a penny for your thoughts.

@SychO9
Copy link
Member

SychO9 commented Aug 15, 2021

@luceos the plan was to go with https://github.com/flarum/core/issues/2610#issuecomment-840677323 and have this done temporarily.

though I don't have any strong opinions on this, feel free to re-open the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

edit a user without "view hidden group bages" permission can break membership of the user
4 participants