[2/2] REST: Add 'web_url' link to API responses

Message ID 20180613093557.6385-2-stephen@that.guru
State Accepted
Headers show
Series
  • [1/2] REST: Add missing tests for '/series'
Related show

Commit Message

Stephen Finucane June 13, 2018, 9:35 a.m.
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(-)

Comments

Daniel Axtens June 16, 2018, 5:13 a.m. | #1
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

Patch

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()