-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
New filter_id query #429
base: master
Are you sure you want to change the base?
New filter_id query #429
Conversation
with {:ok, filters} <- query_string |> parse_filter_ids() |> load_filters(context) do | ||
{:ok, Map.merge(context, %{filters: filters})} | ||
end |
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.
if there's only one point where the pattern diverges, case
is usually preferred over with
query_string
|> parse_filter_ids()
|> load_filters(context)
|> case do
{:ok, filters} ->
{:ok, Map.put(context, :filters, filters)}
err ->
err
end
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 did it like this in case prepare_context
ever needs to do more work than load filters but I can change it if you want
Before you begin
Introduces new search query
filter_id:123
that will retrieve and match the specified filter.filter_id
inside filters is not allowed.filter_id:1
- shows images not hidden by filter 1NOT filter_id:1
- shows images hidden by filter 1filter_id:1 AND filter_id:2
- shows images that pass both filters (i.e. not hidden by either)filter_id:1 OR filter_id:2
- shows images that pass either one of the filters (i.e. not hidden by both)Also: moderator fields are now allowed in watchlists
Should there also be
filter_name
that searches system and user filters?