Message ID | 20180613093557.6385-2-stephen@that.guru |
---|---|
State | Accepted |
Headers | show |
Series | [1/2] REST: Add missing tests for '/series' | expand |
This series all looks good to me - I'm always a fan of tests and I think a web_url is a win for usability. Regards, Daniel Stephen Finucane <stephen@that.guru> writes: > This provides an easy way for clients to navigate to the web view. The > URL is added to four resources: bundles, comments, cover letters and > series. We could also extend this to projects and users in the future, > but the latter would require renaming an existing property while the > latter would require a public "user" page which does not currently > exists. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > Unless anyone complains, I'm probably going to merge this within a day > or two and cut 2.1. This particular shortcoming has been a constant > annoyance for me and I should have addressed this sooner than I did. > --- > patchwork/api/bundle.py | 16 +++++++++--- > patchwork/api/comment.py | 12 +++++++-- > patchwork/api/cover.py | 11 ++++++--- > patchwork/api/embedded.py | 39 +++++++++++++++++++++++------- > patchwork/api/patch.py | 13 +++++++--- > patchwork/api/series.py | 18 ++++++++++---- > patchwork/models.py | 12 +++++++++ > patchwork/tests/api/test_bundle.py | 29 +++++++++++++++++++--- > patchwork/tests/api/test_cover.py | 15 +++++++++--- > patchwork/tests/api/test_patch.py | 16 +++++++++++- > patchwork/tests/api/test_series.py | 18 ++++++++++++++ > 11 files changed, 165 insertions(+), 34 deletions(-) > > diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py > index 733e4881..b0005daa 100644 > --- a/patchwork/api/bundle.py > +++ b/patchwork/api/bundle.py > @@ -20,9 +20,9 @@ > from django.db.models import Q > from rest_framework.generics import ListAPIView > from rest_framework.generics import RetrieveAPIView > -from rest_framework.serializers import HyperlinkedModelSerializer > from rest_framework.serializers import SerializerMethodField > > +from patchwork.api.base import BaseHyperlinkedModelSerializer > from patchwork.api.base import PatchworkPermission > from patchwork.api.filters import BundleFilterSet > from patchwork.api.embedded import PatchSerializer > @@ -32,22 +32,30 @@ from patchwork.compat import is_authenticated > from patchwork.models import Bundle > > > -class BundleSerializer(HyperlinkedModelSerializer): > +class BundleSerializer(BaseHyperlinkedModelSerializer): > > + web_url = SerializerMethodField() > project = ProjectSerializer(read_only=True) > mbox = SerializerMethodField() > owner = UserSerializer(read_only=True) > patches = PatchSerializer(many=True, read_only=True) > > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > def get_mbox(self, instance): > request = self.context.get('request') > return request.build_absolute_uri(instance.get_mbox_url()) > > class Meta: > model = Bundle > - fields = ('id', 'url', 'project', 'name', 'owner', 'patches', > - 'public', 'mbox') > + fields = ('id', 'url', 'web_url', 'project', 'name', 'owner', > + 'patches', 'public', 'mbox') > read_only_fields = ('owner', 'patches', 'mbox') > + versioned_fields = { > + '1.1': ('web_url', ), > + } > extra_kwargs = { > 'url': {'view_name': 'api-bundle-detail'}, > } > diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py > index fbb7cc83..5a5adb1d 100644 > --- a/patchwork/api/comment.py > +++ b/patchwork/api/comment.py > @@ -30,10 +30,15 @@ from patchwork.models import Comment > > class CommentListSerializer(BaseHyperlinkedModelSerializer): > > + web_url = SerializerMethodField() > subject = SerializerMethodField() > headers = SerializerMethodField() > submitter = PersonSerializer(read_only=True) > > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > def get_subject(self, comment): > return email.parser.Parser().parsestr(comment.headers, > True).get('Subject', '') > @@ -54,9 +59,12 @@ class CommentListSerializer(BaseHyperlinkedModelSerializer): > > class Meta: > model = Comment > - fields = ('id', 'msgid', 'date', 'subject', 'submitter', 'content', > - 'headers') > + fields = ('id', 'web_url', 'msgid', 'date', 'subject', 'submitter', > + 'content', 'headers') > read_only_fields = fields > + versioned_fields = { > + '1.1': ('web_url', ), > + } > > > class CommentList(ListAPIView): > diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py > index 246b7cb9..b497fd85 100644 > --- a/patchwork/api/cover.py > +++ b/patchwork/api/cover.py > @@ -34,12 +34,17 @@ from patchwork.models import CoverLetter > > class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): > > + web_url = SerializerMethodField() > project = ProjectSerializer(read_only=True) > submitter = PersonSerializer(read_only=True) > mbox = SerializerMethodField() > series = SeriesSerializer(many=True, read_only=True) > comments = SerializerMethodField() > > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > def get_mbox(self, instance): > request = self.context.get('request') > return request.build_absolute_uri(instance.get_mbox_url()) > @@ -50,11 +55,11 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): > > class Meta: > model = CoverLetter > - fields = ('id', 'url', 'project', 'msgid', 'date', 'name', 'submitter', > - 'mbox', 'series', 'comments') > + fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', > + 'submitter', 'mbox', 'series', 'comments') > read_only_fields = fields > versioned_fields = { > - '1.1': ('mbox', 'comments'), > + '1.1': ('web_url', 'mbox', 'comments'), > } > extra_kwargs = { > 'url': {'view_name': 'api-cover-detail'}, > diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py > index d79724c4..1d5aba84 100644 > --- a/patchwork/api/embedded.py > +++ b/patchwork/api/embedded.py > @@ -32,7 +32,7 @@ from patchwork import models > > > class MboxMixin(BaseHyperlinkedModelSerializer): > - """Embed an link to the mbox URL. > + """Embed a link to the mbox URL. > > This field is just way too useful to leave out of even the embedded > serialization. > @@ -45,12 +45,25 @@ class MboxMixin(BaseHyperlinkedModelSerializer): > return request.build_absolute_uri(instance.get_mbox_url()) > > > -class BundleSerializer(MboxMixin, BaseHyperlinkedModelSerializer): > +class WebURLMixin(BaseHyperlinkedModelSerializer): > + """Embed a link to the web URL.""" > + > + web_url = SerializerMethodField() > + > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > + > +class BundleSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): > > class Meta: > model = models.Bundle > - fields = ('id', 'url', 'name', 'mbox') > + fields = ('id', 'url', 'web_url', 'name', 'mbox') > read_only_fields = fields > + versioned_field = { > + '1.1': ('web_url', ), > + } > extra_kwargs = { > 'url': {'view_name': 'api-bundle-detail'}, > } > @@ -75,26 +88,30 @@ class CheckSerializer(BaseHyperlinkedModelSerializer): > } > > > -class CoverLetterSerializer(MboxMixin, BaseHyperlinkedModelSerializer): > +class CoverLetterSerializer(MboxMixin, WebURLMixin, > + BaseHyperlinkedModelSerializer): > > class Meta: > model = models.CoverLetter > - fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox') > + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') > read_only_fields = fields > versioned_field = { > - '1.1': ('mbox', ), > + '1.1': ('web_url', 'mbox', ), > } > extra_kwargs = { > 'url': {'view_name': 'api-cover-detail'}, > } > > > -class PatchSerializer(MboxMixin, BaseHyperlinkedModelSerializer): > +class PatchSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): > > class Meta: > model = models.Patch > - fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox') > + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') > read_only_fields = fields > + versioned_field = { > + '1.1': ('web_url', ), > + } > extra_kwargs = { > 'url': {'view_name': 'api-patch-detail'}, > } > @@ -127,12 +144,16 @@ class ProjectSerializer(BaseHyperlinkedModelSerializer): > } > > > -class SeriesSerializer(MboxMixin, BaseHyperlinkedModelSerializer): > +class SeriesSerializer(MboxMixin, WebURLMixin, > + BaseHyperlinkedModelSerializer): > > class Meta: > model = models.Series > fields = ('id', 'url', 'date', 'name', 'version', 'mbox') > read_only_fields = fields > + versioned_field = { > + '1.1': ('web_url', ), > + } > extra_kwargs = { > 'url': {'view_name': 'api-series-detail'}, > } > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py > index c807e98a..9d890eb1 100644 > --- a/patchwork/api/patch.py > +++ b/patchwork/api/patch.py > @@ -77,6 +77,7 @@ class StateField(RelatedField): > > class PatchListSerializer(BaseHyperlinkedModelSerializer): > > + web_url = SerializerMethodField() > project = ProjectSerializer(read_only=True) > state = StateField() > submitter = PersonSerializer(read_only=True) > @@ -88,6 +89,10 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): > checks = SerializerMethodField() > tags = SerializerMethodField() > > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > def get_mbox(self, instance): > request = self.context.get('request') > return request.build_absolute_uri(instance.get_mbox_url()) > @@ -110,15 +115,15 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): > > class Meta: > model = Patch > - fields = ('id', 'url', 'project', 'msgid', 'date', 'name', > + fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', > 'commit_ref', 'pull_url', 'state', 'archived', 'hash', > 'submitter', 'delegate', 'mbox', 'series', 'comments', > 'check', 'checks', 'tags') > - read_only_fields = ('project', 'msgid', 'date', 'name', 'hash', > - 'submitter', 'mbox', 'mbox', 'series', 'comments', > + read_only_fields = ('web_url', 'project', 'msgid', 'date', 'name', > + 'hash', 'submitter', 'mbox', 'series', 'comments', > 'check', 'checks', 'tags') > versioned_fields = { > - '1.1': ('comments', ), > + '1.1': ('comments', 'web_url'), > } > extra_kwargs = { > 'url': {'view_name': 'api-patch-detail'}, > diff --git a/patchwork/api/series.py b/patchwork/api/series.py > index ab1b6adb..14768efb 100644 > --- a/patchwork/api/series.py > +++ b/patchwork/api/series.py > @@ -19,9 +19,9 @@ > > from rest_framework.generics import ListAPIView > from rest_framework.generics import RetrieveAPIView > -from rest_framework.serializers import HyperlinkedModelSerializer > from rest_framework.serializers import SerializerMethodField > > +from patchwork.api.base import BaseHyperlinkedModelSerializer > from patchwork.api.base import PatchworkPermission > from patchwork.api.filters import SeriesFilterSet > from patchwork.api.embedded import CoverLetterSerializer > @@ -31,25 +31,33 @@ from patchwork.api.embedded import ProjectSerializer > from patchwork.models import Series > > > -class SeriesSerializer(HyperlinkedModelSerializer): > +class SeriesSerializer(BaseHyperlinkedModelSerializer): > > + web_url = SerializerMethodField() > project = ProjectSerializer(read_only=True) > submitter = PersonSerializer(read_only=True) > mbox = SerializerMethodField() > cover_letter = CoverLetterSerializer(read_only=True) > patches = PatchSerializer(read_only=True, many=True) > > + def get_web_url(self, instance): > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > def get_mbox(self, instance): > request = self.context.get('request') > return request.build_absolute_uri(instance.get_mbox_url()) > > class Meta: > model = Series > - fields = ('id', 'url', 'project', 'name', 'date', 'submitter', > - 'version', 'total', 'received_total', 'received_all', > - 'mbox', 'cover_letter', 'patches') > + fields = ('id', 'url', 'web_url', 'project', 'name', 'date', > + 'submitter', 'version', 'total', 'received_total', > + 'received_all', 'mbox', 'cover_letter', 'patches') > read_only_fields = ('date', 'submitter', 'total', 'received_total', > 'received_all', 'mbox', 'cover_letter', 'patches') > + versioned_fields = { > + '1.1': ('web_url', ), > + } > extra_kwargs = { > 'url': {'view_name': 'api-series-detail'}, > } > diff --git a/patchwork/models.py b/patchwork/models.py > index 38f0f753..947c0d29 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -412,6 +412,9 @@ class SeriesMixin(object): > > class CoverLetter(SeriesMixin, Submission): > > + def get_absolute_url(self): > + return reverse('cover-detail', kwargs={'cover_id': self.id}) > + > def get_mbox_url(self): > return reverse('cover-mbox', kwargs={'cover_id': self.id}) > > @@ -603,6 +606,9 @@ class Comment(EmailMixin, models.Model): > related_query_name='comment', > on_delete=models.CASCADE) > > + def get_absolute_url(self): > + return reverse('comment-redirect', kwargs={'comment_id': self.id}) > + > def save(self, *args, **kwargs): > super(Comment, self).save(*args, **kwargs) > if hasattr(self.submission, 'patch'): > @@ -728,6 +734,12 @@ class Series(FilenameMixin, models.Model): > patch=patch, > number=number) > > + def get_absolute_url(self): > + # TODO(stephenfin): We really need a proper series view > + return reverse('patch-list', > + kwargs={'project_id': self.project.id}) + ( > + 'series=%d' % self.id) > + > def get_mbox_url(self): > return reverse('series-mbox', kwargs={'series_id': self.id}) > > diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py > index 0cf5f4b3..bc583fb5 100644 > --- a/patchwork/tests/api/test_bundle.py > +++ b/patchwork/tests/api/test_bundle.py > @@ -41,16 +41,22 @@ class TestBundleAPI(APITestCase): > fixtures = ['default_tags'] > > @staticmethod > - def api_url(item=None): > + def api_url(item=None, version=None): > + kwargs = {} > + if version: > + kwargs['version'] = version > + > if item is None: > - return reverse('api-bundle-list') > - return reverse('api-bundle-detail', args=[item]) > + return reverse('api-bundle-list', kwargs=kwargs) > + kwargs['pk'] = item > + return reverse('api-bundle-detail', kwargs=kwargs) > > def assertSerialized(self, bundle_obj, bundle_json): > self.assertEqual(bundle_obj.id, bundle_json['id']) > self.assertEqual(bundle_obj.name, bundle_json['name']) > self.assertEqual(bundle_obj.public, bundle_json['public']) > self.assertIn(bundle_obj.get_mbox_url(), bundle_json['mbox']) > + self.assertIn(bundle_obj.get_absolute_url(), bundle_json['web_url']) > > # nested fields > > @@ -109,6 +115,16 @@ class TestBundleAPI(APITestCase): > resp = self.client.get(self.api_url(), {'owner': 'otheruser'}) > self.assertEqual(0, len(resp.data)) > > + def test_list_old_version(self): > + """Validate that newer fields are dropped for older API versions.""" > + create_bundle(public=True) > + > + resp = self.client.get(self.api_url(version='1.0')) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + self.assertIn('url', resp.data[0]) > + self.assertNotIn('web_url', resp.data[0]) > + > def test_detail(self): > """Validate we can get a specific bundle.""" > bundle = create_bundle(public=True) > @@ -117,6 +133,13 @@ class TestBundleAPI(APITestCase): > self.assertEqual(status.HTTP_200_OK, resp.status_code) > self.assertSerialized(bundle, resp.data) > > + def test_detail_version_1_0(self): > + bundle = create_bundle(public=True) > + > + resp = self.client.get(self.api_url(bundle.id, version='1.0')) > + self.assertIn('url', resp.data) > + self.assertNotIn('web_url', resp.data) > + > def test_create_update_delete(self): > """Ensure creates, updates and deletes aren't allowed""" > user = create_maintainer() > diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py > index 15e02372..e4d814e4 100644 > --- a/patchwork/tests/api/test_cover.py > +++ b/patchwork/tests/api/test_cover.py > @@ -57,6 +57,7 @@ class TestCoverLetterAPI(APITestCase): > self.assertEqual(cover_obj.id, cover_json['id']) > self.assertEqual(cover_obj.name, cover_json['name']) > self.assertIn(cover_obj.get_mbox_url(), cover_json['mbox']) > + self.assertIn(cover_obj.get_absolute_url(), cover_json['web_url']) > self.assertIn('comments', cover_json) > > # nested fields > @@ -104,11 +105,15 @@ class TestCoverLetterAPI(APITestCase): > 'submitter': 'test@example.org'}) > self.assertEqual(0, len(resp.data)) > > - # test old version of API > + def test_list_version_1_0(self): > + create_cover() > + > resp = self.client.get(self.api_url(version='1.0')) > self.assertEqual(status.HTTP_200_OK, resp.status_code) > self.assertEqual(1, len(resp.data)) > + self.assertIn('url', resp.data[0]) > self.assertNotIn('mbox', resp.data[0]) > + self.assertNotIn('web_url', resp.data[0]) > > def test_detail(self): > """Validate we can get a specific cover letter.""" > @@ -127,8 +132,12 @@ class TestCoverLetterAPI(APITestCase): > for key, value in parsed_headers.items(): > self.assertIn(value, resp.data['headers'][key]) > > - # test old version of API > - resp = self.client.get(self.api_url(cover_obj.id, version='1.0')) > + def test_detail_version_1_0(self): > + cover = create_cover() > + > + resp = self.client.get(self.api_url(cover.id, version='1.0')) > + self.assertIn('url', resp.data) > + self.assertNotIn('web_url', resp.data) > self.assertNotIn('comments', resp.data) > > def test_create_update_delete(self): > diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py > index 016dab66..27b99248 100644 > --- a/patchwork/tests/api/test_patch.py > +++ b/patchwork/tests/api/test_patch.py > @@ -62,6 +62,7 @@ class TestPatchAPI(APITestCase): > self.assertEqual(patch_obj.msgid, patch_json['msgid']) > self.assertEqual(patch_obj.state.slug, patch_json['state']) > self.assertIn(patch_obj.get_mbox_url(), patch_json['mbox']) > + self.assertIn(patch_obj.get_absolute_url(), patch_json['web_url']) > self.assertIn('comments', patch_json) > > # nested fields > @@ -137,6 +138,15 @@ class TestPatchAPI(APITestCase): > ('state', 'new')]) > self.assertEqual(2, len(resp.data)) > > + def test_list_version_1_0(self): > + create_patch() > + > + resp = self.client.get(self.api_url(version='1.0')) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + self.assertIn('url', resp.data[0]) > + self.assertNotIn('web_url', resp.data[0]) > + > def test_detail(self): > """Validate we can get a specific patch.""" > patch = create_patch( > @@ -158,8 +168,12 @@ class TestPatchAPI(APITestCase): > self.assertEqual(patch.diff, resp.data['diff']) > self.assertEqual(0, len(resp.data['tags'])) > > - # test old version of API > + def test_detail_version_1_0(self): > + patch = create_patch() > + > resp = self.client.get(self.api_url(item=patch.id, version='1.0')) > + self.assertIn('url', resp.data) > + self.assertNotIn('web_url', resp.data) > self.assertNotIn('comments', resp.data) > > def test_create(self): > diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py > index e6f7cdba..11324bc3 100644 > --- a/patchwork/tests/api/test_series.py > +++ b/patchwork/tests/api/test_series.py > @@ -62,6 +62,7 @@ class TestSeriesAPI(APITestCase): > self.assertEqual(series_obj.received_total, > series_json['received_total']) > self.assertIn(series_obj.get_mbox_url(), series_json['mbox']) > + self.assertIn(series_obj.get_absolute_url(), series_json['web_url']) > > # nested fields > > @@ -119,6 +120,16 @@ class TestSeriesAPI(APITestCase): > 'submitter': 'test@example.org'}) > self.assertEqual(0, len(resp.data)) > > + def test_list_old_version(self): > + """Validate that newer fields are dropped for older API versions.""" > + create_series() > + > + resp = self.client.get(self.api_url(version='1.0')) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + self.assertIn('url', resp.data[0]) > + self.assertNotIn('web_url', resp.data[0]) > + > def test_detail(self): > """Validate we can get a specific series.""" > cover = create_cover() > @@ -129,6 +140,13 @@ class TestSeriesAPI(APITestCase): > self.assertEqual(status.HTTP_200_OK, resp.status_code) > self.assertSerialized(series, resp.data) > > + def test_detail_version_1_0(self): > + series = create_series() > + > + resp = self.client.get(self.api_url(series.id, version='1.0')) > + self.assertIn('url', resp.data) > + self.assertNotIn('web_url', resp.data) > + > def test_create_update_delete(self): > """Ensure creates, updates and deletes aren't allowed""" > user = create_maintainer() > -- > 2.17.1 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py index 733e4881..b0005daa 100644 --- a/patchwork/api/bundle.py +++ b/patchwork/api/bundle.py @@ -20,9 +20,9 @@ from django.db.models import Q from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveAPIView -from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import SerializerMethodField +from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission from patchwork.api.filters import BundleFilterSet from patchwork.api.embedded import PatchSerializer @@ -32,22 +32,30 @@ from patchwork.compat import is_authenticated from patchwork.models import Bundle -class BundleSerializer(HyperlinkedModelSerializer): +class BundleSerializer(BaseHyperlinkedModelSerializer): + web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) mbox = SerializerMethodField() owner = UserSerializer(read_only=True) patches = PatchSerializer(many=True, read_only=True) + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + def get_mbox(self, instance): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) class Meta: model = Bundle - fields = ('id', 'url', 'project', 'name', 'owner', 'patches', - 'public', 'mbox') + fields = ('id', 'url', 'web_url', 'project', 'name', 'owner', + 'patches', 'public', 'mbox') read_only_fields = ('owner', 'patches', 'mbox') + versioned_fields = { + '1.1': ('web_url', ), + } extra_kwargs = { 'url': {'view_name': 'api-bundle-detail'}, } diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index fbb7cc83..5a5adb1d 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -30,10 +30,15 @@ from patchwork.models import Comment class CommentListSerializer(BaseHyperlinkedModelSerializer): + web_url = SerializerMethodField() subject = SerializerMethodField() headers = SerializerMethodField() submitter = PersonSerializer(read_only=True) + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + def get_subject(self, comment): return email.parser.Parser().parsestr(comment.headers, True).get('Subject', '') @@ -54,9 +59,12 @@ class CommentListSerializer(BaseHyperlinkedModelSerializer): class Meta: model = Comment - fields = ('id', 'msgid', 'date', 'subject', 'submitter', 'content', - 'headers') + fields = ('id', 'web_url', 'msgid', 'date', 'subject', 'submitter', + 'content', 'headers') read_only_fields = fields + versioned_fields = { + '1.1': ('web_url', ), + } class CommentList(ListAPIView): diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 246b7cb9..b497fd85 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -34,12 +34,17 @@ from patchwork.models import CoverLetter class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): + web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) submitter = PersonSerializer(read_only=True) mbox = SerializerMethodField() series = SeriesSerializer(many=True, read_only=True) comments = SerializerMethodField() + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + def get_mbox(self, instance): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) @@ -50,11 +55,11 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): class Meta: model = CoverLetter - fields = ('id', 'url', 'project', 'msgid', 'date', 'name', 'submitter', - 'mbox', 'series', 'comments') + fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', + 'submitter', 'mbox', 'series', 'comments') read_only_fields = fields versioned_fields = { - '1.1': ('mbox', 'comments'), + '1.1': ('web_url', 'mbox', 'comments'), } extra_kwargs = { 'url': {'view_name': 'api-cover-detail'}, diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index d79724c4..1d5aba84 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -32,7 +32,7 @@ from patchwork import models class MboxMixin(BaseHyperlinkedModelSerializer): - """Embed an link to the mbox URL. + """Embed a link to the mbox URL. This field is just way too useful to leave out of even the embedded serialization. @@ -45,12 +45,25 @@ class MboxMixin(BaseHyperlinkedModelSerializer): return request.build_absolute_uri(instance.get_mbox_url()) -class BundleSerializer(MboxMixin, BaseHyperlinkedModelSerializer): +class WebURLMixin(BaseHyperlinkedModelSerializer): + """Embed a link to the web URL.""" + + web_url = SerializerMethodField() + + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + + +class BundleSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): class Meta: model = models.Bundle - fields = ('id', 'url', 'name', 'mbox') + fields = ('id', 'url', 'web_url', 'name', 'mbox') read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } extra_kwargs = { 'url': {'view_name': 'api-bundle-detail'}, } @@ -75,26 +88,30 @@ class CheckSerializer(BaseHyperlinkedModelSerializer): } -class CoverLetterSerializer(MboxMixin, BaseHyperlinkedModelSerializer): +class CoverLetterSerializer(MboxMixin, WebURLMixin, + BaseHyperlinkedModelSerializer): class Meta: model = models.CoverLetter - fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox') + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') read_only_fields = fields versioned_field = { - '1.1': ('mbox', ), + '1.1': ('web_url', 'mbox', ), } extra_kwargs = { 'url': {'view_name': 'api-cover-detail'}, } -class PatchSerializer(MboxMixin, BaseHyperlinkedModelSerializer): +class PatchSerializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): class Meta: model = models.Patch - fields = ('id', 'url', 'msgid', 'date', 'name', 'mbox') + fields = ('id', 'url', 'web_url', 'msgid', 'date', 'name', 'mbox') read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } extra_kwargs = { 'url': {'view_name': 'api-patch-detail'}, } @@ -127,12 +144,16 @@ class ProjectSerializer(BaseHyperlinkedModelSerializer): } -class SeriesSerializer(MboxMixin, BaseHyperlinkedModelSerializer): +class SeriesSerializer(MboxMixin, WebURLMixin, + BaseHyperlinkedModelSerializer): class Meta: model = models.Series fields = ('id', 'url', 'date', 'name', 'version', 'mbox') read_only_fields = fields + versioned_field = { + '1.1': ('web_url', ), + } extra_kwargs = { 'url': {'view_name': 'api-series-detail'}, } diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index c807e98a..9d890eb1 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -77,6 +77,7 @@ class StateField(RelatedField): class PatchListSerializer(BaseHyperlinkedModelSerializer): + web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) state = StateField() submitter = PersonSerializer(read_only=True) @@ -88,6 +89,10 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): checks = SerializerMethodField() tags = SerializerMethodField() + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + def get_mbox(self, instance): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) @@ -110,15 +115,15 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): class Meta: model = Patch - fields = ('id', 'url', 'project', 'msgid', 'date', 'name', + fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', 'commit_ref', 'pull_url', 'state', 'archived', 'hash', 'submitter', 'delegate', 'mbox', 'series', 'comments', 'check', 'checks', 'tags') - read_only_fields = ('project', 'msgid', 'date', 'name', 'hash', - 'submitter', 'mbox', 'mbox', 'series', 'comments', + read_only_fields = ('web_url', 'project', 'msgid', 'date', 'name', + 'hash', 'submitter', 'mbox', 'series', 'comments', 'check', 'checks', 'tags') versioned_fields = { - '1.1': ('comments', ), + '1.1': ('comments', 'web_url'), } extra_kwargs = { 'url': {'view_name': 'api-patch-detail'}, diff --git a/patchwork/api/series.py b/patchwork/api/series.py index ab1b6adb..14768efb 100644 --- a/patchwork/api/series.py +++ b/patchwork/api/series.py @@ -19,9 +19,9 @@ from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveAPIView -from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import SerializerMethodField +from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission from patchwork.api.filters import SeriesFilterSet from patchwork.api.embedded import CoverLetterSerializer @@ -31,25 +31,33 @@ from patchwork.api.embedded import ProjectSerializer from patchwork.models import Series -class SeriesSerializer(HyperlinkedModelSerializer): +class SeriesSerializer(BaseHyperlinkedModelSerializer): + web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) submitter = PersonSerializer(read_only=True) mbox = SerializerMethodField() cover_letter = CoverLetterSerializer(read_only=True) patches = PatchSerializer(read_only=True, many=True) + def get_web_url(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_absolute_url()) + def get_mbox(self, instance): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) class Meta: model = Series - fields = ('id', 'url', 'project', 'name', 'date', 'submitter', - 'version', 'total', 'received_total', 'received_all', - 'mbox', 'cover_letter', 'patches') + fields = ('id', 'url', 'web_url', 'project', 'name', 'date', + 'submitter', 'version', 'total', 'received_total', + 'received_all', 'mbox', 'cover_letter', 'patches') read_only_fields = ('date', 'submitter', 'total', 'received_total', 'received_all', 'mbox', 'cover_letter', 'patches') + versioned_fields = { + '1.1': ('web_url', ), + } extra_kwargs = { 'url': {'view_name': 'api-series-detail'}, } diff --git a/patchwork/models.py b/patchwork/models.py index 38f0f753..947c0d29 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -412,6 +412,9 @@ class SeriesMixin(object): class CoverLetter(SeriesMixin, Submission): + def get_absolute_url(self): + return reverse('cover-detail', kwargs={'cover_id': self.id}) + def get_mbox_url(self): return reverse('cover-mbox', kwargs={'cover_id': self.id}) @@ -603,6 +606,9 @@ class Comment(EmailMixin, models.Model): related_query_name='comment', on_delete=models.CASCADE) + def get_absolute_url(self): + return reverse('comment-redirect', kwargs={'comment_id': self.id}) + def save(self, *args, **kwargs): super(Comment, self).save(*args, **kwargs) if hasattr(self.submission, 'patch'): @@ -728,6 +734,12 @@ class Series(FilenameMixin, models.Model): patch=patch, number=number) + def get_absolute_url(self): + # TODO(stephenfin): We really need a proper series view + return reverse('patch-list', + kwargs={'project_id': self.project.id}) + ( + 'series=%d' % self.id) + def get_mbox_url(self): return reverse('series-mbox', kwargs={'series_id': self.id}) diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py index 0cf5f4b3..bc583fb5 100644 --- a/patchwork/tests/api/test_bundle.py +++ b/patchwork/tests/api/test_bundle.py @@ -41,16 +41,22 @@ class TestBundleAPI(APITestCase): fixtures = ['default_tags'] @staticmethod - def api_url(item=None): + def api_url(item=None, version=None): + kwargs = {} + if version: + kwargs['version'] = version + if item is None: - return reverse('api-bundle-list') - return reverse('api-bundle-detail', args=[item]) + return reverse('api-bundle-list', kwargs=kwargs) + kwargs['pk'] = item + return reverse('api-bundle-detail', kwargs=kwargs) def assertSerialized(self, bundle_obj, bundle_json): self.assertEqual(bundle_obj.id, bundle_json['id']) self.assertEqual(bundle_obj.name, bundle_json['name']) self.assertEqual(bundle_obj.public, bundle_json['public']) self.assertIn(bundle_obj.get_mbox_url(), bundle_json['mbox']) + self.assertIn(bundle_obj.get_absolute_url(), bundle_json['web_url']) # nested fields @@ -109,6 +115,16 @@ class TestBundleAPI(APITestCase): resp = self.client.get(self.api_url(), {'owner': 'otheruser'}) self.assertEqual(0, len(resp.data)) + def test_list_old_version(self): + """Validate that newer fields are dropped for older API versions.""" + create_bundle(public=True) + + resp = self.client.get(self.api_url(version='1.0')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertIn('url', resp.data[0]) + self.assertNotIn('web_url', resp.data[0]) + def test_detail(self): """Validate we can get a specific bundle.""" bundle = create_bundle(public=True) @@ -117,6 +133,13 @@ class TestBundleAPI(APITestCase): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertSerialized(bundle, resp.data) + def test_detail_version_1_0(self): + bundle = create_bundle(public=True) + + resp = self.client.get(self.api_url(bundle.id, version='1.0')) + self.assertIn('url', resp.data) + self.assertNotIn('web_url', resp.data) + def test_create_update_delete(self): """Ensure creates, updates and deletes aren't allowed""" user = create_maintainer() diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py index 15e02372..e4d814e4 100644 --- a/patchwork/tests/api/test_cover.py +++ b/patchwork/tests/api/test_cover.py @@ -57,6 +57,7 @@ class TestCoverLetterAPI(APITestCase): self.assertEqual(cover_obj.id, cover_json['id']) self.assertEqual(cover_obj.name, cover_json['name']) self.assertIn(cover_obj.get_mbox_url(), cover_json['mbox']) + self.assertIn(cover_obj.get_absolute_url(), cover_json['web_url']) self.assertIn('comments', cover_json) # nested fields @@ -104,11 +105,15 @@ class TestCoverLetterAPI(APITestCase): 'submitter': 'test@example.org'}) self.assertEqual(0, len(resp.data)) - # test old version of API + def test_list_version_1_0(self): + create_cover() + resp = self.client.get(self.api_url(version='1.0')) self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertEqual(1, len(resp.data)) + self.assertIn('url', resp.data[0]) self.assertNotIn('mbox', resp.data[0]) + self.assertNotIn('web_url', resp.data[0]) def test_detail(self): """Validate we can get a specific cover letter.""" @@ -127,8 +132,12 @@ class TestCoverLetterAPI(APITestCase): for key, value in parsed_headers.items(): self.assertIn(value, resp.data['headers'][key]) - # test old version of API - resp = self.client.get(self.api_url(cover_obj.id, version='1.0')) + def test_detail_version_1_0(self): + cover = create_cover() + + resp = self.client.get(self.api_url(cover.id, version='1.0')) + self.assertIn('url', resp.data) + self.assertNotIn('web_url', resp.data) self.assertNotIn('comments', resp.data) def test_create_update_delete(self): diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 016dab66..27b99248 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -62,6 +62,7 @@ class TestPatchAPI(APITestCase): self.assertEqual(patch_obj.msgid, patch_json['msgid']) self.assertEqual(patch_obj.state.slug, patch_json['state']) self.assertIn(patch_obj.get_mbox_url(), patch_json['mbox']) + self.assertIn(patch_obj.get_absolute_url(), patch_json['web_url']) self.assertIn('comments', patch_json) # nested fields @@ -137,6 +138,15 @@ class TestPatchAPI(APITestCase): ('state', 'new')]) self.assertEqual(2, len(resp.data)) + def test_list_version_1_0(self): + create_patch() + + resp = self.client.get(self.api_url(version='1.0')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertIn('url', resp.data[0]) + self.assertNotIn('web_url', resp.data[0]) + def test_detail(self): """Validate we can get a specific patch.""" patch = create_patch( @@ -158,8 +168,12 @@ class TestPatchAPI(APITestCase): self.assertEqual(patch.diff, resp.data['diff']) self.assertEqual(0, len(resp.data['tags'])) - # test old version of API + def test_detail_version_1_0(self): + patch = create_patch() + resp = self.client.get(self.api_url(item=patch.id, version='1.0')) + self.assertIn('url', resp.data) + self.assertNotIn('web_url', resp.data) self.assertNotIn('comments', resp.data) def test_create(self): diff --git a/patchwork/tests/api/test_series.py b/patchwork/tests/api/test_series.py index e6f7cdba..11324bc3 100644 --- a/patchwork/tests/api/test_series.py +++ b/patchwork/tests/api/test_series.py @@ -62,6 +62,7 @@ class TestSeriesAPI(APITestCase): self.assertEqual(series_obj.received_total, series_json['received_total']) self.assertIn(series_obj.get_mbox_url(), series_json['mbox']) + self.assertIn(series_obj.get_absolute_url(), series_json['web_url']) # nested fields @@ -119,6 +120,16 @@ class TestSeriesAPI(APITestCase): 'submitter': 'test@example.org'}) self.assertEqual(0, len(resp.data)) + def test_list_old_version(self): + """Validate that newer fields are dropped for older API versions.""" + create_series() + + resp = self.client.get(self.api_url(version='1.0')) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, len(resp.data)) + self.assertIn('url', resp.data[0]) + self.assertNotIn('web_url', resp.data[0]) + def test_detail(self): """Validate we can get a specific series.""" cover = create_cover() @@ -129,6 +140,13 @@ class TestSeriesAPI(APITestCase): self.assertEqual(status.HTTP_200_OK, resp.status_code) self.assertSerialized(series, resp.data) + def test_detail_version_1_0(self): + series = create_series() + + resp = self.client.get(self.api_url(series.id, version='1.0')) + self.assertIn('url', resp.data) + self.assertNotIn('web_url', resp.data) + def test_create_update_delete(self): """Ensure creates, updates and deletes aren't allowed""" user = create_maintainer()
This provides an easy way for clients to navigate to the web view. The URL is added to four resources: bundles, comments, cover letters and series. We could also extend this to projects and users in the future, but the latter would require renaming an existing property while the latter would require a public "user" page which does not currently exists. Signed-off-by: Stephen Finucane <stephen@that.guru> --- Unless anyone complains, I'm probably going to merge this within a day or two and cut 2.1. This particular shortcoming has been a constant annoyance for me and I should have addressed this sooner than I did. --- patchwork/api/bundle.py | 16 +++++++++--- patchwork/api/comment.py | 12 +++++++-- patchwork/api/cover.py | 11 ++++++--- patchwork/api/embedded.py | 39 +++++++++++++++++++++++------- patchwork/api/patch.py | 13 +++++++--- patchwork/api/series.py | 18 ++++++++++---- patchwork/models.py | 12 +++++++++ patchwork/tests/api/test_bundle.py | 29 +++++++++++++++++++--- patchwork/tests/api/test_cover.py | 15 +++++++++--- patchwork/tests/api/test_patch.py | 16 +++++++++++- patchwork/tests/api/test_series.py | 18 ++++++++++++++ 11 files changed, 165 insertions(+), 34 deletions(-)