Skip to content

Commit

Permalink
feat: eSHE Instructor role
Browse files Browse the repository at this point in the history
Adds the eSHE Instructor role, which inherits Course Staff permissions,
but isn't able to enroll / un-enroll students and can't assing course
team roles unless in combination with Course Staff / Instructor /
Discussion admin roles.
  • Loading branch information
0x29a committed Jul 30, 2023
1 parent a30d849 commit eba8b0c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 5 deletions.
20 changes: 20 additions & 0 deletions common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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"""
Expand Down
2 changes: 2 additions & 0 deletions common/djangoapps/student/tests/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
CourseStaffRole,
CourseFinanceAdminRole,
CourseSalesAdminRole,
eSHEInstructorRole,
LibraryUserRole,
CourseDataResearcherRole,
GlobalStaff,
Expand Down Expand Up @@ -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')),
Expand Down
8 changes: 6 additions & 2 deletions lms/djangoapps/courseware/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_module import CourseBlock # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.error_module import ErrorBlock # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.x_module import XModule # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -48,10 +48,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):
Expand Down
2 changes: 2 additions & 0 deletions lms/djangoapps/instructor/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,6 +31,7 @@
'instructor': CourseInstructorRole,
'staff': CourseStaffRole,
'limited_staff': CourseLimitedStaffRole,
'eshe_instructor': eSHEInstructorRole,
'ccx_coach': CourseCcxCoachRole,
'data_researcher': CourseDataResearcherRole,
}
Expand Down
8 changes: 7 additions & 1 deletion lms/djangoapps/instructor/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
18 changes: 16 additions & 2 deletions lms/djangoapps/instructor/views/instructor_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -124,13 +126,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()

Expand Down Expand Up @@ -488,7 +494,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': (
access['eshe_instructor'] and not (access['explicit_staff'] or access['forum_admin'])
)
}
return section_data

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
14 changes: 14 additions & 0 deletions lms/templates/instructor/instructor_dashboard_2/membership.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -56,6 +57,7 @@
<div class="request-response"></div>
<div class="request-response-error"></div>
</fieldset>
%endif

%if static.get_value('ALLOW_AUTOMATED_SIGNUPS', settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False)):
<hr class="divider" />
Expand Down Expand Up @@ -190,6 +192,18 @@ <h3 class="hd hd-3">${_("Course Team Management")}</h3>
data-add-button-label="${_("Add Limited Staff")}"
></div>

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

## Note that "Admin" is identified as "Instructor" in the Django admin panel.
<div class="auth-list-container"
data-rolename="instructor"
Expand Down

0 comments on commit eba8b0c

Please sign in to comment.