Skip to content

Commit

Permalink
feat: [AXM-287,310,331] Change course progress calculation logic (#2553)
Browse files Browse the repository at this point in the history
* feat: [AXM-287,310,331] Change course progress calculation logic

* style: [AXM-287,310,331] Remove commented code

* fix: [AXM-287,310,331] Change course assignments gather logic
  • Loading branch information
KyryloKireiev committed Jun 21, 2024
1 parent 24f63cc commit cdd2d7e
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 58 deletions.
8 changes: 5 additions & 3 deletions lms/djangoapps/course_api/blocks/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""
from datetime import datetime
from unittest import mock
from unittest.mock import Mock
from unittest.mock import MagicMock, Mock
from urllib.parse import urlencode, urlunparse

import ddt
Expand Down Expand Up @@ -209,8 +209,9 @@ 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_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):
def test_not_authenticated_public_course_with_blank_username(self, get_course_assignment_mock: MagicMock) -> None:
"""
Verify behaviour when accessing course blocks of a public course for anonymous user anonymously.
"""
Expand Down Expand Up @@ -368,7 +369,8 @@ def test_extra_field_when_not_requested(self):
block_data['type'] == 'course'
)

def test_data_researcher_access(self):
@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
27 changes: 26 additions & 1 deletion lms/djangoapps/mobile_api/course_info/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Course Info serializers
"""
from rest_framework import serializers
from typing import Union
from typing import Dict, Union

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
Expand All @@ -13,6 +13,7 @@
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_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
Expand All @@ -31,6 +32,7 @@ class CourseInfoOverviewSerializer(serializers.ModelSerializer):
course_sharing_utm_parameters = serializers.SerializerMethodField()
course_about = serializers.SerializerMethodField('get_course_about_url')
course_modes = serializers.SerializerMethodField()
course_progress = serializers.SerializerMethodField()

class Meta:
model = CourseOverview
Expand All @@ -47,6 +49,7 @@ class Meta:
'course_sharing_utm_parameters',
'course_about',
'course_modes',
'course_progress',
)

@staticmethod
Expand Down Expand Up @@ -75,6 +78,28 @@ def get_course_modes(self, course_overview):
for mode in course_modes
]

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

total_assignments_count = 0
assignments_completed = 0

if course_assignments:
total_assignments_count = len(course_assignments)
assignments_completed = len([assignment for assignment in course_assignments if assignment.complete])

return {
'total_assignments_count': total_assignments_count,
'assignments_completed': assignments_completed,
}


class MobileCourseEnrollmentSerializer(serializers.ModelSerializer):
"""
Expand Down
7 changes: 6 additions & 1 deletion lms/djangoapps/mobile_api/course_info/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ class BlocksInfoInCourseView(BlocksInCourseView):
course, chapter, sequential, vertical, html, problem, video, and
discussion.
display_name: (str) The display name of the block.
course_progress: (dict) Contains information about how many assignments are in the course
and how many assignments the student has completed.
Included here:
* total_assignments_count: (int) Total course's assignments count.
* assignments_completed: (int) Assignments witch the student has completed.
**Returns**
Expand Down Expand Up @@ -366,7 +371,7 @@ def list(self, request, **kwargs): # pylint: disable=W0221
)

course_info_context = {
'user': requested_user
'user': requested_user,
}
user_enrollment = CourseEnrollment.get_enrollment(user=requested_user, course_key=course_key)
course_data.update({
Expand Down
37 changes: 34 additions & 3 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,8 @@ def setUp(self):
self.user = UserFactory()
self.course_overview = CourseOverviewFactory()

def test_get_media(self):
@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

self.assertIn('media', output_data)
Expand All @@ -156,16 +157,46 @@ def test_get_media(self):
self.assertIn('small', output_data['media']['image'])
self.assertIn('large', output_data['media']['image'])

@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, mock_get_link_for_about_page: MagicMock) -> None:
def test_get_course_sharing_utm_parameters(
self,
mock_get_link_for_about_page: MagicMock,
get_course_assignment_mock: MagicMock,
) -> None:
output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data

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)

def test_get_course_modes(self):
@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_assignments', return_value=[])
def test_get_course_progress_no_assignments(self, get_course_assignment_mock: MagicMock) -> None:
expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0}

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(self.course_overview.id, self.user, include_without_due=True)

@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:
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}).data

self.assertIn('course_progress', output_data)
self.assertDictEqual(output_data['course_progress'], expected_course_progress)
get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True)
49 changes: 22 additions & 27 deletions lms/djangoapps/mobile_api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from datetime import datetime
from typing import Dict, List, Optional, Tuple, Union

from django.core.cache import cache
from completion.exceptions import UnavailableCompletionData
from completion.utilities import get_key_to_last_completed_block
from opaque_keys import InvalidKeyError
Expand All @@ -19,10 +18,9 @@
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.grades.api import CourseGradeFactory
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
from lms.djangoapps.mobile_api.utils import API_V4
from openedx.features.course_duration_limits.access import get_user_course_expiration_date
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
Expand Down Expand Up @@ -107,8 +105,6 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer):
audit_access_expires = serializers.SerializerMethodField()
course_modes = serializers.SerializerMethodField()

BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour

def get_audit_access_expires(self, model):
"""
Returns expiration date for a course audit expiration, if any or null
Expand Down Expand Up @@ -140,38 +136,37 @@ def get_course_modes(self, obj):
for mode in course_modes
]

def to_representation(self, instance):
def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # noqa: F821
"""
Override the to_representation method to add the course_status field to the serialized data.
"""
data = super().to_representation(instance)
if 'progress' in self.context.get('requested_fields', []):
data['progress'] = self.calculate_progress(instance)

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

return data

def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]:
"""
Calculate the progress of the user in the course.
:param model:
:return:
"""
is_staff = bool(has_access(model.user, 'staff', model.course.id))
course_assignments = get_course_assignments(
model.course_id,
model.user,
include_without_due=True,
)

cache_key = f'course_block_structure_{str(model.course.id)}_{model.user.id}'
collected_block_structure = cache.get(cache_key)
if not collected_block_structure:
collected_block_structure = get_block_structure_manager(model.course.id).get_collected()
cache.set(cache_key, collected_block_structure, self.BLOCK_STRUCTURE_CACHE_TIMEOUT)
total_assignments_count = 0
assignments_completed = 0

course_grade = CourseGradeFactory().read(model.user, collected_block_structure=collected_block_structure)
if course_assignments:
total_assignments_count = len(course_assignments)
assignments_completed = len([assignment for assignment in course_assignments if assignment.complete])

# recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not)
course_grade.update(visible_grades_only=True, has_staff_access=is_staff)
subsection_grades = list(course_grade.subsection_grades.values())
return {
'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, subsection_grades)),
'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)),
'total_assignments_count': total_assignments_count,
'assignments_completed': assignments_completed,
}

class Meta:
Expand Down Expand Up @@ -199,7 +194,7 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer):
"""

course_status = serializers.SerializerMethodField()
progress = serializers.SerializerMethodField()
course_progress = serializers.SerializerMethodField()
course_assignments = serializers.SerializerMethodField()

def __init__(self, *args, **kwargs):
Expand All @@ -213,7 +208,7 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[
try:
block_id = str(get_key_to_last_completed_block(model.user, model.course.id))
except UnavailableCompletionData:
block_id = ""
block_id = ''

if not block_id:
return None
Expand Down Expand Up @@ -251,7 +246,7 @@ def _get_last_visited_block_path_and_unit_name(

return path, vertical.display_name

def get_progress(self, model: CourseEnrollment) -> Dict[str, int]:
def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]:
"""
Returns the progress of the user in the course.
"""
Expand Down Expand Up @@ -302,7 +297,7 @@ class Meta:
'certificate',
'course_modes',
'course_status',
'progress',
'course_progress',
'course_assignments',
)
lookup_field = 'username'
Expand Down
Loading

0 comments on commit cdd2d7e

Please sign in to comment.