REST: Handle unset series

This was introduced in the recent "convert series from N:M to 1:N"
series. We take the opportunity to make the 'create_patch' and
'create_cover' utility methods a little smarter, in that series will
automatically be created for the patch/cover letter unless told not to.
This requires some related changes to other modules.

Signed-off-by: Stephen Finucane <stephen@that.guru>

diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
index b9e36f23d009422b4e0e8c4f96ab02dec37e3499..7a663226ade84799549d8e82189c4793bceb33ac 100644 (file)
--- a/patchwork/api/cover.py
+++ b/patchwork/api/cover.py
@@ -45,7 +45,7 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer):
         # will be removed in API v2
         data = super(CoverLetterListSerializer, self).to_representation(
             instance)
-        data['series'] = [data['series']]
+        data['series'] = [data['series']] if data['series'] else []
         return data
 
     class Meta:

diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 523378daa2cbab8c07ebd40ef51bcb129e61e24d..05f6cea84c4559f96b415a6323dacaffeb0d4e52 100644 (file)
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -116,7 +116,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
         # after we changed the series-patch relationship from M:N to 1:N. It
         # will be removed in API v2
         data = super(PatchListSerializer, self).to_representation(instance)
-        data['series'] = [data['series']]
+        data['series'] = [data['series']] if data['series'] else []
         return data
 
     class Meta:

diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py
index 57c26369d564af11b53b04178f57d3e558bae601..8f96f3878364dffd2b77e17f4a6fdcb457c9be50 100644 (file)
--- a/patchwork/tests/api/test_cover.py
+++ b/patchwork/tests/api/test_cover.py
@@ -52,6 +52,13 @@ class TestCoverLetterAPI(APITestCase):
         self.assertEqual(cover_obj.submitter.id,
                          cover_json['submitter']['id'])
 
+        if hasattr(cover_obj, 'series'):
+            self.assertEqual(1, len(cover_json['series']))
+            self.assertEqual(cover_obj.series.id,
+                             cover_json['series'][0]['id'])
+        else:
+            self.assertEqual([], cover_json['series'])
+
     def test_list_empty(self):
         """List cover letters when none are present."""
         resp = self.client.get(self.api_url())
@@ -60,7 +67,9 @@ class TestCoverLetterAPI(APITestCase):
 
     def test_list_anonymous(self):
         """List cover letter as anonymous user."""
-        cover = create_cover()
+        # we specifically set series to None to test code that handles legacy
+        # cover letters created before series existed
+        cover = create_cover(series=None)
 
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)

diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index 7d34ba09d9360ed85dbe3f09ea9f5b3c94dc82d9..7ffc1780525d9889a1d29c7711e3602dab245997 100644 (file)
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -59,24 +59,33 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(patch_obj.project.id,
                          patch_json['project']['id'])
 
+        if patch_obj.series:
+            self.assertEqual(1, len(patch_json['series']))
+            self.assertEqual(patch_obj.series.id,
+                             patch_json['series'][0]['id'])
+        else:
+            self.assertEqual([], patch_json['series'])
+
     def test_list_empty(self):
         """List patches when none are present."""
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(0, len(resp.data))
 
-    def _create_patch(self):
+    def _create_patch(self, **kwargs):
         person_obj = create_person(email='test@example.com')
         project_obj = create_project(linkname='myproject')
         state_obj = create_state(name='Under Review')
         patch_obj = create_patch(state=state_obj, project=project_obj,
-                                 submitter=person_obj)
+                                 submitter=person_obj, **kwargs)
 
         return patch_obj
 
     def test_list_anonymous(self):
         """List patches as anonymous user."""
-        patch = self._create_patch()
+        # we specifically set series to None to test code that handles legacy
+        # patches created before series existed
+        patch = self._create_patch(series=None)
 
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)

diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py
index fc82022e175b9822b537c0d1c4d3720f5924aa63..e5c40b558cf1a5c8a6c523089db94a50765bb4ed 100644 (file)
--- a/patchwork/tests/test_events.py
+++ b/patchwork/tests/test_events.py
@@ -32,7 +32,7 @@ class PatchCreatedTest(_BaseTestCase):
 
     def test_patch_created(self):
         """No series, so patch dependencies implicitly exist."""
-        patch = utils.create_patch()
+        patch = utils.create_patch(series=None)
 
         # This should raise the CATEGORY_PATCH_CREATED event only as there is
         # no series
@@ -106,7 +106,8 @@ class PatchCreatedTest(_BaseTestCase):
 class PatchChangedTest(_BaseTestCase):
 
     def test_patch_state_changed(self):
-        patch = utils.create_patch()
+        # purposefully setting series to None to minimize additional events
+        patch = utils.create_patch(series=None)
         old_state = patch.state
         new_state = utils.create_state()
 
@@ -123,7 +124,8 @@ class PatchChangedTest(_BaseTestCase):
                                current_state=new_state)
 
     def test_patch_delegated(self):
-        patch = utils.create_patch()
+        # purposefully setting series to None to minimize additional events
+        patch = utils.create_patch(series=None)
         delegate_a = utils.create_user()
 
         # None -> Delegate A

diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
index 87c75ecad5372f69c20089c1e67bc019a689d5be..3de5e8375f1efa1ca48966f75f4e93434fbd4ac7 100644 (file)
--- a/patchwork/tests/test_mboxviews.py
+++ b/patchwork/tests/test_mboxviews.py
@@ -250,7 +250,7 @@ class MboxSeriesDependencies(TestCase):
     def test_legacy_patch(self):
         """Validate a patch with non-existent dependencies raises a 404."""
         # we're explicitly creating a patch without a series
-        patch = create_patch()
+        patch = create_patch(series=None)
 
         response = self.client.get('%s?series=*' % reverse(
             'patch-mbox', args=[patch.id]))

diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index e15ea561bf6c3cd3ffdf5778b25f9113b729bc25..aad8a2e9b9d9f60b064da17736216b8c0b7bd01e 100644 (file)
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -152,8 +152,15 @@ def create_patch(**kwargs):
     # NOTE(stephenfin): Even though we could simply pass 'series' into the
     # constructor, we don't as that's not what we do in the parser and not what
     # our signal handlers (for events) expect
-    series = kwargs.pop('series', None)
-    number = kwargs.pop('number', None)
+    if 'series' in kwargs:
+        series = kwargs.pop('series')
+    else:
+        series = create_series(project=kwargs.pop('project', create_project()))
+
+    if 'number' in kwargs:
+        number = kwargs.pop('number', None)
+    elif series:
+        number = series.patches.count() + 1
 
     # NOTE(stephenfin): We overwrite the provided project, if there is one, to
     # maintain some degree of sanity
@@ -195,7 +202,10 @@ def create_cover(**kwargs):
     # emulate that here. For more info, see [1].
     #
     # [1] https://stackoverflow.com/q/43119575/
-    series = kwargs.pop('series', None)
+    if 'series' in kwargs:
+        series = kwargs.pop('series')
+    else:
+        series = create_series(project=kwargs.pop('project', create_project()))
 
     # NOTE(stephenfin): We overwrite the provided project, if there is one, to
     # maintain some degree of sanity