[OSIDB-4588] Add in filter for API views #1138
Conversation
Jincxz
changed the title
osoukup
left a comment
osoukup
left a comment
There was a problem hiding this comment.
LGTM. Just out of curiosity, what would happen when a user runs a query like
...?impact=LOW&impact__in=LOW,MODERATE
which is stupid but possible. Both filter apply or somehow interfere?
They are different query parameters, so they should be applied in |
JakubFrejlach
left a comment
JakubFrejlach
left a comment
There was a problem hiding this comment.
Some changes needed, but overall looking good.
| def test_in_filter_with_uuid_fields(self, auth_client, test_api_v2_uri): | ||
| """Test that UUID fields work with auto-in filter""" | ||
| flaw1 = FlawFactory(embargoed=False) | ||
| flaw2 = FlawFactory(embargoed=False) | ||
| affect1 = AffectFactory(flaw=flaw1) | ||
| affect2 = AffectFactory(flaw=flaw2) | ||
|
|
||
| # Test filtering by single UUID first | ||
| response = auth_client().get(f"{test_api_v2_uri}/affects?uuid={affect1.uuid}") | ||
| assert response.status_code == status.HTTP_200_OK | ||
| assert response.json()["count"] == 1 | ||
|
|
||
| # Test filtering by multiple UUIDs with __in suffix | ||
| response = auth_client().get( | ||
| f"{test_api_v2_uri}/affects?uuid__in={affect1.uuid},{affect2.uuid}" | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK | ||
| results = response.json()["results"] | ||
| assert len(results) == 2 | ||
| uuids = {r["uuid"] for r in results} | ||
| assert uuids == {str(affect1.uuid), str(affect2.uuid)} |
There was a problem hiding this comment.
there is currently no filter uuid__in because the InFilterSet creates matching in filters only for CharFilter or ChoiceFilter whereas uuid uses UUIDFilter. This test succeeds because since there is no uuid__in filter defined, supplying the uuid__in query parameter has no affect and the query simply returns all affects and since there are total 2 affects in this test case, the assert len(results) == 2 actually passes.
| f"{test_api_v2_uri}/affects?flaw__source__in=INTERNET,CUSTOMER" | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK | ||
| assert response.json()["count"] == 2 |
There was a problem hiding this comment.
This test has in total 2 affects and the assert checks for existence of 2 affects, I would add one more affect with different Flaw source to really test whether the filter is applied.
| - in: query | ||
| name: affects__affectedness__in | ||
| schema: | ||
| type: array | ||
| items: | ||
| type: string | ||
| enum: | ||
| - '' | ||
| - AFFECTED | ||
| - NEW | ||
| - NOTAFFECTED | ||
| description: |+ | ||
| Multiple values may be separated by commas. | ||
| explode: false | ||
| style: form |
There was a problem hiding this comment.
some of the new in filters has allowed empty values, how would passing that in the in filter work? 🤔 Do we want to/need to support it?
There was a problem hiding this comment.
Looks like we can just query for the empty values by leaving the space in the comma separation empty (e.g. affects?affectedness__in=,AFFECTED). I have added some tests for this.
| "uuid": ["exact"], | ||
| "name": ["exact"], | ||
| "affiliation": ["exact"], | ||
| "from_upstream": ["exact"], | ||
| "created_dt": ["exact"] | ||
| + LT_GT_LOOKUP_EXPRS | ||
| + LTE_GTE_LOOKUP_EXPRS |
There was a problem hiding this comment.
One question, why not just pass in in this array?
Is part of the documentation https://django-filter.readthedocs.io/en/stable/guide/usage.html#generating-filters-with-meta-fields
There was a problem hiding this comment.
I had initially considered that but it was a bit of manually tracking so I programmatically added the fields instead (just in case I would end up missing some).
Jincxz
force-pushed
the
OSIDB-4588-in-filter
branch
from
709f307 to
1d55c89
Compare
Jincxz
force-pushed
the
OSIDB-4588-in-filter
branch
from
1d55c89 to
30b369b
Compare
Did some testing and it does look like a query like |
That sounds like a desired behavior. Thank you for testing it! |
This PR adds "in" filters for CharFilter and ChoiceFilter fields in the API views (not including the V1 API endpoints). The new filters can be accessed via the "in" lookup (e.g. {field_name}__in=A,B).
Closes OSIDB-4588