On 18 May 22:30, Andy Doan wrote: > This exports person objects via the REST API. > > Security Constraints: > * The API is read-only to authenticated users
Question: we currently expose all users from 'api.py' for use by the in Selectize auto-complete widgets in the patch list page. I get the reasons for *not* exposing this information publically, but is there a way to replace 'api.py' functionality with this API? Then again, maybe we don't can't anyway, as the REST API is currently optional. > Signed-off-by: Andy Doan <[email protected]> This raises an AttributeError if any person does not have a linked user account. I've provided a fix below, but we need a unit test also. Stephen > --- > patchwork/rest_serializers.py | 13 +++++++++++- > patchwork/tests/test_rest_api.py | 43 > ++++++++++++++++++++++++++++++++++++++++ > patchwork/views/rest_api.py | 30 +++++++++++++++++++++++----- > 3 files changed, 80 insertions(+), 6 deletions(-) > > diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py > index b399d79..0e2e371 100644 > --- a/patchwork/rest_serializers.py > +++ b/patchwork/rest_serializers.py > @@ -17,11 +17,22 @@ > # 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 Project > +from patchwork.models import Person, Project > > from rest_framework.serializers import ModelSerializer > > > +class PersonSerializer(ModelSerializer): > + class Meta: > + model = Person > + exclude = ('user',) > + > + def to_representation(self, instance): > + data = super(PersonSerializer, self).to_representation(instance) > + data['username'] = instance.user.username You need something like this data['username'] = instance.user.username if instance.user else '' Also, tests to prevent this recurring. > + return data > + > + > class ProjectSerializer(ModelSerializer): > class Meta: > model = Project > diff --git a/patchwork/tests/test_rest_api.py > b/patchwork/tests/test_rest_api.py > index 0dc1fe6..3f98595 100644 > --- a/patchwork/tests/test_rest_api.py > +++ b/patchwork/tests/test_rest_api.py > @@ -114,3 +114,46 @@ class TestProjectAPI(APITestCase): > resp = self.client.delete(self.api_url(1)) > self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > self.assertEqual(1, Project.objects.all().count()) > + > + > [email protected](settings.ENABLE_REST_API, 'requires ENABLE_REST_API') > +class TestPersonAPI(APITestCase): > + fixtures = ['default_states'] > + > + @staticmethod > + def api_url(item=None): > + if item is None: > + return reverse('api_1.0:person-list') > + return reverse('api_1.0:person-detail', args=[item]) > + > + def test_anonymous_list(self): > + """The API should reject anonymous users.""" > + resp = self.client.get(self.api_url()) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + def test_authenticated_list(self): > + """This API requires authenticated users.""" > + user = create_user() > + self.client.force_authenticate(user=user) > + resp = self.client.get(self.api_url()) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + self.assertEqual(user.username, resp.data[0]['name']) > + self.assertEqual(user.username, resp.data[0]['username']) > + self.assertEqual(user.email, resp.data[0]['email']) > + > + def test_readonly(self): > + defaults.project.save() > + user = create_maintainer(defaults.project) > + user.is_superuser = True > + user.save() > + self.client.force_authenticate(user=user) > + > + resp = self.client.delete(self.api_url(1)) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + resp = self.client.patch(self.api_url(1), {'email': '[email protected]'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + resp = self.client.post(self.api_url(), {'email': '[email protected]'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py > index 4938e78..dad9724 100644 > --- a/patchwork/views/rest_api.py > +++ b/patchwork/views/rest_api.py > @@ -19,8 +19,7 @@ > > from django.conf import settings > > -from patchwork.models import Project > -from patchwork.rest_serializers import ProjectSerializer > +from patchwork.rest_serializers import ProjectSerializer, PersonSerializer > > from rest_framework import permissions > from rest_framework.pagination import PageNumberPagination > @@ -65,12 +64,33 @@ class PatchworkPermission(permissions.BasePermission): > return obj.is_editable(request.user) > > > -class ProjectViewSet(ModelViewSet): > +class AuthenticatedReadOnly(permissions.BasePermission): > + def has_permission(self, request, view): > + authenticated = request.user.is_authenticated() > + return authenticated and request.method in permissions.SAFE_METHODS > + > + > +class PatchworkViewSet(ModelViewSet): > + pagination_class = LinkHeaderPagination > + > + def get_queryset(self): > + return self.serializer_class.Meta.model.objects.all() > + > + > +class PeopleViewSet(PatchworkViewSet): > + permission_classes = (AuthenticatedReadOnly, ) > + serializer_class = PersonSerializer > + > + def get_queryset(self): > + qs = super(PeopleViewSet, self).get_queryset() > + return qs.select_related('user__username') I don't really get why we moved from the 'queryset' parameter to the 'get_queryset' function. Would the below not work? queryset = People.objects.select_related('user__username') This works though, so I'm happy to keep it as is. _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
