fix: fix errors attempting value harmonization

cleanup error messages

Author: ltucker

netbox_diode_plugin/api/applier.py

@@ -23,7 +23,7 @@ def apply_changeset(change_set: ChangeSet, request) -> ChangeSetResult:
     _validate_change_set(change_set)
 
     created = {}
-    for i, change in enumerate(change_set.changes):
+    for change in change_set.changes:
         change_type = change.change_type
         object_type = change.object_type
 
@@ -35,9 +35,14 @@ def apply_changeset(change_set: ChangeSet, request) -> ChangeSetResult:
             data = _pre_apply(model_class, change, created)
             _apply_change(data, model_class, change, created, request)
         except ValidationError as e:
-            raise _err_from_validation_error(e, f"changes[{i}]")
+            raise _err_from_validation_error(e, object_type)
         except ObjectDoesNotExist:
-            raise _err(f"{object_type} with id {change.object_id} does not exist", f"changes[{i}]", "object_id")
+            raise _err(f"{object_type} with id {change.object_id} does not exist", object_type, "object_id")
+        except TypeError as e:
+            # this indicates a problem in model validation (should raise ValidationError)
+            # but raised non-validation error (TypeError) -- we don't know which field trigged it.
+            logger.error(f"invalid data type for unspecified field (validation raised non-validation error): {data}: {e}")
+            raise _err("invalid data type for field", object_type, "__all__")
         # ConstraintViolationError ?
         # ...
 
@@ -113,13 +118,15 @@ def _validate_change_set(change_set: ChangeSet):
     if not change_set.changes:
         raise _err("Changes are required", "changeset", "changes")
 
-    for i, change in enumerate(change_set.changes):
+    for change in change_set.changes:
         if change.object_id is None and change.ref_id is None:
-            raise _err("Object ID or Ref ID must be provided", f"changes[{i}]", NON_FIELD_ERRORS)
+            raise _err("Object ID or Ref ID must be provided", change.object_type, NON_FIELD_ERRORS)
         if change.change_type not in ChangeType:
-            raise _err(f"Unsupported change type '{change.change_type}'", f"changes[{i}]", "change_type")
+            raise _err(f"Unsupported change type '{change.change_type}'", change.object_type, "change_type")
 
 def _err(message, object_name, field):
+    if not object_name:
+        object_name = "__all__"
     return ChangeSetException(message, errors={object_name: {field: [message]}})
 
 def _err_from_validation_error(e, object_name):

netbox_diode_plugin/api/differ.py

@@ -10,7 +10,7 @@
 from django.core.exceptions import ValidationError
 from utilities.data import shallow_compare_dict
 
-from .common import Change, ChangeSet, ChangeSetException, ChangeSetResult, ChangeType
+from .common import Change, ChangeSet, ChangeSetException, ChangeSetResult, ChangeType, UnresolvedReference
 from .plugin_utils import get_primary_value, legal_fields
 from .supported_models import extract_supported_models
 from .transformer import cleanup_unresolved_references, set_custom_field_defaults, transform_proto_json
@@ -84,12 +84,20 @@ def prechange_data_from_instance(instance) -> dict: # noqa: C901
 
 def _harmonize_formats(prechange_data: dict, postchange_data: dict):
     for k, v in prechange_data.items():
+        if k.startswith('_'):
+            continue
         if isinstance(v, datetime.datetime):
             prechange_data[k] = v.strftime("%Y-%m-%dT%H:%M:%SZ")
         elif isinstance(v, datetime.date):
             prechange_data[k] = v.strftime("%Y-%m-%d")
         elif isinstance(v, int) and k in postchange_data:
-            postchange_data[k] = int(postchange_data[k])
+            val = postchange_data[k]
+            if isinstance(val, UnresolvedReference):
+                continue
+            try:
+                postchange_data[k] = int(val)
+            except Exception:
+                continue
         elif isinstance(v, dict):
             _harmonize_formats(v, postchange_data.get(k, {}))
 

netbox_diode_plugin/api/transformer.py

@@ -452,7 +452,10 @@ def _prepare_custom_fields(object_type: str, custom_fields: dict) -> tuple[dict,
                 out[key] = value
             elif value_type == "date":
                 # truncate to YYYY-MM-DD
-                out[key] = datetime.datetime.fromisoformat(value).strftime("%Y-%m-%d")
+                try:
+                    out[key] = datetime.datetime.fromisoformat(value).strftime("%Y-%m-%d")
+                except Exception:
+                    out[key] = value
             elif value_type == "integer":
                 out[key] = int(value)
             elif value_type == "json":

netbox_diode_plugin/tests/test_api_apply_change_set.py

@@ -300,7 +300,7 @@ def test_change_type_create_with_error_return_400(self):
 
         self.assertIn(
             'Expected a list of items but got type "int".',
-            _get_error(response, "changes[0]", "asns"),
+            _get_error(response, "dcim.site", "asns"),
         )
         self.assertFalse(site_created.exists())
 
@@ -334,7 +334,7 @@ def test_change_type_update_with_error_return_400(self):
         site_updated = Site.objects.get(id=20)
         self.assertIn(
             'Expected a list of items but got type "int".',
-            _get_error(response, "changes[0]", "asns")
+            _get_error(response, "dcim.site", "asns")
         )
         self.assertEqual(site_updated.name, "Site 2")
 
@@ -478,7 +478,7 @@ def test_change_type_create_and_update_with_error_in_one_object_return_400(self)
 
         self.assertIn(
             "Related object not found using the provided numeric ID: 3",
-            _get_error(response, "changes[1]", "device_type"),
+            _get_error(response, "dcim.device", "device_type"),
         )
         self.assertFalse(site_created.exists())
         self.assertFalse(device_created.exists())
@@ -548,7 +548,7 @@ def test_multiples_create_type_error_in_two_objects_return_400(self):
 
         self.assertIn(
             "Related object not found using the provided numeric ID: 3",
-            _get_error(response, "changes[1]", "device_type"),
+            _get_error(response, "dcim.device", "device_type"),
         )
 
         self.assertFalse(site_created.exists())
@@ -587,7 +587,7 @@ def test_change_type_update_with_object_id_not_exist_return_400(self):
 
         self.assertIn(
             "dcim.site with id 30 does not exist",
-            _get_error(response, "changes[0]", "object_id"),
+            _get_error(response, "dcim.site", "object_id"),
         )
         self.assertEqual(site_updated.name, "Site 2")
 
@@ -655,7 +655,7 @@ def test_change_type_field_not_provided_return_400(
 
         self.assertIn(
             "Unsupported change type ''",
-            _get_error(response, "changes[0]", "change_type"),
+            _get_error(response, "dcim.site", "change_type"),
         )
 
     def test_change_set_id_field_and_change_set_not_provided_return_400(self):
@@ -720,7 +720,7 @@ def test_change_type_and_object_type_provided_return_400(
 
         self.assertIn(
             "Unsupported change type 'None'",
-            _get_error(response, "changes[0]", "change_type"),
+            _get_error(response, "__all__", "change_type"),
         )
         # self.assertEqual(
         #     response.json().get("errors")[0].get("change_type"),
@@ -992,7 +992,7 @@ def test_create_prefix_with_unknown_site_fails(self):
         response = self.send_request(payload, status_code=status.HTTP_400_BAD_REQUEST)
         self.assertIn(
             'Please select a site.',
-            _get_error(response, "changes[0]", "scope"),
+            _get_error(response, "ipam.prefix", "scope"),
         )
         self.assertFalse(Prefix.objects.filter(prefix="192.168.0.0/24").exists())
 

netbox_diode_plugin/tests/test_api_diff_and_apply.py

@@ -113,6 +113,64 @@ def test_generate_diff_and_apply_create_interface_with_tags(self):
         self.assertEqual(new_interface.tags.first().name, "tag 1")
 
 
+    def test_generate_diff_and_apply_create_and_update_device_role(self):
+        """Test generate diff and apply create and update device role."""
+        device_uuid = str(uuid4())
+        role_1_uuid = str(uuid4())
+        role_2_uuid = str(uuid4())
+        site_uuid = str(uuid4())
+        payload = {
+            "timestamp": 1,
+            "object_type": "dcim.device",
+            "entity": {
+                "device": {
+                    "name": f"Device {device_uuid}",
+                    "deviceType": {
+                        "model": f"Device Type {uuid4()}",
+                        "manufacturer": {
+                            "name": f"Manufacturer {uuid4()}"
+                        }
+                    },
+                    "role": {
+                        "name": f"Role {role_1_uuid}"
+                    },
+                    "site": {
+                        "name": f"Site {site_uuid}"
+                    }
+                },
+            }
+        }
+        _, response = self.diff_and_apply(payload)
+        new_device = Device.objects.get(name=f"Device {device_uuid}")
+        self.assertEqual(new_device.site.name, f"Site {site_uuid}")
+        self.assertEqual(new_device.role.name, f"Role {role_1_uuid}")
+        payload = {
+            "timestamp": 1,
+            "object_type": "dcim.device",
+            "entity": {
+                "device": {
+                    "name": f"Device {device_uuid}",
+                    "deviceType": {
+                        "model": f"Device Type {uuid4()}",
+                        "manufacturer": {
+                            "name": f"Manufacturer {uuid4()}"
+                        }
+                    },
+                    "role": {
+                        "name": f"Role {role_2_uuid}"
+                    },
+                    "site": {
+                        "name": f"Site {site_uuid}"
+                    }
+                },
+            }
+        }
+        _, response = self.diff_and_apply(payload)
+        device = Device.objects.get(name=f"Device {device_uuid}")
+        self.assertEqual(device.site.name, f"Site {site_uuid}")
+        self.assertEqual(device.role.name, f"Role {role_2_uuid}")
+
+
     def test_generate_diff_and_apply_create_site_autoslug(self):
         """Test generate diff and apply create site."""
         """Test generate diff create site."""
@@ -321,6 +379,35 @@ def test_generate_diff_and_apply_create_and_update_site_with_custom_field(self):
         diff = response1.json().get("change_set", {})
         self.assertEqual(diff.get("changes", []), [])
 
+    def test_generate_diff_wrong_type_date(self):
+        """Test generate diff wrong type date."""
+        payload = {
+            "timestamp": 1,
+            "object_type": "dcim.site",
+            "entity": {
+                "site": {
+                    "name": "Site Generate Diff 1",
+                    "slug": "site-generate-diff-1",
+                    "customFields": {
+                        "mydate": {
+                            "date": 12,
+                        },
+                    },
+                },
+            }
+        }
+        response1 = self.client.post(
+            self.diff_url, data=payload, format="json", **self.user_header
+        )
+        self.assertEqual(response1.status_code, status.HTTP_200_OK)
+
+        diff = response1.json().get("change_set", {})
+
+        response2 = self.client.post(
+            self.apply_url, data=diff, format="json", **self.user_header
+        )
+        self.assertEqual(response2.status_code, status.HTTP_400_BAD_REQUEST)
+
 
     def diff_and_apply(self, payload):
         """Diff and apply the payload."""