Skip to content

Commit

Permalink
feat: Teaching Assistant role
Browse files Browse the repository at this point in the history
(cherry picked from commit 176de06)
  • Loading branch information
0x29a committed Feb 14, 2024
1 parent c352d5b commit 7ef00c0
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 21 deletions.
13 changes: 12 additions & 1 deletion common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
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 @@
CourseFinanceAdminRole,
CourseSalesAdminRole,
eSHEInstructorRole,
TeachingAssistantRole,
LibraryUserRole,
CourseDataResearcherRole,
GlobalStaff,
Expand Down Expand Up @@ -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')),
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 @@ -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
Expand All @@ -32,6 +33,7 @@
'staff': CourseStaffRole,
'limited_staff': CourseLimitedStaffRole,
'eshe_instructor': eSHEInstructorRole,
'teaching_assistant': TeachingAssistantRole,
'ccx_coach': CourseCcxCoachRole,
'data_researcher': CourseDataResearcherRole,
}
Expand Down
27 changes: 18 additions & 9 deletions lms/djangoapps/instructor/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down
55 changes: 51 additions & 4 deletions lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

Expand All @@ -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)
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion lms/djangoapps/instructor/views/instructor_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
CourseSalesAdminRole,
CourseStaffRole,
eSHEInstructorRole,
TeachingAssistantRole,
strict_role_checking,
)
from common.djangoapps.util.json_request import JsonResponse
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
22 changes: 21 additions & 1 deletion lms/templates/instructor/instructor_dashboard_2/membership.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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"
Expand Down
16 changes: 11 additions & 5 deletions lms/templates/instructor/instructor_dashboard_2/student_admin.html
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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>

Expand Down Expand Up @@ -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>

Expand Down Expand Up @@ -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>
Expand Down

0 comments on commit 7ef00c0

Please sign in to comment.