REST: Don't error if a versioned field we would remove is absent

We remove fields that shouldn't be seen on old versions of the API.
This was done with `pop(field name)`, which will throw an exception
if the named field is absent from the data. However, sometimes if
a patch request is via an old API version, we hit this line without
ever having the field present.

This is odd, but not harmful and we definitely shouldn't 500.

Signed-off-by: Daniel Axtens <[email protected]>
[stephenfin: Merge test into bug fix]
Signed-off-by: Stephen Finucane <[email protected]>
Tested-by: Konstantin Ryabitsev <[email protected]>
Fixes: d944f17 ("REST: Use versioning for modified responses")
Closes: #423
(cherry picked from commit 5c22f9d)

patchwork/api/base.py

@@ -96,6 +96,9 @@ def to_representation(self, instance):
             # field was added, we drop it
             if not utils.has_version(request, version):
                 for field in self.Meta.versioned_fields[version]:
-                    data.pop(field)
+                    # After a PATCH with an older API version, we may not see
+                    # these fields. If they don't exist, don't panic, return
+                    # (and then discard) None.
+                    data.pop(field, None)
 
         return data

patchwork/tests/api/test_patch.py

@@ -334,6 +334,20 @@ def test_update_maintainer(self):
         self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
         self.assertIsNone(Patch.objects.get(id=patch.id).delegate)
 
+    def test_update_maintainer_version_1_0(self):
+        """Update patch as maintainer on v1.1."""
+        project = create_project()
+        patch = create_patch(project=project)
+        state = create_state()
+        user = create_maintainer(project)
+
+        self.client.force_authenticate(user=user)
+        resp = self.client.patch(self.api_url(patch.id, version="1.1"),
+                                 {'state': state.slug, 'delegate': user.id})
+        self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
+        self.assertEqual(Patch.objects.get(id=patch.id).state, state)
+        self.assertEqual(Patch.objects.get(id=patch.id).delegate, user)
+
     @utils.store_samples('patch-update-error-bad-request')
     def test_update_invalid_state(self):
         """Update patch with invalid fields.