feat: [AXM-287,310,331] Change course progress calculation logic

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

@@ -5,7 +5,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
 
 from completion.test_utils import CompletionWaffleTestMixin, submit_completions_for_testing
@@ -207,8 +207,11 @@ 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.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.
         """
@@ -366,7 +369,10 @@ 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_assignment_date_blocks', 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/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,9 +13,11 @@
 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.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):
@@ -31,6 +33,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,8 +50,13 @@ class Meta:
             'course_sharing_utm_parameters',
             'course_about',
             'course_modes',
+            'course_progress',
         )
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.course = modulestore().get_course(self.instance.id)
+
     @staticmethod
     def get_media(obj):
         """
@@ -75,6 +83,29 @@ def get_course_modes(self, course_overview):
             for mode in course_modes
         ]
 
+    def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]:  # noqa: F821
+        """
+        Gets course progress calculated by course assignments.
+        """
+        course_assignments = get_course_assignment_date_blocks(
+            self.course,
+            self.context.get('user'),
+            self.context.get('request'),
+            include_past_dates=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,8 @@ def list(self, request, **kwargs):  # pylint: disable=W0221
                 )
 
                 course_info_context = {
-                    'user': requested_user
+                    'user': requested_user,
+                    'request': request,
                 }
                 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_assignment_date_blocks', 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,59 @@ 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_assignment_date_blocks', 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_assignment_date_blocks', 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=[])
+    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
+
+        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)
+
+    @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignment_date_blocks')
+    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
+
+        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)

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
@@ -21,8 +20,7 @@
 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.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,41 @@ 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:
+            course = modulestore().get_course(instance.course.id)
+            data['course_progress'] = self.calculate_progress(instance, course)
 
         return data
 
-    def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]:
+    def calculate_progress(
+        self, model: CourseEnrollment, course: 'CourseBlockWithMixins'  # noqa: F821
+    ) -> 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_assignment_date_blocks(
+            course,
+            model.user,
+            self.context.get('request'),
+            include_past_dates=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 +198,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 +212,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,11 +250,11 @@ 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.
         """
-        return self.calculate_progress(model)
+        return self.calculate_progress(model, self.course)
 
     def get_course_assignments(self, model: CourseEnrollment) -> Dict[str, Optional[List[Dict[str, str]]]]:
         """
@@ -302,7 +301,7 @@ class Meta:
             'certificate',
             'course_modes',
             'course_status',
-            'progress',
+            'course_progress',
             'course_assignments',
         )
         lookup_field = 'username'