Part duplicate bug fix (#4370)

* Make 'copy_category_parameters' part of actual serializer

* Parameter copying is now handled by the API serializer

* Make field not required

* linting fixes

* pre commit fix

* Fix unit tests

* Further fix for unit test

* Unit tests for category parameter duplication

InvenTree/part/api.py

@@ -2,7 +2,6 @@
 
 import functools
 
-from django.db import transaction
 from django.db.models import Count, F, Q
 from django.http import JsonResponse
 from django.urls import include, path, re_path
@@ -1204,35 +1203,6 @@ class PartList(APIDownloadMixin, ListCreateAPI):
         else:
             return Response(data)
 
-    @transaction.atomic
-    def create(self, request, *args, **kwargs):
-        """We wish to save the user who created this part!
-
-        Note: Implementation copied from DRF class CreateModelMixin
-        """
-        # TODO: Unit tests for this function!
-
-        # Clean up input data
-        data = self.clean_data(request.data)
-
-        serializer = self.get_serializer(data=data)
-        serializer.is_valid(raise_exception=True)
-
-        part = serializer.save()
-        part.creation_user = self.request.user
-
-        # Optionally copy templates from category or parent category
-        copy_templates = {
-            'main': str2bool(data.get('copy_category_templates', False)),
-            'parent': str2bool(data.get('copy_parent_templates', False))
-        }
-
-        part.save(**{'add_category_templates': copy_templates})
-
-        headers = self.get_success_headers(serializer.data)
-
-        return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)
-
     def get_queryset(self, *args, **kwargs):
         """Return an annotated queryset object"""
         queryset = super().get_queryset(*args, **kwargs)

InvenTree/part/models.py

@@ -443,9 +443,6 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
         If the part image has been updated, then check if the "old" (previous) image is still used by another part.
         If not, it is considered "orphaned" and will be deleted.
         """
-        # Get category templates settings
-
-        add_category_templates = kwargs.pop('add_category_templates', False)
 
         if self.pk:
             previous = Part.objects.get(pk=self.pk)
@@ -469,34 +466,6 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
                 'variant_of': _('Invalid choice for parent part'),
             })
 
-        if add_category_templates:
-            # Get part category
-            category = self.category
-
-            if category is not None:
-
-                template_list = []
-
-                parent_categories = category.get_ancestors(include_self=True)
-
-                for category in parent_categories:
-                    for template in category.get_parameter_templates():
-                        # Check that template wasn't already added
-                        if template.parameter_template not in template_list:
-
-                            template_list.append(template.parameter_template)
-
-                            try:
-                                PartParameter.create(
-                                    part=self,
-                                    template=template.parameter_template,
-                                    data=template.default_value,
-                                    save=True
-                                )
-                            except IntegrityError:
-                                # PartParameter already exists
-                                pass
-
     def __str__(self):
         """Return a string representation of the Part (for use in the admin interface)"""
         return f"{self.full_name} - {self.description}"

InvenTree/part/serializers.py

@@ -2,11 +2,12 @@
 
 import imghdr
 import io
+import logging
 from decimal import Decimal
 
 from django.core.files.base import ContentFile
 from django.core.validators import MinValueValidator
-from django.db import models, transaction
+from django.db import IntegrityError, models, transaction
 from django.db.models import ExpressionWrapper, F, FloatField, Q
 from django.db.models.functions import Coalesce
 from django.urls import reverse_lazy
@@ -42,6 +43,8 @@ from .models import (BomItem, BomItemSubstitute, Part, PartAttachment,
                      PartSellPriceBreak, PartStar, PartStocktake,
                      PartStocktakeReport, PartTestTemplate)
 
+logger = logging.getLogger("inventree")
+
 
 class CategorySerializer(InvenTreeModelSerializer):
     """Serializer for PartCategory."""
@@ -454,6 +457,7 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer):
             'duplicate',
             'initial_stock',
             'initial_supplier',
+            'copy_category_parameters'
         ]
 
         read_only_fields = [
@@ -499,6 +503,7 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer):
             'duplicate',
             'initial_stock',
             'initial_supplier',
+            'copy_category_parameters'
         ]
 
         return fields
@@ -613,6 +618,12 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer):
         write_only=True, required=False,
     )
 
+    copy_category_parameters = serializers.BooleanField(
+        default=True, required=False,
+        label=_('Copy Category Parameters'),
+        help_text=_('Copy parameter templates from selected part category'),
+    )
+
     @transaction.atomic
     def create(self, validated_data):
         """Custom method for creating a new Part instance using this serializer"""
@@ -620,9 +631,15 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer):
         duplicate = validated_data.pop('duplicate', None)
         initial_stock = validated_data.pop('initial_stock', None)
         initial_supplier = validated_data.pop('initial_supplier', None)
+        copy_category_parameters = validated_data.pop('copy_category_parameters', False)
 
         instance = super().create(validated_data)
 
+        # Save user information
+        if self.context['request']:
+            instance.creation_user = self.context['request'].user
+            instance.save()
+
         # Copy data from original Part
         if duplicate:
             original = duplicate['part']
@@ -637,6 +654,34 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer):
             if duplicate['copy_parameters']:
                 instance.copy_parameters_from(original)
 
+        # Duplicate parameter data from part category (and parents)
+        if copy_category_parameters and instance.category is not None:
+            # Get flattened list of parent categories
+            categories = instance.category.get_ancestors(include_self=True)
+
+            # All parameter templates within these categories
+            templates = PartCategoryParameterTemplate.objects.filter(
+                category__in=categories
+            )
+
+            for template in templates:
+                # First ensure that the part doesn't have that parameter
+                if PartParameter.objects.filter(
+                    part=instance,
+                    template=template.parameter_template
+                ).exists():
+                    continue
+
+                try:
+                    PartParameter.create(
+                        part=instance,
+                        template=template.parameter_template,
+                        data=template.default_value,
+                        save=True
+                    )
+                except IntegrityError:
+                    logger.error(f"Could not create new PartParameter for part {instance}")
+
         # Create initial stock entry
         if initial_stock:
             quantity = initial_stock['quantity']

InvenTree/part/test_api.py

@@ -1363,6 +1363,51 @@ class PartCreationTests(PartAPITestBase):
                     self.assertEqual(part.bom_items.count(), 4 if bom else 0)
                     self.assertEqual(part.parameters.count(), 2 if params else 0)
 
+    def test_category_parameters(self):
+        """Test that category parameters are correctly applied"""
+
+        cat = PartCategory.objects.get(pk=1)
+
+        # Add some parameter template to the parent category
+        for pk in [1, 2, 3]:
+            PartCategoryParameterTemplate.objects.create(
+                parameter_template=PartParameterTemplate.objects.get(pk=pk),
+                category=cat,
+                default_value=f"Value {pk}"
+            )
+
+        self.assertEqual(cat.parameter_templates.count(), 3)
+
+        # Creat a new Part, without copying category parameters
+        data = self.post(
+            reverse('api-part-list'),
+            {
+                'category': 1,
+                'name': 'Some new part',
+                'description': 'A new part without parameters',
+                'copy_category_parameters': False,
+            },
+            expected_code=201,
+        ).data
+
+        prt = Part.objects.get(pk=data['pk'])
+        self.assertEqual(prt.parameters.count(), 0)
+
+        # Create a new part, this time copying category parameters
+        data = self.post(
+            reverse('api-part-list'),
+            {
+                'category': 1,
+                'name': 'Another new part',
+                'description': 'A new part with parameters',
+                'copy_category_parameters': True,
+            },
+            expected_code=201,
+        ).data
+
+        prt = Part.objects.get(pk=data['pk'])
+        self.assertEqual(prt.parameters.count(), 3)
+
 
 class PartDetailTests(PartAPITestBase):
     """Test that we can create / edit / delete Part objects via the API."""

InvenTree/part/test_param.py

@@ -3,7 +3,7 @@
 import django.core.exceptions as django_exceptions
 from django.test import TestCase, TransactionTestCase
 
-from .models import (Part, PartCategory, PartCategoryParameterTemplate,
+from .models import (PartCategory, PartCategoryParameterTemplate,
                      PartParameter, PartParameterTemplate)
 
 
@@ -70,21 +70,3 @@ class TestCategoryTemplates(TransactionTestCase):
 
         n = PartCategoryParameterTemplate.objects.all().count()
         self.assertEqual(n, 3)
-
-        # Get test part
-        part = Part.objects.get(pk=1)
-
-        # Get part parameters count
-        n_param = part.get_parameters().count()
-
-        add_category_templates = {
-            'main': True,
-            'parent': True,
-        }
-
-        # Save it with category parameters
-        part.save(**{'add_category_templates': add_category_templates})
-
-        # Check new part parameters count
-        # Only 2 parameters should be added as one already existed with same template
-        self.assertEqual(n_param + 2, part.get_parameters().count())

InvenTree/templates/js/translated/part.js

@@ -209,9 +209,6 @@ function partFields(options={}) {
         delete fields['default_supplier'];
 
         fields.copy_category_parameters = {
-            type: 'boolean',
-            label: '{% trans "Copy Category Parameters" %}',
-            help_text: '{% trans "Copy parameter templates from selected part category" %}',
             value: global_settings.PART_CATEGORY_PARAMETERS,
             group: 'create',
         };