Skip to content

Commit

Permalink
fix: enrollment api to remove exam scores for multiple courses
Browse files Browse the repository at this point in the history
  • Loading branch information
tehreem-sadat committed Dec 24, 2024
1 parent a28717f commit 256272a
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 35 deletions.
60 changes: 30 additions & 30 deletions futurex_openedx_extensions/dashboard/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,27 +266,39 @@ def __init__(self, *args: Any, **kwargs: Any):
self._grading_info: Dict[str, Any] = {}
self._subsection_locations: Dict[str, Any] = {}

def collect_grading_info(self, course_ids: list) -> None:
"""Collect the grading info."""
# exam_scores will only be included if there is only one course
if self.context.get('course_id'):
self.collect_grading_info()
else:
tags = self.context.get('requested_optional_field_tags')
if tags:
self.context['requested_optional_field_tags'] = list(set(tags) - {'exam_scores'})

def collect_grading_info(self) -> None:
"""
Collect the grading info.
"""
course_id = CourseLocator.from_string(self.context.get('course_id'))
self._grading_info = {}
self._subsection_locations = {}
index = 0
if not self.is_optional_field_requested('exam_scores'):
return
for course_id in course_ids:
grading_context = grading_context_for_course(get_course_by_id(course_id))
for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].items():
for subsection_index, subsection_info in enumerate(subsection_infos, start=1):
header_enum = f' {subsection_index}' if len(subsection_infos) > 1 else ''
header_name = f'{assignment_type_name}{header_enum}'
if self.is_exam_name_in_header:
header_name += f': {subsection_info["subsection_block"].display_name}'
self._grading_info[str(index)] = {
'header_name': header_name,
'location': str(subsection_info['subsection_block'].location),
}
self._subsection_locations[str(subsection_info['subsection_block'].location)] = str(index)
index += 1

grading_context = grading_context_for_course(get_course_by_id(course_id))
index = 0
for assignment_type_name, subsection_infos in grading_context['all_graded_subsections_by_type'].items():
for subsection_index, subsection_info in enumerate(subsection_infos, start=1):
header_enum = f' {subsection_index}' if len(subsection_infos) > 1 else ''
header_name = f'{assignment_type_name}{header_enum}'
if self.is_exam_name_in_header:
header_name += f': {subsection_info["subsection_block"].display_name}'

self._grading_info[str(index)] = {
'header_name': header_name,
'location': str(subsection_info['subsection_block'].location),
}
self._subsection_locations[str(subsection_info['subsection_block'].location)] = str(index)
index += 1

@property
def is_exam_name_in_header(self) -> bool:
Expand Down Expand Up @@ -387,15 +399,9 @@ class Meta:
model = get_user_model()
fields = LearnerBasicDetailsSerializer.Meta.fields + CourseScoreAndCertificateSerializer.Meta.fields

def __init__(self, *args: Any, **kwargs: Any):
"""Initialize the serializer."""
super().__init__(*args, **kwargs)
self._course_id = CourseLocator.from_string(self.context.get('course_id'))
self.collect_grading_info([self._course_id])

def _get_course_id(self, obj: Any = None) -> CourseLocator:
"""Get the course ID. Its helper method required for CourseScoreAndCertificateSerializer"""
return self._course_id
return CourseLocator.from_string(self.context.get('course_id'))

def _get_user(self, obj: Any = None) -> get_user_model:
"""Get the User. Its helper method required for CourseScoreAndCertificateSerializer"""
Expand All @@ -416,12 +422,6 @@ class Meta:
['course_id']
)

def __init__(self, *args: Any, **kwargs: Any):
"""Initialize the serializer."""
super().__init__(*args, **kwargs)
course_ids = self.context.get('course_ids')
self.collect_grading_info(course_ids)

def _get_course_id(self, obj: Any = None) -> CourseLocator | None:
"""Get the course ID. Its helper method required for CourseScoreAndCertificateSerializer"""
return obj.course_id if obj else None
Expand Down
9 changes: 7 additions & 2 deletions futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ class LearnersEnrollmentView(ExportCSVMixin, FXViewRoleInfoMixin, ListAPIView):
fx_view_name = 'learners_enrollment_details'
fx_default_read_only_roles = ['staff', 'instructor', 'data_researcher', 'org_course_creator_group']
fx_view_description = 'api/fx/learners/v1/enrollments: Get the list of enrollments'
is_single_course_requested = False

def get_queryset(self, *args: Any, **kwargs: Any) -> QuerySet:
"""Get the list of learners for a course"""
Expand All @@ -506,6 +507,9 @@ def get_queryset(self, *args: Any, **kwargs: Any) -> QuerySet:
username.strip() for username in usernames.split(',')
] if usernames else None

if course_ids_list and len(course_ids_list) == 1:
self.is_single_course_requested = True

return get_learners_enrollments_queryset(
fx_permission_info=self.request.fx_permission_info,
user_ids=user_ids_list,
Expand All @@ -518,8 +522,9 @@ def get_queryset(self, *args: Any, **kwargs: Any) -> QuerySet:
def get_serializer_context(self) -> Dict[str, Any]:
"""Get the serializer context"""
context = super().get_serializer_context()
context['course_ids'] = [course_enrollment.course_id for course_enrollment in self.get_queryset()]
context['omit_subsection_name'] = self.request.query_params.get('omit_subsection_name', '0')
if self.is_single_course_requested:
context['course_id'] = str(self.get_queryset().first().course_id)
context['omit_subsection_name'] = self.request.query_params.get('omit_subsection_name', '0')
return context


Expand Down
64 changes: 61 additions & 3 deletions tests/test_dashboard/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,14 @@ class Meta:
@pytest.mark.django_db
@patch('futurex_openedx_extensions.dashboard.serializers.CourseScoreAndCertificateSerializer.collect_grading_info')
def test_learner_enrollments_serializer(mock_collect, base_data,): # pylint: disable=unused-argument
"""Verify that the LearnerDetailsForCourseSerializer returns the needed fields."""
"""Verify that the LearnerEnrollmentSerializer returns the needed fields."""
queryset = CourseEnrollment.objects.filter(user_id=10, course_id='course-v1:ORG3+1+1').annotate(
certificate_available=Value(True),
course_score=Value(0.67),
active_in_course=Value(True),
)
serializer = LearnerEnrollmentSerializer(queryset, context={
'course_ids': ['course-v1:ORG3+1+1']
'course_id': 'course-v1:ORG3+1+1'
}, many=True)
mock_collect.assert_called_once()
data = serializer.data
Expand All @@ -301,6 +301,64 @@ def test_learner_enrollments_serializer(mock_collect, base_data,): # pylint: di
assert data[0]['active_in_course'] is True


@pytest.mark.django_db
@patch('futurex_openedx_extensions.dashboard.serializers.get_course_blocks_completion_summary')
@pytest.mark.parametrize('is_course_id_in_context, optional_field_tags', [
(True, ['exam_scores']),
(True, ['__all__']),
(True, ['csv_export']),
(False, ['exam_scores']),
(False, ['__all__']),
(False, ['csv_export']),
])
def test_learner_enrollment_serializer_exam_scores(
mock_get_completion, is_course_id_in_context, optional_field_tags, grading_context, base_data,
): # pylint: disable=unused-argument, redefined-outer-name
"""
Verify that the LearnerEnrollmentSerializer returns exam_scores
only when course_id is set in serializer_context
"""
queryset = CourseEnrollment.objects.filter(course_id='course-v1:ORG2+1+1').annotate(
certificate_available=Value(True),
course_score=Value(0.67),
active_in_course=Value(True),
)
PersistentSubsectionGrade.objects.create(
user_id=queryset[0].user.id,
course_id='course-v1:ORG2+1+1',
usage_key='block-v1:ORG2+1+1+type@homework+block@1',
earned_graded=0,
possible_graded=0,
earned_all=77.0,
possible_all=88.0,
)
PersistentSubsectionGrade.objects.create(
user_id=queryset[0].user.id,
course_id='course-v1:ORG2+1+1',
usage_key='block-v1:ORG2+1+1+type@homework+block@2',
earned_graded=0,
possible_graded=0,
earned_all=9.0,
possible_all=10.0,
first_attempted=now() - timedelta(days=1),
)
mock_get_completion.return_value = {'complete_count': 2, 'incomplete_count': 1, 'locked_count': 1}
context = {'requested_optional_field_tags': optional_field_tags}

if is_course_id_in_context:
context.update({'course_id': 'course-v1:ORG2+1+1'})

serializer = LearnerEnrollmentSerializer(queryset[0], context=context)
data = serializer.data

if is_course_id_in_context:
assert 'earned - Homework 1: First Homework' in data.keys()
assert 'earned - Homework 2: Second Homework' in data.keys()
else:
assert 'earned - Homework 1: First Homework' not in data.keys()
assert 'earned - Homework 2: Second Homework' not in data.keys()


def test_learner_details_for_course_serializer_optional_fields():
"""Verify that the LearnerDetailsForCourseSerializer returns the correct optional fields."""
serializer = LearnerDetailsForCourseSerializer(context={'course_id': 'course-v1:ORG2+1+1'})
Expand Down Expand Up @@ -367,7 +425,7 @@ def test_learner_details_for_course_serializer_collect_grading_info_not_used(

assert not serializer.grading_info
assert not serializer.subsection_locations
serializer.collect_grading_info(['course-v1:ORG2+1+1'])
serializer.collect_grading_info()
assert not serializer.grading_info
assert not serializer.subsection_locations

Expand Down

0 comments on commit 256272a

Please sign in to comment.