diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index ff6be033f9f5..9da8e5bd33ae 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -300,12 +300,23 @@ class CourseLimitedStaffRole(CourseStaffRole): @register_access_role class eSHEInstructorRole(CourseStaffRole): - """A Staff member of a course without access to Studio.""" + """A Staff member of a course without access to the membership tab and enrollment-related operations.""" ROLE = 'eshe_instructor' BASE_ROLE = CourseStaffRole.ROLE +@register_access_role +class TeachingAssistantRole(CourseStaffRole): + """ + A Staff member of a course without access to the membership tab, enrollment-related operations and + grade-related operations. + """ + + ROLE = 'teaching_assistant' + BASE_ROLE = CourseStaffRole.ROLE + + @register_access_role class CourseInstructorRole(CourseRole): """A course Instructor""" diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 831d33045d66..cc51622913f2 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -17,6 +17,7 @@ CourseFinanceAdminRole, CourseSalesAdminRole, eSHEInstructorRole, + TeachingAssistantRole, LibraryUserRole, CourseDataResearcherRole, GlobalStaff, @@ -170,6 +171,7 @@ class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-clas (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX')), (CourseLimitedStaffRole(IN_KEY), ('limited_staff', IN_KEY, 'edX')), (eSHEInstructorRole(IN_KEY), ('eshe_instructor', IN_KEY, 'edX')), + (TeachingAssistantRole(IN_KEY), ('teaching_assistant', IN_KEY, 'edX')), (CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX')), (OrgStaffRole(IN_KEY.org), ('staff', None, 'edX')), (CourseFinanceAdminRole(IN_KEY), ('finance_admin', IN_KEY, 'edX')), diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index 343b6923ccee..c5bc5fb83370 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -20,6 +20,7 @@ CourseLimitedStaffRole, CourseStaffRole, eSHEInstructorRole, + TeachingAssistantRole, ) from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params from openedx.core.djangoapps.django_comment_common.models import Role @@ -32,6 +33,7 @@ 'staff': CourseStaffRole, 'limited_staff': CourseLimitedStaffRole, 'eshe_instructor': eSHEInstructorRole, + 'teaching_assistant': TeachingAssistantRole, 'ccx_coach': CourseCcxCoachRole, 'data_researcher': CourseDataResearcherRole, } diff --git a/lms/djangoapps/instructor/permissions.py b/lms/djangoapps/instructor/permissions.py index 0d8353653b93..80dc649e5a95 100644 --- a/lms/djangoapps/instructor/permissions.py +++ b/lms/djangoapps/instructor/permissions.py @@ -32,6 +32,21 @@ VIEW_ENROLLMENTS = 'instructor.view_enrollments' VIEW_FORUM_MEMBERS = 'instructor.view_forum_members' +# Due to how the roles iheritance is implemented currently, eshe_instructor and teaching_assistant have implicit +# staff access, but unlike staff, they shouldn't be able to enroll and do grade-related operations as per client's +# requirements. At the same time, all other staff-derived roles, like Limited Staff, should be able to enroll students. +# This solution is far from perfect, but it's probably the best we can do untill the roles system is reworked. +_is_teaching_assistant = HasRolesRule('teaching_assistant') +_is_eshe_instructor = HasRolesRule('eshe_instructor') +_is_eshe_instructor_or_teaching_assistant = _is_teaching_assistant | _is_eshe_instructor +is_staff_but_not_teaching_assistant = ( + (_is_teaching_assistant & HasAccessRule('staff', strict=True)) | + (~_is_teaching_assistant & HasAccessRule('staff')) +) +is_staff_but_not_eshe_instructor_or_teaching_assistant = ( + (_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff', strict=True)) | + (~_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff')) +) perms[ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM] = HasAccessRule('staff') perms[ASSIGN_TO_COHORTS] = HasAccessRule('staff') @@ -41,23 +56,17 @@ perms[ENABLE_CERTIFICATE_GENERATION] = is_staff perms[GENERATE_CERTIFICATE_EXCEPTIONS] = is_staff perms[GENERATE_BULK_CERTIFICATE_EXCEPTIONS] = is_staff -perms[GIVE_STUDENT_EXTENSION] = HasAccessRule('staff') +perms[GIVE_STUDENT_EXTENSION] = is_staff_but_not_teaching_assistant perms[VIEW_ISSUED_CERTIFICATES] = HasAccessRule('staff') | HasRolesRule('data_researcher') # only global staff or those with the data_researcher role can access the data download tab # HasAccessRule('staff') also includes course staff perms[CAN_RESEARCH] = is_staff | HasRolesRule('data_researcher') -# eshe_instructor implicitly gets staff access, but shouldn't be able to enroll -perms[CAN_ENROLL] = ( - # can enroll if a user is an eshe_instructor and has an explicit staff role - (HasRolesRule('eshe_instructor') & HasAccessRule('staff', strict=True)) | - # can enroll if a user is just staff - (~HasRolesRule('eshe_instructor') & HasAccessRule('staff')) -) +perms[CAN_ENROLL] = is_staff_but_not_eshe_instructor_or_teaching_assistant perms[CAN_BETATEST] = HasAccessRule('instructor') perms[ENROLLMENT_REPORT] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[VIEW_COUPONS] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[EXAM_RESULTS] = HasAccessRule('staff') -perms[OVERRIDE_GRADES] = HasAccessRule('staff') +perms[OVERRIDE_GRADES] = is_staff_but_not_teaching_assistant perms[SHOW_TASKS] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[EMAIL] = HasAccessRule('staff') perms[RESCORE_EXAMS] = HasAccessRule('instructor') diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 6572379b6eb9..058b3d579c9b 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -316,9 +316,10 @@ def test_membership_reason_field_visibility(self, enbale_reason_field): else: self.assertNotContains(response, reason_field) - def test_membership_tab_for_eshe_instructor(self): + @ddt.data('eshe_instructor', 'teaching_assistant') + def test_membership_tab_content(self, role): """ - Verify that eSHE instructors don't have access to membership tab and + Verify that eSHE Instructors and Teaching Assistants don't have access to membership tab and work correctly with other roles. """ @@ -336,11 +337,11 @@ def test_membership_tab_for_eshe_instructor(self): user = UserFactory.create() self.client.login(username=user.username, password="test") - # eSHE instructors shouldn't have access to membership tab + # eSHE Instructors / Teaching Assistants shouldn't have access to membership tab CourseAccessRoleFactory( course_id=self.course.id, user=user, - role='eshe_instructor', + role=role, org=self.course.id.org ) response = self.client.get(self.url) @@ -366,6 +367,52 @@ def test_membership_tab_for_eshe_instructor(self): self.assertContains(response, membership_section) self.assertContains(response, batch_enrollment) + def test_student_admin_tab_content(self): + """ + Verify that Teaching Assistants don't have access to the gradebook-related sections + of the student admin tab. + """ + + # Should be visible to Teaching Assistants + view_enrollment_status = '