feat: [AXM-287,310,331] Change course progress calculation logic (#2553)

* 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

Author: KyryloKireiev

lms/djangoapps/course_api/blocks/tests/test_views.py

@@ -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
@@ -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.
         """
@@ -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
         """

lms/djangoapps/courseware/courses.py

@@ -5,7 +5,7 @@
 
 import logging
 from collections import defaultdict, namedtuple
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import six
 import pytz
@@ -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.
 
@@ -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(

lms/djangoapps/mobile_api/course_info/serializers.py

@@ -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
@@ -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
@@ -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
@@ -47,6 +49,7 @@ class Meta:
             'course_sharing_utm_parameters',
             'course_about',
             'course_modes',
+            'course_progress',
         )
 
     @staticmethod
@@ -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):
     """

lms/djangoapps/mobile_api/course_info/views.py

@@ -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**
 
@@ -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({

lms/djangoapps/mobile_api/tests/test_course_info_serializers.py

@@ -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)
@@ -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)

lms/djangoapps/mobile_api/users/serializers.py

@@ -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
@@ -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
@@ -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
@@ -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:
@@ -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):
@@ -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
@@ -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.
         """
@@ -302,7 +297,7 @@ class Meta:
             'certificate',
             'course_modes',
             'course_status',
-            'progress',
+            'course_progress',
             'course_assignments',
         )
         lookup_field = 'username'