Repository: netbox-community/netbox
Pull request: Fixes #2921: Replace tags filter with Select2 widget
### Fixes: #2921

Replaces the tags form with a Select2 widget inside of the search form.

Bonus: You no longer need to manually include the `inc/tags_panel.html` in the list view templates; the field will automatically get thrown to the bottom of the search form if the model supports tags. The inventory items list is a case in point.

Problem statement:
Replace tags list on object filters with a Select2 selection widget
### Environment
* Python version:  3.6.7
* NetBox version:  2.5.6

### Steps to Reproduce
1. Create prefixes in different sites, some with tags, some without
2. On the `Prefixes` page and in the `Tags` section of the WebUI, click to filter by one of the tags
3. You should now see only those prefixes that are assigned that tag value
4. Update the filter to limit your results to an individual site and click `Apply`
5. The previous tag filter is removed showing only the filter by site

<!-- What did you expect to happen? -->
### Expected Behavior
To be able to cumulatively add filters and tags as the same time to drill down and find what the object you may be looking for.

<!-- What happened instead? -->
### Observed Behavior
Explained above.

Hints from issue discussion:
This is not just present on the Prefix, but anywhere where the tag cloud exists.

I am not sure if this should be classed as a bug  as this appears to be how it was intended to operate.
I expected that it wasn't limited to just the `Prefix` model, but didn't look everywhere.
Also, I wasn't sure if it was a bug or an feature request, so I opened it as a bug as it **seemed** to be intended to do the same thing as the `filter` did.

To make sure I follow the correct process, do I need to open an FR or by having the `enhancement` tag added, is everything taken care of and this functionality will be added sometime in the future?

Let me know if I need to do anything else.
My vote would be to replace the current UI tag filtering method with a multi-select2 widget as a part of the standard filter forms.
That would be perfect!

Having separate methods for filtering that get in the way of each other is not ideal.
I was also surprised that when I selected a tag to drill down to a subset of devices, and then entered part of a device's name, that the tag filtering was lost.

However today there's no way to *remove* tag filtering either, apart from doing an unrelated search.

I agree that having a multi-select would be good, where you can select tag(s) and deselect them as separate operations.

It needs to be clear whether it acts as "and" or "or".  I guess a multi-select normally acts as "or", but there's also a valid use case for "and".  Perhaps there should be selection labelled "any" or "all".
I've identified a tag display bug (v2.6.6) that would be remediated by this fix.

Tag two objects (2 sites or 2 circuits for example) with tags that partially match: `zeta` and `zeta-tag`. In the list view, click the tag for `zeta-tag` to filter the results. You'll see both tags highlighted as if they've both been selected. I think the intent is that only the truly selected tag would be highlighted.

Template line in question: https://github.com/netbox-community/netbox/blob/develop/netbox/templates/inc/tags_panel.html#L10
I was tackling this issue by adding a new class that the filter forms can inherit from but ran into the problem with ordering as some of them have their field order set part-implicitly (some fields are in `field_order`, the rest follow the order that they were defined in).

Inheritance without modifying the order will place the tags field between the explicit and implicit fields in the filter form, so I'd have to order all of them manually. In addition, the field will become a part of the primary filter panel (not sure if we should keep it in a separate panel for distinction).

The straight-forward solution would be to just change `templates/inc/tags_panel.html`, which I'm not sure is the most elegant one, but it is the one involving the least changes.

Commit message: Fixes #2921: Replace tags filter with Select2 widget

Patch to review:
diff --git a/docs/release-notes/version-2.6.md b/docs/release-notes/version-2.6.md
index 88cd9c12060..c313d7e1b87 100644
--- a/docs/release-notes/version-2.6.md
+++ b/docs/release-notes/version-2.6.md
@@ -6,6 +6,7 @@
 * [#2050](https://github.com/netbox-community/netbox/issues/2050) - Preview image attachments when hovering the link
 * [#2113](https://github.com/netbox-community/netbox/issues/2113) - Allow NAPALM driver settings to be changed with request headers
 * [#2589](https://github.com/netbox-community/netbox/issues/2589) - Toggle for showing available prefixes/ip addresses
+* [#2921](https://github.com/netbox-community/netbox/issues/2921) - Replace tags filter with Select2 widget
 * [#3009](https://github.com/netbox-community/netbox/issues/3009) - Search by description when assigning IP address
 * [#3090](https://github.com/netbox-community/netbox/issues/3090) - Add filter field for device interfaces
 * [#3187](https://github.com/netbox-community/netbox/issues/3187) - Add rack selection field to rack elevations
diff --git a/netbox/templates/circuits/circuit_list.html b/netbox/templates/circuits/circuit_list.html
index d686bdf7a42..169aab0721a 100644
--- a/netbox/templates/circuits/circuit_list.html
+++ b/netbox/templates/circuits/circuit_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Circuits{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/circuits/provider_list.html b/netbox/templates/circuits/provider_list.html
index e4ee7fb2bbb..4126f75ec04 100644
--- a/netbox/templates/circuits/provider_list.html
+++ b/netbox/templates/circuits/provider_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Providers{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/device_list.html b/netbox/templates/dcim/device_list.html
index 623d69aa25b..8b991689ff6 100644
--- a/netbox/templates/dcim/device_list.html
+++ b/netbox/templates/dcim/device_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Devices{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/devicetype_list.html b/netbox/templates/dcim/devicetype_list.html
index 3b8988ed867..75f587f5dfb 100644
--- a/netbox/templates/dcim/devicetype_list.html
+++ b/netbox/templates/dcim/devicetype_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Device Types{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/powerfeed_list.html b/netbox/templates/dcim/powerfeed_list.html
index cfe2c989ce0..e384cb2c231 100644
--- a/netbox/templates/dcim/powerfeed_list.html
+++ b/netbox/templates/dcim/powerfeed_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Power Feeds{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/rack_list.html b/netbox/templates/dcim/rack_list.html
index 72da3048e5b..2724e4427ae 100644
--- a/netbox/templates/dcim/rack_list.html
+++ b/netbox/templates/dcim/rack_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Racks{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/site_list.html b/netbox/templates/dcim/site_list.html
index 64948a6f96c..ef9e0e4115c 100644
--- a/netbox/templates/dcim/site_list.html
+++ b/netbox/templates/dcim/site_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Sites{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/dcim/virtualchassis_list.html b/netbox/templates/dcim/virtualchassis_list.html
index 8c26f3c3e9c..55cfc16910b 100644
--- a/netbox/templates/dcim/virtualchassis_list.html
+++ b/netbox/templates/dcim/virtualchassis_list.html
@@ -13,7 +13,6 @@ <h1>{% block title %}Virtual Chassis{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/inc/tags_panel.html b/netbox/templates/inc/tags_panel.html
deleted file mode 100644
index a7923fbedc9..00000000000
--- a/netbox/templates/inc/tags_panel.html
+++ /dev/null
@@ -1,13 +0,0 @@
-{% load helpers %}
-
-<div class="panel panel-default">
-    <div class="panel-heading">
-        <span class="fa fa-tags" aria-hidden="true"></span>
-        <strong>Tags</strong>
-    </div>
-    <div class="panel-body text-center">
-        {% for tag in tags %}
-            <a href="{% querystring request tag=tag.slug %}" class="btn btn-sm {% if tag.slug in request.GET.tag %}btn-primary{% else %}btn-link{% endif %}">{{ tag }} <span class="badge">{{ tag.count }}</span></a>
-        {% endfor %}
-    </div>
-</div>
diff --git a/netbox/templates/ipam/aggregate_list.html b/netbox/templates/ipam/aggregate_list.html
index aad747b2dce..27363a56d4a 100644
--- a/netbox/templates/ipam/aggregate_list.html
+++ b/netbox/templates/ipam/aggregate_list.html
@@ -17,7 +17,6 @@ <h1>{% block title %}Aggregates{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
         <div class="panel panel-default">
             <div class="panel-heading">
                 <strong><i class="fa fa-bar-chart"></i> Statistics</strong>
diff --git a/netbox/templates/ipam/ipaddress_list.html b/netbox/templates/ipam/ipaddress_list.html
index 12f22730149..b7920a43466 100644
--- a/netbox/templates/ipam/ipaddress_list.html
+++ b/netbox/templates/ipam/ipaddress_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}IP Addresses{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
 	</div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/ipam/prefix_list.html b/netbox/templates/ipam/prefix_list.html
index b80af8e1d29..f0754d37bb5 100644
--- a/netbox/templates/ipam/prefix_list.html
+++ b/netbox/templates/ipam/prefix_list.html
@@ -21,7 +21,6 @@ <h1>{% block title %}Prefixes{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
 	</div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/ipam/service_list.html b/netbox/templates/ipam/service_list.html
index a39bec22e87..4aac520d9f8 100644
--- a/netbox/templates/ipam/service_list.html
+++ b/netbox/templates/ipam/service_list.html
@@ -12,7 +12,6 @@ <h1>{% block title %}Services{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
 	</div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/ipam/vlan_list.html b/netbox/templates/ipam/vlan_list.html
index b4d313a8cdd..24d538f88ea 100644
--- a/netbox/templates/ipam/vlan_list.html
+++ b/netbox/templates/ipam/vlan_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}VLANs{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
 	</div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/ipam/vrf_list.html b/netbox/templates/ipam/vrf_list.html
index 566e2f3e6cd..975c73a376b 100644
--- a/netbox/templates/ipam/vrf_list.html
+++ b/netbox/templates/ipam/vrf_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}VRFs{% endblock %}</h1>
 	</div>
 	<div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
 	</div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/secrets/secret_list.html b/netbox/templates/secrets/secret_list.html
index b6d792765fb..ee631b4392a 100644
--- a/netbox/templates/secrets/secret_list.html
+++ b/netbox/templates/secrets/secret_list.html
@@ -15,7 +15,6 @@ <h1>{% block title %}Secrets{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/tenancy/tenant_list.html b/netbox/templates/tenancy/tenant_list.html
index 91463c52c09..a77636a5b4a 100644
--- a/netbox/templates/tenancy/tenant_list.html
+++ b/netbox/templates/tenancy/tenant_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Tenants{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/virtualization/cluster_list.html b/netbox/templates/virtualization/cluster_list.html
index 3fef90c03d6..6f5f058ada7 100644
--- a/netbox/templates/virtualization/cluster_list.html
+++ b/netbox/templates/virtualization/cluster_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Clusters{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/templates/virtualization/virtualmachine_list.html b/netbox/templates/virtualization/virtualmachine_list.html
index b10341547d5..821f956a2dc 100644
--- a/netbox/templates/virtualization/virtualmachine_list.html
+++ b/netbox/templates/virtualization/virtualmachine_list.html
@@ -16,7 +16,6 @@ <h1>{% block title %}Virtual Machines{% endblock %}</h1>
     </div>
     <div class="col-md-3 noprint">
 		{% include 'inc/search_panel.html' %}
-		{% include 'inc/tags_panel.html' %}
     </div>
 </div>
 {% endblock %}
diff --git a/netbox/utilities/views.py b/netbox/utilities/views.py
index 1aa358fba6f..525fd92a9a3 100644
--- a/netbox/utilities/views.py
+++ b/netbox/utilities/views.py
@@ -8,7 +8,7 @@
 from django.db import transaction, IntegrityError
 from django.db.models import Count, ProtectedError
 from django.db.models.query import QuerySet
-from django.forms import CharField, Form, ModelMultipleChoiceField, MultipleHiddenInput, Textarea
+from django.forms import CharField, Form, ModelMultipleChoiceField, MultipleChoiceField, MultipleHiddenInput, Textarea
 from django.http import HttpResponse, HttpResponseServerError
 from django.shortcuts import get_object_or_404, redirect, render
 from django.template import loader
@@ -24,7 +24,7 @@
 
 from extras.models import CustomField, CustomFieldValue, ExportTemplate
 from extras.querysets import CustomFieldQueryset
-from utilities.forms import BootstrapMixin, CSVDataField
+from utilities.forms import BootstrapMixin, CSVDataField, StaticSelect2Multiple
 from utilities.utils import csv_format
 from .error_handlers import handle_protectederror
 from .forms import ConfirmationForm
@@ -94,6 +94,7 @@ def get(self, request):
 
         model = self.queryset.model
         content_type = ContentType.objects.get_for_model(model)
+        filter_form = self.filter_form(request.GET, label_suffix='') if self.filter_form else None
 
         if self.filter:
             self.queryset = self.filter(request.GET, self.queryset).qs
@@ -142,11 +143,17 @@ def get(self, request):
         if 'pk' in table.base_columns and (permissions['change'] or permissions['delete']):
             table.columns.show('pk')
 
-        # Construct queryset for tags list
-        if hasattr(model, 'tags'):
+        # Add the tags filter field to the from if the model has tags
+        if hasattr(model, 'tags') and filter_form:
             tags = model.tags.annotate(count=Count('extras_taggeditem_items')).order_by('name')
-        else:
-            tags = None
+            choices = [(str(tag.slug), '{} ({})'.format(tag.name, tag.count)) for tag in tags]
+
+            filter_form.fields['tag'] = MultipleChoiceField(
+                label='Tags',
+                choices=choices,
+                required=False,
+                widget=StaticSelect2Multiple(),
+            )
 
         # Apply the request context
         paginate = {
@@ -159,8 +166,7 @@ def get(self, request):
             'content_type': content_type,
             'table': table,
             'permissions': permissions,
-            'filter_form': self.filter_form(request.GET, label_suffix='') if self.filter_form else None,
-            'tags': tags,
+            'filter_form': filter_form,
         }
         context.update(self.extra_context())
 


Code review comments:
diff_hunk: @@ -142,11 +143,17 @@ def get(self, request):
         if 'pk' in table.base_columns and (permissions['change'] or permissions['delete']):
             table.columns.show('pk')
 
-        # Construct queryset for tags list
-        if hasattr(model, 'tags'):
+        # Add the tags filter field to the from if the model has tags
+        if hasattr(model, 'tags') and filter_form:
             tags = model.tags.annotate(count=Count('extras_taggeditem_items')).order_by('name')
-        else:
-            tags = None
+            choices = [(str(tag.slug), '{} ({})'.format(tag.name, tag.count)) for tag in tags]
+
+            filter_form.fields['tag'] = MultipleChoiceField(
path: netbox/utilities/views.py
text: @user1:
Love the overall approach (moving tags into a form field), but IMO we should handle this in the form itself rather than in the view. Both because it's a bit cleaner, and to ensure that the tag field is present anywhere else the form might be used.

Not sure if this is better done with a mixin or using a custom form field. We do want to preserve the current behavior of listing only assigned tags; not sure how tricky that will be.

@author:
The reason I put it in the view is to avoid modifying so much of the code in order to put the `Tags` field at the bottom of the form. If you look at `DeviceFilterForm`, you'll notice that the `field_order` is missing some of the fields, meaning they'll be thrown in the order that they were created at.

If I were to create a class to which the `*FilterForm` will inherit from, I'd have to explicitly define for the rest of the fields in `field_order` such that the `tag` field is placed at the bottom of the search form. If you're okay with me doing that, I'm happy to do it.

Regarding the listing-only-what's-assigned, that's not too much different than what's already happening; don't worry about it.

@user1:
Makes sense.

> I'd have to explicitly define for the rest of the fields in `field_order`

Works for me if you don't mind

@author:
Alright. I'll get crackin'.

@author:
Fixed now. Didn't have to play around with the `field_order` after all.