From 9524bc6388a2783b377aea46e25e805aa6f9fbc5 Mon Sep 17 00:00:00 2001
From: Thilo Wischmeyer <thwischm@gmail.com>
Date: Wed, 30 Jun 2021 14:24:52 +0200
Subject: [PATCH] Replaced change_is_reviewer with change_role

---
 core/tests/test_user_account_views.py | 36 +++++++++++++++++----------
 core/views/common_views.py            | 35 +++++++++++++++-----------
 2 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/core/tests/test_user_account_views.py b/core/tests/test_user_account_views.py
index aaf5b856..e64beb3c 100644
--- a/core/tests/test_user_account_views.py
+++ b/core/tests/test_user_account_views.py
@@ -105,27 +105,27 @@ class ReviewerCanChangeCorrectorRoleTests(APITestCase):
         self.reviewer1 = self.user_factory.make_reviewer()
         self.client = APIClient()
 
-    def _set_reviewer_rights(self, new_value, changing_user, user_to_change):
+    def _set_role(self, new_value, changing_user, user_to_change):
         self.client.force_authenticate(user=changing_user)
-        url = f"/api/user/{user_to_change.pk}/change_is_reviewer/"
-        return self.client.patch(url, data={'is_reviewer': new_value})
+        url = f"/api/user/{user_to_change.pk}/change_role/"
+        return self.client.patch(url, data={'role': new_value})
 
-    def _grant_reviewer_rights(self, changing_user, user_to_change):
-        return self._set_reviewer_rights(True, changing_user, user_to_change)
+    def _make_reviewer(self, changing_user, user_to_change):
+        return self._set_role('Reviewer', changing_user, user_to_change)
 
-    def _revoke_reviewer_rights(self, changing_user, user_to_change):
-        return self._set_reviewer_rights(False, changing_user, user_to_change)
+    def _make_tutor(self, changing_user, user_to_change):
+        return self._set_role('Tutor', changing_user, user_to_change)
 
     def test_reviewer_can_promote_tutor_to_reviewer(self):
         tutor = self.user_factory.make_tutor()
-        response = self._grant_reviewer_rights(self.reviewer1, tutor)
+        response = self._make_reviewer(self.reviewer1, tutor)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         tutor.refresh_from_db()
         self.assertTrue(tutor.is_reviewer())
 
     def test_reviewer_can_demote_other_reviewer_to_tutor(self):
         reviewer2 = self.user_factory.make_reviewer()
-        response = self._revoke_reviewer_rights(self.reviewer1, reviewer2)
+        response = self._make_tutor(self.reviewer1, reviewer2)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         reviewer2.refresh_from_db()
         self.assertFalse(reviewer2.is_reviewer())
@@ -137,7 +137,17 @@ class ReviewerCanChangeCorrectorRoleTests(APITestCase):
                 'pass_score': 60,
             }])[0]
         student = self.user_factory.make_student(exam=exam)
-        response = self._grant_reviewer_rights(self.reviewer1, student)
+        response = self._make_reviewer(self.reviewer1, student)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_reviewer_cannot_promote_student_to_tutor(self):
+        exam = make_exams(exams=[{
+                'module_reference': 'Test Exam 01',
+                'total_score': 100,
+                'pass_score': 60,
+            }])[0]
+        student = self.user_factory.make_student(exam=exam)
+        response = self._make_tutor(self.reviewer1, student)
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_student_cannot_change_access_rights(self):
@@ -147,14 +157,14 @@ class ReviewerCanChangeCorrectorRoleTests(APITestCase):
                 'pass_score': 60,
             }])[0]
         student = self.user_factory.make_student(exam=exam)
-        response = self._grant_reviewer_rights(student, self.reviewer1)
+        response = self._make_reviewer(student, self.reviewer1)
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_tutor_cannot_change_access_rights(self):
         tutor = self.user_factory.make_tutor()
-        response = self._grant_reviewer_rights(tutor, self.reviewer1)
+        response = self._make_reviewer(tutor, self.reviewer1)
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_reviewer_cannot_demote_self_to_tutor(self):
-        response = self._revoke_reviewer_rights(self.reviewer1, self.reviewer1)
+        response = self._make_tutor(self.reviewer1, self.reviewer1)
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
diff --git a/core/views/common_views.py b/core/views/common_views.py
index a1fb1fef..3ea1f890 100644
--- a/core/views/common_views.py
+++ b/core/views/common_views.py
@@ -328,26 +328,31 @@ class UserAccountViewSet(viewsets.ReadOnlyModelViewSet):
         user.save()
         return Response(status.HTTP_200_OK)
 
-    @action(detail=True, methods=['patch'])
-    def change_is_reviewer(self, request, *args, **kwargs):
-        changing_to_reviewer = request.data.get('is_reviewer')
+    @action(detail=True, methods=["patch"])
+    def change_role(self, request, *args, **kwargs):
+        new_role = request.data.get('role')
         user = self.get_object()
+        valid_values = [
+            models.UserAccount.STUDENT,
+            models.UserAccount.REVIEWER,
+            models.UserAccount.TUTOR,
+        ]
+        if new_role not in valid_values:
+            error_msg = (
+                "You need to provide a 'role' field with one of these values: "
+                + ', '.join(valid_values)
+            )
+            return Response({'Error': error_msg}, status.HTTP_400_BAD_REQUEST)
         if not request.user.is_reviewer():
-            error_msg = "Only reviewers can manage access rights."
+            error_msg = 'Only reviewers can manage access rights.'
             return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN)
-        if changing_to_reviewer is None:
-            error_msg = "You need to provide an 'is_reviewer' field"
-            return Response({'Error': error_msg}, status.HTTP_400_BAD_REQUEST)
-        if user.is_student() and changing_to_reviewer:
-            error_msg = "Cannot promote a student to reviewer."
+        if user.is_student():
+            error_msg = 'Cannot promote a student to another role.'
             return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN)
-        if user == request.user and not changing_to_reviewer:
-            error_msg = "As a reviewer, you cannot demote yourself."
+        if user == request.user and not new_role == models.UserAccount.REVIEWER:
+            error_msg = 'As a reviewer, you cannot demote yourself.'
             return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN)
-        if changing_to_reviewer:
-            user.role = models.UserAccount.REVIEWER
-        else:
-            user.role = models.UserAccount.STUDENT
+        user.role = new_role
         user.save()
         return Response(status.HTTP_200_OK)
 
-- 
GitLab