Enhancement: Advanced Filtering API #1468
Conversation
|
This is nuts. Looks really powerful and something I want to get into Mealie, It looks like a pretty solid implementation I have a couple of questions.
|
|
Thank you! I was really happy with how it came out. I wrote the implementation myself, but I took inspiration from Freshdesk's relatively-new filter API which is functionally very similar: For a frontend wrapper, if we wanted to fully support it I've seen things that are essentially query builders: Or something like this (this is for calculations, not queries, but we could adapt a similar UI): The alternative would be something more lightweight that doesn't expose the full functionality. We could pull the schema, list all the fields, and allow users to select the operator/value. Something like: You can't get as granular with this (mainly because of parenthesis) but it would be simpler to implement. I'm happy to add some more tests! Pytest is a bit of an enigma to me but using what you've already done as reference has made it pretty easy. What sort of isolated cases did you have in mind? Just more specific stuff? |
There was a problem hiding this comment.
Very Cool, I added some notes on how I'd like to see the test broken out, if you don't get to it by Sunday Morning, I'll take a stab at it some I can get this merged before I take some time off.
Thanks for the additional context on your though process. A frontend query builder could be pretty interesting, I'll have to take a stab at that at some point.
| # check that basic filtering is working | ||
| query = PaginationQuery(page=1, per_page=-1, query_filter='name="test unit 2"') | ||
| unit_results = units_repo.page_all(query).items | ||
| assert len(unit_results) == 1 | ||
| assert unit_results[0].id == unit_2.id | ||
|
|
||
| # check that datetimes are resolving properly | ||
| dt = unit_2.created_at.isoformat() | ||
| query = PaginationQuery(page=1, per_page=-1, query_filter=f'createdAt>="{dt}"') | ||
| unit_results = units_repo.page_all(query).items | ||
| assert len(unit_results) == 2 | ||
| assert unit_1.id not in [unit.id for unit in unit_results] | ||
|
|
||
| # check that booleans are resolving properly | ||
| query = PaginationQuery(page=1, per_page=-1, query_filter="useAbbreviation=true") | ||
| unit_results = units_repo.page_all(query).items | ||
| assert len(unit_results) == 1 | ||
| assert unit_results[0].id == unit_1.id | ||
|
|
||
| # check an advanced filter | ||
| dt = unit_3.created_at.isoformat() | ||
| qf = f'name="test unit 1" OR (useAbbreviation=f AND (name="test unit 2" OR createdAt > "{dt}"))' | ||
| query = PaginationQuery(page=1, per_page=-1, query_filter=qf) | ||
| unit_results = units_repo.page_all(query).items | ||
| assert len(unit_results) == 2 | ||
| assert unit_3.id not in [unit.id for unit in unit_results] | ||
|
|
||
| # verify that improper queries throw 400 errors | ||
| route = "/api/units" | ||
|
|
||
| # unbalanced parenthesis | ||
| qf = '(name="test name" AND useAbbreviation=f))' | ||
| response = api_client.get(route, params={"queryFilter": qf}, headers=unique_user.token) | ||
| assert response.status_code == 400 | ||
|
|
||
| # invalid datetime format | ||
| qf = 'createdAt="this is not a valid datetime format"' | ||
| response = api_client.get(route, params={"queryFilter": qf}, headers=unique_user.token) | ||
| assert response.status_code == 400 | ||
|
|
||
| # invalid attribute | ||
| qf = 'badAttribute="test value"' | ||
| response = api_client.get(route, params={"queryFilter": qf}, headers=unique_user.token) | ||
| assert response.status_code == 400 |
There was a problem hiding this comment.
I'm happy to add some more tests! Pytest is a bit of an enigma to me but using what you've already done as reference has made it pretty easy. What sort of isolated cases did you have in mind? Just more specific stuff?
I think you cover most of the cases you've introduced. Basically I'm looking for each one of these comments to be their own test so it's easier to isolate specific cases that I end up breaking. You could probably accomplish this by running the setup in a fixture and injecting what you need into the function from there and just set the fixture up to only run once.
There was a problem hiding this comment.
Ah I see, so separate them out into their own functions so it handles them as separate tests. That makes sense! I'll see if I can get it done tonight or tomorrow morning; I'll be out of town for a few days (leaving tomorrow) so if I don't get to it by then it's all yours :)
This extends the
get_allroutes to include a robust query filter. It allows the user to combine multiple relational and logical operators and group them arbitrarily.Supported Relational Operators :
=, <>, >, <, >=, <=Supported Logical Operators:
AND, ORThis enables simple query strings such as:
name = "Pasta Fagioli" OR name = "My Favorite Recipe"createdAt > "2021-02-22"But parenthesis can be used for more robust filters such as:
name="cup" OR (useAbbreviation=f AND (name="ounce" OR createdAt > "2022-07-04T19:49:27Z"))The implementation uses a combination of enums (for operators), attribute validation (to prevent calling the database with an attribute that doesn't exist) and parameterized queries (to prevent SQL injection) for validation. It also throws a
400error when predictable mistakes are made (invalid attributes, unbalanced parenthesis, or invalid date/datetime formats).The meat of this PR is in
query_filter.pywhich does all the parsing and validation; the remaining codebase remains largely unchanged.Assuming this PR is accepted, I will do a proper writeup in the docs