[PATCH v2 6/6] REST: Add '/bundle' endpoint

[Andy Doan]

On 03/07/2017 01:22 PM, Stephen Finucane wrote:
> I had initially resisted adding a '/bundle' endpoint to the API as I
> wanted to kill this feature in favour of series. However, series are not
> a like-for-like replacement for bundles. Among other things, series do
> not provide the composability of bundles: bundles can be manually
> created, meaning you can use bundles to group not only multiple patches
> but also multiple series (or at least the patches in those series).
>
> Seeing as we're not getting rid of this feature, we should expose it via
> the API. Bundles are little unusual, in that they can be public or
> private, thus, we should only show bundles that are public or belonging
> to the currently authenticated user, if any. For now, this is a
> read-only endpoint. We may well allow creation of bundles via the API
> once we figure out how to do this cleanly.
>
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> Cc: Andy Doan <andy.doan at linaro.org>

This looks fine to me. Two things though:

1) Can you add some unit tests for this?
2) Are you planning a follow on patch to allow updates via this endpoint?

> ---
> v2:
> - Add '/bundles' to endpoint list at '/'
> ---
>  patchwork/api/bundle.py  | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
>  patchwork/api/filters.py |  8 +++++
>  patchwork/api/index.py   |  1 +
>  patchwork/urls.py        |  7 +++++
>  4 files changed, 96 insertions(+)
>  create mode 100644 patchwork/api/bundle.py
>
> diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py
> new file mode 100644
> index 0000000..2b17d05
> --- /dev/null
> +++ b/patchwork/api/bundle.py
> @@ -0,0 +1,80 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2017 Stephen Finucane <stephen at that.guru>
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> +
> +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 PatchworkPermission
> +from patchwork.api.filters import BundleFilter
> +from patchwork.models import Bundle
> +
> +
> +class BundleSerializer(HyperlinkedModelSerializer):
> +
> +    mbox = SerializerMethodField()
> +
> +    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')
> +        read_only_fields = ('project', 'owner', 'patches', 'mbox')
> +        extra_kwargs = {
> +            'url': {'view_name': 'api-bundle-detail'},
> +            'project': {'view_name': 'api-project-detail'},
> +            'owner': {'view_name': 'api-user-detail'},
> +            'patches': {'view_name': 'api-patch-detail'},
> +        }
> +
> +
> +class BundleMixin(object):
> +
> +    permission_classes = (PatchworkPermission,)
> +    serializer_class = BundleSerializer
> +
> +    def get_queryset(self):
> +        if not self.request.user.is_anonymous:
> +            bundle_filter = Q(owner=self.request.user) | Q(public=True)
> +        else:
> +            bundle_filter = Q(public=True)
> +
> +        return Bundle.objects\
> +            .filter(bundle_filter)\
> +            .prefetch_related('patches',)\
> +            .select_related('owner', 'project')
> +
> +
> +class BundleList(BundleMixin, ListAPIView):
> +    """List bundles."""
> +
> +    filter_class = BundleFilter
> +    search_fields = ('name',)
> +    ordering_fields = ('id', 'name', 'owner')
> +
> +
> +class BundleDetail(BundleMixin, RetrieveAPIView):
> +    """Show a bundle."""
> +
> +    pass
> diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> index 0f2e6e9..eff7ceb 100644
> --- a/patchwork/api/filters.py
> +++ b/patchwork/api/filters.py
> @@ -20,6 +20,7 @@
>  from django_filters import FilterSet
>  from django_filters import IsoDateTimeFilter
>
> +from patchwork.models import Bundle
>  from patchwork.models import Check
>  from patchwork.models import CoverLetter
>  from patchwork.models import Event
> @@ -68,3 +69,10 @@ class EventFilter(FilterSet):
>      class Meta:
>          model = Event
>          fields = ('project', 'series', 'patch', 'cover')
> +
> +
> +class BundleFilter(FilterSet):
> +
> +    class Meta:
> +        model = Bundle
> +        fields = ('project', 'owner', 'public')
> diff --git a/patchwork/api/index.py b/patchwork/api/index.py
> index 210c32e..513e8b6 100644
> --- a/patchwork/api/index.py
> +++ b/patchwork/api/index.py
> @@ -34,4 +34,5 @@ class IndexView(APIView):
>              'covers': request.build_absolute_uri(reverse('api-cover-list')),
>              'series': request.build_absolute_uri(reverse('api-series-list')),
>              'events': request.build_absolute_uri(reverse('api-event-list')),
> +            'bundles': request.build_absolute_uri(reverse('api-bundle-list')),
>          })
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 09b8b31..57d5cd7 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -158,6 +158,7 @@ if settings.ENABLE_REST_API:
>          raise RuntimeError(
>              'djangorestframework must be installed to enable the REST API.')
>
> +    from patchwork.api import bundle as api_bundle_views
>      from patchwork.api import check as api_check_views
>      from patchwork.api import cover as api_cover_views
>      from patchwork.api import event as api_event_views
> @@ -208,6 +209,12 @@ if settings.ENABLE_REST_API:
>          url(r'^series/(?P<pk>[^/]+)/$',
>              api_series_views.SeriesDetail.as_view(),
>              name='api-series-detail'),
> +        url(r'^bundles/$',
> +            api_bundle_views.BundleList.as_view(),
> +            name='api-bundle-list'),
> +        url(r'^bundles/(?P<pk>[^/]+)/$',
> +            api_bundle_views.BundleDetail.as_view(),
> +            name='api-bundle-detail'),
>          url(r'^projects/$',
>              api_project_views.ProjectList.as_view(),
>              name='api-project-list'),
>