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

[Finucane, Stephen]

On 10 May 17:39, Andy Doan wrote:
> This exports patches via the REST API.

supernit: s/exports/exposes :)

> Security Constraints:
>  * Anyone (logged in or not) can read all objects.
>  * No one can create/delete objects.
>  * Project maintainers are allowed to update (ie "patch"
>    attributes)
> 
> NOTE: Patch.save was overridden incorrectly and had to be
> fixed to work with DRF.

On further inspection, I think the responses still need a little work.
Sorry for not spotting this the first go round.

Stephen

> Signed-off-by: Andy Doan <andy.doan at linaro.org>
> ---
>  patchwork/models.py              |   4 +-
>  patchwork/rest_serializers.py    |   9 +++-
>  patchwork/tests/test_rest_api.py | 110 ++++++++++++++++++++++++++++++++++++++-
>  patchwork/views/rest_api.py      |   9 +++-
>  4 files changed, 126 insertions(+), 6 deletions(-)
> 
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 4d454c7..6324273 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -359,14 +359,14 @@ class Patch(Submission):
>          for tag in tags:
>              self._set_tag(tag, counter[tag])
>  
> -    def save(self):
> +    def save(self, **kwargs):
>          if not hasattr(self, 'state') or not self.state:
>              self.state = get_default_initial_patch_state()
>  
>          if self.hash is None and self.diff is not None:
>              self.hash = hash_patch(self.diff).hexdigest()
>  
> -        super(Patch, self).save()
> +        super(Patch, self).save(**kwargs)
>  
>          self.refresh_tag_counts()
>  
> diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py
> index 60633b4..52a0306 100644
> --- a/patchwork/rest_serializers.py
> +++ b/patchwork/rest_serializers.py
> @@ -17,7 +17,7 @@
>  # along with Patchwork; if not, write to the Free Software
>  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  
> -from patchwork.models import Person, Project
> +from patchwork.models import Patch, Person, Project
>  
>  from rest_framework.serializers import ModelSerializer
>  
> @@ -30,3 +30,10 @@ class PersonSerializer(ModelSerializer):
>  class ProjectSerializer(ModelSerializer):
>      class Meta:
>          model = Project
> +
> +
> +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": []
    },

(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).
(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?
(3) Should 'header' be a list, like 'tags'?
(4) Could/should 'mbox_url' use an absolute link instead of the
relative one?

I'm open to feedback on all of these, of course :) Also, apologies if
the spec misguided you: I'll do some work on this over the weekend.