[PATCHv2 05/10] REST: Add Patches to the API

[Andy Doan]

On 05/17/2016 08:25 AM, Finucane, Stephen wrote:
>>> +class PatchSerializer(ModelSerializer):
>>> > >+    class Meta:
>>> > >+        model = Patch
>>> > >+        read_only_fields = ('project', 'name', 'date', 'submitter', 'diff',
>>> > >+                            'content', 'hash', 'msgid')
>> >
>> >So I played around with think the responses could do with a little more
>> >work. Firstly, here's a sample response (with the really long fields
>> >"SNIPPED!").
>> >
>> >     {
>> >         "id": 1,
>> >         "mbox_url": "/patch/1/mbox/",
>> >         "msgid": "<1391726961-32072-1-git-send-email-markus.mayer at linaro.org>",
>> >         "date": "2014-02-06T22:49:21",
>> >         "headers": "SNIPPED!",
>> >         "content": "SNIPPED!",
>> >         "name": "[RFC] parsemail.py: Don't search for patches in replies",
>> >         "diff": "SNIPPED!",
>> >         "commit_ref": null,
>> >         "pull_url": null,
>> >         "archived": false,
>> >         "hash": "312958438b253a6436397bb06816c09616d0833e",
>> >         "submitter": 1,
>> >         "project": 1,
>> >         "delegate": null,
>> >         "state": "New",
>> >         "tags": []
>> >     },

Great feedback:

>> >(1) We should have two serializers: one for 'list' and one for 'detail'
>> >(or 'get'). I would exclude 'headers', 'content' and 'diff' from the
>> >'list' response (and test this).

done in v3

>> >(2) We should use links for the 'project'/'submitter' fields, rather
>> >than IDs.  Alternatively, we could have a 'project_id' and
>> >'project_url' field instead of simply 'project'. What do you prefer?

I found a nice way to use absolute urls and I think we should just do 
that rather than have both "_id" and "_url" fields.

>> >(3) Should 'header' be a list, like 'tags'?

Its easy enough and a nice touch.

>> >(4) Could/should 'mbox_url' use an absolute link instead of the
>> >relative one?

yep, v3.

> (5) Use 'prefetch_related' on 'patch.tags', in addition to
> 'patch.checks'. There is a cascade of queries to the 'patchwork_tag'
> field when accessing '/api/1.0/patches/' at the moment. In general,
> we could probably do with making better use of 'defer',
> 'prefetch_related' and 'select_related'. The existing views code [1]
> would be a good place to start here.

thanks for the pointer.