Address review feedback

netbox/dcim/api/serializers_/devices.py

@@ -185,11 +185,18 @@ class ModuleSerializer(PrimaryModelSerializer):
         replicate_components = data.pop('replicate_components', True)
         adopt_components = data.pop('adopt_components', False)
         data = super().validate(data)
+
+        # For updates these fields are not meaningful; omit them from validated_data so that
+        # ModelSerializer.update() does not set unexpected attributes on the instance.
+        if self.instance:
+            return data
+
+        # Always pass the flags to create() so it can set the correct private attributes.
         data['replicate_components'] = replicate_components
         data['adopt_components'] = adopt_components
 
-        # Only check for component conflicts when creating a new module with replication or adoption enabled
-        if self.instance or (not replicate_components and not adopt_components):
+        # Skip conflict checks when no component operations are requested.
+        if not replicate_components and not adopt_components:
             return data
 
         device = data.get('device')
@@ -228,6 +235,15 @@ class ModuleSerializer(PrimaryModelSerializer):
                         raise serializers.ValidationError(
                             _("Cannot install module with placeholder values in a module bay with no position defined.")
                         )
+                    if template.name.count(MODULE_TOKEN) != len(module_bays):
+                        raise serializers.ValidationError(
+                            _(
+                                "Cannot install module with placeholder values in a module bay tree {level} in tree "
+                                "but {tokens} placeholders given."
+                            ).format(
+                                level=len(module_bays), tokens=template.name.count(MODULE_TOKEN)
+                            )
+                        )
                     for bay in module_bays:
                         resolved_name = resolved_name.replace(MODULE_TOKEN, bay.position, 1)
 
@@ -255,7 +271,7 @@ class ModuleSerializer(PrimaryModelSerializer):
         replicate_components = validated_data.pop('replicate_components', True)
         adopt_components = validated_data.pop('adopt_components', False)
 
-        # Tags are handled after save; pop them here to manage manually
+        # Tags are handled after save; pop them here to pass to _save_tags()
         tags = validated_data.pop('tags', None)
 
         # Build the instance without saving so we can set private attributes
@@ -268,7 +284,7 @@ class ModuleSerializer(PrimaryModelSerializer):
         instance.save()
 
         if tags is not None:
-            instance.tags.set([t.name for t in tags])
+            self._save_tags(instance, tags)
 
         return instance
 

netbox/dcim/tests/test_api.py

@@ -1794,6 +1794,61 @@ class ModuleTest(APIViewTestCases.APIViewTestCase):
         response = self.client.post(url, data, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
 
+    def test_adopt_components_already_owned(self):
+        """
+        Installing a module with adopt_components=True when an existing component already
+        belongs to another module should return a validation error.
+        """
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Adopt Owned Test')
+        owner_module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Owner Module Type')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Adopt Owned Test Module Type')
+        InterfaceTemplate.objects.create(module_type=module_type, name='eth0', type='1000base-t')
+        owner_bay = ModuleBay.objects.create(device=device, name='Owner Bay')
+        target_bay = ModuleBay.objects.create(device=device, name='Adopt Owned Bay')
+
+        # Install a module that owns the interface
+        owner_module = Module.objects.create(device=device, module_bay=owner_bay, module_type=owner_module_type)
+        Interface.objects.create(device=device, name='eth0', type='1000base-t', module=owner_module)
+
+        url = reverse('dcim-api:module-list')
+        data = {
+            'device': device.pk,
+            'module_bay': target_bay.pk,
+            'module_type': module_type.pk,
+            'adopt_components': True,
+        }
+        response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+
+    def test_patch_ignores_replicate_and_adopt(self):
+        """
+        PATCH requests that include replicate_components or adopt_components should not
+        trigger component replication or adoption (these fields are create-only).
+        """
+        self.add_permissions('dcim.change_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for PATCH Test')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='PATCH Test Module Type')
+        InterfaceTemplate.objects.create(module_type=module_type, name='eth0', type='1000base-t')
+        module_bay = ModuleBay.objects.create(device=device, name='PATCH Bay')
+        # Create the module without replication so we can verify PATCH doesn't trigger it
+        module = Module(device=device, module_bay=module_bay, module_type=module_type)
+        module._disable_replication = True
+        module.save()
+
+        url = reverse('dcim-api:module-detail', kwargs={'pk': module.pk})
+        data = {
+            'replicate_components': True,
+            'adopt_components': True,
+            'serial': 'PATCHED',
+        }
+        response = self.client.patch(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        # No interfaces should have been created by the PATCH
+        self.assertFalse(device.interfaces.exists())
+
 
 class ConsolePortTest(Mixins.ComponentTraceMixin, APIViewTestCases.APIViewTestCase):
     model = ConsolePort