Skip to content
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

Changed the behaviour of Prefix table: allow sorting #5452

Merged
merged 9 commits into from
Mar 21, 2024
2 changes: 1 addition & 1 deletion changes/4811.changed
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Changed the behavior of tree model tables: now they are sortable, and after sorting is applied, all hierarchy indentations are removed.
Changed the behavior of tree model tables and Prefix table: now they are sortable, and after sorting is applied, all hierarchy indentations are removed.
timizuoebideri1 marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 2 additions & 21 deletions nautobot/ipam/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,28 +42,10 @@
{% if record.present_in_database %}{% utilization_graph record.get_utilization %}{% else %}—{% endif %}
"""

PREFIX_LINK = """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this removed because it was unused? My only hesitation is whether we might have any apps that are relying on it that we don't know about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not exposed to the apps/plugin, or do plugin developers use constants not exposed to apps?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't supposed to but that doesn't mean they can't. Just being overly cautious here.

{% load helpers %}
{% for i in record.ancestors.count|as_range %}
<i class="mdi mdi-circle-small"></i>
{% endfor %}
<a href="\
{% if record.present_in_database %}\
{% url 'ipam:prefix' pk=record.pk %}\
{% else %}\
{% url 'ipam:prefix_add' %}\
?prefix={{ record }}&namespace={{ object.namespace.pk }}\
{% for loc in object.locations.all %}&locations={{ loc.pk }}{% endfor %}\
{% if object.tenant %}&tenant_group={{ object.tenant.tenant_group.pk }}&tenant={{ object.tenant.pk }}{% endif %}\
{% endif %}\
">{{ record.prefix }}</a>
"""

PREFIX_COPY_LINK = """
{% load helpers %}
{% for i in record.ancestors.count|as_range %}
<i class="mdi mdi-circle-small"></i>
{% endfor %}
{% tree_hierarchy_ui_representation record.ancestors.count|as_range table.order_by %}
<span class="hover_copy">
<a href="\
{% if record.present_in_database %}\
Expand Down Expand Up @@ -369,15 +351,14 @@ class PrefixTable(StatusTableMixin, RoleTableMixin, BaseTable):
namespace = tables.Column(linkify=True)
vlan = tables.Column(linkify=True, verbose_name="VLAN")
rir = tables.Column(linkify=True)
children = tables.Column(accessor="descendants_count")
children = tables.Column(accessor="descendants_count", orderable=False)
date_allocated = tables.DateTimeColumn()
location_count = LinkedCountColumn(
viewname="dcim:location_list", url_params={"prefixes": "pk"}, verbose_name="Locations"
)

class Meta(BaseTable.Meta):
model = Prefix
orderable = False
fields = (
"pk",
"prefix",
Expand Down
42 changes: 42 additions & 0 deletions nautobot/ipam/tests/test_tables.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from django.test import TestCase

from nautobot.core.models.querysets import count_related
from nautobot.dcim.models.locations import Location
from nautobot.ipam.models import Prefix
from nautobot.ipam.tables import PrefixTable


class PrefixTableTestCase(TestCase):
def _validate_sorted_queryset_same_with_table_queryset(self, queryset, table_class, field_name):
with self.subTest(f"Assert sorting {table_class.__name__} on '{field_name}'"):
table = table_class(queryset, order_by=field_name)
table_queryset_data = table.data.data.values_list("pk", flat=True)
sorted_queryset = queryset.order_by(field_name).values_list("pk", flat=True)
self.assertEqual(list(table_queryset_data), list(sorted_queryset))
glennmatthews marked this conversation as resolved.
Show resolved Hide resolved

def test_prefix_table_sort(self):
"""Assert TreeNode model table are orderable."""
pk_list = Prefix.objects.all().values_list("pk")[:20]
queryset = Prefix.objects.filter(pk__in=pk_list)
glennmatthews marked this conversation as resolved.
Show resolved Hide resolved

# Each of the table has at-least two sortable field_names in the field_names
table_avail_fields = ["prefix", "tenant", "namespace", "vlan", "rir"]
timizuoebideri1 marked this conversation as resolved.
Show resolved Hide resolved

# Assets model names
HanlinMiao marked this conversation as resolved.
Show resolved Hide resolved
table_avail_fields = ["tenant", "vlan", "namespace"]
for table_field_name in table_avail_fields:
self._validate_sorted_queryset_same_with_table_queryset(queryset, PrefixTable, table_field_name)
self._validate_sorted_queryset_same_with_table_queryset(queryset, PrefixTable, f"-{table_field_name}")

# Assert `prefix`
table_queryset_data = PrefixTable(queryset, order_by="prefix").data.data.values_list("pk", flat=True)
prefix_queryset = queryset.order_by("network", "prefix_length").values_list("pk", flat=True)
self.assertEqual(list(table_queryset_data), list(prefix_queryset))
table_queryset_data = PrefixTable(queryset, order_by="-prefix").data.data.values_list("pk", flat=True)
prefix_queryset = queryset.order_by("-network", "-prefix_length").values_list("pk", flat=True)
self.assertEqual(list(table_queryset_data), list(prefix_queryset))

# Assets `location_count`
location_count_queryset = queryset.annotate(location_count=count_related(Location, "prefixes")).all()
self._validate_sorted_queryset_same_with_table_queryset(location_count_queryset, PrefixTable, "location_count")
self._validate_sorted_queryset_same_with_table_queryset(location_count_queryset, PrefixTable, "-location_count")