-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add suspended gambit #23
Conversation
What would the use case for this gambit be? |
In the user directory extension, we discussed adding a filter to see who is / isn't suspended. It was proposed that this gambit be added here instead of in user directory so that it could be available more universally |
Wouldn't this allow anyone to see who is suspended? Not everyone has permission to see that... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest wrapping the two wheres in a group to prevent any issue with other gambits:
->where(function(Builder $builder) {
$builder->where()->orWhere();
})
Also as @datitisev points out it needs a permission check.
I think we can re-use "who can suspend" that's already used to decide who can see the suspend badge https://github.com/flarum/suspend/blob/master/src/Listener/AddUserSuspendAttributes.php#L32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs permission checks
89a3d23
to
24ff144
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not tested, but looking good 👍
protected function conditions(AbstractSearch $search, array $matches, $negate) | ||
{ | ||
if (! $search instanceof UserSearch) { | ||
throw new LogicException('This gambit can only be applied on a DiscussionSearch'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant to write UserSearch here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's awkward 🙈, I'll push a fix shortly. Thanks for catching this!
* Add is:suspended gambit * Wrap where clauses in function * Added permission check for suspended gambit * Apply fixes from StyleCI Co-authored-by: luceos <daniel+github@klabbers.email>
* Add is:suspended gambit * Wrap where clauses in function * Added permission check for suspended gambit * Apply fixes from StyleCI Co-authored-by: luceos <daniel+github@klabbers.email>
Add gambit for filtering suspended / not suspended users