From 139274f356de7c04e998f9ddc4028c83fa8c072d Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Sat, 18 Feb 2023 11:42:53 +1100
Subject: [PATCH] Bug fix for ensuring location and category names are unique
 for common parent (#4361)

* Update Meta class for StockLocation and PartCategory

* Migration files

* Add extra unique requirements to  InvenTreeTree model

- unique_together does not work as expected with null values
---
 InvenTree/InvenTree/models.py                 | 40 +++++++++++++------
 InvenTree/part/models.py                      | 12 +++---
 .../migrations/0093_auto_20230217_2140.py     | 17 ++++++++
 InvenTree/stock/models.py                     |  6 +++
 4 files changed, 57 insertions(+), 18 deletions(-)
 create mode 100644 InvenTree/stock/migrations/0093_auto_20230217_2140.py

diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py
index 3a3dacd04b..1d44776e8d 100644
--- a/InvenTree/InvenTree/models.py
+++ b/InvenTree/InvenTree/models.py
@@ -501,6 +501,34 @@ class InvenTreeTree(MPTTModel):
         parent: The item immediately above this one. An item with a null parent is a top-level item
     """
 
+    class Meta:
+        """Metaclass defines extra model properties."""
+
+        abstract = True
+
+    class MPTTMeta:
+        """Set insert order."""
+        order_insertion_by = ['name']
+
+    def validate_unique(self, exclude=None):
+        """Validate that this tree instance satisfies our uniqueness requirements.
+
+        Note that a 'unique_together' requirement for ('name', 'parent') is insufficient,
+        as it ignores cases where parent=None (i.e. top-level items)
+        """
+
+        super().validate_unique(exclude)
+
+        results = self.__class__.objects.filter(
+            name=self.name,
+            parent=self.parent
+        ).exclude(pk=self.pk)
+
+        if results.exists():
+            raise ValidationError({
+                'name': _('Duplicate names cannot exist under the same parent')
+            })
+
     def api_instance_filters(self):
         """Instance filters for InvenTreeTree models."""
         return {
@@ -539,18 +567,6 @@ class InvenTreeTree(MPTTModel):
             for child in self.get_children():
                 child.save(*args, **kwargs)
 
-    class Meta:
-        """Metaclass defines extra model properties."""
-
-        abstract = True
-
-        # Names must be unique at any given level in the tree
-        unique_together = ('name', 'parent')
-
-    class MPTTMeta:
-        """Set insert order."""
-        order_insertion_by = ['name']
-
     name = models.CharField(
         blank=False,
         max_length=100,
diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py
index 04e230eb2a..c7faa25d6b 100644
--- a/InvenTree/part/models.py
+++ b/InvenTree/part/models.py
@@ -66,6 +66,11 @@ class PartCategory(MetadataMixin, InvenTreeTree):
         default_keywords: Default keywords for parts created in this category
     """
 
+    class Meta:
+        """Metaclass defines extra model properties"""
+        verbose_name = _("Part Category")
+        verbose_name_plural = _("Part Categories")
+
     def delete_recursive(self, *args, **kwargs):
         """This function handles the recursive deletion of subcategories depending on kwargs contents"""
         delete_parts = kwargs.get('delete_parts', False)
@@ -154,11 +159,6 @@ class PartCategory(MetadataMixin, InvenTreeTree):
                   "are already assigned to it!"))
         super().clean()
 
-    class Meta:
-        """Metaclass defines extra model properties"""
-        verbose_name = _("Part Category")
-        verbose_name_plural = _("Part Categories")
-
     def get_parts(self, cascade=True) -> set[Part]:
         """Return a queryset for all parts under this category.
 
@@ -747,7 +747,7 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
             return helpers.getBlankThumbnail()
 
     def validate_unique(self, exclude=None):
-        """Validate that a part is 'unique'.
+        """Validate that this Part instance is 'unique'.
 
         Uniqueness is checked across the following (case insensitive) fields:
         - Name
diff --git a/InvenTree/stock/migrations/0093_auto_20230217_2140.py b/InvenTree/stock/migrations/0093_auto_20230217_2140.py
new file mode 100644
index 0000000000..5629f8b4ee
--- /dev/null
+++ b/InvenTree/stock/migrations/0093_auto_20230217_2140.py
@@ -0,0 +1,17 @@
+# Generated by Django 3.2.16 on 2023-02-17 21:40
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('stock', '0092_alter_stockitem_updated'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='stocklocation',
+            options={'verbose_name': 'Stock Location', 'verbose_name_plural': 'Stock Locations'},
+        ),
+    ]
diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py
index 2d0cfdb86d..582151b17f 100644
--- a/InvenTree/stock/models.py
+++ b/InvenTree/stock/models.py
@@ -47,6 +47,12 @@ class StockLocation(InvenTreeBarcodeMixin, MetadataMixin, InvenTreeTree):
     Stock locations can be hierarchical as required
     """
 
+    class Meta:
+        """Metaclass defines extra model properties"""
+
+        verbose_name = _('Stock Location')
+        verbose_name_plural = _('Stock Locations')
+
     def delete_recursive(self, *args, **kwargs):
         """This function handles the recursive deletion of sub-locations depending on kwargs contents"""
         delete_stock_items = kwargs.get('delete_stock_items', False)