From b43980d6607babee9222d4c5d19b0896c8a72914 Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Fri, 3 Dec 2021 15:09:56 -0500 Subject: [PATCH] Fixes #7960: Prevent creation of regions/site groups/locations with duplicate names (see #7354) --- docs/release-notes/version-3.1.md | 1 + .../0137_relax_uniqueness_constraints.py | 56 +++++++-- netbox/dcim/models/sites.py | 110 ++++++++++++++++-- 3 files changed, 148 insertions(+), 19 deletions(-) diff --git a/docs/release-notes/version-3.1.md b/docs/release-notes/version-3.1.md index 86058f6e3..0ab4a611d 100644 --- a/docs/release-notes/version-3.1.md +++ b/docs/release-notes/version-3.1.md @@ -27,6 +27,7 @@ * [#7771](https://github.com/netbox-community/netbox/issues/7771) - Group assignment should be optional when creating contacts via REST API * [#7849](https://github.com/netbox-community/netbox/issues/7849) - Fix exception when creating an FHRPGroup with an invalid IP address * [#7880](https://github.com/netbox-community/netbox/issues/7880) - Include assigned IP addresses in FHRP group object representation +* [#7960](https://github.com/netbox-community/netbox/issues/7960) - Prevent creation of regions/site groups/locations with duplicate names (see #7354) ### REST API Changes diff --git a/netbox/dcim/migrations/0137_relax_uniqueness_constraints.py b/netbox/dcim/migrations/0137_relax_uniqueness_constraints.py index 8f7d40026..7cedb1b08 100644 --- a/netbox/dcim/migrations/0137_relax_uniqueness_constraints.py +++ b/netbox/dcim/migrations/0137_relax_uniqueness_constraints.py @@ -1,5 +1,3 @@ -# Generated by Django 3.2.8 on 2021-10-19 17:41 - from django.db import migrations, models @@ -32,14 +30,54 @@ class Migration(migrations.Migration): ), migrations.AlterUniqueTogether( name='location', - unique_together={('site', 'parent', 'name'), ('site', 'parent', 'slug')}, + unique_together=set(), ), - migrations.AlterUniqueTogether( - name='region', - unique_together={('parent', 'slug'), ('parent', 'name')}, + migrations.AddConstraint( + model_name='location', + constraint=models.UniqueConstraint(fields=('site', 'parent', 'name'), name='dcim_location_parent_name'), ), - migrations.AlterUniqueTogether( - name='sitegroup', - unique_together={('parent', 'slug'), ('parent', 'name')}, + migrations.AddConstraint( + model_name='location', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('site', 'name'), name='dcim_location_name'), + ), + migrations.AddConstraint( + model_name='location', + constraint=models.UniqueConstraint(fields=('site', 'parent', 'slug'), name='dcim_location_parent_slug'), + ), + migrations.AddConstraint( + model_name='location', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('site', 'slug'), name='dcim_location_slug'), + ), + migrations.AddConstraint( + model_name='region', + constraint=models.UniqueConstraint(fields=('parent', 'name'), name='dcim_region_parent_name'), + ), + migrations.AddConstraint( + model_name='region', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('name',), name='dcim_region_name'), + ), + migrations.AddConstraint( + model_name='region', + constraint=models.UniqueConstraint(fields=('parent', 'slug'), name='dcim_region_parent_slug'), + ), + migrations.AddConstraint( + model_name='region', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('slug',), name='dcim_region_slug'), + ), + migrations.AddConstraint( + model_name='sitegroup', + constraint=models.UniqueConstraint(fields=('parent', 'name'), name='dcim_sitegroup_parent_name'), + ), + migrations.AddConstraint( + model_name='sitegroup', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('name',), name='dcim_sitegroup_name'), + ), + migrations.AddConstraint( + model_name='sitegroup', + constraint=models.UniqueConstraint(fields=('parent', 'slug'), name='dcim_sitegroup_parent_slug'), + ), + migrations.AddConstraint( + model_name='sitegroup', + constraint=models.UniqueConstraint(condition=models.Q(('parent', None)), fields=('slug',), name='dcim_sitegroup_slug'), ), ] diff --git a/netbox/dcim/models/sites.py b/netbox/dcim/models/sites.py index fd40b30c4..a19ae8050 100644 --- a/netbox/dcim/models/sites.py +++ b/netbox/dcim/models/sites.py @@ -62,11 +62,41 @@ class Region(NestedGroupModel): ) class Meta: - unique_together = ( - ('parent', 'name'), - ('parent', 'slug'), + constraints = ( + models.UniqueConstraint( + fields=('parent', 'name'), + name='dcim_region_parent_name' + ), + models.UniqueConstraint( + fields=('name',), + name='dcim_region_name', + condition=Q(parent=None) + ), + models.UniqueConstraint( + fields=('parent', 'slug'), + name='dcim_region_parent_slug' + ), + models.UniqueConstraint( + fields=('slug',), + name='dcim_region_slug', + condition=Q(parent=None) + ), ) + def validate_unique(self, exclude=None): + if self.parent is None: + regions = Region.objects.exclude(pk=self.pk) + if regions.filter(name=self.name, parent__isnull=True).exists(): + raise ValidationError({ + 'name': 'A region with this name already exists.' + }) + if regions.filter(slug=self.slug, parent__isnull=True).exists(): + raise ValidationError({ + 'name': 'A region with this slug already exists.' + }) + + super().validate_unique(exclude=exclude) + def get_absolute_url(self): return reverse('dcim:region', args=[self.pk]) @@ -119,11 +149,41 @@ class SiteGroup(NestedGroupModel): ) class Meta: - unique_together = ( - ('parent', 'name'), - ('parent', 'slug'), + constraints = ( + models.UniqueConstraint( + fields=('parent', 'name'), + name='dcim_sitegroup_parent_name' + ), + models.UniqueConstraint( + fields=('name',), + name='dcim_sitegroup_name', + condition=Q(parent=None) + ), + models.UniqueConstraint( + fields=('parent', 'slug'), + name='dcim_sitegroup_parent_slug' + ), + models.UniqueConstraint( + fields=('slug',), + name='dcim_sitegroup_slug', + condition=Q(parent=None) + ), ) + def validate_unique(self, exclude=None): + if self.parent is None: + site_groups = SiteGroup.objects.exclude(pk=self.pk) + if site_groups.filter(name=self.name, parent__isnull=True).exists(): + raise ValidationError({ + 'name': 'A site group with this name already exists.' + }) + if site_groups.filter(slug=self.slug, parent__isnull=True).exists(): + raise ValidationError({ + 'name': 'A site group with this slug already exists.' + }) + + super().validate_unique(exclude=exclude) + def get_absolute_url(self): return reverse('dcim:sitegroup', args=[self.pk]) @@ -335,10 +395,40 @@ class Location(NestedGroupModel): class Meta: ordering = ['site', 'name'] - unique_together = ([ - ('site', 'parent', 'name'), - ('site', 'parent', 'slug'), - ]) + constraints = ( + models.UniqueConstraint( + fields=('site', 'parent', 'name'), + name='dcim_location_parent_name' + ), + models.UniqueConstraint( + fields=('site', 'name'), + name='dcim_location_name', + condition=Q(parent=None) + ), + models.UniqueConstraint( + fields=('site', 'parent', 'slug'), + name='dcim_location_parent_slug' + ), + models.UniqueConstraint( + fields=('site', 'slug'), + name='dcim_location_slug', + condition=Q(parent=None) + ), + ) + + def validate_unique(self, exclude=None): + if self.parent is None: + locations = Location.objects.exclude(pk=self.pk) + if locations.filter(name=self.name, site=self.site, parent__isnull=True).exists(): + raise ValidationError({ + "name": f"A location with this name in site {self.site} already exists." + }) + if locations.filter(slug=self.slug, site=self.site, parent__isnull=True).exists(): + raise ValidationError({ + "name": f"A location with this slug in site {self.site} already exists." + }) + + super().validate_unique(exclude=exclude) def get_absolute_url(self): return reverse('dcim:location', args=[self.pk])