fix: change filter logic to avoid early returning #1852
Conversation
Signed-off-by: Adam Setch <[email protected]>
|
| return settings.filterIncludeHandles.some((handle) => | ||
| filterNotificationByHandle(notification, handle), | ||
| ); | ||
| passesFilters = |
There was a problem hiding this comment.
You probably want included handles to override the user type check, eg. if you don't check User but have an included handle you'd want to pass.
so: !exclude && (type || include) && reason
edit: although that would mean include is useless if User is checked. Maybe the presence of include handles should imply User is unchecked? (ie. that you only want the specified users)
There was a problem hiding this comment.
Why not leave the logic as-is and treat all selected filters as an AND 🤔
There was a problem hiding this comment.
It does mean as you pointed out (thank you for doing so), there is some interplay between User Type and the Handle Filters - but that seems OK to me.
@afonsojramos @bmulholland - interested to hear your thoughts 👂
There was a problem hiding this comment.
I think it looks good as is. It can probably be improved with a little modularity, but it is fine as it is.
resolves #1848