[PATCH 2/2] REST: Allow configuration of user settings via API
Stephen Finucane
stephen at that.guru
Sun Dec 1 05:48:36 AEDT 2019
Expose the embedded UserProfile field via the REST API.
Signed-off-by: Stephen Finucane <stephen at that.guru>
---
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."""
--
2.23.0
More information about the Patchwork
mailing list