Repository: netbox-community/netbox
Pull request: Fixes: #18833 Inventory Item Bulk Import - 'InventoryItemImportForm' has no field named 'component_id'.
### Fixes: #18833 Inventory Item Bulk Import - 'InventoryItemImportForm' has no field named 'component_id'.

### Root Cause:

In Django's `BaseModelForm` `_post_clean` method, an instance of the model is created with data from `self.cleaned_data`. After the model instantiation, the `full_clean()` method is executed on that model instance.  
Since `component_type` is a field of the `InventoryItem` model, the execution of `full_clean()` on the instance was failing without being captured by any form validation method.

### Solution:

In Django's form validation process, the `clean_<fieldname>()` methods are executed before the `clean_form()` method. Inside the `clean_<fieldname>()` method, there are no guarantees about which field names have already been processed and is available at `self.cleaned_data`.And to fully validate `component_type` use cases, the `device`, `component_type`, and `component_name` fields must be available.  

To fix the validation process, the `clean_component_name` method was replaced by the `clean` method, and the fields `component_name` and `component_type` were removed from `cleaned_data` when incorrect data was provided.  




Problem statement:
Inventory Item Bulk Import - 'InventoryItemImportForm' has no field named 'component_id'.
### Deployment Type

Self-hosted

### NetBox Version

v4.2.4

### Python Version

3.10

### Steps to Reproduce

1. Enter Data in Direct Import tab on Inventory Item Bulk Import page
2. Click Submit

Data is in CSV format with headers:

device	name	status	role	part_id	serial	description	component_type	component_name


### Expected Behavior

Inventory items should have been imported

### Observed Behavior

Server Error
There was a problem with your request. Please contact an administrator.

The complete exception is provided below:

<class 'ValueError'>

'InventoryItemImportForm' has no field named 'component_id'.

Python version: 3.10.12
NetBox version: 4.2.4
Plugins: 
  netbox_topology_views: 4.2.0

Hints from issue discussion:
~~Interesting. I've got this same bug all of a sudden on NetBox 4.1.7 and I imported a bunch of identical inventory items earlier...~~  

~~device,name,label,role,manufacturer,parent,part_id,serial,asset_tag,discovered,description,tags,component_type,component_name,id~~

Scratch that entirely. I was trying to add inventory items to non-existent interfaces on a switch. Once I figured out my numbering error in the CSV data, I was able to import without problems. 
Hello @lp-mo, @cmcknz77 can you post your input form data?
I wasn't able to reproduce the error  in 4.2.5
My data was:
```  device,name,label,role,manufacturer,parent,part_id,serial,asset_tag,discovered,description,tags,component_type,component_name,id
SWITCH4,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890867,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/49,
SWITCH4,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890872,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/50,
SWITCH4,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890874,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/51,
SWITCH4,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890873,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/52,
SWITCH5,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890881,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/49,
SWITCH5,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890876,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/50,
SWITCH5,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890875,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/51,
SWITCH5,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890880,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/52,
SWITCH6,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890886,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/49,
SWITCH6,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890877,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/50,
SWITCH6,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890878,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/51,
SWITCH6,10G-LR,, 10Gb SM SFP+ Module,HPE Aruba,,J9151E,1234567890879,,FALSE,"NIKOMAX - GL-OT-ST08LC2-1310-1310(HP), Single Mode, LC, 8dB, 10km, DDM",,dcim.interface,1/1/52,```

And the issue was caused by the fact that I'd not realised that SWITCH5 is a 24-port switch. Once I changed the interface numbering for those four lines I was able to import without generating the error.
@cmcknz77 going deeper in my analysis it seens that error only happens if there're some error with the other fields data. it seens that as you said `Once I changed the interface numbering for those four lines I was able to import without generating the error.` that right, ok?
> [@cmcknz77](https://github.com/cmcknz77) going deeper in my analysis it seens that error only happens if there're some error with the other fields data. it seens that as you said `Once I changed the interface numbering for those four lines I was able to import without generating the error.` that right, ok?

That's sounds right for my issue. Hopefully @lp-mo will check his data, confirm that there's not a similar issue with it, and then confirm issue resolution.
I can confirm that the error is when the component_name does not match the device and if there is any other error in the data. I was expecting the error handling to tell me that the component_name does not match. 

On the demo instance I created a device called SWITCH1 which has interface TenGigabitEthernet1/0/37. Then I did an import with this data

```
device	name	status	part_id	serial	description	component_type	component_name
SWITCH1	SFP-H10GB-CU3M     	active	SFP-H10GB-CU3M     	 LRMXXXXXX    	 SFP-10GBase-CX	dcim.interface	TenGigabitEthernet1/0/30
```

This failed with the 'InventoryItemImportForm' error. That is because TenGigabitEthernet1/0/30 does not exist. If I change it to TenGigabitEthernet1/0/37 then it imports successfully since TenGigabitEthernet1/0/37 exists.

```
device	name	status	part_id	serial	description	component_type	component_name
SWITCH1	SFP-H10GB-CU3M     	active	SFP-H10GB-CU3M     	 LRMXXXXXX    	 SFP-10GBase-CX	dcim.interface	TenGigabitEthernet1/0/37
```

I also get the 'InventoryItemImportForm' error when I tried submitting data for a device name that doesn't exist. In this example SWITCH9 does not exist.

```
device	name	status	part_id	serial	description	component_type	component_name
SWITCH9	SFP-H10GB-CU3M     	active	SFP-H10GB-CU3M     	 LRMXXXXXX    	 SFP-10GBase-CX	dcim.interface	TenGigabitEthernet1/0/37
```

I would expect it to return an error like if there was no component_type and component_name header:

> Record 1 device: Object not found: SWITCH9

Instead, I get: 

> '<class 'ValueError'>
>
>'InventoryItemImportForm' has no field named 'component_id'.
>
>Python version: 3.12.9
>NetBox version: 4.2.4
>Plugins: 
 > netbox_demo: 0.5.0'

Same error if I remove the component_name header from the data:

```
device	name	status	part_id	serial	description	component_type
SWITCH9	SFP-H10GB-CU3M     	active	SFP-H10GB-CU3M     	 LRMXXXXXX    	 SFP-10GBase-CX	dcim.interface
```

Commit message: Refactor InventoryItemImportForm clean method

Patch to review:
diff --git a/netbox/dcim/forms/bulk_import.py b/netbox/dcim/forms/bulk_import.py
index 92f7220da91..d6135098777 100644
--- a/netbox/dcim/forms/bulk_import.py
+++ b/netbox/dcim/forms/bulk_import.py
@@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
         device = self.cleaned_data.get("device")
 
-        if not device and hasattr(self, 'instance') and hasattr(self.instance, 'device'):
-            device = self.instance.device
-
-        if not all([device, content_type, component_name]):
-            return None
-
-        model = content_type.model_class()
-        try:
-            component = model.objects.get(device=device, name=component_name)
-            self.instance.component = component
-        except ObjectDoesNotExist:
-            raise forms.ValidationError(
-                _("Component not found: {device} - {component_name}").format(
-                    device=device, component_name=component_name
+        if content_type:
+            if device is None:
+                cleaned_data.pop('component_type')
+            if component_name is None:
+                cleaned_data.pop('component_type')
+                raise forms.ValidationError(
+                    _("Component name must be specified when component type is specified")
                 )
-            )
+            if all([device, component_name]):
+                try:
+                    model = content_type.model_class()
+                    component = model.objects.get(device=device, name=component_name)
+                    self.instance.component = component
+                except ObjectDoesNotExist:
+                    cleaned_data.pop('component_type')
+                    cleaned_data.pop('component_name')
+                    raise forms.ValidationError(
+                        _("Component not found: {device} - {component_name}").format(
+                            device=device, component_name=component_name
+                        )
+                    )
+
+        return cleaned_data
 
 
 #


Code review comments:
diff_hunk: @@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
         device = self.cleaned_data.get("device")
 
-        if not device and hasattr(self, 'instance') and hasattr(self.instance, 'device'):
-            device = self.instance.device
-
-        if not all([device, content_type, component_name]):
-            return None
-
-        model = content_type.model_class()
-        try:
-            component = model.objects.get(device=device, name=component_name)
-            self.instance.component = component
-        except ObjectDoesNotExist:
-            raise forms.ValidationError(
-                _("Component not found: {device} - {component_name}").format(
-                    device=device, component_name=component_name
+        if content_type:
+            if device is None:
+                cleaned_data.pop('component_type')
+            if component_name is None:
+                cleaned_data.pop('component_type')
+                raise forms.ValidationError(
+                    _("Component name must be specified when component type is specified")
                 )
-            )
+            if all([device, component_name]):
+                try:
+                    model = content_type.model_class()
+                    component = model.objects.get(device=device, name=component_name)
+                    self.instance.component = component
path: netbox/dcim/forms/bulk_import.py
text: @user1:
```suggestion
                    self.instance.component = model.objects.get(device=device, name=component_name)
```

diff_hunk: @@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
         device = self.cleaned_data.get("device")
 
-        if not device and hasattr(self, 'instance') and hasattr(self.instance, 'device'):
-            device = self.instance.device
-
-        if not all([device, content_type, component_name]):
-            return None
-
-        model = content_type.model_class()
-        try:
-            component = model.objects.get(device=device, name=component_name)
-            self.instance.component = component
-        except ObjectDoesNotExist:
-            raise forms.ValidationError(
-                _("Component not found: {device} - {component_name}").format(
-                    device=device, component_name=component_name
+        if content_type:
+            if device is None:
+                cleaned_data.pop('component_type')
path: netbox/dcim/forms/bulk_import.py
text: @user1:
We generally want to avoid mutating the cleaned data. If `device` (a required field) hasn't been specified, we should be able to abort here as it will have already raised a validation error.

diff_hunk: @@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
path: netbox/dcim/forms/bulk_import.py
text: @user1:
Suggest renaming this to `component_type` for clarity.

diff_hunk: @@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
         device = self.cleaned_data.get("device")
 
-        if not device and hasattr(self, 'instance') and hasattr(self.instance, 'device'):
-            device = self.instance.device
-
-        if not all([device, content_type, component_name]):
-            return None
-
-        model = content_type.model_class()
-        try:
-            component = model.objects.get(device=device, name=component_name)
-            self.instance.component = component
-        except ObjectDoesNotExist:
-            raise forms.ValidationError(
-                _("Component not found: {device} - {component_name}").format(
-                    device=device, component_name=component_name
+        if content_type:
path: netbox/dcim/forms/bulk_import.py
text: @user1:
We should return a validation error if `component_name` is specified but `component_type` is not.

diff_hunk: @@ -1161,27 +1161,35 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields['parent'].queryset = InventoryItem.objects.none()
 
-    def clean_component_name(self):
-        content_type = self.cleaned_data.get('component_type')
-        component_name = self.cleaned_data.get('component_name')
+    def clean(self):
+        cleaned_data = self.cleaned_data
+        content_type = cleaned_data.get('component_type')
+        component_name = cleaned_data.get('component_name')
path: netbox/dcim/forms/bulk_import.py
text: @user1:
```suggestion
        content_type = self.cleaned_data.get('component_type')
        component_name = self.cleaned_data.get('component_name')
```