From e3b0bffa503f7942b9e3aa31b3eee0be293b0dec Mon Sep 17 00:00:00 2001
From: Sarina Canelake <sarina@edx.org>
Date: Tue, 23 Feb 2021 09:03:31 -0500
Subject: [PATCH] Revert "Remove the "role of users being enrolled" field from
Instructor Dashboard"
---
cms/envs/common.py | 5 ++
common/djangoapps/student/api.py | 12 ++++-
common/djangoapps/student/models_api.py | 4 +-
lms/djangoapps/instructor/tests/test_api.py | 7 +--
.../tests/views/test_instructor_dashboard.py | 47 +++++++++++++++++++
lms/djangoapps/instructor/views/api.py | 13 ++++-
.../instructor/views/instructor_dashboard.py | 3 ++
lms/envs/common.py | 4 ++
.../instructor_dashboard_2/membership.html | 11 +++++
9 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/cms/envs/common.py b/cms/envs/common.py
index 7a4194a92ce..04c8ee5e520 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -2023,6 +2023,11 @@ INTEGRATED_CHANNELS_API_CHUNK_TRANSMISSION_LIMIT = {}
BASE_COOKIE_DOMAIN = 'localhost'
+# This limits the type of roles that are submittable via the `student` app's manual enrollment
+# audit API. While this isn't used in CMS, it is used via Enterprise which is installed in
+# the CMS. Without this, we get errors.
+MANUAL_ENROLLMENT_ROLE_CHOICES = ['Learner', 'Support', 'Partner']
+
############## Settings for the Discovery App ######################
COURSE_CATALOG_URL_ROOT = 'http://localhost:8008'
diff --git a/common/djangoapps/student/api.py b/common/djangoapps/student/api.py
index 4ae91a43d12..6ccdcef7489 100644
--- a/common/djangoapps/student/api.py
+++ b/common/djangoapps/student/api.py
@@ -45,6 +45,11 @@ TRANSITION_STATES = (
DEFAULT_TRANSITION_STATE,
)
+MANUAL_ENROLLMENT_ROLE_CHOICES = configuration_helpers.get_value(
+ 'MANUAL_ENROLLMENT_ROLE_CHOICES',
+ settings.MANUAL_ENROLLMENT_ROLE_CHOICES
+)
+
COURSE_DASHBOARD_PLUGIN_VIEW_NAME = "course_dashboard"
@@ -54,6 +59,7 @@ def create_manual_enrollment_audit(
transition_state,
reason,
course_run_key=None,
+ role=None
):
"""
Creates an audit item for a manual enrollment.
@@ -63,10 +69,13 @@ def create_manual_enrollment_audit(
transition_state: <str> state of enrollment transition state from _TRANSITIONS_STATES
reason: <str> Reason why user was manually enrolled
course_run_key: <str> Used to link the audit enrollment to the actual enrollment
+ role: <str> role of the enrolled user from MANUAL_ENROLLMENT_ROLE_CHOICES
Note: We purposefully *exclude* passing items like CourseEnrollment objects to prevent callers from needed to
know about model level code.
"""
+ if role and role not in MANUAL_ENROLLMENT_ROLE_CHOICES:
+ raise ValueError("Role `{}` not in allowed roles: `{}".format(role, MANUAL_ENROLLMENT_ROLE_CHOICES))
if transition_state not in TRANSITION_STATES:
raise ValueError("State `{}` not in allow states: `{}`".format(transition_state, TRANSITION_STATES))
@@ -86,7 +95,8 @@ def create_manual_enrollment_audit(
user_email,
transition_state,
reason,
- enrollment
+ enrollment,
+ role
)
diff --git a/common/djangoapps/student/models_api.py b/common/djangoapps/student/models_api.py
index 20e0363f465..e5a7e51ce55 100644
--- a/common/djangoapps/student/models_api.py
+++ b/common/djangoapps/student/models_api.py
@@ -35,7 +35,8 @@ def create_manual_enrollment_audit( # lint-amnesty, pylint: disable=missing-fun
user_email,
state_transition,
reason,
- course_enrollment
+ course_enrollment,
+ role
):
_ManualEnrollmentAudit.create_manual_enrollment_audit(
user=enrolled_by,
@@ -43,6 +44,7 @@ def create_manual_enrollment_audit( # lint-amnesty, pylint: disable=missing-fun
state_transition=state_transition,
reason=reason,
enrollment=course_enrollment,
+ role=role,
)
diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py
index 7123b9758ef..5a74c5f81c4 100644
--- a/lms/djangoapps/instructor/tests/test_api.py
+++ b/lms/djangoapps/instructor/tests/test_api.py
@@ -1776,18 +1776,19 @@ class TestInstructorAPIEnrollment(SharedModuleStoreTestCase, LoginEnrollmentTest
)
assert course_enrollment.mode == CourseMode.DEFAULT_MODE_SLUG
- def test_reason_is_persisted(self):
+ def test_role_and_reason_are_persisted(self):
"""
- test that reason field is persisted in the database
+ test that role and reason fields are persisted in the database
"""
paid_course = self.create_paid_course()
url = reverse('students_update_enrollment', kwargs={'course_id': str(paid_course.id)})
params = {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': False,
- 'auto_enroll': False, 'reason': 'testing'}
+ 'auto_enroll': False, 'reason': 'testing', 'role': 'Learner'}
response = self.client.post(url, params)
manual_enrollment = ManualEnrollmentAudit.objects.first()
assert manual_enrollment.reason == 'testing'
+ assert manual_enrollment.role == 'Learner'
assert response.status_code == 200
def _change_student_enrollment(self, user, course, action):
diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
index fa1477d7223..783f5da8f7c 100644
--- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
+++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
@@ -245,6 +245,53 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT
else:
self.assertNotContains(response, reason_field)
+ def test_membership_site_configuration_role(self):
+ """
+ Verify that the role choices set via site configuration are loaded in the membership tab
+ of the instructor dashboard
+ """
+
+ configuration_values = {
+ "MANUAL_ENROLLMENT_ROLE_CHOICES": [
+ "role1",
+ "role2",
+ ]
+ }
+ site = Site.objects.first()
+ SiteConfiguration.objects.create(
+ site=site,
+ site_values=configuration_values,
+ enabled=True
+ )
+ url = reverse(
+ 'instructor_dashboard',
+ kwargs={
+ 'course_id': str(self.course_info.id)
+ }
+ )
+
+ response = self.client.get(url)
+ self.assertContains(response, '<option value="role1">role1</option>')
+ self.assertContains(response, '<option value="role2">role2</option>')
+
+ def test_membership_default_role(self):
+ """
+ Verify that in the absence of site configuration role choices, default values of role choices are loaded
+ in the membership tab of the instructor dashboard
+ """
+
+ url = reverse(
+ 'instructor_dashboard',
+ kwargs={
+ 'course_id': str(self.course_info.id)
+ }
+ )
+
+ response = self.client.get(url)
+ self.assertContains(response, '<option value="Learner">Learner</option>')
+ self.assertContains(response, '<option value="Support">Support</option>')
+ self.assertContains(response, '<option value="Partner">Partner</option>')
+
def test_student_admin_staff_instructor(self):
"""
Verify that staff users are not able to see course-wide options, while still
diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py
index 0213c06cfc7..88cb6c93105 100644
--- a/lms/djangoapps/instructor/views/api.py
+++ b/lms/djangoapps/instructor/views/api.py
@@ -649,6 +649,17 @@ def students_update_enrollment(request, course_id): # lint-amnesty, pylint: dis
auto_enroll = _get_boolean_param(request, 'auto_enroll')
email_students = _get_boolean_param(request, 'email_students')
reason = request.POST.get('reason')
+ role = request.POST.get('role')
+
+ allowed_role_choices = configuration_helpers.get_value('MANUAL_ENROLLMENT_ROLE_CHOICES',
+ settings.MANUAL_ENROLLMENT_ROLE_CHOICES)
+ if role and role not in allowed_role_choices:
+ return JsonResponse(
+ {
+ 'action': action,
+ 'results': [{'error': True, 'message': 'Not a valid role choice'}],
+ 'auto_enroll': auto_enroll,
+ }, status=400)
enrollment_obj = None
state_transition = DEFAULT_TRANSITION_STATE
@@ -741,7 +752,7 @@ def students_update_enrollment(request, course_id): # lint-amnesty, pylint: dis
else:
ManualEnrollmentAudit.create_manual_enrollment_audit(
- request.user, email, state_transition, reason, enrollment_obj
+ request.user, email, state_transition, reason, enrollment_obj, role
)
results.append({
'identifier': identifier,
diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py
index 9ee10ab442a..29d83fdb9ec 100644
--- a/lms/djangoapps/instructor/views/instructor_dashboard.py
+++ b/lms/djangoapps/instructor/views/instructor_dashboard.py
@@ -456,6 +456,8 @@ def _section_membership(course, access):
""" Provide data for the corresponding dashboard section """
course_key = course.id
ccx_enabled = settings.FEATURES.get('CUSTOM_COURSES_EDX', False) and course.enable_ccx
+ enrollment_role_choices = configuration_helpers.get_value('MANUAL_ENROLLMENT_ROLE_CHOICES',
+ settings.MANUAL_ENROLLMENT_ROLE_CHOICES)
section_data = {
'section_key': 'membership',
@@ -482,6 +484,7 @@ def _section_membership(course, access):
'update_forum_role_membership',
kwargs={'course_id': str(course_key)}
),
+ 'enrollment_role_choices': enrollment_role_choices,
'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False)
}
return section_data
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 0e126d2c6f9..51f3a823803 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -86,6 +86,10 @@ LMS_ROOT_URL = 'https://localhost:18000'
LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL
LMS_ENROLLMENT_API_PATH = "/api/enrollment/v1/"
+# Default choices for role dropdown in the membership tab of the instructor dashboard
+# This setting is used when a site does not define its own choices via site configuration
+MANUAL_ENROLLMENT_ROLE_CHOICES = ['Learner', 'Support', 'Partner']
+
# List of logout URIs for each IDA that the learner should be logged out of when they logout of the LMS. Only applies to
# IDA for which the social auth flow uses DOT (Django OAuth Toolkit).
IDA_LOGOUT_URI_LIST = []
diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html
index baa11767a9e..7945827b673 100644
--- a/lms/templates/instructor/instructor_dashboard_2/membership.html
+++ b/lms/templates/instructor/instructor_dashboard_2/membership.html
@@ -13,6 +13,17 @@ from openedx.core.djangolib.markup import HTML, Text
<textarea rows="6" name="student-ids" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
</label>
+ <div class="role">
+ <label>
+ ${_("Role of the users being enrolled.")}
+ <select name="role">
+ % for role in section_data['enrollment_role_choices']:
+ <option value="${role}">${role}</option>
+ % endfor
+ </select>
+ </label>
+ </div>
+
% if section_data['is_reason_field_enabled']:
<label>
${_("Enter the reason why the students are to be manually enrolled or unenrolled.")}
--
GitLab