From 885ea8a4d5f7191afd564501e3b013b1c9ff85f6 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 7 Feb 2020 18:04:40 -0500 Subject: [PATCH 1/6] Override get_bound_field() on FilterChoiceFieldMixin to restrict the queryset of bound fields --- netbox/utilities/forms.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index 355484673..28b906140 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -8,6 +8,7 @@ from django import forms from django.conf import settings from django.contrib.postgres.forms.jsonb import JSONField as _JSONField, InvalidJSONInput from django.db.models import Count +from django.forms import BoundField from mptt.forms import TreeNodeMultipleChoiceField from .choices import unpack_grouped_choices @@ -607,12 +608,26 @@ class FilterChoiceFieldMixin(object): kwargs['widget'] = forms.SelectMultiple(attrs={'size': 6}) super().__init__(*args, **kwargs) - def label_from_instance(self, obj): - label = super().label_from_instance(obj) - obj_count = getattr(obj, self.count_attr, None) - if obj_count is not None: - return '{} ({})'.format(label, obj_count) - return label + # def label_from_instance(self, obj): + # label = super().label_from_instance(obj) + # obj_count = getattr(obj, self.count_attr, None) + # if obj_count is not None: + # return '{} ({})'.format(label, obj_count) + # return label + + def get_bound_field(self, form, field_name): + + bound_field = BoundField(form, self, field_name) + + # Modify the QuerySet of the field before we return it. Limit choices to any data already bound: Options + # will be populated on-demand via the APISelect widget. + if bound_field.data: + kwargs = {'{}__in'.format(self.to_field_name or 'pk'): bound_field.data} + self.queryset = self.queryset.filter(**kwargs) + else: + self.queryset = self.queryset.none() + + return bound_field class FilterChoiceField(FilterChoiceFieldMixin, forms.ModelMultipleChoiceField): From 5ddfde2214f535239652fd6ab3ef00f7436a24a9 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 10 Feb 2020 09:40:34 -0500 Subject: [PATCH 2/6] Clean up unneeded code relevant to FilterChoiceField --- netbox/dcim/forms.py | 11 -------- netbox/ipam/forms.py | 8 ------ netbox/tenancy/forms.py | 3 --- netbox/utilities/forms.py | 47 ++++++---------------------------- netbox/virtualization/forms.py | 7 ----- 5 files changed, 8 insertions(+), 68 deletions(-) diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 3c3ae8b2e..81a0be2d2 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -734,7 +734,6 @@ class RackFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): 'site' ), label='Rack group', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/rack-groups/", null_option=True @@ -748,7 +747,6 @@ class RackFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): role = FilterChoiceField( queryset=RackRole.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/rack-roles/", value_field="slug", @@ -874,7 +872,6 @@ class RackReservationFilterForm(BootstrapMixin, TenancyFilterForm): group_id = FilterChoiceField( queryset=RackGroup.objects.prefetch_related('site'), label='Rack group', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/rack-groups/", null_option=True, @@ -2182,7 +2179,6 @@ class DeviceFilterForm(BootstrapMixin, LocalConfigContextFilterForm, TenancyFilt rack_id = FilterChoiceField( queryset=Rack.objects.all(), label='Rack', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/racks/", null_option=True, @@ -2219,7 +2215,6 @@ class DeviceFilterForm(BootstrapMixin, LocalConfigContextFilterForm, TenancyFilt platform = FilterChoiceField( queryset=Platform.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/platforms/", value_field="slug", @@ -3913,7 +3908,6 @@ class CableFilterForm(BootstrapMixin, forms.Form): rack_id = FilterChoiceField( queryset=Rack.objects.all(), label='Rack', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/racks/", null_option=True, @@ -4471,7 +4465,6 @@ class VirtualChassisFilterForm(BootstrapMixin, CustomFieldFilterForm): tenant_group = FilterChoiceField( queryset=TenantGroup.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/tenancy/tenant-groups/", value_field="slug", @@ -4484,7 +4477,6 @@ class VirtualChassisFilterForm(BootstrapMixin, CustomFieldFilterForm): tenant = FilterChoiceField( queryset=Tenant.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/tenancy/tenants/", value_field="slug", @@ -4592,7 +4584,6 @@ class PowerPanelFilterForm(BootstrapMixin, CustomFieldFilterForm): rack_group_id = FilterChoiceField( queryset=RackGroup.objects.all(), label='Rack group (ID)', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/rack-groups/", null_option=True, @@ -4826,7 +4817,6 @@ class PowerFeedFilterForm(BootstrapMixin, CustomFieldFilterForm): power_panel_id = FilterChoiceField( queryset=PowerPanel.objects.all(), label='Power panel', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/power-panels/", null_option=True, @@ -4835,7 +4825,6 @@ class PowerFeedFilterForm(BootstrapMixin, CustomFieldFilterForm): rack_id = FilterChoiceField( queryset=Rack.objects.all(), label='Rack', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/racks/", null_option=True, diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index 24f044f79..71aa73d18 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -528,7 +528,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm) vrf_id = FilterChoiceField( queryset=VRF.objects.all(), label='VRF', - null_label='-- Global --', widget=APISelectMultiple( api_url="/api/ipam/vrfs/", null_option=True, @@ -554,7 +553,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm) site = FilterChoiceField( queryset=Site.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/dcim/sites/", value_field="slug", @@ -564,7 +562,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm) role = FilterChoiceField( queryset=Role.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/ipam/roles/", value_field="slug", @@ -999,7 +996,6 @@ class IPAddressFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterFo vrf_id = FilterChoiceField( queryset=VRF.objects.all(), label='VRF', - null_label='-- Global --', widget=APISelectMultiple( api_url="/api/ipam/vrfs/", null_option=True, @@ -1080,7 +1076,6 @@ class VLANGroupFilterForm(BootstrapMixin, forms.Form): site = FilterChoiceField( queryset=Site.objects.all(), to_field_name='slug', - null_label='-- Global --', widget=APISelectMultiple( api_url="/api/dcim/sites/", value_field="slug", @@ -1279,7 +1274,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): site = FilterChoiceField( queryset=Site.objects.all(), to_field_name='slug', - null_label='-- Global --', widget=APISelectMultiple( api_url="/api/dcim/sites/", value_field="slug", @@ -1289,7 +1283,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): group_id = FilterChoiceField( queryset=VLANGroup.objects.all(), label='VLAN group', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/ipam/vlan-groups/", null_option=True, @@ -1303,7 +1296,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): role = FilterChoiceField( queryset=Role.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/ipam/roles/", value_field="slug", diff --git a/netbox/tenancy/forms.py b/netbox/tenancy/forms.py index b0468b37a..4babd753f 100644 --- a/netbox/tenancy/forms.py +++ b/netbox/tenancy/forms.py @@ -108,7 +108,6 @@ class TenantFilterForm(BootstrapMixin, CustomFieldFilterForm): group = FilterChoiceField( queryset=TenantGroup.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/tenancy/tenant-groups/", value_field="slug", @@ -163,7 +162,6 @@ class TenancyFilterForm(forms.Form): tenant_group = FilterChoiceField( queryset=TenantGroup.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/tenancy/tenant-groups/", value_field="slug", @@ -176,7 +174,6 @@ class TenancyFilterForm(forms.Form): tenant = FilterChoiceField( queryset=Tenant.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url="/api/tenancy/tenants/", value_field="slug", diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index 28b906140..464495fa0 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -582,47 +582,24 @@ class TagFilterField(forms.MultipleChoiceField): super().__init__(label='Tags', choices=get_choices, required=False, *args, **kwargs) -class FilterChoiceIterator(forms.models.ModelChoiceIterator): - - def __iter__(self): - # Filter on "empty" choice using FILTERS_NULL_CHOICE_VALUE (instead of an empty string) - if self.field.null_label is not None: - yield (settings.FILTERS_NULL_CHOICE_VALUE, self.field.null_label) - queryset = self.queryset.all() - # Can't use iterator() when queryset uses prefetch_related() - if not queryset._prefetch_related_lookups: - queryset = queryset.iterator() - for obj in queryset: - yield self.choice(obj) - - -class FilterChoiceFieldMixin(object): - iterator = FilterChoiceIterator - - def __init__(self, null_label=None, count_attr='filter_count', *args, **kwargs): - self.null_label = null_label - self.count_attr = count_attr +class FilterChoiceField(forms.ModelMultipleChoiceField): + """ + Override get_bound_field() to avoid pre-populating field choices with a SQL query. The field will be + rendered only with choices set via bound data. Choices are populated on-demand via the APISelect widget. + """ + def __init__(self, *args, **kwargs): + # Filter fields are not required by default if 'required' not in kwargs: kwargs['required'] = False - if 'widget' not in kwargs: - kwargs['widget'] = forms.SelectMultiple(attrs={'size': 6}) super().__init__(*args, **kwargs) - # def label_from_instance(self, obj): - # label = super().label_from_instance(obj) - # obj_count = getattr(obj, self.count_attr, None) - # if obj_count is not None: - # return '{} ({})'.format(label, obj_count) - # return label - def get_bound_field(self, form, field_name): - bound_field = BoundField(form, self, field_name) # Modify the QuerySet of the field before we return it. Limit choices to any data already bound: Options # will be populated on-demand via the APISelect widget. if bound_field.data: - kwargs = {'{}__in'.format(self.to_field_name or 'pk'): bound_field.data} + kwargs = {'{}__in'.format(self.to_field_name): bound_field.data} self.queryset = self.queryset.filter(**kwargs) else: self.queryset = self.queryset.none() @@ -630,14 +607,6 @@ class FilterChoiceFieldMixin(object): return bound_field -class FilterChoiceField(FilterChoiceFieldMixin, forms.ModelMultipleChoiceField): - pass - - -class FilterTreeNodeMultipleChoiceField(FilterChoiceFieldMixin, TreeNodeMultipleChoiceField): - pass - - class LaxURLField(forms.URLField): """ Modifies Django's built-in URLField in two ways: diff --git a/netbox/virtualization/forms.py b/netbox/virtualization/forms.py index 1560a683f..96136b4de 100644 --- a/netbox/virtualization/forms.py +++ b/netbox/virtualization/forms.py @@ -213,7 +213,6 @@ class ClusterFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm site = FilterChoiceField( queryset=Site.objects.all(), to_field_name='slug', - null_label='-- None --', required=False, widget=APISelectMultiple( api_url="/api/dcim/sites/", @@ -224,7 +223,6 @@ class ClusterFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm group = FilterChoiceField( queryset=ClusterGroup.objects.all(), to_field_name='slug', - null_label='-- None --', required=False, widget=APISelectMultiple( api_url="/api/virtualization/cluster-groups/", @@ -562,7 +560,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil cluster_group = FilterChoiceField( queryset=ClusterGroup.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url='/api/virtualization/cluster-groups/', value_field="slug", @@ -572,7 +569,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil cluster_type = FilterChoiceField( queryset=ClusterType.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url='/api/virtualization/cluster-types/', value_field="slug", @@ -601,7 +597,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil site = FilterChoiceField( queryset=Site.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url='/api/dcim/sites/', value_field="slug", @@ -611,7 +606,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil role = FilterChoiceField( queryset=DeviceRole.objects.filter(vm_role=True), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url='/api/dcim/device-roles/', value_field="slug", @@ -629,7 +623,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil platform = FilterChoiceField( queryset=Platform.objects.all(), to_field_name='slug', - null_label='-- None --', widget=APISelectMultiple( api_url='/api/dcim/platforms/', value_field="slug", From 55f5ede9708d23b5daaa79d687efde19929b3569 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 10 Feb 2020 09:58:33 -0500 Subject: [PATCH 3/6] Standardize usage of FilterChoiceField --- netbox/circuits/forms.py | 2 +- netbox/dcim/forms.py | 3 +-- netbox/extras/forms.py | 7 +++++-- netbox/utilities/forms.py | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/netbox/circuits/forms.py b/netbox/circuits/forms.py index caf8d9d36..39b694b1c 100644 --- a/netbox/circuits/forms.py +++ b/netbox/circuits/forms.py @@ -311,7 +311,7 @@ class CircuitFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm required=False, widget=StaticSelect2Multiple() ) - region = forms.ModelMultipleChoiceField( + region = FilterChoiceField( queryset=Region.objects.all(), to_field_name='slug', required=False, diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 81a0be2d2..b12d273a9 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -369,10 +369,9 @@ class SiteFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm): required=False, widget=StaticSelect2Multiple() ) - region = forms.ModelMultipleChoiceField( + region = FilterChoiceField( queryset=Region.objects.all(), to_field_name='slug', - required=False, widget=APISelectMultiple( api_url="/api/dcim/regions/", value_field="slug", diff --git a/netbox/extras/forms.py b/netbox/extras/forms.py index 8c9113d39..f9b765379 100644 --- a/netbox/extras/forms.py +++ b/netbox/extras/forms.py @@ -387,11 +387,14 @@ class ObjectChangeFilterForm(BootstrapMixin, forms.Form): ) action = forms.ChoiceField( choices=add_blank_choice(ObjectChangeActionChoices), - required=False + required=False, + widget=StaticSelect2() ) + # TODO: Convert to FilterChoiceField once we have an API endpoint for users user = forms.ModelChoiceField( queryset=User.objects.order_by('username'), - required=False + required=False, + widget=StaticSelect2() ) changed_object_type = forms.ModelChoiceField( queryset=ContentType.objects.order_by('model'), diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index 464495fa0..7b31f1e94 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -212,7 +212,7 @@ class SelectWithPK(StaticSelect2): option_template_name = 'widgets/select_option_with_pk.html' -class ContentTypeSelect(forms.Select): +class ContentTypeSelect(StaticSelect2): """ Appends an `api-value` attribute equal to the slugified model name for each ContentType. For example: From 009fc4f301e9a74016b03ecbaea6e9095c8f6f9c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 10 Feb 2020 10:02:42 -0500 Subject: [PATCH 4/6] Remove custom template for APISelect widget --- netbox/utilities/forms.py | 3 --- netbox/utilities/templates/widgets/select_api.html | 9 --------- 2 files changed, 12 deletions(-) delete mode 100644 netbox/utilities/templates/widgets/select_api.html diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index 7b31f1e94..85fb762bb 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -260,9 +260,6 @@ class APISelect(SelectWithDisabled): name of the query param and the value if the query param's value. :param null_option: If true, include the static null option in the selection list. """ - # Only preload the selected option(s); new options are dynamically displayed and added via the API - template_name = 'widgets/select_api.html' - def __init__( self, api_url, diff --git a/netbox/utilities/templates/widgets/select_api.html b/netbox/utilities/templates/widgets/select_api.html deleted file mode 100644 index d9516086b..000000000 --- a/netbox/utilities/templates/widgets/select_api.html +++ /dev/null @@ -1,9 +0,0 @@ - From 5008526db145059f66031f2e7b0fc6180327ed22 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 10 Feb 2020 10:08:20 -0500 Subject: [PATCH 5/6] Set a default self.to_field_name for FilterChoiceField --- netbox/utilities/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index 85fb762bb..cd78b249e 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -596,7 +596,7 @@ class FilterChoiceField(forms.ModelMultipleChoiceField): # Modify the QuerySet of the field before we return it. Limit choices to any data already bound: Options # will be populated on-demand via the APISelect widget. if bound_field.data: - kwargs = {'{}__in'.format(self.to_field_name): bound_field.data} + kwargs = {'{}__in'.format(self.to_field_name or 'pk'): bound_field.data} self.queryset = self.queryset.filter(**kwargs) else: self.queryset = self.queryset.none() From d4789b7c9e14ed5b180230c0511774c4c8c01de2 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 10 Feb 2020 10:20:06 -0500 Subject: [PATCH 6/6] Changelog for #4108 --- docs/release-notes/version-2.7.md | 1 + netbox/utilities/forms.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/version-2.7.md b/docs/release-notes/version-2.7.md index 44298fec3..fedf9f170 100644 --- a/docs/release-notes/version-2.7.md +++ b/docs/release-notes/version-2.7.md @@ -13,6 +13,7 @@ * [#4090](https://github.com/netbox-community/netbox/issues/4090) - Render URL custom fields as links under object view * [#4091](https://github.com/netbox-community/netbox/issues/4091) - Fix filtering of objects by custom fields using UI search form * [#4099](https://github.com/netbox-community/netbox/issues/4099) - Linkify interfaces on global interfaces list +* [#4108](https://github.com/netbox-community/netbox/issues/4108) - Avoid extraneous database queries when rendering search forms # v2.7.4 (2020-02-04) diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py index cd78b249e..8c0f0d8d1 100644 --- a/netbox/utilities/forms.py +++ b/netbox/utilities/forms.py @@ -9,7 +9,6 @@ from django.conf import settings from django.contrib.postgres.forms.jsonb import JSONField as _JSONField, InvalidJSONInput from django.db.models import Count from django.forms import BoundField -from mptt.forms import TreeNodeMultipleChoiceField from .choices import unpack_grouped_choices from .constants import *