From: Stephen Finucane Date: Fri, 16 Nov 2018 15:47:43 +0000 (+0100) Subject: REST: Handle unset series X-Git-Tag: v2.2.0-rc1~201 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=58256ec248d75a39b1c5e130a6f6c0497ccd0770;p=thirdparty%2Fpatchwork.git 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 --- diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index b9e36f23..7a663226 100644 --- 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 523378da..05f6cea8 100644 --- 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 57c26369..8f96f387 100644 --- 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 7d34ba09..7ffc1780 100644 --- 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 fc82022e..e5c40b55 100644 --- 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 87c75eca..3de5e837 100644 --- 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 e15ea561..aad8a2e9 100644 --- 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