Skip to content

Commit

Permalink
fix: [AXM-287,310,331] Change course assignments gather logic
Browse files Browse the repository at this point in the history
  • Loading branch information
KyryloKireiev committed May 10, 2024
1 parent 938e832 commit 9b02f1d
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 58 deletions.
8 changes: 2 additions & 6 deletions lms/djangoapps/course_api/blocks/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,7 @@ def test_not_authenticated_public_course_with_all_blocks(self):
self.query_params['all_blocks'] = True
self.verify_response(403)

@mock.patch(
'lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[]
)
@mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
@mock.patch("lms.djangoapps.course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True))
def test_not_authenticated_public_course_with_blank_username(self, get_course_assignment_mock: MagicMock) -> None:
"""
Expand Down Expand Up @@ -369,9 +367,7 @@ def test_extra_field_when_not_requested(self):
block_data['type'] == 'course'
)

@mock.patch(
'lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[]
)
@mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
def test_data_researcher_access(self, get_course_assignment_mock: MagicMock) -> None:
"""
Test if data researcher has access to the api endpoint
Expand Down
8 changes: 6 additions & 2 deletions lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import logging
from collections import defaultdict, namedtuple
from datetime import datetime
from datetime import datetime, timedelta

import six
import pytz
Expand Down Expand Up @@ -587,7 +587,7 @@ def get_course_blocks_completion_summary(course_key, user):


@request_cached()
def get_course_assignments(course_key, user, include_access=False): # lint-amnesty, pylint: disable=too-many-statements
def get_course_assignments(course_key, user, include_access=False, include_without_due=False,): # lint-amnesty, pylint: disable=too-many-statements
"""
Returns a list of assignment (at the subsection/sequential level) due dates for the given course.
Expand All @@ -607,6 +607,10 @@ def get_course_assignments(course_key, user, include_access=False): # lint-amne
for subsection_key in block_data.get_children(section_key):
due = block_data.get_xblock_field(subsection_key, 'due')
graded = block_data.get_xblock_field(subsection_key, 'graded', False)

if not due and include_without_due:
due = now + timedelta(days=1000)

if due and graded:
first_component_block_id = get_first_component_of_block(subsection_key, block_data)
contains_gated_content = include_access and block_data.get_xblock_field(
Expand Down
16 changes: 5 additions & 11 deletions lms/djangoapps/mobile_api/course_info/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user
from lms.djangoapps.courseware.access_utils import check_course_open_for_learner
from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks
from lms.djangoapps.courseware.courses import get_course_assignments
from lms.djangoapps.mobile_api.users.serializers import ModeSerializer
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.features.course_duration_limits.access import get_user_course_expiration_date
from xmodule.modulestore.django import modulestore


class CourseInfoOverviewSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -53,10 +52,6 @@ class Meta:
'course_progress',
)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.course = modulestore().get_course(self.instance.id)

@staticmethod
def get_media(obj):
"""
Expand All @@ -83,15 +78,14 @@ def get_course_modes(self, course_overview):
for mode in course_modes
]

def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821
def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821 #here
"""
Gets course progress calculated by course assignments.
"""
course_assignments = get_course_assignment_date_blocks(
self.course,
course_assignments = get_course_assignments(
obj.id,
self.context.get('user'),
self.context.get('request'),
include_past_dates=True
include_without_due=True,
)

total_assignments_count = 0
Expand Down
1 change: 0 additions & 1 deletion lms/djangoapps/mobile_api/course_info/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ def list(self, request, **kwargs): # pylint: disable=W0221

course_info_context = {
'user': requested_user,
'request': request,
}
user_enrollment = CourseEnrollment.get_enrollment(user=requested_user, course_key=course_key)
course_data.update({
Expand Down
31 changes: 9 additions & 22 deletions lms/djangoapps/mobile_api/tests/test_course_info_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def setUp(self):
self.user = UserFactory()
self.course_overview = CourseOverviewFactory()

@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[])
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
def test_get_media(self, get_course_assignment_mock: MagicMock) -> None:
output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data

Expand All @@ -157,7 +157,7 @@ def test_get_media(self, get_course_assignment_mock: MagicMock) -> None:
self.assertIn('small', output_data['media']['image'])
self.assertIn('large', output_data['media']['image'])

@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[])
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', return_value='mock_about_link')
def test_get_course_sharing_utm_parameters(
self,
Expand All @@ -169,47 +169,34 @@ def test_get_course_sharing_utm_parameters(
self.assertEqual(output_data['course_about'], mock_get_link_for_about_page.return_value)
mock_get_link_for_about_page.assert_called_once_with(self.course_overview)

@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[])
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
def test_get_course_modes(self, get_course_assignment_mock: MagicMock) -> None:
expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}]

output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data

self.assertListEqual(output_data['course_modes'], expected_course_modes)

@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks', return_value=[])
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[])
def test_get_course_progress_no_assignments(self, get_course_assignment_mock: MagicMock) -> None:
request_mock = Mock()
expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0}

output_data = CourseInfoOverviewSerializer(
self.course_overview, context={
'user': self.user,
'request': request_mock,
}
).data
output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data

self.assertIn('course_progress', output_data)
self.assertDictEqual(output_data['course_progress'], expected_course_progress)
get_course_assignment_mock.assert_called_once_with(None, self.user, request_mock, include_past_dates=True)
get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True)

@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks')
@patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments')
def test_get_course_progress_with_assignments(self, get_course_assignment_mock: MagicMock) -> None:
request_mock = Mock()
assignments_mock = [
Mock(complete=False), Mock(complete=False), Mock(complete=True), Mock(complete=True), Mock(complete=True)
]
get_course_assignment_mock.return_value = assignments_mock

expected_course_progress = {'total_assignments_count': 5, 'assignments_completed': 3}

output_data = CourseInfoOverviewSerializer(
self.course_overview, context={
'user': self.user,
'request': request_mock,
}
).data
output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data

self.assertIn('course_progress', output_data)
self.assertDictEqual(output_data['course_progress'], expected_course_progress)
get_course_assignment_mock.assert_called_once_with(None, self.user, request_mock, include_past_dates=True)
get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True)
18 changes: 7 additions & 11 deletions lms/djangoapps/mobile_api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from lms.djangoapps.certificates.api import certificate_downloadable_status
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc
from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks
from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks, get_course_assignments
from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer
from lms.djangoapps.mobile_api.utils import API_V4
from openedx.features.course_duration_limits.access import get_user_course_expiration_date
Expand Down Expand Up @@ -143,22 +143,18 @@ def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # noq
data = super().to_representation(instance)

if 'course_progress' in self.context.get('requested_fields', []) and self.context.get('api_version') == API_V4:
course = modulestore().get_course(instance.course.id)
data['course_progress'] = self.calculate_progress(instance, course)
data['course_progress'] = self.calculate_progress(instance)

return data

def calculate_progress(
self, model: CourseEnrollment, course: 'CourseBlockWithMixins' # noqa: F821
) -> Dict[str, int]:
def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]:
"""
Calculate the progress of the user in the course.
"""
course_assignments = get_course_assignment_date_blocks(
course,
course_assignments = get_course_assignments(
model.course_id,
model.user,
self.context.get('request'),
include_past_dates=True
include_without_due=True,
)

total_assignments_count = 0
Expand Down Expand Up @@ -254,7 +250,7 @@ def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]:
"""
Returns the progress of the user in the course.
"""
return self.calculate_progress(model, self.course)
return self.calculate_progress(model)

def get_course_assignments(self, model: CourseEnrollment) -> Dict[str, Optional[List[Dict[str, str]]]]:
"""
Expand Down
10 changes: 5 additions & 5 deletions lms/djangoapps/mobile_api/users/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ def test_response_contains_primary_enrollment_assignments_info(self):
self.assertListEqual(response.data['primary']['course_assignments']['past_assignments'], [])
self.assertListEqual(response.data['primary']['course_assignments']['future_assignments'], [])

@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignment_date_blocks', return_value=[])
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments', return_value=[])
def test_course_progress_in_primary_enrollment_with_no_assignments(
self,
get_course_assignment_mock: MagicMock,
Expand All @@ -927,7 +927,7 @@ def test_course_progress_in_primary_enrollment_with_no_assignments(
'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary'
'.get_course_assignments'
)
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignment_date_blocks')
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments')
def test_course_progress_in_primary_enrollment_with_assignments(
self,
get_course_assignment_mock: MagicMock,
Expand All @@ -953,7 +953,7 @@ def test_course_progress_in_primary_enrollment_with_assignments(
self.assertIn('course_progress', response.data['primary'])
self.assertDictEqual(response.data['primary']['course_progress'], expected_course_progress)

@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignment_date_blocks')
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments')
def test_course_progress_for_secondary_enrollments_no_query_param(
self,
get_course_assignment_mock: MagicMock,
Expand All @@ -969,7 +969,7 @@ def test_course_progress_for_secondary_enrollments_no_query_param(
for enrollment in response.data['enrollments']['results']:
self.assertNotIn('course_progress', enrollment)

@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignment_date_blocks')
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments')
def test_course_progress_for_secondary_enrollments_with_query_param(
self,
get_course_assignment_mock: MagicMock,
Expand All @@ -991,7 +991,7 @@ def test_course_progress_for_secondary_enrollments_with_query_param(
'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary'
'.get_course_assignments'
)
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignment_date_blocks')
@patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments')
def test_course_progress_for_secondary_enrollments_with_query_param_and_assignments(
self,
get_course_assignment_mock: MagicMock,
Expand Down
4 changes: 4 additions & 0 deletions lms/djangoapps/mobile_api/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ class UserCourseEnrollmentsList(generics.ListAPIView):
* mode: The type of certificate registration for this course (honor or
certified).
* url: URL to the downloadable version of the certificate, if exists.
* course_progress: Contains information about how many assignments are in the course
and how many assignments the student has completed.
* total_assignments_count: Total course's assignments count.
* assignments_completed: Assignments witch the student has completed.
"""

lookup_field = 'username'
Expand Down

0 comments on commit 9b02f1d

Please sign in to comment.