-
Notifications
You must be signed in to change notification settings - Fork 8
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
Question about filters #16
Comments
No need to apologize! I appreciate your interest in Sinja and our chats have been enlightening. Maybe we'd be better off discussing some of the more informal questions on Gitter, but it's not really a big deal to me.
If you want to filter the collection returned by Here's a simple example that works with Sequel (and in fact this is almost exactly what Sinja::Sequel::Core does): helpers do
def filter(collection, fields)
collection.where(fields) # 2. add a where clause to the dataset
# ?filter[date]=2017-09-24&filter[otherfilter][]=1&filter[otherfilter][]=2 =>
# collection.where(:date=>'2017-09-24', :otherfilter=>[1, 2])
end
def finalize(collection)
collection.all # 3. turn the dataset into an array of models
end
end
index filter_by: [:date, :otherfilter] do
Model # 1. return the collection as a dataset, not an array of models
end |
Is it a way to require those two fields, like raise an error? I guess the halt command works in filter? |
There's no built-in way to require parameters, but yeah, (This is wrong, see below for corrected version.)helpers do
def filter(_, fields)
unless fields.key?(:date) && fields.key?(:otherfilter)
halt 400, 'You must provide date and otherfilter filters'
end
super
end
end
index filter_by: [:date, :otherfilter] do
# ..
end |
Sadly i'm hitting And /call is still being able to be hit |
Hrrm, what's the full URL and what does the |
The full url is Whole resource resource :batch do
helpers do
def filter(_, fields)
unless fields.key?(:date) && fields.key?(:channel_id)
halt 400, 'You must provide date and otherfilter filters'
end
super
end
def find(id)
Batch[id.to_i]
end
end
show
index(filter_by: [:date, :channel_id]) do
Batch
end
end The filter work on other calls but not on that specific one, even if I remove the special filter requirement. |
That certainly looks like a bug. Let me try to get a local repro and I'll work on a fix. Thanks! |
Alrighty, Let's say I have a movie model with a imdb_id as integer (instead of tt), the default provider then does a search for the thing as a string instead of a integer. |
RE: integer filters, here's the easiest way to do that, currently. In your resource controller: helpers do
def filter(_, fields)
fields[:imdb_id] = fields[:imdb_id].to_i
super
end
end |
I'm so stupid. Thanks for that. |
Slight correction to the above. To require filter fields, you'll want to put the check in helpers do
def before_index
unless params[:filter].key?(:date) && params[:filter].key?(:channel_id)
halt 400, 'You must provide date and otherfilter filters'
end
end
end |
Okey, now it complains if those filters isn't there, thanks. It still complains that the filter query is invalid if those two are provided. I also understand that channel_ids turns it into an array but using fitler[channel_id][] doesnt turn it into an array. Dump of invalid filter query:
|
Seems to be an issue with my index(filter_by: [:date, :channel_id]) do
Batch
end If I remove filter_by it works. And doing
if I run the model manually with the data with channel_id as a integer it works. |
These are all good findings that I want to fix! Bear with me! |
@jnylen Could you share with me the full SQL log for the bad query mentioned in your note from last week? The relevant part (about |
Hi @mwpastore,
With .to_i:
Doing it manually in padrino console works. SQL:
|
@jnylen The The problem is that somewhere along the line the filter keys are being transformed from symbols to strings before being passed to Sequel, so Postgres is literally trying to compare the string This is also why you're getting the error about invalid filter query parameters: Sinja is whitelisting symbols, but seeing strings. So now the question is: why and where is this happening? I thought you switched to Sinatra::Base for your controllers, are you still using Padrino? What versions of Sinatra and Padrino are present in your gem bundle? |
I think I need to cut a new release of Sinja with some of the work that I've pushed up over the past couple months. Do me a favor: try running the bleeding edge and seeing if that fixes the problem:
|
So, @mwpastore. Using the git repo instead of the one on rubygems works.
And using [] on non *_ids works aswell. So everything that failed seems to work on the git repo. So closing this. ;) |
Great! I'll push out a conservative patchlevel release and a more aggressive minor release over the weekend. Thank you so much for helping me troubleshoot this and get it figured out. |
Hi @mwpastore,
Sorry about all the issue tickets, this is merely a question about filters.
I want to be able to do ?filter[date]=2017-09-24&filter[otherfilter][]=1&filter[otherfilter][]=2
But i'm not sure how to do it. There is both show_many and index with filters. show_many doesn't seem to do (filter_by: [:date, :otherfilter]).
What is the correct way of handling it in Sinja? The docs are quite small on the filters part.
The text was updated successfully, but these errors were encountered: