Skip to content

Commit

Permalink
fix: allow only enrolled users in course forum roles
Browse files Browse the repository at this point in the history
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 not the expected behaviour based on the help text displayed in
the section.

This PR updates update_forum_role_membership api to check whether user
is enrolled before adding them to a role.

Cherry pick from openedx#32436

(cherry picked from commit 10377ea7d0b1182f5d0b6d389161fab2bac1722a)
  • Loading branch information
navinkarkera committed Jun 26, 2023
1 parent 9475431 commit 259dbf6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 0 deletions.
33 changes: 33 additions & 0 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2212,11 +2212,16 @@ def setUp(self):
super().setUp()

self.instructor = InstructorFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.instructor, self.course.id)
self.client.login(username=self.instructor.username, password='test')

self.other_instructor = InstructorFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.other_instructor, self.course.id)
self.other_staff = StaffFactory(course_key=self.course.id)
CourseEnrollment.enroll(self.other_staff, self.course.id)
self.other_user = UserFactory()
CourseEnrollment.enroll(self.other_user, self.course.id)
self.unenrolled_user = UserFactory()

def test_modify_access_noparams(self):
""" Test missing all query parameters. """
Expand Down Expand Up @@ -2432,6 +2437,34 @@ def assert_update_forum_role_membership(self, current_user, identifier, rolename
elif action == 'revoke':
assert rolename not in user_roles

def test_update_forum_role_with_unenrolled_user(self):
"""
Check response of update_forum_role_membership api with unenrolled user.
"""

def _check_allow_access_with_unenrolled_user(url, role):
"""
Helper function for testing api.
"""
response = self.client.post(url, {
'unique_student_identifier': self.unenrolled_user.username,
'rolename': role,
'action': 'allow',
})
assert response.status_code == 200
expected = {
'unique_student_identifier': self.unenrolled_user.username,
'userNotEnrolled': True,
}
res_json = json.loads(response.content.decode('utf-8'))
assert res_json == expected
user_roles = self.unenrolled_user.roles.filter(course_id=self.course.id).values_list("name", flat=True)
assert role not in user_roles

forum_role_url = reverse('update_forum_role_membership', kwargs={'course_id': str(self.course.id)})
_check_allow_access_with_unenrolled_user(forum_role_url, 'Community TA')
_check_allow_access_with_unenrolled_user(forum_role_url, 'Administrator')


@ddt.ddt
class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2807,6 +2807,12 @@ def update_forum_role_membership(request, course_id):
))

user = get_student_from_identifier(unique_student_identifier)
if action == 'allow' and not is_user_enrolled_in_course(user, course_id):
response_payload = {
'unique_student_identifier': unique_student_identifier,
'userNotEnrolled': True,
}
return JsonResponse(response_payload)

try:
update_forum_role(course_id, user, rolename, action)
Expand Down
5 changes: 5 additions & 0 deletions lms/static/js/instructor_dashboard/membership.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@ such that the value can be defined later than this assignment (file load order).
return this.show_errors(
gettext('Error: You cannot remove yourself from the Instructor group!')
);
} else if (data.userNotEnrolled) {
msg = gettext("Error: User '<%- username %>' is not enrolled in this course.")
return this.show_errors(_.template(msg)({
username: data.unique_student_identifier
}));
} else {
return this.reload_list();
}
Expand Down

0 comments on commit 259dbf6

Please sign in to comment.