Presently, when updating a patch we assume that patches are provided. This isn't necessary - you might just want to make it public - and isn't enforced by the API itself. However, because we make this assumption, we see a HTTP 500. Resolve the issue and add tests to prevent a regression.
Signed-off-by: Stephen Finucane <step...@that.guru> Resolves: #357 --- patchwork/api/bundle.py | 8 +++--- patchwork/tests/api/test_bundle.py | 25 +++++++++++++++++++ .../notes/issue-357-1bef23dbfda2722d.yaml | 6 +++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/issue-357-1bef23dbfda2722d.yaml diff --git patchwork/api/bundle.py patchwork/api/bundle.py index 54a9266e..93e32316 100644 --- patchwork/api/bundle.py +++ patchwork/api/bundle.py @@ -80,10 +80,11 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): return instance def update(self, instance, validated_data): - patches = validated_data.pop('patches') + patches = validated_data.pop('patches', None) instance = super(BundleSerializer, self).update( instance, validated_data) - instance.overwrite_patches(patches) + if patches: + instance.overwrite_patches(patches) return instance def validate_patches(self, value): @@ -97,7 +98,8 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): return value def validate(self, data): - data['project'] = data['patches'][0].project + if data.get('patches'): + data['project'] = data['patches'][0].project return super(BundleSerializer, self).validate(data) diff --git patchwork/tests/api/test_bundle.py patchwork/tests/api/test_bundle.py index d03f26f1..79924486 100644 --- patchwork/tests/api/test_bundle.py +++ patchwork/tests/api/test_bundle.py @@ -288,9 +288,34 @@ class TestBundleAPI(utils.APITestCase): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(2, len(resp.data['patches'])) self.assertEqual('hello-bundle', resp.data['name']) + self.assertFalse(resp.data['public']) self.assertEqual(1, Bundle.objects.all().count()) self.assertEqual(2, len(Bundle.objects.first().patches.all())) self.assertEqual('hello-bundle', Bundle.objects.first().name) + self.assertFalse(Bundle.objects.first().public) + + def test_update_no_patches(self): + """Validate we handle updating only the name.""" + user, project, patch_a, patch_b = self._test_create_update() + bundle = create_bundle(owner=user, project=project) + + bundle.append_patch(patch_a) + bundle.append_patch(patch_b) + + self.assertEqual(1, Bundle.objects.all().count()) + self.assertEqual(2, len(Bundle.objects.first().patches.all())) + + resp = self.client.patch(self.api_url(bundle.id), { + 'name': 'hello-bundle', 'public': True, + }) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(2, len(resp.data['patches'])) + self.assertEqual('hello-bundle', resp.data['name']) + self.assertTrue(resp.data['public']) + self.assertEqual(1, Bundle.objects.all().count()) + self.assertEqual(2, len(Bundle.objects.first().patches.all())) + self.assertEqual('hello-bundle', Bundle.objects.first().name) + self.assertTrue(Bundle.objects.first().public) @utils.store_samples('bundle-delete-not-found') def test_delete_anonymous(self): diff --git releasenotes/notes/issue-357-1bef23dbfda2722d.yaml releasenotes/notes/issue-357-1bef23dbfda2722d.yaml new file mode 100644 index 00000000..1f337c72 --- /dev/null +++ releasenotes/notes/issue-357-1bef23dbfda2722d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + An issue that preventing updating bundles via the REST API without + updating the included patches has been resolved. + (`#357 <https://github.com/getpatchwork/patchwork/issues/357>`__) -- 2.25.2 _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork