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

Ignored queries options #2

Open
leemeichin opened this issue Jan 10, 2014 · 2 comments
Open

Ignored queries options #2

leemeichin opened this issue Jan 10, 2014 · 2 comments

Comments

@leemeichin
Copy link
Contributor

Some parameters are used to determine pagination and don't have any effect on the filters or queries passed in, eg. per_page and page. We use logic like this on Amnesty to save some time:

queries.except(*IGNORED_QUERIES).reject{|_,v| v.blank?}.each do |name, value|
  s.filter :term, name => Tire::Utils.escape(value)
end

Should some things have their own methods (ie. stuff that deals with pagination), or should there be an option to exclude them as actual queries?

Would it be worth having either an ignore option for search_option, or otherwise

@ismasan
Copy link

ismasan commented Jan 10, 2014

I'm thinking there 2 separate concerns here.

1). DSL for declaring allowed parameters and their options, as a mixin that can be added to your own presenters, search classes or even controllers.

2). Abstraction for easier building of ElasticSearch queries.

So from the point of view of the param-filtering DSL it shouldn't have to care what the params are used for.

re. programatically adding ES filters like this, I'm not sure how useful it is. For example in an app of mine I sometimes use :term and other times :terms (they have different behaviours). I think it's clearer to just add filters to the query case by case.

@leemeichin
Copy link
Contributor Author

Hmmm, they seem distinct enough to deal with entirely separately. I did think adapters would be a good way to go if it made any sense, but I'm not sure if that would just be bloat.

Our reason for programmatically declaring filters was that we had a hell of a lot of them (20 by my count, including the search field) that all behaved the same way. For us it was just a case of not wanting to repeat every parameter several times. Considering how the Tire gem works and without diving into its internals I'm not quite sure how to keep that DRY while offering some clarity :(

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

No branches or pull requests

2 participants