From 5aaaf06230890213a37dda27c7a9c073dc3fd80b Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 13 Jun 2023 16:06:25 +0530 Subject: [PATCH] fix: auto enroll users when added as staff The course team management section under Instructor > Membership tab allows users to be added a role even if are not enrolled in the course. This is behaviour does not match the help text displayed in the section. This PR updates modify_access api to enrolls user if they are not enrolled after adding them to a role as well as changes the help text to reflect actual changes. --- cms/templates/manage_users.html | 2 +- lms/djangoapps/instructor/tests/test_api.py | 3 +++ lms/djangoapps/instructor/views/api.py | 7 +++++-- .../instructor_dashboard/membership.html | 4 ++-- lms/static/js/instructor_dashboard/membership.js | 2 +- .../instructor_dashboard_2/membership.html | 16 ++++++++++------ 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/cms/templates/manage_users.html b/cms/templates/manage_users.html index b0c278d11414..5da57980a459 100644 --- a/cms/templates/manage_users.html +++ b/cms/templates/manage_users.html @@ -101,7 +101,7 @@

${_("Course Team Roles")}

${_("Course team members with the Staff role are course co-authors. They have full writing and editing privileges on all course content.")}

## Note that the "Admin" role below is identified as "Instructor" in the Django admin panel.

${_("Admins are course team members who can add and remove other course team members.")}

-

${_("All course team members can access content in Studio, the LMS, and Insights, but are not automatically enrolled in the course.")}

+

${_("All course team members can access content in Studio, the LMS, and Insights, and they are automatically enrolled in the course.")}

% if show_transfer_ownership_hint: diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 0cb7e009454e..bb2be51e9640 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -2253,6 +2253,7 @@ def test_modify_access_bad_role(self): assert response.status_code == 400 def test_modify_access_allow(self): + assert CourseEnrollment.is_enrolled(self.other_user, self.course.id) is False url = reverse('modify_access', kwargs={'course_id': str(self.course.id)}) response = self.client.post(url, { 'unique_student_identifier': self.other_user.email, @@ -2260,6 +2261,8 @@ def test_modify_access_allow(self): 'action': 'allow', }) assert response.status_code == 200 + # User should be auto enrolled in the course + assert CourseEnrollment.is_enrolled(self.other_user, self.course.id) def test_modify_access_allow_with_uname(self): url = reverse('modify_access', kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index f2d52b0c3a71..ddcfa7b7aba3 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1005,11 +1005,12 @@ def modify_access(request, course_id): course = get_course_with_access( request.user, 'instructor', course_id, depth=None ) + unique_student_identifier = request.POST.get('unique_student_identifier') try: - user = get_student_from_identifier(request.POST.get('unique_student_identifier')) + user = get_student_from_identifier(unique_student_identifier) except User.DoesNotExist: response_payload = { - 'unique_student_identifier': request.POST.get('unique_student_identifier'), + 'unique_student_identifier': unique_student_identifier, 'userDoesNotExist': True, } return JsonResponse(response_payload) @@ -1044,6 +1045,8 @@ def modify_access(request, course_id): if action == 'allow': allow_access(course, user, rolename) + if not is_user_enrolled_in_course(user, course_id): + CourseEnrollment.enroll(user, course_id) elif action == 'revoke': revoke_access(course, user, rolename) else: diff --git a/lms/static/js/fixtures/instructor_dashboard/membership.html b/lms/static/js/fixtures/instructor_dashboard/membership.html index 962bfac5b4f1..85ab52c9665e 100644 --- a/lms/static/js/fixtures/instructor_dashboard/membership.html +++ b/lms/static/js/fixtures/instructor_dashboard/membership.html @@ -22,7 +22,7 @@

Course Team Management

@@ -33,7 +33,7 @@

Course Team Management

Course team members with the Staff role help you manage your course. Staff can enroll and unenroll learners, as well as modify their grades and access all course data. Staff also have access to your course in Studio and - Insights. You can only give course team roles to enrolled users. + Insights. Any users not yet enrolled in the course will be automatically enrolled when added as Staff.
diff --git a/lms/static/js/instructor_dashboard/membership.js b/lms/static/js/instructor_dashboard/membership.js index 2d4c54d88c98..f9e157607bf1 100644 --- a/lms/static/js/instructor_dashboard/membership.js +++ b/lms/static/js/instructor_dashboard/membership.js @@ -510,7 +510,7 @@ such that the value can be defined later than this assignment (file load order). text: identifier })); } - } + } return displayResponse.$task_response.append($taskResSection); }; if (errors.length === 0 && successes.length === 0 && noUsers.length === 0) { diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 7fba151bbff3..5a477665868a 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -172,7 +172,8 @@

${_("Course Team Management")}

${_("Course team members with the Staff role help you manage your course. " "Staff can enroll and unenroll learners, as well as modify their grades and " "access all course data. Staff also have access to your course in Studio and " - "Insights. You can only give course team roles to enrolled users.")}" + "Insights. Any users not yet enrolled in the course will be automatically " + "enrolled when added as Staff.")}" data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="${_("Add Staff")}" @@ -185,7 +186,8 @@

${_("Course Team Management")}

${_("Course team members with the Limited Staff role help you manage your course. " "Limited Staff can enroll and unenroll learners, as well as modify their grades and " "access all course data. Limited Staff don't have access to your course in Studio. " - "You can only give course team roles to enrolled users.")}" + "Any users not yet enrolled in the course will be automatically enrolled when added " + "as Limited Staff.")}" data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="${_("Add Limited Staff")}" @@ -199,8 +201,8 @@

${_("Course Team Management")}

${_("Course team members with the Admin role help you manage your course. " "They can do all of the tasks that Staff can do, and can also add and " "remove the Staff and Admin roles, discussion moderation roles, and the " - "beta tester role to manage course team membership. You can only give " - "course team roles to enrolled users.")}" + "beta tester role to manage course team membership. Any users not yet " + "enrolled in the course will be automatically enrolled when added as Admin.")}" data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="${_("Add Admin")}" @@ -213,7 +215,8 @@

${_("Course Team Management")}

data-info-text=" ${_("Beta Testers can see course content before other learners. " "They can make sure that the content works, but have no additional " - "privileges. You can only give course team roles to enrolled users.")}" + "privileges. Any users not yet enrolled in the course will be automatically " + "enrolled when added as Beta Tester.")}" data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="${_("Add Beta Tester")}" @@ -237,7 +240,8 @@

${_("Course Team Management")}

data-rolename="data_researcher" data-display-name="${_("Course Data Researcher")}" data-info-text=" - ${_("Course Data Researchers can access the data download tab.")}" + ${_("Course Data Researchers can access the data download tab. Any users not yet " + "enrolled in the course will be automatically enrolled when added as Course Data Researcher.")}" data-list-endpoint="${ section_data['list_course_role_members_url'] }" data-modify-endpoint="${ section_data['modify_access_url'] }" data-add-button-label="${_("Add Course Data Researcher")}"