fix: response seralize() should counts new properties added by user (#725)

Author: NeverBehave

pynetbox/core/response.py

@@ -295,6 +295,18 @@ class Record:
 
     url = None
 
+    # Internal Record metadata that should not be serialized for API updates.
+    # These are object bookkeeping attributes, not NetBox API fields.
+    _INTERNAL_ATTRS = frozenset(
+        [
+            "api",  # API client instance
+            "endpoint",  # Endpoint object reference
+            "url",  # Object URL (read-only field provided by API)
+            "has_details",  # Flag for lazy-loading full details
+            "default_ret",  # Default Record class for nested objects
+        ]
+    )
+
     def __init__(self, values, api, endpoint):
         self.has_details = False
         self._full_cache = []
@@ -538,6 +550,14 @@ def serialize(self, nested=False, init=False):
         If an attribute's value is a ``Record`` type it's replaced with
         the ``id`` field of that object.
 
+        When ``init=False`` (default), includes both original fields from the
+        API response and any fields that have been set on the object after
+        initialization. This allows proper change detection for fields set
+        to None or other values.
+
+        When ``init=True``, returns only the original fields from the initial
+        API response, used for comparing against the current state to detect
+        changes.
 
         .. note::
 
@@ -550,13 +570,31 @@ def serialize(self, nested=False, init=False):
         if nested:
             return get_return(self)
 
+        # Determine which fields to serialize
         if init:
-            init_vals = dict(self._init_cache)
+            # For initial state, use only _init_cache
+            init_cache_dict = dict(self._init_cache)
+            fields_to_serialize = init_cache_dict.keys()
+            init_vals = init_cache_dict
+        else:
+            # For current state, include all fields (original + modified)
+            init_cache_keys = {k for k, _ in self._init_cache}
+
+            # Get all non-internal field names from object's __dict__
+            obj_keys = {
+                k
+                for k in self.__dict__.keys()
+                if not k.startswith("_") and k not in self._INTERNAL_ATTRS
+            }
+
+            # Combine both sets
+            fields_to_serialize = init_cache_keys | obj_keys
+            init_vals = {}  # Not used when init=False
 
         ret = {}
 
-        for i in dict(self):
-            current_val = getattr(self, i) if not init else init_vals.get(i)
+        for i in fields_to_serialize:
+            current_val = getattr(self, i, None) if not init else init_vals.get(i)
             if i == "custom_fields":
                 ret[i] = flatten_custom(current_val)
             else:

tests/unit/test_response.py

@@ -416,6 +416,100 @@ def test_save_updates_cache(self):
         # This should now contain the bridge update
         self.assertEqual(updates, {"bridge": 1})
 
+    def test_serialize_includes_new_field_set_to_none(self):
+        """Regression test for issue #708: serialize() should include fields set after init."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+        test_obj.new_field = None
+
+        current = test_obj.serialize()
+        self.assertIn("new_field", current)
+        self.assertIsNone(current["new_field"])
+
+        init = test_obj.serialize(init=True)
+        self.assertNotIn("new_field", init)
+
+    def test_serialize_includes_new_field_set_to_value(self):
+        """Regression test for issue #708: serialize() should include all new fields."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+        test_obj.new_string = "value"
+        test_obj.new_int = 42
+        test_obj.new_none = None
+
+        current = test_obj.serialize()
+        self.assertEqual(current["new_string"], "value")
+        self.assertEqual(current["new_int"], 42)
+        self.assertIsNone(current["new_none"])
+
+        init = test_obj.serialize(init=True)
+        self.assertNotIn("new_string", init)
+        self.assertNotIn("new_int", init)
+        self.assertNotIn("new_none", init)
+
+    def test_diff_detects_new_field_set_to_none(self):
+        """Regression test for issue #708: _diff() should detect new fields."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+        test_obj.primary_mac_address = None
+
+        diff = test_obj._diff()
+        self.assertIn("primary_mac_address", diff)
+
+    def test_updates_includes_new_field_set_to_none(self):
+        """Regression test for issue #708: updates() should include new fields set to None."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+        test_obj.primary_mac_address = None
+
+        updates = test_obj.updates()
+        self.assertIn("primary_mac_address", updates)
+        self.assertIsNone(updates["primary_mac_address"])
+
+    def test_updates_includes_new_field_set_to_value(self):
+        """Regression test for issue #708: updates() should include all new fields."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+        test_obj.new_field = "new_value"
+        test_obj.another_field = 42
+
+        updates = test_obj.updates()
+        self.assertEqual(updates["new_field"], "new_value")
+        self.assertEqual(updates["another_field"], 42)
+
+    def test_nested_object_field_update_issue_708(self):
+        """Regression test for issue #708: nested objects with limited fields."""
+        api = Mock()
+        api.base_url = "http://localhost:8000/api"
+        interface = Record(
+            {
+                "id": 1,
+                "name": "eth0",
+                "url": "http://localhost:8000/api/dcim/interfaces/1/",
+            },
+            api,
+            None,
+        )
+        interface.primary_mac_address = None
+
+        updates = interface.updates()
+        self.assertIn("primary_mac_address", updates)
+        self.assertIsNone(updates["primary_mac_address"])
+
+        diff = interface._diff()
+        self.assertIn("primary_mac_address", diff)
+
+    def test_serialize_excludes_internal_attributes(self):
+        """Ensure serialize() filters out internal Record metadata."""
+        test_obj = Record({"id": 123, "name": "test"}, None, None)
+
+        serialized = test_obj.serialize()
+        for attr in [
+            "api",
+            "endpoint",
+            "url",
+            "has_details",
+            "default_ret",
+            "_init_cache",
+            "_full_cache",
+        ]:
+            self.assertNotIn(attr, serialized)
+
 
 class RecordSetTestCase(unittest.TestCase):
     ids = [1, 3, 5]