Merge pull request #634 from open-craft/0x29a/bb8580/teaching-assista…

…nt-quince

feat: Teaching Assistant role [BB-8580][QUINCE]

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"""

common/djangoapps/student/tests/test_roles.py

@@ -16,6 +16,7 @@
     CourseFinanceAdminRole,
     CourseSalesAdminRole,
     eSHEInstructorRole,
+    TeachingAssistantRole,
     LibraryUserRole,
     CourseDataResearcherRole,
     GlobalStaff,
@@ -169,6 +170,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')),

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,
 }

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')

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 = '<div class="student-enrollment-status-container action-type-container">'
+        view_progress = '<div class="student-progress-container action-type-container">'
+
+        # Should not be visible to Teaching Assistants
+        view_gradebook = '<div class="action-type-container ">'
+        adjust_learner_grade = '<div class="student-grade-container action-type-container ">'
+        adjust_all_learners_grades = '<div class="course-specific-container action-type-container ">'
+
+        user = UserFactory.create()
+        self.client.login(username=user.username, password="test")
+
+        # Teaching Assistants shouldn't have access to the gradebook-related sections
+        CourseAccessRoleFactory(
+            course_id=self.course.id,
+            user=user,
+            role='teaching_assistant',
+            org=self.course.id.org
+        )
+        response = self.client.get(self.url)
+        self.assertContains(response, view_enrollment_status)
+        self.assertContains(response, view_progress)
+        self.assertNotContains(response, view_gradebook)
+        self.assertNotContains(response, adjust_learner_grade)
+        self.assertNotContains(response, adjust_all_learners_grades)
+
+        # However if combined with instructor, they should have access to all sections
+        CourseAccessRoleFactory(
+            course_id=self.course.id,
+            user=user,
+            role='instructor',
+            org=self.course.id.org
+        )
+        response = self.client.get(self.url)
+        self.assertContains(response, view_enrollment_status)
+        self.assertContains(response, view_progress)
+        self.assertContains(response, view_gradebook)
+        self.assertContains(response, adjust_learner_grade)
+        self.assertContains(response, adjust_all_learners_grades)
+
     def test_student_admin_staff_instructor(self):
         """
         Verify that staff users are not able to see course-wide options, while still

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -33,6 +33,7 @@
     CourseSalesAdminRole,
     CourseStaffRole,
     eSHEInstructorRole,
+    TeachingAssistantRole,
     strict_role_checking,
 )
 from common.djangoapps.util.json_request import JsonResponse
@@ -125,6 +126,7 @@ def instructor_dashboard_2(request, course_id):  # lint-amnesty, pylint: disable
         'admin': request.user.is_staff,
         'instructor': bool(has_access(request.user, 'instructor', course)),
         'eshe_instructor': eSHEInstructorRole(course_key).has_user(request.user),
+        'teaching_assistant': TeachingAssistantRole(course_key).has_user(request.user),
         'finance_admin': CourseFinanceAdminRole(course_key).has_user(request.user),
         'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user),
         'staff': bool(has_access(request.user, 'staff', course)),
@@ -520,7 +522,11 @@ def _section_membership(course, access):
         # section if the user doesn't have the Course Staff role set explicitly
         # or have the Discussion Admin role.
         'is_hidden': (
-            not access['forum_admin'] and (access['eshe_instructor'] and not access['explicit_staff'])
+            not access['forum_admin']
+            and (
+                (access['eshe_instructor'] or access['teaching_assistant'])
+                and not access['explicit_staff']
+            )
         ),
     }
     return section_data

lms/envs/common.py

@@ -1071,6 +1071,16 @@
     # .. toggle_target_removal_date: None
     # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/561/files'
     'ENABLE_ESHE_INSTRUCTOR_ROLE': False,
+
+    # .. toggle_name: FEATURES['ENABLE_TEACHING_ASSISTANT_ROLE']
+    # .. toggle_implementation: DjangoSetting
+    # .. toggle_default: False
+    # .. toggle_description: Whether to enable the Teaching Assistant role
+    # .. toggle_use_cases: open_edx
+    # .. toggle_creation_date: 2024-02-12
+    # .. toggle_target_removal_date: None
+    # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/632/files'
+    'ENABLE_TEACHING_ASSISTANT_ROLE': False,
 }
 
 # Specifies extra XBlock fields that should available when requested via the Course Blocks API

lms/static/js/fixtures/instructor_dashboard/membership.html

@@ -12,6 +12,7 @@ <h3 class="hd hd-3">Course Team Management</h3>
           <option value="staff">Staff</option>
           <option value="limited_staff">Limited Staff</option>
           <option value="eshe_instructor">eSHE Instructor</option>
+          <option value="teaching_assistant">Teaching Assistant</option>
           <option value="instructor">Admin</option>
           <option value="beta">Beta Testers</option>
           <option value="Administrator">Discussion Admins</option>

lms/templates/instructor/instructor_dashboard_2/membership.html

@@ -5,7 +5,13 @@
 from django.utils.translation import pgettext
 from openedx.core.djangolib.markup import HTML, Text
 %>
-% if not section_data['access']['eshe_instructor'] or section_data['access']['explicit_staff']:
+
+<%
+  eshe_instructor_or_teaching_assistant_access = section_data['access']['eshe_instructor'] or section_data['access']['teaching_assistant']
+  membership_section_visible = not eshe_instructor_or_teaching_assistant_access or section_data['access']['explicit_staff']
+%>
+
+% if membership_section_visible:
 <fieldset class="batch-enrollment membership-section">
     <legend id="heading-batch-enrollment" class="hd hd-3">${_("Batch Enrollment")}</legend>
     <label>
@@ -208,6 +214,20 @@ <h3 class="hd hd-3">${_("Course Team Management")}</h3>
     ></div>
     %endif
 
+    %if settings.FEATURES.get('ENABLE_TEACHING_ASSISTANT_ROLE', None):
+    <div class="auth-list-container"
+      data-rolename="teaching_assistant"
+      data-display-name="${_("Teaching Assistant")}"
+      data-info-text="
+        ${_("Course team members with the Teaching Assistant role help you manage your course. "
+        "Teaching Assistant permissions are similar to Staff, but they can't assign course team roles, "
+        "enroll and unenroll learners and view or override grades")}"
+      data-list-endpoint="${ section_data['list_course_role_members_url'] }"
+      data-modify-endpoint="${ section_data['modify_access_url'] }"
+      data-add-button-label="${_("Add Teaching Assistant")}"
+    ></div>
+    %endif%
+
     ## Note that "Admin" is identified as "Instructor" in the Django admin panel.
     <div class="auth-list-container"
       data-rolename="instructor"

lms/templates/instructor/instructor_dashboard_2/student_admin.html

@@ -1,7 +1,13 @@
 <%page args="section_data" expression_filter="h"/>
 <%! from django.utils.translation import gettext as _ %>
+
+<%
+  teaching_assistant_access = section_data['access']['teaching_assistant']
+  gradebook_related_sections_hidden = teaching_assistant_access and not section_data['access']['explicit_staff']
+%>
+
 %if section_data['access']['staff'] or section_data['access']['instructor']:
-<div class="action-type-container">
+<div class="action-type-container ${'hidden' if gradebook_related_sections_hidden else ''}">
     %if section_data.get('writable_gradebook_url') or section_data.get('is_small_course'):
         <br><br>
         <h4 class="hd hd-4">${_("View gradebook for enrolled learners")}</h4>
@@ -59,7 +65,7 @@ <h4 class="hd hd-4">${_("View a specific learner's grades and progress")}</h4>
   <hr>
 </div>
 
-<div class="student-grade-container action-type-container">
+<div class="student-grade-container action-type-container ${'hidden' if gradebook_related_sections_hidden else ''}">
     <h4 class="hd hd-4">${_("Adjust a learner's grade for a specific problem")}</h4>
     <div class="request-response-error"></div>
     <label for="student-select-grade">
@@ -132,7 +138,7 @@ <h5 class="hd hd-5">${_("Task Status")}</h5>
 </div>
 
 % if course.entrance_exam_enabled:
-<div class="entrance-exam-grade-container action-type-container">
+<div class="entrance-exam-grade-container action-type-container ${'hidden' if gradebook_related_sections_hidden else ''}">
     <h4 class="hd hd-4">${_("Adjust a learner's entrance exam results")}</h4>
     <div class="request-response-error"></div>
 
@@ -194,7 +200,7 @@ <h5 class="hd hd-5">${_("Task Status")}</h5>
 
 %if section_data['access']['instructor']:
 %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
-<div class="course-specific-container action-type-container">
+<div class="course-specific-container action-type-container ${'hidden' if gradebook_related_sections_hidden else ''}">
     <h4 class="hd hd-4">${_("Adjust all enrolled learners' grades for a specific problem")}</h4>
     <div class="request-response-error"></div>
 
@@ -232,7 +238,7 @@ <h5 class="hd hd-5">${_("Task Status")}</h5>
 %endif
 
 %if settings.FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
-<div class="running-tasks-container action-type-container">
+<div class="running-tasks-container action-type-container ${'hidden' if gradebook_related_sections_hidden else ''}">
     <h4 class="hd hd-4">${_("Pending Tasks")}</h4>
     <div class="running-tasks-section">
         <label>${_("The status for any active tasks appears in a table below.")}</label>