Repository: netbox-community/netbox
Pull request: Fixes #9280 - Add option to adopt existing DeviceComponents
### Fixes: #9280

This PR adds a new boolean field (adopt_components) to the Module form. The creation logic has been changed to check for and change the module field on any existing components.

~~This is still a draft. I need to verify that the API works, and I haven't checked a few cases.~~ Looking for input for the implementation also.

Problem statement:
Add an option to adopt existing DeviceComponents when installing modules
### NetBox version

v3.2.2

### Feature type

Change to existing functionality

### Proposed functionality

Add an option to adopt existing DeviceComponents when installing modules. The option would change the module of all DeviceComponents on the target device with the same name as the component being added by the module.

### Use case

Making it easier to migrate to using modules for devices with manually added DeviceComponents.

### Database changes

None.

### External dependencies

None.

Hints from issue discussion:
I like it. We should end up with four possible conditions depending on which boxes are checked:

| Replicate | Adopt | Result |
|---|---|---|
| - | - | No components get created or updated |
| x | - | New components get created but existing components are unmodified |
| - | x | Existing components with matching names are assigned to the module, but no new components are created |
| x | x | If a matching component exists, assign it to the module; otherwise create a new one |

Commit message: Fix PEP8

Patch to review:
diff --git a/netbox/dcim/forms/models.py b/netbox/dcim/forms/models.py
index 31c5b957d73..c8ca1daf191 100644
--- a/netbox/dcim/forms/models.py
+++ b/netbox/dcim/forms/models.py
@@ -633,12 +633,18 @@ class ModuleForm(NetBoxModelForm):
         help_text="Automatically populate components associated with this module type"
     )
 
+    adopt_components = forms.BooleanField(
+        required=False,
+        initial=False,
+        help_text="Adopt already existing components"
+    )
+
     fieldsets = (
         ('Module', (
             'device', 'module_bay', 'manufacturer', 'module_type', 'tags',
         )),
         ('Hardware', (
-            'serial', 'asset_tag', 'replicate_components',
+            'serial', 'asset_tag', 'replicate_components', 'adopt_components',
         )),
     )
 
@@ -646,7 +652,7 @@ class Meta:
         model = Module
         fields = [
             'device', 'module_bay', 'manufacturer', 'module_type', 'serial', 'asset_tag', 'tags',
-            'replicate_components', 'comments',
+            'replicate_components', 'adopt_components', 'comments',
         ]
 
     def __init__(self, *args, **kwargs):
@@ -655,6 +661,8 @@ def __init__(self, *args, **kwargs):
         if self.instance.pk:
             self.fields['replicate_components'].initial = False
             self.fields['replicate_components'].disabled = True
+            self.fields['adopt_components'].initial = False
+            self.fields['adopt_components'].disabled = True
 
     def save(self, *args, **kwargs):
 
@@ -662,6 +670,9 @@ def save(self, *args, **kwargs):
         if self.instance.pk or not self.cleaned_data['replicate_components']:
             self.instance._disable_replication = True
 
+        if self.cleaned_data['adopt_components']:
+            self.instance._adopt_components = True
+
         return super().save(*args, **kwargs)
 
 
diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py
index 6ed7b349fca..023d3a83f9b 100644
--- a/netbox/dcim/models/devices.py
+++ b/netbox/dcim/models/devices.py
@@ -1065,30 +1065,47 @@ def save(self, *args, **kwargs):
 
         super().save(*args, **kwargs)
 
-        # If this is a new Module and component replication has not been disabled, instantiate all its
-        # related components per the ModuleType definition
-        if is_new and not getattr(self, '_disable_replication', False):
-            ConsolePort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.consoleporttemplates.all()]
-            )
-            ConsoleServerPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.consoleserverporttemplates.all()]
-            )
-            PowerPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.powerporttemplates.all()]
-            )
-            PowerOutlet.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.poweroutlettemplates.all()]
-            )
-            Interface.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.interfacetemplates.all()]
-            )
-            RearPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.rearporttemplates.all()]
-            )
-            FrontPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.frontporttemplates.all()]
-            )
+        adopt_components = getattr(self, '_adopt_components', False)
+        disable_replication = getattr(self, '_disable_replication', False)
+
+        # We skip adding components if the module is being edited or
+        # both replication and component adoption is disabled
+        if not is_new or (disable_replication and not adopt_components):
+            return
+
+        # Iterate all component types
+        for templates, component_attribute, component_model in [
+            ("consoleporttemplates", "consoleports", ConsolePort),
+            ("consoleserverporttemplates", "consoleserverports", ConsoleServerPort),
+            ("interfacetemplates", "interfaces", Interface),
+            ("powerporttemplates", "powerports", PowerPort),
+            ("poweroutlettemplates", "poweroutlets", PowerOutlet),
+            ("rearporttemplates", "rearports", RearPort),
+            ("frontporttemplates", "frontports", FrontPort)
+        ]:
+            create_instances = []
+            update_instances = []
+
+            # Get the template for the module type.
+            for template in getattr(self.module_type, templates).all():
+                template_instance = template.instantiate(device=self.device, module=self)
+
+                if adopt_components:
+                    existing_item = getattr(self.device, component_attribute).filter(name=template_instance.name).first()
+
+                    # Check if there's a component with the same name already
+                    if existing_item:
+                        # Assign it to the module
+                        existing_item.module = self
+                        update_instances.append(existing_item)
+                        continue
+
+                # Only create new components if replication is enabled
+                if not disable_replication:
+                    create_instances.append(template_instance)
+
+            component_model.objects.bulk_create(create_instances)
+            component_model.objects.bulk_update(update_instances, ['module'])
 
 
 #
diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py
index 70eb4b6596c..4104bd20636 100644
--- a/netbox/dcim/tests/test_views.py
+++ b/netbox/dcim/tests/test_views.py
@@ -1869,6 +1869,54 @@ def test_module_component_replication(self):
         self.assertHttpStatus(self.client.post(**request), 302)
         self.assertEqual(Interface.objects.filter(device=device).count(), 5)
 
+    @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
+    def test_module_component_adoption(self):
+        self.add_permissions('dcim.add_module')
+
+        interface_name = "Interface-1"
+
+        # Add an interface to the ModuleType
+        module_type = ModuleType.objects.first()
+        InterfaceTemplate(module_type=module_type, name=interface_name).save()
+
+        form_data = self.form_data.copy()
+        device = Device.objects.get(pk=form_data['device'])
+
+        # Create a module with replicated components
+        form_data['module_bay'] = ModuleBay.objects.filter(device=device)[0]
+        form_data['replicate_components'] = True
+        request = {
+            'path': self._get_url('add'),
+            'data': post_data(form_data),
+        }
+        self.assertHttpStatus(self.client.post(**request), 302)
+
+        # Check that the interface was created
+        initial_interface = Interface.objects.filter(device=device, name=interface_name).first()
+        self.assertIsNotNone(initial_interface)
+
+        # Save the module id associated with the interface
+        initial_module_id = initial_interface.module.id
+
+        # Create a second module (in the next bay) with adopted components
+        # The module id of the interface should change
+        form_data['module_bay'] = ModuleBay.objects.filter(device=device)[1]
+        form_data['replicate_components'] = False
+        form_data['adopt_components'] = True
+        request = {
+            'path': self._get_url('add'),
+            'data': post_data(form_data),
+        }
+
+        self.assertHttpStatus(self.client.post(**request), 302)
+
+        # Re-retrieve interface to get new module id
+        initial_interface.refresh_from_db()
+        updated_module_id = initial_interface.module.id
+
+        # Check that the module id has changed
+        self.assertNotEqual(initial_module_id, updated_module_id)
+
 
 class ConsolePortTestCase(ViewTestCases.DeviceComponentViewTestCase):
     model = ConsolePort


Code review comments:
diff_hunk: @@ -1065,30 +1065,47 @@ def save(self, *args, **kwargs):
 
         super().save(*args, **kwargs)
 
-        # If this is a new Module and component replication has not been disabled, instantiate all its
-        # related components per the ModuleType definition
-        if is_new and not getattr(self, '_disable_replication', False):
-            ConsolePort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.consoleporttemplates.all()]
-            )
-            ConsoleServerPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.consoleserverporttemplates.all()]
-            )
-            PowerPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.powerporttemplates.all()]
-            )
-            PowerOutlet.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.poweroutlettemplates.all()]
-            )
-            Interface.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.interfacetemplates.all()]
-            )
-            RearPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.rearporttemplates.all()]
-            )
-            FrontPort.objects.bulk_create(
-                [x.instantiate(device=self.device, module=self) for x in self.module_type.frontporttemplates.all()]
-            )
+        adopt_components = getattr(self, '_adopt_components', False)
+        disable_replication = getattr(self, '_disable_replication', False)
+
+        # We skip adding components if the module is being edited or
+        # both replication and component adoption is disabled
+        if not is_new or (disable_replication and not adopt_components):
+            return
+
+        # Iterate all component types
+        for templates, component_attribute, component_model in [
+            ("consoleporttemplates", "consoleports", ConsolePort),
+            ("consoleserverporttemplates", "consoleserverports", ConsoleServerPort),
+            ("interfacetemplates", "interfaces", Interface),
+            ("powerporttemplates", "powerports", PowerPort),
+            ("poweroutlettemplates", "poweroutlets", PowerOutlet),
+            ("rearporttemplates", "rearports", RearPort),
+            ("frontporttemplates", "frontports", FrontPort)
+        ]:
+            create_instances = []
+            update_instances = []
+
+            # Get the template for the module type.
+            for template in getattr(self.module_type, templates).all():
+                template_instance = template.instantiate(device=self.device, module=self)
+
+                if adopt_components:
+                    existing_item = getattr(self.device, component_attribute).filter(name=template_instance.name).first()
path: netbox/dcim/models/devices.py
text: @user1:
To avoid dealing with duplicate components already assigned to another module, it may be reasonable to ignore them completely.

```suggestion
                    existing_item = getattr(self.device, component_attribute).filter(module__isnull=True, name=template_instance.name).first()
```

Also, a minor point of optimization: We could retrieve all of these components in a single query outside the `for` loop and cache them in a dictionary keyed by name. Then we could just look up each template in the dictionary.