-
Notifications
You must be signed in to change notification settings - Fork 767
QueryArrayWidget does not work with the foo=bar,baz syntax #1685 #1686
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
base: main
Are you sure you want to change the base?
Conversation
|
What should I do to get this merged in? |
carltongibson
left a comment
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.
I'm still not following the desire here. The QueryArrayWidget is for old PHP-style price[] query string parameters.
tests/test_widgets.py
Outdated
| <input type="text" name="price" value="1,2" />""", | ||
| ) | ||
|
|
||
| import django.http |
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.
Why is this import here?
| result = w.value_from_datadict({}, {}, "price") | ||
| self.assertEqual(result, []) | ||
|
|
||
| data = QueryDict("price=1,2") |
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.
This isn't a query array example. 🤔
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.
The idea is that all input would work, not just QueryArrays. Also ?price=1,2. QueryArray only supports ?price=1,price=2.
|
Thanks for the review! Will update the PR. The reason for this PR is that:
This makes it hard for the frontend to know when to use which and also the frontend like to use ?a=x,y also for the ModelMultipleChoiceFilter. With this fixes, you can in both cases use a QueryArrayWidget and all three search filters, for all types of filtering that uses multiple entries (whether __in or ModelMultipleChoiceFilter):
|
|
@carltongibson I know you are very busy, but do you a minute to express your feelings about this MR? It would solve some frontend head-aches. whereas otherwise it would be: apart from code cleanliness it is hard for the frontend to find out when they have to use which filter type, it will simply be omitted if you choose the wrong type. |
This is the failing test.