REST: Allow configuration of user settings via API

Expose the embedded UserProfile field via the REST API.

Signed-off-by: Stephen Finucane <stephen@that.guru>

5 files changed, 232 insertions, 34 deletions

diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
index a5e235b..4d660d5 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
@@ -2165,6 +2165,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 196d784..c2f5ea6 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
@@ -2209,6 +2233,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 d7b4d29..50db73c 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
@@ -2165,6 +2165,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 29944e2..4ea2322 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 dfc4ddf..af340df 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."""