EDUCATOR-5571: filter gradebook api by course staff role (#26994)
feat: add excluded_course_roles parameter to gradebook view to allow omitting staff
This commit is contained in:
@@ -21,7 +21,7 @@ from rest_framework.response import Response
|
|||||||
from rest_framework.views import APIView
|
from rest_framework.views import APIView
|
||||||
|
|
||||||
from common.djangoapps.student.auth import has_course_author_access
|
from common.djangoapps.student.auth import has_course_author_access
|
||||||
from common.djangoapps.student.models import CourseEnrollment
|
from common.djangoapps.student.models import CourseEnrollment, CourseAccessRole
|
||||||
from common.djangoapps.student.roles import BulkRoleCache
|
from common.djangoapps.student.roles import BulkRoleCache
|
||||||
from common.djangoapps.track.event_transaction_utils import (
|
from common.djangoapps.track.event_transaction_utils import (
|
||||||
create_new_event_transaction_id,
|
create_new_event_transaction_id,
|
||||||
@@ -618,7 +618,19 @@ class GradebookView(GradeViewMixin, PaginatedAPIView):
|
|||||||
q_object |= Q(course_grade_in_range=True)
|
q_object |= Q(course_grade_in_range=True)
|
||||||
|
|
||||||
q_objects.append(q_object)
|
q_objects.append(q_object)
|
||||||
|
if request.GET.get('excluded_course_roles'):
|
||||||
|
excluded_course_roles = request.GET.getlist('excluded_course_roles')
|
||||||
|
course_access_role_filters = dict(
|
||||||
|
user=OuterRef('user'),
|
||||||
|
course_id=course_key,
|
||||||
|
)
|
||||||
|
if 'all' not in excluded_course_roles:
|
||||||
|
course_access_role_filters['role__in'] = excluded_course_roles
|
||||||
|
annotations['has_excluded_role'] = Exists(
|
||||||
|
CourseAccessRole.objects.filter(**course_access_role_filters)
|
||||||
|
)
|
||||||
|
# TODO: In django 3.0+, we can directly filter on this 'exists' rather than annotating
|
||||||
|
q_objects.append(Q(has_excluded_role=False))
|
||||||
entries = []
|
entries = []
|
||||||
related_models = ['user']
|
related_models = ['user']
|
||||||
users = self._paginate_users(course_key, q_objects, related_models, annotations=annotations)
|
users = self._paginate_users(course_key, q_objects, related_models, annotations=annotations)
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ Tests for the course grading API view
|
|||||||
|
|
||||||
import json
|
import json
|
||||||
from collections import OrderedDict, namedtuple
|
from collections import OrderedDict, namedtuple
|
||||||
|
from contextlib import contextmanager
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
@@ -19,6 +20,13 @@ from rest_framework import status
|
|||||||
from rest_framework.test import APITestCase
|
from rest_framework.test import APITestCase
|
||||||
|
|
||||||
from common.djangoapps.course_modes.models import CourseMode
|
from common.djangoapps.course_modes.models import CourseMode
|
||||||
|
from common.djangoapps.student.roles import (
|
||||||
|
CourseBetaTesterRole,
|
||||||
|
CourseCcxCoachRole,
|
||||||
|
CourseDataResearcherRole,
|
||||||
|
CourseInstructorRole,
|
||||||
|
CourseStaffRole
|
||||||
|
)
|
||||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
|
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
|
||||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||||
from lms.djangoapps.courseware.tests.factories import InstructorFactory, StaffFactory
|
from lms.djangoapps.courseware.tests.factories import InstructorFactory, StaffFactory
|
||||||
@@ -1280,6 +1288,142 @@ class GradebookViewTest(GradebookViewTestBase):
|
|||||||
assert actual_data['total_users_count'] == num_enrollments
|
assert actual_data['total_users_count'] == num_enrollments
|
||||||
assert actual_data['filtered_users_count'] == num_enrollments
|
assert actual_data['filtered_users_count'] == num_enrollments
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def _mock_all_course_grade_reads(self, percent=0.9):
|
||||||
|
"""
|
||||||
|
A context manager for mocking CourseGradeFactory.read and returning the same grade for all learners.
|
||||||
|
"""
|
||||||
|
# pylint: disable=unused-argument
|
||||||
|
def fake_course_grade_read(*args, **kwargs):
|
||||||
|
return self.mock_course_grade(kwargs['user'], passed=True, percent=percent)
|
||||||
|
|
||||||
|
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_read:
|
||||||
|
mock_read.side_effect = fake_course_grade_read
|
||||||
|
yield
|
||||||
|
|
||||||
|
def _assert_usernames(self, response, expected_usernames):
|
||||||
|
""" Helper method to assert that the expected users were returned from the endpoint """
|
||||||
|
assert status.HTTP_200_OK == response.status_code
|
||||||
|
response_data = dict(response.data)
|
||||||
|
actual_usernames = [row['username'] for row in response_data['results']]
|
||||||
|
assert set(actual_usernames) == set(expected_usernames)
|
||||||
|
|
||||||
|
def test_users_with_course_roles(self):
|
||||||
|
""" Test that a staff member erolled in the course will be included in grade results. """
|
||||||
|
# This function creates and enrolls a course staff (not global staff) user
|
||||||
|
staff_user = self.login_course_staff()
|
||||||
|
with override_waffle_flag(self.waffle_flag, active=True):
|
||||||
|
with self._mock_all_course_grade_reads():
|
||||||
|
response = self.client.get(self.get_url(course_key=self.course.id))
|
||||||
|
course_students = [
|
||||||
|
self.student.username,
|
||||||
|
self.other_student.username,
|
||||||
|
self.program_student.username,
|
||||||
|
self.program_masters_student.username
|
||||||
|
]
|
||||||
|
self._assert_usernames(
|
||||||
|
response,
|
||||||
|
course_students + [staff_user.username]
|
||||||
|
)
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
None,
|
||||||
|
[],
|
||||||
|
['all'],
|
||||||
|
[CourseInstructorRole.ROLE, CourseBetaTesterRole.ROLE, CourseCcxCoachRole.ROLE],
|
||||||
|
[CourseInstructorRole.ROLE, 'all'],
|
||||||
|
[CourseBetaTesterRole.ROLE, 'nonexistant-role'],
|
||||||
|
[
|
||||||
|
CourseInstructorRole.ROLE,
|
||||||
|
CourseStaffRole.ROLE,
|
||||||
|
CourseBetaTesterRole.ROLE,
|
||||||
|
CourseCcxCoachRole.ROLE,
|
||||||
|
CourseDataResearcherRole.ROLE
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_filter_course_roles(self, excluded_course_roles):
|
||||||
|
""" Test that excluded_course_roles=all filters out any user with a course role """
|
||||||
|
# Create test users, enroll them in the course, and give them roles.
|
||||||
|
role_user_usernames = dict()
|
||||||
|
course_roles_to_create = [
|
||||||
|
CourseInstructorRole,
|
||||||
|
CourseStaffRole,
|
||||||
|
CourseBetaTesterRole,
|
||||||
|
CourseCcxCoachRole,
|
||||||
|
CourseDataResearcherRole,
|
||||||
|
]
|
||||||
|
for role in course_roles_to_create:
|
||||||
|
user = UserFactory.create(username="test_filter_course_roles__" + role.ROLE)
|
||||||
|
role(self.course.id).add_users(user)
|
||||||
|
self._create_user_enrollments(user)
|
||||||
|
role_user_usernames[role.ROLE] = user.username
|
||||||
|
|
||||||
|
# This will create global staff and not enroll them in the course
|
||||||
|
self.login_staff()
|
||||||
|
with self._mock_all_course_grade_reads():
|
||||||
|
with override_waffle_flag(self.waffle_flag, active=True):
|
||||||
|
response = self.client.get(
|
||||||
|
self.get_url(course_key=self.course.id),
|
||||||
|
{'excluded_course_roles': excluded_course_roles} if excluded_course_roles is not None else {}
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_usernames = [
|
||||||
|
self.student.username,
|
||||||
|
self.other_student.username,
|
||||||
|
self.program_student.username,
|
||||||
|
self.program_masters_student.username,
|
||||||
|
]
|
||||||
|
if not excluded_course_roles:
|
||||||
|
# Don't filter out any course roles
|
||||||
|
expected_usernames += list(role_user_usernames.values())
|
||||||
|
elif 'all' in excluded_course_roles:
|
||||||
|
# Filter out every course role
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
# Filter out some number of course roles
|
||||||
|
for role, username in role_user_usernames.items():
|
||||||
|
if role not in excluded_course_roles:
|
||||||
|
expected_usernames.append(username)
|
||||||
|
|
||||||
|
self._assert_usernames(response, expected_usernames)
|
||||||
|
|
||||||
|
@ddt.data(False, True)
|
||||||
|
def test_exclude_course_roles_for_another_course(self, other_student_role_in_self_course):
|
||||||
|
"""
|
||||||
|
Test for filtering errors when users have roles in other courses.
|
||||||
|
"""
|
||||||
|
# Conditionally make other_student a beta tester (arbitrary role) in self.course
|
||||||
|
if other_student_role_in_self_course:
|
||||||
|
CourseBetaTesterRole(self.course.id).add_users(self.other_student)
|
||||||
|
|
||||||
|
# Create another course, enroll other_student, and make other_student course staff in other course
|
||||||
|
another_course = CourseFactory.create(display_name='another-course', run='run-1')
|
||||||
|
CourseEnrollmentFactory(
|
||||||
|
course_id=another_course.id,
|
||||||
|
user=self.other_student,
|
||||||
|
)
|
||||||
|
CourseStaffRole(another_course.id).add_users(self.other_student)
|
||||||
|
|
||||||
|
# Query the gradebook view for self.course, excluding staff.
|
||||||
|
# other_student is staff in another-course, not self.course, so
|
||||||
|
# they should still be included.
|
||||||
|
self.login_staff()
|
||||||
|
with self._mock_all_course_grade_reads():
|
||||||
|
with override_waffle_flag(self.waffle_flag, active=True):
|
||||||
|
response = self.client.get(
|
||||||
|
self.get_url(course_key=self.course.id),
|
||||||
|
{'excluded_course_roles': CourseStaffRole.ROLE}
|
||||||
|
)
|
||||||
|
self._assert_usernames(
|
||||||
|
response,
|
||||||
|
[
|
||||||
|
self.student.username,
|
||||||
|
self.other_student.username,
|
||||||
|
self.program_student.username,
|
||||||
|
self.program_masters_student.username,
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@ddt.ddt
|
@ddt.ddt
|
||||||
class GradebookBulkUpdateViewTest(GradebookViewTestBase):
|
class GradebookBulkUpdateViewTest(GradebookViewTestBase):
|
||||||
|
|||||||
Reference in New Issue
Block a user