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

Implement filtering options for Boolean types #388

Closed
Tracked by #247
kgodey opened this issue Jul 15, 2021 · 6 comments · Fixed by #618 or #1090
Closed
Tracked by #247

Implement filtering options for Boolean types #388

kgodey opened this issue Jul 15, 2021 · 6 comments · Fixed by #618 or #1090
Assignees
Labels
type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL

Comments

@kgodey
Copy link
Contributor

kgodey commented Jul 15, 2021

Problem

We need to ensure that records that include columns of Boolean type support the following filters via API:

  • is {true/false}
  • is not {true/false}
  • is empty
  • is not empty

This may already be done, please verify that it works and add some test cases specifically for boolean data types.

This involves:

  • Implementing the filters in the backend
  • Updating the /api/v0/databases/<id>/types/ endpoint to store available filters on this type
    • Filter information should include the number of parameters needing to be passed in (e.g. is needs 1 parameter, is empty needs 0)

Additional context

@kgodey kgodey added ready Ready for implementation type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL work: database labels Jul 15, 2021
@kgodey kgodey added this to the 07. Initial Data Types milestone Jul 15, 2021
@powellc
Copy link
Contributor

powellc commented Aug 19, 2021

This seems to be functional already. Adding tests and the available filters on the type endpoint.

@eito-fis
Copy link
Contributor

@powellc You might've seen this already, but there are already some tests for filtering at db/tests/records/test_filtering.py. It lists all the operators that are implemented by sqlalchemy-filters which might be useful.

And if you need to update our fork of sqlalchemy-filters, I've been working off this branch. If it needs to change, it might be worth properly organizing that repo and getting things merged into master and updating the readme - let me know and I can help out.

@powellc
Copy link
Contributor

powellc commented Aug 19, 2021

That's great @eito-fis, thanks! I hadn't see the filter tests yet. Do you think it makes sense to still test the filtering via the API, or would these test suffice to cover that logic? I think I'm leaning towards also adding tests to the mathesar/tests/views//api/test_records_api.py file, but I could be conviced not to 😄

@eito-fis
Copy link
Contributor

I was previously testing all the filtering on the backend and then just ensuring the API called the function with the right arguments. At the time it wasn't very principled, but I liked that the db tests ran faster than the API tests (django db gets reset between api tests) so I went with that.

@kgodey
Copy link
Contributor Author

kgodey commented Aug 19, 2021

@powellc @eito-fis I think it makes sense to add tests to the API for each data type to ensure that we support the specific filters for that data type.

@powellc
Copy link
Contributor

powellc commented Aug 19, 2021

Sounds good! I'll push up some API tests for filtering around booleans in a bit. Thanks!

powellc added a commit that referenced this issue Aug 25, 2021
powellc added a commit that referenced this issue Aug 25, 2021
powellc added a commit that referenced this issue Aug 25, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Aug 31, 2021
powellc added a commit that referenced this issue Sep 1, 2021
This commit adds a first pass at trying to add options for filtering to
the /api/v0/databases/<id>/types/ output.

The idea here is to make it pretty trivial to add the options for new
types to the endpoint. I'm not totally sold on the this being the best
format per-se, but I like how it gets injected automatically for a given
type and depends only on providing a definition. The definition can
change, but the place they go stays constant.
powellc added a commit that referenced this issue Sep 1, 2021
This commit adds a first pass at trying to add options for filtering to
the /api/v0/databases/<id>/types/ output.

The idea here is to make it pretty trivial to add the options for new
types to the endpoint. I'm not totally sold on the this being the best
format per-se, but I like how it gets injected automatically for a given
type and depends only on providing a definition. The definition can
change, but the place they go stays constant.
powellc added a commit that referenced this issue Sep 1, 2021
This commit uses a constant to identify the correct filter option for a
type, and also adds a better format.
powellc added a commit that referenced this issue Sep 1, 2021
This commit uses a constant to identify the correct filter option for a
type, and also adds a better format.
powellc added a commit that referenced this issue Sep 1, 2021
@dmos62 dmos62 reopened this Feb 21, 2022
@kgodey kgodey assigned dmos62 and unassigned powellc Feb 21, 2022
@kgodey kgodey added status: started and removed ready Ready for implementation labels Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL
Projects
No open projects
4 participants