Repository: digitalocean/pynetbox (python)

Issue:
Appending to Tagged VLANs is not Idempotent
Appending a VLAN object ID to `tagged_vlans` that's already present causes save() to still send a patch.

Pull request:
Fixes #137 - Appending to Tagged VLANs
Fixes behavior where appending an existing VLAN object id to tagged_vlans would cause .save() to still trigger a PATCH operation. The fix treats the field similarly to the tags field and converts to a set then back to a list in serialize.

Patch:
diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py
index 67fb2c6..8c38024 100644
--- a/pynetbox/core/response.py
+++ b/pynetbox/core/response.py
@@ -20,6 +20,9 @@ from pynetbox.core.util import Hashabledict
 # Record objects.
 JSON_FIELDS = ("custom_fields", "data", "config_context")
 
+# List of fields that are lists but should be treated as sets.
+LIST_AS_SET = ("tags", "tagged_vlans")
+
 
 def get_return(lookup, return_fields=None):
     """Returns simple representations for items passed to lookup.
@@ -309,8 +312,6 @@ class Record(object):
             current_val = getattr(self, i) if not init else init_vals.get(i)
             if i == "custom_fields":
                 ret[i] = flatten_custom(current_val)
-            elif i == "tags":
-                ret[i] = list(set(current_val))
             else:
                 if isinstance(current_val, Record):
                     current_val = getattr(current_val, "serialize")(
@@ -322,6 +323,8 @@ class Record(object):
                         v.id if isinstance(v, Record) else v
                         for v in current_val
                     ]
+                    if i in LIST_AS_SET:
+                        current_val = list(set(current_val))
                 ret[i] = current_val
         return ret
 


Test patch:
diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py
index a996d06..cdd66ad 100644
--- a/tests/unit/test_response.py
+++ b/tests/unit/test_response.py
@@ -4,130 +4,103 @@ from pynetbox.core.response import Record
 
 
 class RecordTestCase(unittest.TestCase):
-
     def test_serialize_list_of_records(self):
         test_values = {
-            'id': 123,
+            "id": 123,
             "tagged_vlans": [
                 {
                     "id": 1,
                     "url": "http://localhost:8000/api/ipam/vlans/1/",
                     "vid": 1,
                     "name": "test1",
-                    "display_name": "test1"
+                    "display_name": "test1",
                 },
                 {
                     "id": 2,
                     "url": "http://localhost:8000/api/ipam/vlans/2/",
                     "vid": 2,
                     "name": "test 2",
-                    "display_name": "test2"
-                }
+                    "display_name": "test2",
+                },
             ],
         }
         test_obj = Record(test_values, None, None)
         test = test_obj.serialize()
-        self.assertEqual(test['tagged_vlans'], [1, 2])
+        self.assertEqual(test["tagged_vlans"], [1, 2])
 
     def test_serialize_list_of_ints(self):
-        test_values = {
-            'id': 123,
-            'units': [12],
-        }
+        test_values = {"id": 123, "units": [12]}
         test_obj = Record(test_values, None, None)
         test = test_obj.serialize()
-        self.assertEqual(test['units'], [12])
+        self.assertEqual(test["units"], [12])
 
     def test_serialize_tag_set(self):
-        test_values = {
-            'id': 123,
-            'tags': [
-                'foo',
-                'bar',
-                'foo',
-            ],
-        }
+        test_values = {"id": 123, "tags": ["foo", "bar", "foo"]}
         test = Record(test_values, None, None).serialize()
-        self.assertEqual(len(test['tags']), 2)
+        self.assertEqual(len(test["tags"]), 2)
 
     def test_diff(self):
         test_values = {
-            'id': 123,
-            'custom_fields': {
-                'foo': 'bar'
-            },
-            'string_field': 'foobar',
-            'int_field': 1,
-            "nested_dict": {
-                "id": 222,
-                "name": 'bar',
-            },
-            'tags': [
-                'foo',
-                'bar',
-            ],
-            'int_list': [
-                123,
-                321,
-                231,
-            ],
+            "id": 123,
+            "custom_fields": {"foo": "bar"},
+            "string_field": "foobar",
+            "int_field": 1,
+            "nested_dict": {"id": 222, "name": "bar"},
+            "tags": ["foo", "bar"],
+            "int_list": [123, 321, 231],
         }
         test = Record(test_values, None, None)
-        test.tags.append('baz')
+        test.tags.append("baz")
         test.nested_dict = 1
-        test.string_field = 'foobaz'
-        self.assertEqual(test._diff(), {'tags', 'nested_dict', 'string_field'})
+        test.string_field = "foobaz"
+        self.assertEqual(test._diff(), {"tags", "nested_dict", "string_field"})
 
-    def test_dict(self):
+    def test_diff_append_records_list(self):
         test_values = {
-            'id': 123,
-            'custom_fields': {
-                'foo': 'bar'
-            },
-            'string_field': 'foobar',
-            'int_field': 1,
-            "nested_dict": {
-                "id": 222,
-                "name": 'bar',
-            },
-            'tags': [
-                'foo',
-                'bar',
-            ],
-            'int_list': [
-                123,
-                321,
-                231,
+            "id": 123,
+            "tagged_vlans": [
+                {
+                    "id": 1,
+                    "url": "http://localhost:8000/api/ipam/vlans/1/",
+                    "vid": 1,
+                    "name": "test1",
+                    "display_name": "test1",
+                }
             ],
-            'empty_list': [],
-            'record_list': [
+        }
+        test_obj = Record(test_values, None, None)
+        test_obj.tagged_vlans.append(1)
+        test = test_obj._diff()
+        self.assertFalse(test)
+
+    def test_dict(self):
+        test_values = {
+            "id": 123,
+            "custom_fields": {"foo": "bar"},
+            "string_field": "foobar",
+            "int_field": 1,
+            "nested_dict": {"id": 222, "name": "bar"},
+            "tags": ["foo", "bar"],
+            "int_list": [123, 321, 231],
+            "empty_list": [],
+            "record_list": [
                 {
-                    'id': 123,
-                    'name': 'Test',
-                    'str_attr': 'foo',
-                    'int_attr': 123,
-                    'custom_fields': {
-                        'foo': 'bar'
-                    },
-                    'tags': [
-                        'foo',
-                        'bar',
-                    ],
+                    "id": 123,
+                    "name": "Test",
+                    "str_attr": "foo",
+                    "int_attr": 123,
+                    "custom_fields": {"foo": "bar"},
+                    "tags": ["foo", "bar"],
                 },
                 {
-                    'id': 321,
-                    'name': 'Test 1',
-                    'str_attr': 'bar',
-                    'int_attr': 321,
-                    'custom_fields': {
-                        'foo': 'bar'
-                    },
-                    'tags': [
-                        'foo',
-                        'bar',
-                    ],
+                    "id": 321,
+                    "name": "Test 1",
+                    "str_attr": "bar",
+                    "int_attr": 321,
+                    "custom_fields": {"foo": "bar"},
+                    "tags": ["foo", "bar"],
                 },
-            ]
+            ],
         }
         test = Record(test_values, None, None)
         self.assertEqual(dict(test), test_values)