From 84de0458aa440ee8b79fa76484ac345c67bbbcf5 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 14:40:29 -0400 Subject: [PATCH] Implement nested RackGroups --- netbox/dcim/api/serializers.py | 3 +- netbox/dcim/filters.py | 10 +++++ netbox/dcim/forms.py | 38 ++++++++++++++-- .../dcim/migrations/0101_nested_rackgroups.py | 43 ++++++++++++++++++ .../0102_nested_rackgroups_rebuild.py | 21 +++++++++ netbox/dcim/models/__init__.py | 31 ++++++++++++- netbox/dcim/tables.py | 11 +++-- netbox/dcim/tests/test_api.py | 29 +++++++----- netbox/dcim/tests/test_filters.py | 44 ++++++++++++++----- netbox/dcim/tests/test_views.py | 12 +++-- netbox/dcim/views.py | 8 +++- 11 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 netbox/dcim/migrations/0101_nested_rackgroups.py create mode 100644 netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 5483904f5..85ff1895c 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -96,11 +96,12 @@ class SiteSerializer(TaggitSerializer, CustomFieldModelSerializer): class RackGroupSerializer(ValidatedModelSerializer): site = NestedSiteSerializer() + parent = NestedRackGroupSerializer(required=False, allow_null=True) rack_count = serializers.IntegerField(read_only=True) class Meta: model = RackGroup - fields = ['id', 'name', 'slug', 'site', 'rack_count'] + fields = ['id', 'name', 'slug', 'site', 'parent', 'rack_count'] class RackRoleSerializer(ValidatedModelSerializer): diff --git a/netbox/dcim/filters.py b/netbox/dcim/filters.py index a268e3114..3a21beb47 100644 --- a/netbox/dcim/filters.py +++ b/netbox/dcim/filters.py @@ -153,6 +153,16 @@ class RackGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): to_field_name='slug', label='Site (slug)', ) + parent_id = django_filters.ModelMultipleChoiceFilter( + queryset=RackGroup.objects.all(), + label='Rack group (ID)', + ) + parent = django_filters.ModelMultipleChoiceFilter( + field_name='parent__slug', + queryset=RackGroup.objects.all(), + to_field_name='slug', + label='Rack group (slug)', + ) class Meta: model = RackGroup diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 88d72eeda..1a6d60b86 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -386,7 +386,17 @@ class RackGroupForm(BootstrapMixin, forms.ModelForm): site = DynamicModelChoiceField( queryset=Site.objects.all(), widget=APISelect( - api_url="/api/dcim/sites/" + api_url="/api/dcim/sites/", + filter_for={ + 'parent': 'site_id', + } + ) + ) + parent = DynamicModelChoiceField( + queryset=RackGroup.objects.all(), + required=False, + widget=APISelect( + api_url="/api/dcim/rack-groups/" ) ) slug = SlugField() @@ -394,7 +404,7 @@ class RackGroupForm(BootstrapMixin, forms.ModelForm): class Meta: model = RackGroup fields = ( - 'site', 'name', 'slug', + 'site', 'parent', 'name', 'slug', ) @@ -407,6 +417,15 @@ class RackGroupCSVForm(forms.ModelForm): 'invalid_choice': 'Site not found.', } ) + parent = forms.ModelChoiceField( + queryset=RackGroup.objects.all(), + required=False, + to_field_name='name', + help_text='Name of parent rack group', + error_messages={ + 'invalid_choice': 'Rack group not found.', + } + ) class Meta: model = RackGroup @@ -426,7 +445,8 @@ class RackGroupFilterForm(BootstrapMixin, forms.Form): api_url="/api/dcim/regions/", value_field="slug", filter_for={ - 'site': 'region' + 'site': 'region', + 'parent': 'region', } ) ) @@ -437,6 +457,18 @@ class RackGroupFilterForm(BootstrapMixin, forms.Form): widget=APISelectMultiple( api_url="/api/dcim/sites/", value_field="slug", + filter_for={ + 'parent': 'site', + } + ) + ) + parent = DynamicModelMultipleChoiceField( + queryset=RackGroup.objects.all(), + to_field_name='slug', + required=False, + widget=APISelectMultiple( + api_url="/api/dcim/rack-groups/", + value_field="slug", ) ) diff --git a/netbox/dcim/migrations/0101_nested_rackgroups.py b/netbox/dcim/migrations/0101_nested_rackgroups.py new file mode 100644 index 000000000..dd5f8af26 --- /dev/null +++ b/netbox/dcim/migrations/0101_nested_rackgroups.py @@ -0,0 +1,43 @@ +from django.db import migrations, models +import django.db.models.deletion +import mptt.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0100_mptt_remove_indexes'), + ] + + operations = [ + migrations.AddField( + model_name='rackgroup', + name='parent', + field=mptt.fields.TreeForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='dcim.RackGroup'), + ), + migrations.AddField( + model_name='rackgroup', + name='level', + field=models.PositiveIntegerField(default=0, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='rackgroup', + name='lft', + field=models.PositiveIntegerField(default=1, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='rackgroup', + name='rght', + field=models.PositiveIntegerField(default=2, editable=False), + preserve_default=False, + ), + # tree_id will be set to a valid value during the following migration (which needs to be a separate migration) + migrations.AddField( + model_name='rackgroup', + name='tree_id', + field=models.PositiveIntegerField(db_index=True, default=0, editable=False), + preserve_default=False, + ), + ] diff --git a/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py b/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py new file mode 100644 index 000000000..411bb3abb --- /dev/null +++ b/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py @@ -0,0 +1,21 @@ +from django.db import migrations + + +def rebuild_mptt(apps, schema_editor): + RackGroup = apps.get_model('dcim', 'RackGroup') + for i, rackgroup in enumerate(RackGroup.objects.all(), start=1): + RackGroup.objects.filter(pk=rackgroup.pk).update(tree_id=i) + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0101_nested_rackgroups'), + ] + + operations = [ + migrations.RunPython( + code=rebuild_mptt, + reverse_code=migrations.RunPython.noop + ), + ] diff --git a/netbox/dcim/models/__init__.py b/netbox/dcim/models/__init__.py index af785f0d2..1dbfdb76b 100644 --- a/netbox/dcim/models/__init__.py +++ b/netbox/dcim/models/__init__.py @@ -283,7 +283,7 @@ class Site(ChangeLoggedModel, CustomFieldModel): # Racks # -class RackGroup(ChangeLoggedModel): +class RackGroup(MPTTModel, ChangeLoggedModel): """ Racks can be grouped as subsets within a Site. The scope of a group will depend on how Sites are defined. For example, if a Site spans a corporate campus, a RackGroup might be defined to represent each building within that @@ -298,8 +298,16 @@ class RackGroup(ChangeLoggedModel): on_delete=models.CASCADE, related_name='rack_groups' ) + parent = TreeForeignKey( + to='self', + on_delete=models.CASCADE, + related_name='children', + blank=True, + null=True, + db_index=True + ) - csv_headers = ['site', 'name', 'slug'] + csv_headers = ['site', 'parent', 'name', 'slug'] class Meta: ordering = ['site', 'name'] @@ -308,6 +316,9 @@ class RackGroup(ChangeLoggedModel): ['site', 'slug'], ] + class MPTTMeta: + order_insertion_by = ['name'] + def __str__(self): return self.name @@ -317,10 +328,26 @@ class RackGroup(ChangeLoggedModel): def to_csv(self): return ( self.site, + self.parent.name if self.parent else '', self.name, self.slug, ) + def to_objectchange(self, action): + # Remove MPTT-internal fields + return ObjectChange( + changed_object=self, + object_repr=str(self), + action=action, + object_data=serialize_object(self, exclude=['level', 'lft', 'rght', 'tree_id']) + ) + + def clean(self): + + # Parent RackGroup (if any) must belong to the same Site + if self.parent and self.parent.site != self.site: + raise ValidationError(f"Parent rack group ({self.parent}) must belong to the same site ({self.site})") + class RackRole(ChangeLoggedModel): """ diff --git a/netbox/dcim/tables.py b/netbox/dcim/tables.py index bc91dd70c..ebda79dc0 100644 --- a/netbox/dcim/tables.py +++ b/netbox/dcim/tables.py @@ -11,13 +11,13 @@ from .models import ( VirtualChassis, ) -REGION_LINK = """ +MPTT_LINK = """ {% if record.get_children %} {% else %} {% endif %} - {{ record.name }} + {{ record.name }} """ @@ -214,7 +214,7 @@ def get_component_template_actions(model_name): class RegionTable(BaseTable): pk = ToggleColumn() - name = tables.TemplateColumn(template_code=REGION_LINK, orderable=False) + name = tables.TemplateColumn(template_code=MPTT_LINK, orderable=False) site_count = tables.Column(verbose_name='Sites') slug = tables.Column(verbose_name='Slug') actions = tables.TemplateColumn( @@ -250,7 +250,10 @@ class SiteTable(BaseTable): class RackGroupTable(BaseTable): pk = ToggleColumn() - name = tables.LinkColumn() + name = tables.TemplateColumn( + template_code=MPTT_LINK, + orderable=False + ) site = tables.LinkColumn( viewname='dcim:site', args=[Accessor('site.slug')], diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index a0fb442f3..ddb9c0b52 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -349,9 +349,11 @@ class RackGroupTest(APITestCase): self.site1 = Site.objects.create(name='Test Site 1', slug='test-site-1') self.site2 = Site.objects.create(name='Test Site 2', slug='test-site-2') - self.rackgroup1 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 1', slug='test-rack-group-1') - self.rackgroup2 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 2', slug='test-rack-group-2') - self.rackgroup3 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 3', slug='test-rack-group-3') + self.parent_rackgroup1 = RackGroup.objects.create(site=self.site1, name='Parent Rack Group 1', slug='parent-rack-group-1') + self.parent_rackgroup2 = RackGroup.objects.create(site=self.site2, name='Parent Rack Group 2', slug='parent-rack-group-2') + self.rackgroup1 = RackGroup.objects.create(site=self.site1, name='Rack Group 1', slug='rack-group-1', parent=self.parent_rackgroup1) + self.rackgroup2 = RackGroup.objects.create(site=self.site1, name='Rack Group 2', slug='rack-group-2', parent=self.parent_rackgroup1) + self.rackgroup3 = RackGroup.objects.create(site=self.site1, name='Rack Group 3', slug='rack-group-3', parent=self.parent_rackgroup1) def test_get_rackgroup(self): @@ -365,7 +367,7 @@ class RackGroupTest(APITestCase): url = reverse('dcim-api:rackgroup-list') response = self.client.get(url, **self.header) - self.assertEqual(response.data['count'], 3) + self.assertEqual(response.data['count'], 5) def test_list_rackgroups_brief(self): @@ -380,20 +382,22 @@ class RackGroupTest(APITestCase): def test_create_rackgroup(self): data = { - 'name': 'Test Rack Group 4', - 'slug': 'test-rack-group-4', + 'name': 'Rack Group 4', + 'slug': 'rack-group-4', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, } url = reverse('dcim-api:rackgroup-list') response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(RackGroup.objects.count(), 4) + self.assertEqual(RackGroup.objects.count(), 6) rackgroup4 = RackGroup.objects.get(pk=response.data['id']) self.assertEqual(rackgroup4.name, data['name']) self.assertEqual(rackgroup4.slug, data['slug']) self.assertEqual(rackgroup4.site_id, data['site']) + self.assertEqual(rackgroup4.parent_id, data['parent']) def test_create_rackgroup_bulk(self): @@ -402,16 +406,19 @@ class RackGroupTest(APITestCase): 'name': 'Test Rack Group 4', 'slug': 'test-rack-group-4', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, { 'name': 'Test Rack Group 5', 'slug': 'test-rack-group-5', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, { 'name': 'Test Rack Group 6', 'slug': 'test-rack-group-6', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, ] @@ -419,7 +426,7 @@ class RackGroupTest(APITestCase): response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(RackGroup.objects.count(), 6) + self.assertEqual(RackGroup.objects.count(), 8) self.assertEqual(response.data[0]['name'], data[0]['name']) self.assertEqual(response.data[1]['name'], data[1]['name']) self.assertEqual(response.data[2]['name'], data[2]['name']) @@ -430,17 +437,19 @@ class RackGroupTest(APITestCase): 'name': 'Test Rack Group X', 'slug': 'test-rack-group-x', 'site': self.site2.pk, + 'parent': self.parent_rackgroup2.pk, } url = reverse('dcim-api:rackgroup-detail', kwargs={'pk': self.rackgroup1.pk}) response = self.client.put(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(RackGroup.objects.count(), 3) + self.assertEqual(RackGroup.objects.count(), 5) rackgroup1 = RackGroup.objects.get(pk=response.data['id']) self.assertEqual(rackgroup1.name, data['name']) self.assertEqual(rackgroup1.slug, data['slug']) self.assertEqual(rackgroup1.site_id, data['site']) + self.assertEqual(rackgroup1.parent_id, data['parent']) def test_delete_rackgroup(self): @@ -448,7 +457,7 @@ class RackGroupTest(APITestCase): response = self.client.delete(url, **self.header) self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) - self.assertEqual(RackGroup.objects.count(), 2) + self.assertEqual(RackGroup.objects.count(), 4) class RackRoleTest(APITestCase): diff --git a/netbox/dcim/tests/test_filters.py b/netbox/dcim/tests/test_filters.py index 02de313fc..edd366a58 100644 --- a/netbox/dcim/tests/test_filters.py +++ b/netbox/dcim/tests/test_filters.py @@ -186,12 +186,21 @@ class RackGroupTestCase(TestCase): ) Site.objects.bulk_create(sites) - rack_groups = ( - RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), - RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), - RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), + parent_rack_groups = ( + RackGroup(name='Parent Rack Group 1', slug='parent-rack-group-1', site=sites[0]), + RackGroup(name='Parent Rack Group 2', slug='parent-rack-group-2', site=sites[1]), + RackGroup(name='Parent Rack Group 3', slug='parent-rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in parent_rack_groups: + rackgroup.save() + + rack_groups = ( + RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0], parent=parent_rack_groups[0]), + RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1], parent=parent_rack_groups[1]), + RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2], parent=parent_rack_groups[2]), + ) + for rackgroup in rack_groups: + rackgroup.save() def test_id(self): id_list = self.queryset.values_list('id', flat=True)[:2] @@ -209,15 +218,22 @@ class RackGroupTestCase(TestCase): def test_region(self): regions = Region.objects.all()[:2] params = {'region_id': [regions[0].pk, regions[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) params = {'region': [regions[0].slug, regions[1].slug]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) def test_site(self): sites = Site.objects.all()[:2] params = {'site_id': [sites[0].pk, sites[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) params = {'site': [sites[0].slug, sites[1].slug]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) + + def test_parent(self): + parent_groups = RackGroup.objects.filter(name__startswith='Parent')[:2] + params = {'parent_id': [parent_groups[0].pk, parent_groups[1].pk]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'parent': [parent_groups[0].slug, parent_groups[1].slug]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) @@ -280,7 +296,8 @@ class RackTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() rack_roles = ( RackRole(name='Rack Role 1', slug='rack-role-1'), @@ -432,7 +449,8 @@ class RackReservationTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() racks = ( Rack(name='Rack 1', site=sites[0], group=rack_groups[0]), @@ -1146,7 +1164,8 @@ class DeviceTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() racks = ( Rack(name='Rack 1', site=sites[0], group=rack_groups[0]), @@ -2559,7 +2578,8 @@ class PowerPanelTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() power_panels = ( PowerPanel(name='Power Panel 1', site=sites[0], rack_group=rack_groups[0]), diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 2263a472b..d7434c808 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -122,11 +122,13 @@ class RackGroupTestCase(ViewTestCases.OrganizationalObjectViewTestCase): site = Site(name='Site 1', slug='site-1') site.save() - RackGroup.objects.bulk_create([ + rack_groups = ( RackGroup(name='Rack Group 1', slug='rack-group-1', site=site), RackGroup(name='Rack Group 2', slug='rack-group-2', site=site), RackGroup(name='Rack Group 3', slug='rack-group-3', site=site), - ]) + ) + for rackgroup in rack_groups: + rackgroup.save() cls.form_data = { 'name': 'Rack Group X', @@ -231,7 +233,8 @@ class RackTestCase(ViewTestCases.PrimaryObjectViewTestCase): RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]) ) - RackGroup.objects.bulk_create(rackgroups) + for rackgroup in rackgroups: + rackgroup.save() rackroles = ( RackRole(name='Rack Role 1', slug='rack-role-1'), @@ -1570,7 +1573,8 @@ class PowerPanelTestCase(ViewTestCases.PrimaryObjectViewTestCase): RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), ) - RackGroup.objects.bulk_create(rackgroups) + for rackgroup in rackgroups: + rackgroup.save() PowerPanel.objects.bulk_create(( PowerPanel(site=sites[0], rack_group=rackgroups[0], name='Power Panel 1'), diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index a639849ba..1e5ae9772 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -266,7 +266,13 @@ class SiteBulkDeleteView(PermissionRequiredMixin, BulkDeleteView): class RackGroupListView(PermissionRequiredMixin, ObjectListView): permission_required = 'dcim.view_rackgroup' - queryset = RackGroup.objects.prefetch_related('site').annotate(rack_count=Count('racks')) + queryset = RackGroup.objects.add_related_count( + RackGroup.objects.all(), + Rack, + 'group', + 'rack_count', + cumulative=True + ).prefetch_related('site') filterset = filters.RackGroupFilterSet filterset_form = forms.RackGroupFilterForm table = tables.RackGroupTable