From 173bbce5006b93b090014cb43afd26dd6cab081d Mon Sep 17 00:00:00 2001
From: sarina <sarina@edx.org>
Date: Fri, 19 Feb 2021 15:18:45 -0500
Subject: [PATCH] feat!: Remove the "role of users being enrolled" field from
Instructor Dashboard
See DEPR-142
---
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, 6 insertions(+), 100 deletions(-)
diff --git a/cms/envs/common.py b/cms/envs/common.py
index 746f08a5aec..1f1e233f92b 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -2023,11 +2023,6 @@ 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 6ccdcef7489..4ae91a43d12 100644
--- a/common/djangoapps/student/api.py
+++ b/common/djangoapps/student/api.py
@@ -45,11 +45,6 @@ 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"
@@ -59,7 +54,6 @@ def create_manual_enrollment_audit(
transition_state,
reason,
course_run_key=None,
- role=None
):
"""
Creates an audit item for a manual enrollment.
@@ -69,13 +63,10 @@ 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))
@@ -95,8 +86,7 @@ def create_manual_enrollment_audit(
user_email,
transition_state,
reason,
- enrollment,
- role
+ enrollment
)
diff --git a/common/djangoapps/student/models_api.py b/common/djangoapps/student/models_api.py
index e5a7e51ce55..20e0363f465 100644
--- a/common/djangoapps/student/models_api.py
+++ b/common/djangoapps/student/models_api.py
@@ -35,8 +35,7 @@ def create_manual_enrollment_audit( # lint-amnesty, pylint: disable=missing-fun
user_email,
state_transition,
reason,
- course_enrollment,
- role
+ course_enrollment
):
_ManualEnrollmentAudit.create_manual_enrollment_audit(
user=enrolled_by,
@@ -44,7 +43,6 @@ 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 dc4790e281c..e9cf54bc90c 100644
--- a/lms/djangoapps/instructor/tests/test_api.py
+++ b/lms/djangoapps/instructor/tests/test_api.py
@@ -1778,19 +1778,18 @@ class TestInstructorAPIEnrollment(SharedModuleStoreTestCase, LoginEnrollmentTest
)
assert course_enrollment.mode == CourseMode.DEFAULT_MODE_SLUG
- def test_role_and_reason_are_persisted(self):
+ def test_reason_is_persisted(self):
"""
- test that role and reason fields are persisted in the database
+ test that reason field is 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', 'role': 'Learner'}
+ 'auto_enroll': False, 'reason': 'testing'}
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 783f5da8f7c..fa1477d7223 100644
--- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
+++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
@@ -245,53 +245,6 @@ 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 0008b01312e..5ac4bd6a1d5 100644
--- a/lms/djangoapps/instructor/views/api.py
+++ b/lms/djangoapps/instructor/views/api.py
@@ -649,17 +649,6 @@ 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
@@ -752,7 +741,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, role
+ request.user, email, state_transition, reason, enrollment_obj
)
results.append({
'identifier': identifier,
diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py
index 755da85b7a2..3401724aa38 100644
--- a/lms/djangoapps/instructor/views/instructor_dashboard.py
+++ b/lms/djangoapps/instructor/views/instructor_dashboard.py
@@ -456,8 +456,6 @@ 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',
@@ -484,7 +482,6 @@ 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 bf2af92ea3c..6b16b30a6ff 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -73,10 +73,6 @@ 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 7945827b673..baa11767a9e 100644
--- a/lms/templates/instructor/instructor_dashboard_2/membership.html
+++ b/lms/templates/instructor/instructor_dashboard_2/membership.html
@@ -13,17 +13,6 @@ 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