Merge pull request #570 from open-craft/0x29a/bb7489/eshe-instructor-…

…palm

feat: eSHE Instructor role [BB-7489]

common/djangoapps/student/roles.py

@@ -7,6 +7,7 @@
 import logging
 from abc import ABCMeta, abstractmethod
 from collections import defaultdict
+from contextlib import contextmanager
 
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from opaque_keys.edx.django.models import CourseKeyField
@@ -44,6 +45,17 @@ def register_access_role(cls):
     return cls
 
 
+@contextmanager
+def strict_role_checking():
+    """
+    Context manager that temporarily disables role inheritance.
+    """
+    OLD_ACCESS_ROLES_INHERITANCE = ACCESS_ROLES_INHERITANCE.copy()
+    ACCESS_ROLES_INHERITANCE.clear()
+    yield
+    ACCESS_ROLES_INHERITANCE.update(OLD_ACCESS_ROLES_INHERITANCE)
+
+
 class BulkRoleCache:  # lint-amnesty, pylint: disable=missing-class-docstring
     CACHE_NAMESPACE = "student.roles.BulkRoleCache"
     CACHE_KEY = 'roles_by_user'
@@ -78,7 +90,7 @@ def __init__(self, user):
             )
 
     @staticmethod
-    def _get_roles(role):
+    def get_roles(role):
         """
         Return the roles that should have the same permissions as the specified role.
         """
@@ -90,7 +102,7 @@ def has_role(self, role, course_id, org):
         or a role that inherits from the specified role, course_id and org.
         """
         return any(
-            access_role.role in self._get_roles(role) and
+            access_role.role in self.get_roles(role) and
             access_role.course_id == course_id and
             access_role.org == org
             for access_role in self._roles
@@ -286,6 +298,14 @@ class CourseLimitedStaffRole(CourseStaffRole):
     BASE_ROLE = CourseStaffRole.ROLE
 
 
+@register_access_role
+class eSHEInstructorRole(CourseStaffRole):
+    """A Staff member of a course without access to Studio."""
+
+    ROLE = 'eshe_instructor'
+    BASE_ROLE = CourseStaffRole.ROLE
+
+
 @register_access_role
 class CourseInstructorRole(CourseRole):
     """A course Instructor"""
@@ -463,11 +483,11 @@ def remove_courses(self, *course_keys):
 
     def courses_with_role(self):
         """
-        Return a django QuerySet for all of the courses with this user x role. You can access
+        Return a django QuerySet for all of the courses with this user x (or derived from x) role. You can access
         any of these properties on each result record:
         * user (will be self.user--thus uninteresting)
         * org
         * course_id
         * role (will be self.role--thus uninteresting)
         """
-        return CourseAccessRole.objects.filter(role=self.role, user=self.user)
+        return CourseAccessRole.objects.filter(role__in=RoleCache.get_roles(self.role), user=self.user)

common/djangoapps/student/tests/test_roles.py

@@ -16,6 +16,7 @@
     CourseStaffRole,
     CourseFinanceAdminRole,
     CourseSalesAdminRole,
+    eSHEInstructorRole,
     LibraryUserRole,
     CourseDataResearcherRole,
     GlobalStaff,
@@ -168,6 +169,7 @@ class RoleCacheTestCase(TestCase):  # lint-amnesty, pylint: disable=missing-clas
     ROLES = (
         (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX')),
         (CourseLimitedStaffRole(IN_KEY), ('limited_staff', IN_KEY, 'edX')),
+        (eSHEInstructorRole(IN_KEY), ('eshe_instructor', 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/courseware/rules.py

@@ -18,7 +18,7 @@
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.enrollments.api import is_enrollment_valid_for_proctoring
 from common.djangoapps.student.models import CourseAccessRole
-from common.djangoapps.student.roles import CourseRole, OrgRole
+from common.djangoapps.student.roles import CourseRole, OrgRole, strict_role_checking
 from xmodule.course_block import CourseBlock  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.error_block import ErrorBlock  # lint-amnesty, pylint: disable=wrong-import-order
 
@@ -47,10 +47,14 @@ class HasAccessRule(Rule):
     """
     A rule that calls `has_access` to determine whether it passes
     """
-    def __init__(self, action):
+    def __init__(self, action, strict=False):
         self.action = action
+        self.strict = strict
 
     def check(self, user, instance=None):
+        if self.strict:
+            with strict_role_checking():
+                return has_access(user, self.action, instance)
         return has_access(user, self.action, instance)
 
     def query(self, user):

lms/djangoapps/instructor/access.py

@@ -19,6 +19,7 @@
     CourseInstructorRole,
     CourseLimitedStaffRole,
     CourseStaffRole,
+    eSHEInstructorRole,
 )
 from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params
 from openedx.core.djangoapps.django_comment_common.models import Role
@@ -30,6 +31,7 @@
     'instructor': CourseInstructorRole,
     'staff': CourseStaffRole,
     'limited_staff': CourseLimitedStaffRole,
+    'eshe_instructor': eSHEInstructorRole,
     'ccx_coach': CourseCcxCoachRole,
     'data_researcher': CourseDataResearcherRole,
 }

lms/djangoapps/instructor/permissions.py

@@ -44,7 +44,13 @@
 # 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')
-perms[CAN_ENROLL] = HasAccessRule('staff')
+# 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_BETATEST] = HasAccessRule('instructor')
 perms[ENROLLMENT_REPORT] = HasAccessRule('staff') | HasRolesRule('data_researcher')
 perms[VIEW_COUPONS] = HasAccessRule('staff') | HasRolesRule('data_researcher')

lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py

@@ -29,6 +29,7 @@
 from lms.djangoapps.courseware.tabs import get_course_tab_list
 from lms.djangoapps.courseware.tests.factories import StudentModuleFactory
 from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase
+from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
 from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK
 from lms.djangoapps.instructor.toggles import DATA_DOWNLOAD_V2
 from lms.djangoapps.instructor.views.gradebook_api import calculate_page_info
@@ -37,6 +38,7 @@
     ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND,
     OVERRIDE_DISCUSSION_LEGACY_SETTINGS_FLAG
 )
+from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
 from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
 
 
@@ -308,6 +310,56 @@ def test_membership_reason_field_visibility(self, enbale_reason_field):
         else:
             self.assertNotContains(response, reason_field)
 
+    def test_membership_tab_for_eshe_instructor(self):
+        """
+        Verify that eSHE instructors don't have access to membership tab and
+        work correctly with other roles.
+        """
+
+        membership_section = (
+            '<li class="nav-item">'
+            '<button type="button" class="btn-link membership" data-section="membership">'
+            'Membership'
+            '</button>'
+            '</li>'
+        )
+        batch_enrollment = (
+            '<fieldset class="batch-enrollment membership-section">'
+        )
+
+        user = UserFactory.create()
+        self.client.login(username=user.username, password="test")
+
+        # eSHE instructors shouldn't have access to membership tab
+        CourseAccessRoleFactory(
+            course_id=self.course.id,
+            user=user,
+            role='eshe_instructor',
+            org=self.course.id.org
+        )
+        response = self.client.get(self.url)
+        self.assertNotContains(response, membership_section)
+
+        # However if combined with forum_admin, they should have access to this
+        # tab, but not to batch enrollment
+        forum_admin_role = RoleFactory(name=FORUM_ROLE_ADMINISTRATOR, course_id=self.course.id)
+        forum_admin_role.users.add(user)
+        response = self.client.get(self.url)
+        self.assertContains(response, membership_section)
+        self.assertNotContains(response, batch_enrollment)
+
+        # Combined with course staff, should have union of all three roles
+        # permissions sets
+        CourseAccessRoleFactory(
+            course_id=self.course.id,
+            user=user,
+            role='staff',
+            org=self.course.id.org
+        )
+        response = self.client.get(self.url)
+        self.assertContains(response, membership_section)
+        self.assertContains(response, batch_enrollment)
+
     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

@@ -35,7 +35,9 @@
     CourseFinanceAdminRole,
     CourseInstructorRole,
     CourseSalesAdminRole,
-    CourseStaffRole
+    CourseStaffRole,
+    eSHEInstructorRole,
+    strict_role_checking,
 )
 from common.djangoapps.util.json_request import JsonResponse
 from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled
@@ -126,13 +128,17 @@ def instructor_dashboard_2(request, course_id):  # lint-amnesty, pylint: disable
     access = {
         'admin': request.user.is_staff,
         'instructor': bool(has_access(request.user, 'instructor', course)),
+        'eshe_instructor': eSHEInstructorRole(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)),
         'forum_admin': has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR),
         'data_researcher': request.user.has_perm(permissions.CAN_RESEARCH, course_key),
     }
 
+    with strict_role_checking():
+        access['explicit_staff'] = bool(has_access(request.user, 'staff', course))
+
     if not request.user.has_perm(permissions.VIEW_DASHBOARD, course_key):
         raise Http404()
 
@@ -490,7 +496,15 @@ def _section_membership(course, access):
             'update_forum_role_membership',
             kwargs={'course_id': str(course_key)}
         ),
-        'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False)
+        'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False),
+
+        # Membership section should be hidden for eSHE instructors.
+        # Since they get Course Staff role implicitly, we need to hide this
+        # 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'])
+        ),
     }
     return section_data
 

lms/envs/common.py

@@ -1048,6 +1048,16 @@
     # .. toggle_target_removal_date: None
     # .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832'
     'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False,
+
+    # .. toggle_name: FEATURES['ENABLE_ESHE_INSTRUCTOR_ROLE']
+    # .. toggle_implementation: DjangoSetting
+    # .. toggle_default: False
+    # .. toggle_description: Whether to enable the ESHE Instructor role
+    # .. toggle_use_cases: open_edx
+    # .. toggle_creation_date: 2023-07-31
+    # .. toggle_target_removal_date: None
+    # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/561/files'
+    'ENABLE_ESHE_INSTRUCTOR_ROLE': False,
 }
 
 # Specifies extra XBlock fields that should available when requested via the Course Blocks API

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

@@ -11,6 +11,7 @@ <h3 class="hd hd-3">Course Team Management</h3>
         <select id="member-lists-selector" class="member-lists-selector">
           <option value="staff">Staff</option>
           <option value="limited_staff">Limited Staff</option>
+          <option value="eshe_instructor">eSHE Instructor</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,6 +5,7 @@
 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']:
 <fieldset class="batch-enrollment membership-section">
     <legend id="heading-batch-enrollment" class="hd hd-3">${_("Batch Enrollment")}</legend>
     <label>
@@ -92,6 +93,8 @@ <h3 class="hd hd-3">${_("Register/Enroll Students")}</h3>
 </div>
 %endif
 
+%endif
+
 <hr class="divider" />
 
 %if section_data['access']['instructor']:
@@ -191,6 +194,20 @@ <h3 class="hd hd-3">${_("Course Team Management")}</h3>
       data-add-button-label="${_("Add Limited Staff")}"
     ></div>
 
+    %if settings.FEATURES.get('ENABLE_ESHE_INSTRUCTOR_ROLE', None):
+    <div class="auth-list-container"
+      data-rolename="eshe_instructor"
+      data-display-name="${_("eSHE Instructor")}"
+      data-info-text="
+        ${_("Course team members with the eSHE Instructor role help you manage your course. "
+        "eSHE Instructor permissions are similar to Staff, but they can't assign course team roles and "
+        "can't enroll and unenroll learners")}"
+      data-list-endpoint="${ section_data['list_course_role_members_url'] }"
+      data-modify-endpoint="${ section_data['modify_access_url'] }"
+      data-add-button-label="${_("Add eSHE Instructor")}"
+    ></div>
+    %endif
+
     ## Note that "Admin" is identified as "Instructor" in the Django admin panel.
     <div class="auth-list-container"
       data-rolename="instructor"