Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
SearchFilter with unaccent#7733
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
SearchFilter with unaccent#7733
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Sharpek commentedSep 4, 2021
Hello, what about this PR ? It's look good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
can you also add a small unit test for this change?
As I explain in the PR description:
I've followed what was done in the Full-text search, which is basically that there is no test. Adding test for this would involve spinning up a PostgreSQL in order to do so, and it feels outside the scope of the intended change. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@auvipy could you take another look at this? |
it should be mr.@tomchristie |
alixleger commentedSep 21, 2022
Hello, what about this PR ? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hey@tomchristie , do you think you could take a look at this PR? |
I will also have a look |
| @@ -45,6 +45,7 @@ class SearchFilter(BaseFilterBackend): | |||
| '=': 'iexact', | |||
| '@': 'search', | |||
| '$': 'iregex', | |||
| '&': 'unaccent', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
can you please add relevant test for this search feature? or they are already covered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@auvipy hey! echoing a bit whatI mentioned last year:
This feature is only for Django's PostgreSQL backend, which does not include tests in this repo.
Adding tests could be more complex than the scope of this PR, I think it's a perfectly valid issue to open, but would not block this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
https://github.com/encode/django-rest-framework/blob/master/tests/test_filters.py#L52 not possible to add any tests here right?
I tried to trigger new CI but it seems the repository from which the PR was originated has been removed! so can not re open it! might do it myself in another PR |
Description
Implements#7727
New restriction for SearchFilter:
&Accent-insensitive search.As this is only supported by Django'sPostgreSQL backend.), no test was added (same case as the Full-text search restriction).
This uses the
__unaccentlookup, allowing for accent-insensitive searches.