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

#1207 Module - canUpdate - check if user has manage permissions #1208

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michalcharvat
Copy link
Contributor

No description provided.

@michalcharvat michalcharvat removed the request for review from dehart January 6, 2025 14:17
@michalcharvat
Copy link
Contributor Author

@mschering Can I merge/commit changes like this or shall I wait until you approve them?

@dehart
Copy link
Contributor

dehart commented Jan 6, 2025

I think the change to the default behaviour is good. However mayManage might not exist as it is configurable per module. When the user is admin the existing permissions should all be true so this check should be the fallback eg.

protected function canUpdate(Entity $entity): bool
{
    return $entity->getUserRights()->mayManage ?? go()->getAuthState()->isAdmin();
}

@mschering thoughts?

@mschering
Copy link
Member

Yes I think Michael's solution will work.

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.

3 participants