Stephen Finucane <[email protected]> writes: > This was a design decision made when implementing the REST API. The > idea was that these items were URLs to related objects and should be > indicated as such. However, this was a faulty assumption as the > Patchwork API, unlike other some other APIs (GitHub), does not also > include a full representation of said objects, like so: > > { > "url": "http://localhost:8000/api/1.0/patches/1/";, > ... > "delegate_url": "http://localhost:8000/api/1.0/users/1";, > "delegate": { > "url": "http://localhost:8000/api/1.0/users/1/";, > "username": "admin", > "first_name": "", > "last_name": "", > "email": "" > } > } > > Since there is no intention to support this design yet, there isn't > really any reason to fight django-rest-framework in appending these > suffixes. Simply remove them. This changes the output for most endpoints > from something like this: > > { > "url": "http://localhost:8000/api/1.0/patches/1";, > ... > "delegate_url": "http://localhost:8000/api/1.0/users/1"; > } > > to: > > { > "url": "http://localhost:8000/api/1.0/patches/1";, > ... > "delegate": "http://localhost:8000/api/1.0/users/1"; > }
Thanks! Reviewed-by: Daniel Axtens <[email protected]> Regards, Daniel > > This will affect all endpoints with nested resources. > > Note that the API version is not bumped as the API is still considered > unreleased. > > Signed-off-by: Stephen Finucane <[email protected]> > Reviewed-by: Andy Doan <[email protected]> > --- > patchwork/api/base.py | 13 ------------- > patchwork/api/check.py | 1 - > patchwork/api/patch.py | 10 +++++----- > patchwork/api/person.py | 5 +++-- > patchwork/tests/test_rest_api.py | 10 +++++----- > 5 files changed, 13 insertions(+), 26 deletions(-) > > diff --git a/patchwork/api/base.py b/patchwork/api/base.py > index 7333a7f..3639ebe 100644 > --- a/patchwork/api/base.py > +++ b/patchwork/api/base.py > @@ -21,22 +21,9 @@ from django.conf import settings > from rest_framework import permissions > from rest_framework.pagination import PageNumberPagination > from rest_framework.response import Response > -from rest_framework.serializers import HyperlinkedModelSerializer > -from rest_framework.serializers import HyperlinkedRelatedField > from rest_framework.viewsets import ModelViewSet > > > -class URLSerializer(HyperlinkedModelSerializer): > - """Just like parent but puts _url for fields""" > - > - def to_representation(self, instance): > - data = super(URLSerializer, self).to_representation(instance) > - for name, field in self.fields.items(): > - if isinstance(field, HyperlinkedRelatedField) and name != 'url': > - data[name + '_url'] = data.pop(name) > - return data > - > - > class LinkHeaderPagination(PageNumberPagination): > """Provide pagination based on rfc5988. > > diff --git a/patchwork/api/check.py b/patchwork/api/check.py > index 31ade07..26a2595 100644 > --- a/patchwork/api/check.py > +++ b/patchwork/api/check.py > @@ -58,7 +58,6 @@ class CheckSerializer(ModelSerializer): > url = self.context['request'].build_absolute_uri(reverse( > 'api_1.0:patch-detail', args=[instance.patch.id])) > data['url'] = url + 'checks/%s/' % instance.id > - data['users_url'] = data.pop('user') > return data > > class Meta: > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py > index c36a11b..2dc77d1 100644 > --- a/patchwork/api/patch.py > +++ b/patchwork/api/patch.py > @@ -19,12 +19,12 @@ > > import email.parser > > +from rest_framework.serializers import HyperlinkedModelSerializer > from rest_framework.serializers import ListSerializer > from rest_framework.serializers import SerializerMethodField > > from patchwork.api.base import PatchworkPermission > from patchwork.api.base import PatchworkViewSet > -from patchwork.api.base import URLSerializer > from patchwork.models import Patch > > > @@ -37,8 +37,8 @@ class PatchListSerializer(ListSerializer): > return super(PatchListSerializer, self).to_representation(data) > > > -class PatchSerializer(URLSerializer): > - mbox_url = SerializerMethodField() > +class PatchSerializer(HyperlinkedModelSerializer): > + mbox = SerializerMethodField() > state = SerializerMethodField() > > class Meta: > @@ -53,13 +53,13 @@ class PatchSerializer(URLSerializer): > def get_state(self, obj): > return obj.state.name > > - def get_mbox_url(self, patch): > + def get_mbox(self, patch): > request = self.context.get('request', None) > return request.build_absolute_uri(patch.get_mbox_url()) > > def to_representation(self, instance): > data = super(PatchSerializer, self).to_representation(instance) > - data['checks_url'] = data['url'] + 'checks/' > + data['checks'] = data['url'] + 'checks/' > data['check'] = instance.combined_check_state > headers = data.get('headers') > if headers is not None: > diff --git a/patchwork/api/person.py b/patchwork/api/person.py > index b1168f7..758b21d 100644 > --- a/patchwork/api/person.py > +++ b/patchwork/api/person.py > @@ -17,13 +17,14 @@ > # along with Patchwork; if not, write to the Free Software > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > +from rest_framework.serializers import HyperlinkedModelSerializer > + > from patchwork.api.base import AuthenticatedReadOnly > from patchwork.api.base import PatchworkViewSet > -from patchwork.api.base import URLSerializer > from patchwork.models import Person > > > -class PersonSerializer(URLSerializer): > +class PersonSerializer(HyperlinkedModelSerializer): > class Meta: > model = Person > fields = ('email', 'name', 'user') > diff --git a/patchwork/tests/test_rest_api.py > b/patchwork/tests/test_rest_api.py > index 44a2859..3be8ecf 100644 > --- a/patchwork/tests/test_rest_api.py > +++ b/patchwork/tests/test_rest_api.py > @@ -175,7 +175,7 @@ class TestPersonAPI(APITestCase): > self.assertEqual(1, len(resp.data)) > self.assertEqual(user.username, resp.data[0]['name']) > self.assertEqual(user.email, resp.data[0]['email']) > - self.assertIn('users/%d/' % user.id, resp.data[0]['user_url']) > + self.assertIn('users/%d/' % user.id, resp.data[0]['user']) > > def test_unlinked_user(self): > person = create_person() > @@ -187,7 +187,7 @@ class TestPersonAPI(APITestCase): > self.assertEqual(2, len(resp.data)) > self.assertEqual(person.name, > resp.data[0]['name']) > - self.assertIsNone(resp.data[0]['user_url']) > + self.assertIsNone(resp.data[0]['user']) > > def test_readonly(self): > user = create_maintainer() > @@ -291,13 +291,13 @@ class TestPatchAPI(APITestCase): > self.assertEqual(status.HTTP_200_OK, resp.status_code) > self.assertEqual(patch.name, resp.data['name']) > self.assertIn(TestProjectAPI.api_url(patch.project.id), > - resp.data['project_url']) > + resp.data['project']) > self.assertEqual(patch.msgid, resp.data['msgid']) > self.assertEqual(patch.diff, resp.data['diff']) > self.assertIn(TestPersonAPI.api_url(patch.submitter.id), > - resp.data['submitter_url']) > + resp.data['submitter']) > self.assertEqual(patch.state.name, resp.data['state']) > - self.assertIn(patch.get_mbox_url(), resp.data['mbox_url']) > + self.assertIn(patch.get_mbox_url(), resp.data['mbox']) > > def test_detail_tags(self): > patch = create_patch( > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > [email protected] > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
