MST-264 Fix bug for course staff user in unrelated course (#24099)

* MST-264 Fix the wrong assumption that a course_staff of a course not related to programs should be in course_staff logic
This commit is contained in:
Simon Chen
2020-06-01 11:11:37 -04:00
committed by GitHub
parent 215e2d0530
commit d0f99ec778
2 changed files with 72 additions and 20 deletions

View File

@@ -1490,13 +1490,28 @@ class UserProgramReadOnlyAccessGetTests(EnrollmentsDataMixin, APITestCase):
with mock.patch(
_VIEW_PATCH_FORMAT.format('get_programs'),
autospec=True,
return_value=[self.mock_program_data[0]]
side_effect=[[self.mock_program_data[0]], []]
) as mock_get_programs:
response = self.client.get(reverse(self.view_name) + '?type=masters')
assert status.HTTP_200_OK == response.status_code
assert len(response.data) == 1
mock_get_programs.assert_called_once_with(course=self.course_id)
mock_get_programs.assert_has_calls([
mock.call(course=self.course_id),
mock.call(uuids=[]),
], any_order=True)
def _enroll_user_into_course_as_course_staff(self, user, course_key_string):
"""
This is a helper function to create a course run based on the course key string,
then enroll the user to the course run as a course staff.
"""
course_key_to_create = CourseKey.from_string(course_key_string)
CourseOverviewFactory(id=course_key_to_create)
CourseRunFactory.create(key=text_type(course_key_to_create))
CourseEnrollmentFactory.create(course_id=course_key_to_create, user=user)
CourseStaffRole(course_key_to_create).add_users(user)
return course_key_to_create
@ddt.data(
(
@@ -1520,11 +1535,11 @@ class UserProgramReadOnlyAccessGetTests(EnrollmentsDataMixin, APITestCase):
return program
return None
other_course_key = CourseKey.from_string('course-v1:edX+ToyX+Other_Course')
CourseOverviewFactory(id=other_course_key)
CourseRunFactory.create(key=text_type(other_course_key))
CourseEnrollmentFactory.create(course_id=other_course_key, user=self.course_staff)
CourseStaffRole(other_course_key).add_users(self.course_staff)
other_course_key = self._enroll_user_into_course_as_course_staff(
self.course_staff,
'course-v1:edX+ToyX+Other_Course'
)
self.client.login(username=self.course_staff.username, password=self.password)
programs_to_return_first = [
@@ -1543,7 +1558,7 @@ class UserProgramReadOnlyAccessGetTests(EnrollmentsDataMixin, APITestCase):
with mock.patch(
_VIEW_PATCH_FORMAT.format('get_programs'),
autospec=True,
side_effect=[programs_to_return_first, programs_to_return_second]
side_effect=[[], programs_to_return_first, programs_to_return_second]
) as mock_get_programs:
response = self.client.get(reverse(self.view_name) + '?type=masters')
@@ -1554,6 +1569,37 @@ class UserProgramReadOnlyAccessGetTests(EnrollmentsDataMixin, APITestCase):
mock.call(course=other_course_key),
], any_order=True)
def test_course_staff_of_non_program_course(self):
created_course_key = self._enroll_user_into_course_as_course_staff(
self.student,
'course-v1:edX+ToyX+Other_Course'
)
program_to_enroll = self.mock_program_data[0]
ProgramEnrollmentFactory.create(
program_uuid=program_to_enroll['uuid'],
curriculum_uuid=self.curriculum_uuid,
user=self.student,
status='enrolled',
external_user_key='user-{}'.format(self.student.id),
)
self.client.login(username=self.student.username, password=self.password)
with mock.patch(
_VIEW_PATCH_FORMAT.format('get_programs'),
autospec=True,
side_effect=[[], [program_to_enroll]]
) as mock_get_programs:
response = self.client.get(reverse(self.view_name))
assert status.HTTP_200_OK == response.status_code
assert len(response.data) == 1
mock_get_programs.assert_has_calls([
mock.call(course=created_course_key),
mock.call(uuids=[UUID(program_to_enroll['uuid'])]),
])
@mock.patch(_VIEW_PATCH_FORMAT.format('get_programs'), autospec=True, return_value=None)
def test_learner_200_if_no_programs_enrolled(self, mock_get_programs):
self.client.login(username=self.student.username, password=self.password)

View File

@@ -709,15 +709,17 @@ class UserProgramReadOnlyAccessView(DeveloperErrorViewMixin, PaginatedAPIView):
if request_user.is_staff:
programs = get_programs_by_type(request.site, requested_program_type)
elif self.is_course_staff(request_user):
programs = self.get_programs_user_is_course_staff_for(request_user, requested_program_type)
else:
program_enrollments = fetch_program_enrollments_by_student(
user=request.user,
program_enrollment_statuses=ProgramEnrollmentStatuses.__ACTIVE__,
)
uuids = [enrollment.program_uuid for enrollment in program_enrollments]
programs = get_programs(uuids=uuids) or []
program_dict = {}
# Check if the user is a course staff of any course which is a part of a program.
for staff_program in self.get_programs_user_is_course_staff_for(request_user, requested_program_type):
program_dict.setdefault(staff_program['uuid'], staff_program)
# Now get the program enrollments for user purely as a learner add to the list
for learner_program in self._get_enrolled_programs_from_model(request_user):
program_dict.setdefault(learner_program['uuid'], learner_program)
programs = list(program_dict.values())
programs_in_which_user_has_access = [
{'uuid': program['uuid'], 'slug': program['marketing_slug']}
@@ -726,12 +728,16 @@ class UserProgramReadOnlyAccessView(DeveloperErrorViewMixin, PaginatedAPIView):
return Response(programs_in_which_user_has_access, status.HTTP_200_OK)
def is_course_staff(self, user):
def _get_enrolled_programs_from_model(self, user):
"""
Returns true if the user is a course_staff member of any course within a program
Return the Program Enrollments linked to the learner within the data model.
"""
staff_course_keys = self.get_course_keys_user_is_staff_for(user)
return len(staff_course_keys)
program_enrollments = fetch_program_enrollments_by_student(
user=user,
program_enrollment_statuses=ProgramEnrollmentStatuses.__ACTIVE__,
)
uuids = [enrollment.program_uuid for enrollment in program_enrollments]
return get_programs(uuids=uuids) or []
def get_course_keys_user_is_staff_for(self, user):
"""