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

Clean up authorize assertions #11656

Merged
merged 6 commits into from
Nov 18, 2024
Merged

Conversation

nanaya
Copy link
Collaborator

@nanaya nanaya commented Nov 15, 2024

  • reject null user in some cases
  • don't return anything

- reject null user in some cases
- don't return anything
Copy link
Collaborator

@notbakaneko notbakaneko left a comment

Choose a reason for hiding this comment

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

Maybe now is a good time to make the methods private as well

if ($user->isRestricted()) {
throw new AuthorizationCheckException($prefix.'restricted');
}

if ($user->isSilenced()) {
throw new AuthorizationCheckException($prefix.'silenced');
}

return 'ok';
}

/**
* @param User|null $user
Copy link
Collaborator

Choose a reason for hiding this comment

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

also needs updating, or just removed since it's not adding any new info, same with the other one

@notbakaneko notbakaneko merged commit 1fbc73b into ppy:master Nov 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants