-
Notifications
You must be signed in to change notification settings - Fork 76
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
Allow for filtering of multiple values + fix bug farhan0581#15 for Django 2.2 #18
base: master
Are you sure you want to change the base?
Conversation
Allow for filtering of multiple values + fix bug farhan0581#15 for Django 2.2
Update filters.py
admin_auto_filters/filters.py
Outdated
@@ -100,12 +103,15 @@ def has_output(self): | |||
def lookups(self, request, model_admin): | |||
return () | |||
|
|||
def value(self): | |||
return self.used_parameters.get(self.parameter_name).split(',') if self.used_parameters.get(self.parameter_name) else [] |
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.
Wouldn't this be better with classical if - else block?
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.
Sure I changed it.
Also had to add var jQuery = django.jQuery, $ = jQuery;
to the beginning of the js file for it to work consistently.
To fix the bug of having to have an empty Media class in the Admin class, it probably would be better to attach the media to a form class surrounding the select as in django-admin-rangefilter.
It does not seem correct to have a select outside of a form anyway.
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.
My recommendation would be
parameter_name = self.used_parameters.get(self.parameter_name)
if parameter_name:
return parameter_name.split(',')
return []
@farhan0581 @jphilip Any ETA on getting this merged? This seems to fix multiple issues (#21, #17, #16). |
widget = AutocompleteSelect(remote_field, | ||
model_admin.admin_site, | ||
custom_url=self.get_autocomplete_url(request, model_admin),) | ||
widget = AutocompleteSelectMultiple( |
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.
we need to provide the functionality for custom url for autocomplete, but it would be completely removed in this change. Require changes here.
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.
Should be done in this PR: #81
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.
functionality to provide custom_url.
This change allows for filtering by one or more values instead of just one provided few changes.