Address PR feedback, ensure that parent GFK underlying fields are not nullable

This changes an existing (within the branch) migration, so if you're
testing this you'll need to back up migrations and re-run.

netbox/ipam/forms/bulk_import.py

@@ -571,6 +571,10 @@ class ServiceImportForm(NetBoxModelImportForm):
         to_field_name='name',
         help_text=_('Parent object name')
     )
+    parent_object_id = forms.IntegerField(
+        required=False,
+        help_text=_('Parent object ID'),
+    )
     protocol = CSVChoiceField(
         label=_('Protocol'),
         choices=ServiceProtocolChoices,
@@ -586,8 +590,7 @@ class ServiceImportForm(NetBoxModelImportForm):
     class Meta:
         model = Service
         fields = (
-            'parent_object_type', 'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments',
+            'ipaddresses', 'name', 'protocol', 'ports', 'description', 'comments', 'tags',
-            'tags', 'parent_object_id',
         )
 
     def __init__(self, data=None, *args, **kwargs):
@@ -619,16 +622,20 @@ class ServiceImportForm(NetBoxModelImportForm):
                 parent = parent_ct.model_class().objects.filter(id=parent_id).first()
                 self.cleaned_data['parent'] = parent
             else:
-                # If a parent object type is passed and we've made it to here, then raise a validation
+                # If a parent object type is passed and we've made it here, then raise a validation
-                # error since an associated parent object or parent object id has nto be passed
+                # error since an associated parent object or parent object id has not been passed
                 raise forms.ValidationError(
-                    _("One of parent or parent_object_id needs to be included with parent_object_type")
+                    _("One of parent or parent_object_id must be included with parent_object_type")
                 )
 
+        # making sure parent is defined. In cases where an import is resulting in an update, the
+        # import data might not include the parent object and so the above logic might not be
+        # triggered
+        parent = self.cleaned_data.get('parent')
         for ip_address in self.cleaned_data.get('ipaddresses', []):
             if not ip_address.assigned_object or getattr(ip_address.assigned_object, 'parent_object') != parent:
                 raise forms.ValidationError(
-                    _("{ip} is not assigned to this device/VM.").format(ip=ip_address)
+                    _("{ip} is not assigned to this parent.").format(ip=ip_address)
                 )
 
         return self.cleaned_data

netbox/ipam/migrations/0080_remove_service_device_virtual_machine_add_parent_gfk_index.py

@@ -18,6 +18,18 @@ class Migration(migrations.Migration):
             model_name='service',
             name='virtual_machine',
         ),
+        migrations.AlterField(
+            model_name='service',
+            name='parent_object_id',
+            field=models.PositiveBigIntegerField(),
+        ),
+        migrations.AlterField(
+            model_name='service',
+            name='parent_object_type',
+            field=models.ForeignKey(
+                on_delete=models.deletion.PROTECT, related_name='+', to='contenttypes.contenttype'
+            ),
+        ),
         migrations.AddIndex(
             model_name='service',
             index=models.Index(

netbox/ipam/models/services.py

@@ -68,13 +68,8 @@ class Service(ContactsMixin, ServiceBase, PrimaryModel):
         to='contenttypes.ContentType',
         on_delete=models.PROTECT,
         related_name='+',
-        blank=True,
-        null=True
-    )
-    parent_object_id = models.PositiveBigIntegerField(
-        blank=True,
-        null=True
     )
+    parent_object_id = models.PositiveBigIntegerField()
     parent = GenericForeignKey(
         ct_field='parent_object_type',
         fk_field='parent_object_id'

netbox/ipam/tests/test_filtersets.py

@@ -1258,9 +1258,24 @@ class IPAddressTestCase(TestCase, ChangeLoggedFilterSetTests):
         IPAddress.objects.bulk_create(ipaddresses)
 
         services = (
-            Service(name='Service 1', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]),
+            Service(
-            Service(name='Service 2', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]),
+                parent=devices[0],
-            Service(name='Service 3', protocol=ServiceProtocolChoices.PROTOCOL_TCP, ports=[1]),
+                name='Service 1',
+                protocol=ServiceProtocolChoices.PROTOCOL_TCP,
+                ports=[1],
+            ),
+            Service(
+                parent=devices[1],
+                name='Service 2',
+                protocol=ServiceProtocolChoices.PROTOCOL_TCP,
+                ports=[1],
+            ),
+            Service(
+                parent=devices[2],
+                name='Service 3',
+                protocol=ServiceProtocolChoices.PROTOCOL_TCP,
+                ports=[1],
+            ),
         )
         Service.objects.bulk_create(services)
         services[0].ipaddresses.add(ipaddresses[0])