Skip to content

Commit

Permalink
Fix #4149 - correct logic in RackForm
Browse files Browse the repository at this point in the history
  • Loading branch information
glennmatthews committed Aug 9, 2023
1 parent 9ed631a commit 214a807
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 13 deletions.
1 change: 1 addition & 0 deletions changes/4149.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug that prevented renaming a `Rack` if it contained any devices whose names were not globally unique.
24 changes: 12 additions & 12 deletions nautobot/dcim/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,21 +696,21 @@ def clean(self):
cleaned_data = self.cleaned_data
site = cleaned_data.get("site")

if self.instance:
if self.instance and site != self.instance.site:
# If the site is changed, the rack post save signal attempts to update the rack devices,
# which may result in an Exception.
# To avoid an unhandled exception in signal, catch this error here.
duplicate_devices_names = (
Device.objects.values_list("name", flat=True)
.annotate(name_count=Count("name"))
.filter(name_count__gt=1)
)
duplicate_devices = Device.objects.filter(site=site, name__in=list(duplicate_devices_names)).values_list(
"name", flat=True
)
# which may result in an Exception if the updated devices conflict with existing devices at this site.
# To avoid an unhandled exception in the signal, check for this scenario here.
duplicate_devices = set()
for device in self.instance.devices.all():
qs = Device.objects.exclude(pk=device.pk).filter(site=site, tenant=device.tenant, name=device.name)
if qs.exists():
duplicate_devices.add(qs.first().name)
if duplicate_devices:
raise ValidationError(
{"site": f"Device with `name` in {list(duplicate_devices)} and site={site} already exists."}
{
"site": f"Device(s) {sorted(duplicate_devices)} already exist in site {site} and "
"would conflict with same-named devices in this rack."
}
)
return cleaned_data

Expand Down
18 changes: 17 additions & 1 deletion nautobot/dcim/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,5 +444,21 @@ def test_update_rack_site(self):
form = RackForm(data=data, instance=racks[0])
self.assertEqual(
str(form.errors.as_data()["site"][0]),
str(["Device with `name` in ['device1'] and site=Site-2 already exists."]),
str(
[
"Device(s) ['device1'] already exist in site Site-2 and "
"would conflict with same-named devices in this rack."
]
),
)

# Check for https://github.com/nautobot/nautobot/issues/4149
data = {
"name": "New name",
"site": racks[0].site.pk,
"status": racks[0].status.pk,
"u_height": 48,
"width": RackWidthChoices.WIDTH_19IN,
}
form = RackForm(data=data, instance=racks[0])
self.assertTrue(form.is_valid())

0 comments on commit 214a807

Please sign in to comment.