| Message ID | 1480097915-12158-5-git-send-email-stephen@that.guru |
|---|---|
| State | Accepted |
| Headers | show |
> + def to_representation(self, instance): > + data = super(PatchSerializer, self).to_representation(instance) > + data['checks'] = data['url'] + 'checks/' Should this URL be constructed with a method, rather than string concatenation? > + return data Apart from that, looks good to me but I'm not a DRF expert so I hesitate to formally review it. Regards, Daniel > class Meta: > model = Patch > @@ -50,24 +78,6 @@ class PatchSerializer(HyperlinkedModelSerializer): > # render this field > exclude = ('tags',) > > - def get_state(self, obj): > - return obj.state.name > - > - 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'] = data['url'] + 'checks/' > - data['check'] = instance.combined_check_state > - headers = data.get('headers') > - if headers is not None: > - data['headers'] = email.parser.Parser().parsestr(headers, True) > - data['tags'] = [{'name': x.tag.name, 'count': x.count} > - for x in instance.patchtag_set.all()] > - return data > - > > class PatchViewSet(PatchworkViewSet): > permission_classes = (PatchworkPermission,) > diff --git a/patchwork/api/project.py b/patchwork/api/project.py > index 7def2ed..b4debb6 100644 > --- a/patchwork/api/project.py > +++ b/patchwork/api/project.py > @@ -25,10 +25,6 @@ from patchwork.models import Project > > > class ProjectSerializer(HyperlinkedModelSerializer): > - class Meta: > - model = Project > - exclude = ('send_notifications', 'use_tags') > - > def to_representation(self, instance): > data = super(ProjectSerializer, self).to_representation(instance) > data['link_name'] = data.pop('linkname') > @@ -36,6 +32,10 @@ class ProjectSerializer(HyperlinkedModelSerializer): > data['list_id'] = data.pop('listid') > return data > > + class Meta: > + model = Project > + exclude = ('send_notifications', 'use_tags') > + > > class ProjectViewSet(PatchworkViewSet): > permission_classes = (PatchworkPermission,) > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 2dc77d1..3f464b2 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -40,6 +40,34 @@ class PatchListSerializer(ListSerializer): class PatchSerializer(HyperlinkedModelSerializer): mbox = SerializerMethodField() state = SerializerMethodField() + tags = SerializerMethodField() + headers = SerializerMethodField() + check = SerializerMethodField() + + def get_state(self, instance): + return instance.state.name + + def get_mbox(self, instance): + request = self.context.get('request') + return request.build_absolute_uri(instance.get_mbox_url()) + + def get_tags(self, instance): + # TODO(stephenfin): I don't think this is correct - too many queries + return [{'name': x.tag.name, 'count': x.count} + for x in instance.patchtag_set.all()] + + def get_headers(self, instance): + if instance.headers: + return + email.parser.Parser().parsestr(instance.headers, True) + + def get_check(self, instance): + return instance.combined_check_state + + def to_representation(self, instance): + data = super(PatchSerializer, self).to_representation(instance) + data['checks'] = data['url'] + 'checks/' + return data class Meta: model = Patch @@ -50,24 +78,6 @@ class PatchSerializer(HyperlinkedModelSerializer): # render this field exclude = ('tags',) - def get_state(self, obj): - return obj.state.name - - 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'] = data['url'] + 'checks/' - data['check'] = instance.combined_check_state - headers = data.get('headers') - if headers is not None: - data['headers'] = email.parser.Parser().parsestr(headers, True) - data['tags'] = [{'name': x.tag.name, 'count': x.count} - for x in instance.patchtag_set.all()] - return data - class PatchViewSet(PatchworkViewSet): permission_classes = (PatchworkPermission,) diff --git a/patchwork/api/project.py b/patchwork/api/project.py index 7def2ed..b4debb6 100644 --- a/patchwork/api/project.py +++ b/patchwork/api/project.py @@ -25,10 +25,6 @@ from patchwork.models import Project class ProjectSerializer(HyperlinkedModelSerializer): - class Meta: - model = Project - exclude = ('send_notifications', 'use_tags') - def to_representation(self, instance): data = super(ProjectSerializer, self).to_representation(instance) data['link_name'] = data.pop('linkname') @@ -36,6 +32,10 @@ class ProjectSerializer(HyperlinkedModelSerializer): data['list_id'] = data.pop('listid') return data + class Meta: + model = Project + exclude = ('send_notifications', 'use_tags') + class ProjectViewSet(PatchworkViewSet): permission_classes = (PatchworkPermission,)
Use 'SerializerMethodField' to override how we generate fields, rather than hacking the 'to_representation' method. This is the more idiomatic way to do this. The Meta class for the Project and Patch serializers are moved to the bottom of the serializers and some variables renamed, both for consistency's sake. Signed-off-by: Stephen Finucane <stephen@that.guru> --- patchwork/api/patch.py | 46 ++++++++++++++++++++++++++++------------------ patchwork/api/project.py | 8 ++++---- 2 files changed, 32 insertions(+), 22 deletions(-)