Skip to content

Commit 21b5656

Browse files
fix: Update permissions for course team tab in Instructor Dashboard
1 parent 160e7e6 commit 21b5656

4 files changed

Lines changed: 175 additions & 20 deletions

File tree

lms/djangoapps/instructor/permissions.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,24 @@ def has_permission(self, request, view):
9494
return request.user.has_perm(permission, course)
9595

9696

97+
class CourseTeamPermission(BasePermission):
98+
"""
99+
Allow access to course team management endpoints for users with
100+
instructor (Admin) access or the Discussion Admin (forum Administrator) role.
101+
"""
102+
def has_permission(self, request, view):
103+
try:
104+
course_key = CourseKey.from_string(view.kwargs.get('course_id'))
105+
except InvalidKeyError as exc:
106+
raise Http404('Invalid course key') from exc
107+
course = get_course_by_id(course_key)
108+
if has_access(request.user, 'instructor', course):
109+
return True
110+
if has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR):
111+
return True
112+
return False
113+
114+
97115
class ForumAdminRequiresInstructorAccess(BasePermission):
98116
"""
99117
default roles require either (staff & forum admin) or (instructor)

lms/djangoapps/instructor/tests/test_api_v2.py

Lines changed: 142 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,9 @@ def _test_staff_tabs(self, tabs):
327327
"""Helper to test tabs visible to staff users."""
328328
tab_ids = [tab['tab_id'] for tab in tabs]
329329

330-
# Staff should see these basic tabs
331-
expected_basic_tabs = ['course_info', 'enrollments', 'course_team', 'grading', 'cohorts']
332-
self.assertListEqual(tab_ids, expected_basic_tabs) # noqa: PT009
330+
# Staff should see these basic tabs (course_team is restricted to instructor/forum_admin)
331+
expected_basic_tabs = ['course_info', 'enrollments', 'grading', 'cohorts']
332+
assert tab_ids == expected_basic_tabs
333333

334334
def test_staff_sees_basic_tabs(self):
335335
"""
@@ -343,7 +343,10 @@ def test_instructor_sees_all_basic_tabs(self):
343343
Test that instructors see all tabs that staff see.
344344
"""
345345
instructor_tabs = self._get_tabs_from_response(self.instructor)
346-
self._test_staff_tabs(instructor_tabs)
346+
tab_ids = [tab['tab_id'] for tab in instructor_tabs]
347+
348+
expected_tabs = ['course_info', 'enrollments', 'course_team', 'grading', 'cohorts']
349+
assert tab_ids == expected_tabs
347350

348351
def test_researcher_sees_all_basic_tabs(self):
349352
"""
@@ -3369,3 +3372,138 @@ def test_revoke_forum_role(self):
33693372
assert response.data['roles'] == ['Moderator']
33703373
assert response.data['action'] == 'revoke'
33713374
assert not role.users.filter(pk=target.pk).exists()
3375+
3376+
3377+
class CourseTeamTabVisibilityTest(SharedModuleStoreTestCase):
3378+
"""
3379+
Tests that the course_team tab is only visible to Admin (instructor role)
3380+
and Discussion Admin (forum Administrator role).
3381+
3382+
See: https://github.com/openedx/openedx-platform/issues/38439
3383+
"""
3384+
3385+
@classmethod
3386+
def setUpClass(cls):
3387+
super().setUpClass()
3388+
cls.course = CourseFactory.create(
3389+
org='edX',
3390+
number='TabVis',
3391+
run='2024',
3392+
display_name='Tab Visibility Test Course',
3393+
)
3394+
cls.course_key = cls.course.id
3395+
3396+
def setUp(self):
3397+
super().setUp()
3398+
self.client = APIClient()
3399+
self.url = reverse('instructor_api_v2:course_metadata', kwargs={'course_id': str(self.course_key)})
3400+
3401+
# Instructor (Admin) — should see course_team tab
3402+
self.instructor = InstructorFactory.create(course_key=self.course_key)
3403+
3404+
# Discussion Admin (forum Administrator) — should see course_team tab
3405+
self.forum_admin = StaffFactory.create(course_key=self.course_key)
3406+
seed_permissions_roles(self.course_key)
3407+
admin_role = Role.objects.get(course_id=self.course_key, name='Administrator')
3408+
admin_role.users.add(self.forum_admin)
3409+
3410+
# Staff — should NOT see course_team tab
3411+
self.staff_user = StaffFactory.create(course_key=self.course_key)
3412+
3413+
def _get_tab_ids(self, user):
3414+
self.client.force_authenticate(user=user)
3415+
response = self.client.get(self.url)
3416+
assert response.status_code == status.HTTP_200_OK
3417+
return [tab['tab_id'] for tab in response.data.get('tabs', [])]
3418+
3419+
def test_instructor_sees_course_team_tab(self):
3420+
"""Admin (instructor role) should see the course_team tab."""
3421+
tab_ids = self._get_tab_ids(self.instructor)
3422+
assert 'course_team' in tab_ids
3423+
3424+
def test_forum_admin_sees_course_team_tab(self):
3425+
"""Discussion Admin (forum Administrator role) should see the course_team tab."""
3426+
tab_ids = self._get_tab_ids(self.forum_admin)
3427+
assert 'course_team' in tab_ids
3428+
3429+
def test_staff_does_not_see_course_team_tab(self):
3430+
"""Staff without instructor or forum admin role should NOT see the course_team tab."""
3431+
tab_ids = self._get_tab_ids(self.staff_user)
3432+
assert 'course_team' not in tab_ids
3433+
3434+
3435+
class CourseTeamEndpointForumAdminAccessTest(SharedModuleStoreTestCase):
3436+
"""
3437+
Tests that Discussion Admin (forum Administrator role) can access
3438+
course team endpoints, not just the instructor role.
3439+
3440+
See: https://github.com/openedx/openedx-platform/issues/38439
3441+
"""
3442+
3443+
@classmethod
3444+
def setUpClass(cls):
3445+
super().setUpClass()
3446+
cls.course = CourseFactory.create(
3447+
org='edX',
3448+
number='ForumAccess',
3449+
run='2024',
3450+
display_name='Forum Admin Access Test Course',
3451+
)
3452+
cls.course_key = cls.course.id
3453+
3454+
def setUp(self):
3455+
super().setUp()
3456+
self.client = APIClient()
3457+
3458+
# Discussion Admin: staff + forum Administrator role
3459+
self.forum_admin = StaffFactory.create(course_key=self.course_key)
3460+
seed_permissions_roles(self.course_key)
3461+
admin_role = Role.objects.get(course_id=self.course_key, name='Administrator')
3462+
admin_role.users.add(self.forum_admin)
3463+
3464+
# Plain staff user (no forum admin, no instructor) — should be denied
3465+
self.staff_user = StaffFactory.create(course_key=self.course_key)
3466+
3467+
def test_forum_admin_can_list_team_roles(self):
3468+
"""Discussion Admin should be able to GET /team/roles."""
3469+
url = reverse('instructor_api_v2:course_team_roles', kwargs={'course_id': str(self.course_key)})
3470+
self.client.force_authenticate(user=self.forum_admin)
3471+
response = self.client.get(url)
3472+
assert response.status_code == status.HTTP_200_OK
3473+
3474+
def test_forum_admin_can_list_team_members(self):
3475+
"""Discussion Admin should be able to GET /team."""
3476+
url = reverse('instructor_api_v2:course_team', kwargs={'course_id': str(self.course_key)})
3477+
self.client.force_authenticate(user=self.forum_admin)
3478+
response = self.client.get(url)
3479+
assert response.status_code == status.HTTP_200_OK
3480+
3481+
def test_forum_admin_can_grant_role(self):
3482+
"""Discussion Admin should be able to POST /team to grant a role."""
3483+
url = reverse('instructor_api_v2:course_team', kwargs={'course_id': str(self.course_key)})
3484+
target = UserFactory.create()
3485+
self.client.force_authenticate(user=self.forum_admin)
3486+
response = self.client.post(url, {
3487+
'identifiers': [target.username],
3488+
'role': 'staff',
3489+
'action': 'allow',
3490+
}, format='json')
3491+
assert response.status_code == status.HTTP_200_OK
3492+
3493+
def test_forum_admin_can_revoke_role(self):
3494+
"""Discussion Admin should be able to DELETE /team/{username}."""
3495+
target = StaffFactory.create(course_key=self.course_key)
3496+
url = reverse(
3497+
'instructor_api_v2:course_team_member',
3498+
kwargs={'course_id': str(self.course_key), 'email_or_username': target.username},
3499+
)
3500+
self.client.force_authenticate(user=self.forum_admin)
3501+
response = self.client.delete(url, {'roles': ['staff']}, format='json')
3502+
assert response.status_code == status.HTTP_200_OK
3503+
3504+
def test_plain_staff_cannot_access_team_endpoints(self):
3505+
"""Staff without instructor or forum admin role should get 403."""
3506+
url = reverse('instructor_api_v2:course_team', kwargs={'course_id': str(self.course_key)})
3507+
self.client.force_authenticate(user=self.staff_user)
3508+
response = self.client.get(url)
3509+
assert response.status_code == status.HTTP_403_FORBIDDEN

lms/djangoapps/instructor/views/api_v2.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2420,8 +2420,7 @@ class CourseTeamRolesView(DeveloperErrorViewMixin, APIView):
24202420
* 401: User is not authenticated
24212421
* 403: User lacks instructor permissions
24222422
"""
2423-
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
2424-
permission_name = permissions.EDIT_COURSE_ACCESS
2423+
permission_classes = (IsAuthenticated, permissions.CourseTeamPermission)
24252424

24262425
def get(self, request, course_id):
24272426
"""Return the list of available course team roles for this course."""
@@ -2547,8 +2546,7 @@ class CourseTeamView(DeveloperErrorViewMixin, APIView):
25472546
* 403: User lacks instructor permissions
25482547
* 404: Course not found
25492548
"""
2550-
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
2551-
permission_name = permissions.EDIT_COURSE_ACCESS
2549+
permission_classes = (IsAuthenticated, permissions.CourseTeamPermission)
25522550

25532551
def get(self, request, course_id):
25542552
"""
@@ -2716,8 +2714,7 @@ class CourseTeamMemberView(DeveloperErrorViewMixin, APIView):
27162714
* 404: Course or user not found
27172715
* 409: Cannot remove own instructor access
27182716
"""
2719-
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
2720-
permission_name = permissions.EDIT_COURSE_ACCESS
2717+
permission_classes = (IsAuthenticated, permissions.CourseTeamPermission)
27212718

27222719
def delete(self, request, course_id, email_or_username):
27232720
"""Revoke one or more course roles from a user."""

lms/djangoapps/instructor/views/serializers_v2.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,6 @@ def get_tabs(self, data):
168168
),
169169
'sort_order': 20,
170170
},
171-
{
172-
'tab_id': 'course_team',
173-
'title': _('Course Team'),
174-
'url': self._build_tab_url(
175-
'INSTRUCTOR_MICROFRONTEND_URL',
176-
course_key,
177-
'course_team'
178-
),
179-
'sort_order': 30,
180-
},
181171
{
182172
'tab_id': 'grading',
183173
'title': _('Grading'),
@@ -200,6 +190,18 @@ def get_tabs(self, data):
200190
},
201191
])
202192

193+
if access['instructor'] or access['forum_admin']:
194+
tabs.append({
195+
'tab_id': 'course_team',
196+
'title': _('Course Team'),
197+
'url': self._build_tab_url(
198+
'INSTRUCTOR_MICROFRONTEND_URL',
199+
course_key,
200+
'course_team'
201+
),
202+
'sort_order': 30,
203+
})
204+
203205
if access['staff'] and is_bulk_email_feature_enabled(course_key):
204206
tabs.append(
205207
{

0 commit comments

Comments
 (0)