From patchwork Sat Nov 30 18:48:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1202716 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47QL5M61rMz9sP3 for ; Sun, 1 Dec 2019 05:49:27 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="hAhGoEX0"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 47QL5M0h8xzDqty for ; Sun, 1 Dec 2019 05:49:27 +1100 (AEDT) X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=199.181.239.144; helo=relay0144.mxlogin.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="hAhGoEX0"; dkim-atps=neutral Received: from relay0144.mxlogin.com (relay0144.mxlogin.com [199.181.239.144]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 47QL4f3y1szDqst for ; Sun, 1 Dec 2019 05:48:50 +1100 (AEDT) Received: from filter004.mxroute.com (unknown [116.203.155.46]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay0144.mxlogin.com (Postfix) with ESMTPS id B5489CC30328 for ; Sat, 30 Nov 2019 12:48:46 -0600 (CST) Received: from one.mxroute.com (one.mxroute.com [195.201.59.211]) by filter004.mxroute.com (Postfix) with ESMTPS id 3DF1A3EADB for ; Sat, 30 Nov 2019 18:48:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=NZSfqSLImdoux9vZAn3nG38E64NsxKNyKTQnHS1VfZA=; b=hAhGoEX0v0o+VVMK1+0H1G4kqy X9CmeGupQclBr0P+yzRQXJVdK6lAzHoe8HEyckm7LHtUTcpVZgvYOq9ToRUeHoRMtfuAsiQI9kjQO U4rrvJvY44u8SNp7aWGdzMKkDPURS3VX2nY36szqQWFer9/rC8Oe84ju1oy1GLB3XtBK6BVzxD2YI gOKSq6jXhJKcqQz+0JC+IJzInUO54mKtr1E8wpJCRLTwCQyvHCgymPJZ/AIj+XCaoPhfx4WPDwhxy mSrkunAqAWQftLTHkulOgcDVU5hDGunszOoEYbWuaEz/XWXulRJ4NyyRlCf1CjK2n/isPsxEJ9nTO lRxMpkvw==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 2/2] REST: Allow configuration of user settings via API Date: Sat, 30 Nov 2019 18:48:36 +0000 Message-Id: <20191130184836.65723-2-stephen@that.guru> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191130184836.65723-1-stephen@that.guru> References: <20191130184836.65723-1-stephen@that.guru> MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" Expose the embedded UserProfile field via the REST API. Signed-off-by: Stephen Finucane --- docs/api/schemas/latest/patchwork.yaml | 39 +++++++++++-- docs/api/schemas/patchwork.j2 | 53 +++++++++++++++++ docs/api/schemas/v1.2/patchwork.yaml | 39 +++++++++++-- patchwork/api/user.py | 55 +++++++++++++++++- patchwork/tests/api/test_user.py | 80 ++++++++++++++++++++------ 5 files changed, 232 insertions(+), 34 deletions(-) diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index 6c7564ba..872a6d63 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -1092,7 +1092,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '403': description: Forbidden content: @@ -1129,7 +1129,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '400': description: Bad request content: @@ -1172,7 +1172,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '400': description: Bad request content: @@ -1307,13 +1307,13 @@ components: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' multipart/form-data: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' schemas: Index: type: object @@ -2153,6 +2153,33 @@ components: format: email readOnly: true minLength: 1 + UserSelf: + type: object + allOf: + - $ref: '#/components/schemas/User' + - type: object + properties: + settings: + type: object + properties: + send_email: + title: Send email + description: > + Whether Patchwork should send email on your behalf. + Only present and configurable for your account. + type: boolean + items_per_page: + title: Items per page + description: > + Number of items to display per page (web UI). + Only present and configurable for your account. + type: integer + show_ids: + title: Show IDs + description: + Show click-to-copy IDs in the list view (web UI). + Only present and configurable for your account. + type: boolean CheckEmbedded: type: object properties: diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index e2c8a8c1..0b057a4b 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -1101,7 +1101,11 @@ paths: content: application/json: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} '403': description: Forbidden content: @@ -1138,7 +1142,11 @@ paths: content: application/json: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} '400': description: Bad request content: @@ -1181,7 +1189,11 @@ paths: content: application/json: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} '400': description: Bad request content: @@ -1318,13 +1330,25 @@ components: content: application/json: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} multipart/form-data: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} application/x-www-form-urlencoded: schema: +{% if version >= (1, 2) %} + $ref: '#/components/schemas/UserDetail' +{% else %} $ref: '#/components/schemas/User' +{% endif %} schemas: Index: type: object @@ -2195,6 +2219,35 @@ components: format: email readOnly: true minLength: 1 +{% if version >= (1, 2) %} + UserDetail: + type: object + allOf: + - $ref: '#/components/schemas/User' + - type: object + properties: + settings: + type: object + properties: + send_email: + title: Send email + description: > + Whether Patchwork should send email on your behalf. + Only present and configurable for your account. + type: boolean + items_per_page: + title: Items per page + description: > + Number of items to display per page (web UI). + Only present and configurable for your account. + type: integer + show_ids: + title: Show IDs + description: + Show click-to-copy IDs in the list view (web UI). + Only present and configurable for your account. + type: boolean +{% endif %} CheckEmbedded: type: object properties: diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml index 7dc95793..5546f88e 100644 --- a/docs/api/schemas/v1.2/patchwork.yaml +++ b/docs/api/schemas/v1.2/patchwork.yaml @@ -1092,7 +1092,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '403': description: Forbidden content: @@ -1129,7 +1129,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '400': description: Bad request content: @@ -1172,7 +1172,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' '400': description: Bad request content: @@ -1307,13 +1307,13 @@ components: content: application/json: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' multipart/form-data: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' application/x-www-form-urlencoded: schema: - $ref: '#/components/schemas/User' + $ref: '#/components/schemas/UserSelf' schemas: Index: type: object @@ -2153,6 +2153,33 @@ components: format: email readOnly: true minLength: 1 + UserSelf: + type: object + allOf: + - $ref: '#/components/schemas/User' + - type: object + properties: + settings: + type: object + properties: + send_email: + title: Send email + description: > + Whether Patchwork should send email on your behalf. + Only present and configurable for your account. + type: boolean + items_per_page: + title: Items per page + description: > + Number of items to display per page (web UI). + Only present and configurable for your account. + type: integer + show_ids: + title: Show IDs + description: + Show click-to-copy IDs in the list view (web UI). + Only present and configurable for your account. + type: boolean CheckEmbedded: type: object properties: diff --git a/patchwork/api/user.py b/patchwork/api/user.py index 29944e22..4ea2322e 100644 --- a/patchwork/api/user.py +++ b/patchwork/api/user.py @@ -7,8 +7,12 @@ from django.contrib.auth.models import User from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveUpdateAPIView from rest_framework import permissions +from rest_framework.serializers import ModelSerializer from rest_framework.serializers import HyperlinkedModelSerializer +from patchwork.models import UserProfile +from patchwork.api.utils import has_version + class IsOwnerOrReadOnly(permissions.BasePermission): @@ -19,7 +23,14 @@ class IsOwnerOrReadOnly(permissions.BasePermission): return obj == request.user -class UserSerializer(HyperlinkedModelSerializer): +class UserProfileSerializer(ModelSerializer): + + class Meta: + model = UserProfile + fields = ('send_email', 'items_per_page', 'show_ids') + + +class UserListSerializer(HyperlinkedModelSerializer): class Meta: model = User @@ -32,11 +43,48 @@ class UserSerializer(HyperlinkedModelSerializer): } +class UserDetailSerializer(UserListSerializer): + settings = UserProfileSerializer(source='profile') + + def update(self, instance, validated_data): + settings_data = validated_data.pop('profile', None) + + request = self.context['request'] + if settings_data and has_version(request, '1.2') and ( + request.user.id == instance.id): + # TODO(stephenfin): We ignore this field rather than raise an error + # to be consistent with the rest of the API. We should change this + # when we change the overall settings + self.fields['settings'].update(instance.profile, settings_data) + + return super(UserDetailSerializer, self).update( + instance, validated_data) + + def to_representation(self, instance): + data = super(UserDetailSerializer, self).to_representation(instance) + + request = self.context['request'] + if not has_version(request, '1.2') or request.user.id != instance.id: + del data['settings'] + + return data + + class Meta: + model = User + fields = UserListSerializer.Meta.fields + ('settings',) + # we don't allow updating of emails via the API, as we need to + # validate that the User actually owns said email first + read_only_fields = UserListSerializer.Meta.read_only_fields + versioned_fields = { + '1.2': ('settings',), + } + extra_kwargs = UserListSerializer.Meta.extra_kwargs + + class UserMixin(object): queryset = User.objects.all() permission_classes = (permissions.IsAuthenticated, IsOwnerOrReadOnly) - serializer_class = UserSerializer class UserList(UserMixin, ListAPIView): @@ -45,6 +93,7 @@ class UserList(UserMixin, ListAPIView): search_fields = ('username', 'first_name', 'last_name', 'email') ordering_fields = ('id', 'username', 'email') ordering = 'id' + serializer_class = UserListSerializer class UserDetail(UserMixin, RetrieveUpdateAPIView): @@ -59,4 +108,4 @@ class UserDetail(UserMixin, RetrieveUpdateAPIView): Update a user. """ - pass + serializer_class = UserDetailSerializer diff --git a/patchwork/tests/api/test_user.py b/patchwork/tests/api/test_user.py index dfc4ddf1..af340dfe 100644 --- a/patchwork/tests/api/test_user.py +++ b/patchwork/tests/api/test_user.py @@ -20,17 +20,36 @@ if settings.ENABLE_REST_API: class TestUserAPI(utils.APITestCase): @staticmethod - def api_url(item=None): + def api_url(item=None, version=None): + kwargs = {} + if version: + kwargs['version'] = version + if item is None: - return reverse('api-user-list') - return reverse('api-user-detail', args=[item]) + return reverse('api-user-list', kwargs=kwargs) + kwargs['pk'] = item + return reverse('api-user-detail', kwargs=kwargs) + + def assertSerialized(self, user_obj, user_json, has_settings=False): + user_obj.refresh_from_db() + user_obj.profile.refresh_from_db() - def assertSerialized(self, user_obj, user_json): self.assertEqual(user_obj.id, user_json['id']) self.assertEqual(user_obj.username, user_json['username']) self.assertNotIn('password', user_json) self.assertNotIn('is_superuser', user_json) + if has_settings: + self.assertIn('settings', user_json) + self.assertEqual(user_json['settings']['send_email'], + user_obj.profile.send_email) + self.assertEqual(user_json['settings']['items_per_page'], + user_obj.profile.items_per_page) + self.assertEqual(user_json['settings']['show_ids'], + user_obj.profile.show_ids) + else: + self.assertNotIn('settings', user_json) + @utils.store_samples('users-list-error-forbidden') def test_list_anonymous(self): """List users as anonymous user.""" @@ -60,13 +79,24 @@ class TestUserAPI(utils.APITestCase): @utils.store_samples('users-detail') def test_detail_authenticated(self): - """Show user as authenticated user.""" + """Show user as a user other than self.""" + user_a = create_user() + user_b = create_user() + + self.client.force_authenticate(user=user_a) + resp = self.client.get(self.api_url(user_b.id)) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertSerialized(user_b, resp.data, has_settings=False) + + @utils.store_samples('users-detail-self') + def test_detail_self(self): + """Show user as self.""" user = create_user() self.client.force_authenticate(user=user) resp = self.client.get(self.api_url(user.id)) self.assertEqual(status.HTTP_200_OK, resp.status_code) - self.assertSerialized(user, resp.data) + self.assertSerialized(user, resp.data, has_settings=True) @utils.store_samples('users-update-error-forbidden') def test_update_anonymous(self): @@ -76,8 +106,9 @@ class TestUserAPI(utils.APITestCase): resp = self.client.patch(self.api_url(user.id), {'first_name': 'Tan'}) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + @utils.store_samples('users-update') def test_update_other_user(self): - """Update user as another, non-maintainer user.""" + """Update user as a user other than self.""" user_a = create_user() user_b = create_user() @@ -86,26 +117,37 @@ class TestUserAPI(utils.APITestCase): {'first_name': 'Tan'}) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) - def test_update_maintainer(self): - """Update user as maintainer.""" - user = create_maintainer() - user.is_superuser = True - user.save() + @utils.store_samples('users-update-self') + def test_update_self(self): + """Update user as self.""" + user = create_user() + self.assertFalse(user.profile.send_email) self.client.force_authenticate(user=user) - resp = self.client.patch(self.api_url(user.id), {'first_name': 'Tan'}) + resp = self.client.patch(self.api_url(user.id), { + 'first_name': 'Tan', 'settings': {'send_email': True}}) self.assertEqual(status.HTTP_200_OK, resp.status_code) - self.assertSerialized(user, resp.data) + self.assertSerialized(user, resp.data, has_settings=True) + self.assertEqual('Tan', user.first_name) + self.assertTrue(user.profile.send_email) - @utils.store_samples('users-update') - def test_update_self(self): - """Update user as self.""" + def test_update_self_version_1_1(self): + """Update user as self using the old API. + + Ensure the profile changes are ignored. + """ user = create_user() + self.assertFalse(user.profile.send_email) self.client.force_authenticate(user=user) - resp = self.client.patch(self.api_url(user.id), {'first_name': 'Tan'}) + resp = self.client.patch( + self.api_url(user.id, version='1.1'), + {'first_name': 'Tan', 'settings': {'send_email': True}}, + validate_request=False) self.assertEqual(status.HTTP_200_OK, resp.status_code) - self.assertSerialized(user, resp.data) + self.assertSerialized(user, resp.data, has_settings=False) + self.assertEqual('Tan', user.first_name) + self.assertFalse(user.profile.send_email) def test_create_delete(self): """Ensure creations and deletions and not allowed."""