-
Notifications
You must be signed in to change notification settings - Fork 263
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
Fix advance filter error on multiple custom filter #3574
Fix advance filter error on multiple custom filter #3574
Conversation
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.
Also, this seems to only address the MultiSelect symptom reported in #3391. What about fixing the Select symptom as well?
on custom field of the type selection
Only one entry can be selected.
|
||
if not required or default_choice is None: | ||
choices = add_blank_choice(choices) | ||
|
||
# Set the initial value to the first available choice (if any) | ||
if set_initial and default_choice: | ||
initial = default_choice.value | ||
|
||
if self.type == CustomFieldTypeChoices.TYPE_SELECT: | ||
if not required or default_choice is None: | ||
choices = add_blank_choice(choices) | ||
field_class = CSVChoiceField if for_csv_import else forms.ChoiceField |
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 change feels wrong to me - the form field (as opposed to the filter form field) for both Select and MultiSelect custom fields should have a blank choice, shouldn't it? Can you help me understand why this should be specific to Select fields only?
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.
By default multi select fields have blanks
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.
Adding it makes blank a choice in the multi select, which was causing error
@@ -70,7 +70,7 @@ class CustomFieldJSONFilter(CustomFieldFilterMixin, django_filters.Filter): | |||
"""Custom field single value filter for backwards compatibility""" | |||
|
|||
|
|||
class CustomFieldMultiSelectFilter(CustomFieldFilterMixin, django_filters.Filter): | |||
class CustomFieldMultiSelectFilter(CustomFieldFilterMixin, django_filters.MultipleChoiceFilter): | |||
"""Custom field single value filter for backwards compatibility""" |
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.
Is the docstring here no longer accurate/relevant? Do we in fact need this to remain a non-multiple-choice filter for some compatibility reason?
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.
040edaf
to
8738527
Compare
…mizuoebideri1-3391-cf-multiple_advance_filter
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.
LGTM.
…ttps://github.com/nautobot/nautobot into u/timizuoebideri1-3391-cf-multiple_advance_filter
Closes: #3391
What's Changed
Error reported was caused by
is_single_choice_field
returningTrue
forCustomFieldMultiSelectFilter
becauseCustomFieldMultiSelectFilter
inherits fromdjango_filters.Filter
and notdjango_filters.MultipleChoiceFilter
.Also fixed an additional bug where the page freeze after selecting a custom multiple field choice.
TODO