fix: Limit roles that can be seen in Instructor Dashboard. (#38460)

Author: brianjbuck-wgu

lms/djangoapps/instructor/tests/test_api_v2.py

@@ -3563,28 +3563,55 @@ def test_forum_admin_can_list_team_members(self):
         response = self.client.get(url)
         assert response.status_code == status.HTTP_200_OK
 
-    def test_forum_admin_can_grant_role(self):
-        """Discussion Admin should be able to POST /team to grant a role."""
+    def test_forum_admin_can_grant_forum_role(self):
+        """Discussion Admin should be able to grant a forum role."""
         url = reverse('instructor_api_v2:course_team', kwargs={'course_id': str(self.course_key)})
         target = UserFactory.create()
         self.client.force_authenticate(user=self.forum_admin)
         response = self.client.post(url, {
             'identifiers': [target.username],
-            'role': 'staff',
+            'role': 'Moderator',
             'action': 'allow',
         }, format='json')
         assert response.status_code == status.HTTP_200_OK
 
-    def test_forum_admin_can_revoke_role(self):
-        """Discussion Admin should be able to DELETE /team/{username}."""
+    def test_forum_admin_can_revoke_forum_role(self):
+        """Discussion Admin should be able to revoke a forum role."""
+        target = UserFactory.create()
+        role = Role.objects.get(course_id=self.course_key, name='Moderator')
+        role.users.add(target)
+        url = reverse(
+            'instructor_api_v2:course_team_member',
+            kwargs={'course_id': str(self.course_key), 'email_or_username': target.username},
+        )
+        self.client.force_authenticate(user=self.forum_admin)
+        response = self.client.delete(url, {'roles': ['Moderator']}, format='json')
+        assert response.status_code == status.HTTP_200_OK
+
+    def test_forum_admin_cannot_grant_course_role(self):
+        """Discussion Admin should not be able to grant a non-forum course role like staff."""
+        url = reverse('instructor_api_v2:course_team', kwargs={'course_id': str(self.course_key)})
+        target = UserFactory.create()
+        self.client.force_authenticate(user=self.forum_admin)
+        response = self.client.post(url, {
+            'identifiers': [target.username],
+            'role': 'staff',
+            'action': 'allow',
+        }, format='json')
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert 'You do not have permissions to change this role' in response.data['error']
+
+    def test_forum_admin_cannot_revoke_course_role(self):
+        """Discussion Admin should not be able to revoke a non-forum course role like staff."""
         target = StaffFactory.create(course_key=self.course_key)
         url = reverse(
             'instructor_api_v2:course_team_member',
             kwargs={'course_id': str(self.course_key), 'email_or_username': target.username},
         )
         self.client.force_authenticate(user=self.forum_admin)
         response = self.client.delete(url, {'roles': ['staff']}, format='json')
-        assert response.status_code == status.HTTP_200_OK
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert 'You do not have permissions to change the requested roles' in response.data['error']
 
     def test_plain_staff_cannot_access_team_endpoints(self):
         """Staff without instructor or forum admin role should get 403."""
@@ -3615,10 +3642,10 @@ def test_forum_admin_cannot_grant_instructor_role(self):
             'action': 'allow',
         }, format='json')
         assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert 'You do not have permissions to change this role' in response.data['error']
 
     def test_forum_admin_cannot_revoke_instructor_role(self):
         """Discussion Admin should not be able to revoke the instructor role."""
-        # Create an instructor to target
         instructor = InstructorFactory.create(course_key=self.course_key)
         url = reverse(
             'instructor_api_v2:course_team_member',
@@ -3627,3 +3654,38 @@ def test_forum_admin_cannot_revoke_instructor_role(self):
         self.client.force_authenticate(user=self.forum_admin)
         response = self.client.delete(url, {'roles': ['instructor']}, format='json')
         assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert 'You do not have permissions to change the requested roles' in response.data['error']
+
+    def test_roles_editable_param_filters_for_forum_admin(self):
+        """GET /team/roles?editable=true returns only forum roles for Discussion Admin."""
+        url = reverse('instructor_api_v2:course_team_roles', kwargs={'course_id': str(self.course_key)})
+        self.client.force_authenticate(user=self.forum_admin)
+        response = self.client.get(url, {'editable': 'true'})
+        assert response.status_code == status.HTTP_200_OK
+        returned_roles = {r['role'] for r in response.data['results']}
+        assert returned_roles == {'Administrator', 'Moderator', 'Group Moderator', 'Community TA'}
+
+    def test_roles_editable_param_returns_all_for_instructor(self):
+        """GET /team/roles?editable=true returns all roles for an instructor."""
+        instructor = InstructorFactory.create(course_key=self.course_key)
+        url = reverse('instructor_api_v2:course_team_roles', kwargs={'course_id': str(self.course_key)})
+        self.client.force_authenticate(user=instructor)
+        response = self.client.get(url, {'editable': 'true'})
+        assert response.status_code == status.HTTP_200_OK
+        returned_roles = {r['role'] for r in response.data['results']}
+        # Instructor should see both course roles and forum roles
+        assert 'instructor' in returned_roles
+        assert 'staff' in returned_roles
+        assert 'Administrator' in returned_roles
+
+    def test_roles_without_editable_param_returns_all(self):
+        """GET /team/roles without editable param returns all roles regardless of user."""
+        url = reverse('instructor_api_v2:course_team_roles', kwargs={'course_id': str(self.course_key)})
+        self.client.force_authenticate(user=self.forum_admin)
+        response = self.client.get(url)
+        assert response.status_code == status.HTTP_200_OK
+        returned_roles = {r['role'] for r in response.data['results']}
+        # Without editable param, all roles are returned
+        assert 'instructor' in returned_roles
+        assert 'staff' in returned_roles
+        assert 'Administrator' in returned_roles

lms/djangoapps/instructor/views/api_v2.py

@@ -3065,9 +3065,15 @@ class CourseTeamRolesView(DeveloperErrorViewMixin, APIView):
     ``CUSTOM_COURSES_EDX`` feature flag is enabled **and** the course
     has CCX enabled (``course.enable_ccx``).
 
+    When the `editable=true` query parameter is passed, the results
+    are further filtered to only include roles the requesting user has
+    permission to assign. Discussion Administrators will only see forum
+    roles; instructors will see all roles.
+
     **GET Example Request**
 
         GET /api/instructor/v2/courses/{course_id}/team/roles
+        GET /api/instructor/v2/courses/{course_id}/team/roles?editable=true
 
     **GET Response Values**
 
@@ -3095,12 +3101,17 @@ def get(self, request, course_id):
         course_key = CourseKey.from_string(course_id)
         course = get_course_by_id(course_key)
 
+        editable = request.query_params.get('editable', 'false').lower() == 'true'
+
         roles = set(ROLES.keys()) | set(FORUM_ROLES)
 
         ccx_enabled = settings.FEATURES.get('CUSTOM_COURSES_EDX', False) and course.enable_ccx
         if not ccx_enabled:
             roles.discard('ccx_coach')
 
+        if editable and not has_access(request.user, 'instructor', course):
+            roles = set(FORUM_ROLES)
+
         results = [
             {'role': rolename, 'display_name': str(ROLE_DISPLAY_NAMES[rolename])}
             for rolename in sorted(roles)
@@ -3299,9 +3310,9 @@ def post(self, request, course_id):
         rolename = serializer.validated_data['role']
         action = serializer.validated_data['action']
 
-        if rolename == 'instructor' and not has_access(request.user, 'instructor', course):
+        if not is_forum_role(rolename) and not has_access(request.user, 'instructor', course):
             return Response(
-                {'error': _('Managing the instructor role requires instructor access.')},
+                {'error': _('You do not have permissions to change this role.')},
                 status=status.HTTP_403_FORBIDDEN,
             )
 
@@ -3401,11 +3412,13 @@ def delete(self, request, course_id, email_or_username):
 
         roles = revoke_serializer.validated_data['roles']
 
-        if 'instructor' in roles and not has_access(request.user, 'instructor', course):
-            return Response(
-                {'error': _('Managing the instructor role requires instructor access.')},
-                status=status.HTTP_403_FORBIDDEN,
-            )
+        if not has_access(request.user, 'instructor', course):
+            non_forum_roles = [r for r in roles if not is_forum_role(r)]
+            if non_forum_roles:
+                return Response(
+                    {'error': _('You do not have permissions to change the requested roles.')},
+                    status=status.HTTP_403_FORBIDDEN,
+                )
 
         try:
             user = get_student_from_identifier(email_or_username)