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

feat: Add gte and lte filters for sqla #1787

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fredthomsen
Copy link
Contributor

Description

Add <= and >= filters for SQLA. These seem common enough that including them here makes sense.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

@codecov
Copy link

codecov bot commented Jan 14, 2022

Codecov Report

Merging #1787 (6c1d020) into master (e2b744c) will decrease coverage by 30.54%.
The diff coverage is 44.44%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1787       +/-   ##
===========================================
- Coverage   77.05%   46.50%   -30.55%     
===========================================
  Files          56       56               
  Lines        8175     8046      -129     
===========================================
- Hits         6299     3742     -2557     
- Misses       1876     4304     +2428     
Flag Coverage Δ
python 46.50% <44.44%> (-30.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
flask_appbuilder/models/sqla/filters.py 44.32% <44.44%> (-36.52%) ⬇️
flask_appbuilder/security/sqla/models.py 0.00% <0.00%> (-97.94%) ⬇️
flask_appbuilder/models/generic/interface.py 0.00% <0.00%> (-92.31%) ⬇️
flask_appbuilder/models/generic/filters.py 0.00% <0.00%> (-78.38%) ⬇️
flask_appbuilder/api/convert.py 20.14% <0.00%> (-72.00%) ⬇️
flask_appbuilder/security/sqla/manager.py 0.00% <0.00%> (-71.39%) ⬇️
flask_appbuilder/models/sqla/interface.py 19.20% <0.00%> (-59.64%) ⬇️
flask_appbuilder/hooks.py 43.33% <0.00%> (-56.67%) ⬇️
flask_appbuilder/api/__init__.py 40.52% <0.00%> (-55.88%) ⬇️
flask_appbuilder/api/schemas.py 47.61% <0.00%> (-52.39%) ⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2b744c...6c1d020. Read the comment docs.

@fedepad
Copy link
Contributor

fedepad commented Jan 28, 2022

The tests seem to fail because you need to update the field_* here: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/tests/test_api.py#L1783

by adding the new filters for the field_* types for which you defined them in the conversion table.
As far as I can see from your code you will need to add for field_date, field_integer, field_float.
If you need to update other tests you will see.
Check if you need to implement for Mongo too if the same test is run for both...

@stale
Copy link

stale bot commented Apr 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

@stale stale bot added the stale label Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants