Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [FC-0047] Extend mobile API with course progress and primary courses on dashboard view #34848

Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
348fdc3
feat: [AXM-24] Update structure for course enrollments API (#2515)
KyryloKireiev Mar 19, 2024
96f951b
feat: [AXM-47] Add course_status field to primary object (#2517)
KyryloKireiev Mar 22, 2024
8644c21
feat: [AXM-40] add courses progress to enrollment endpoint (#2519)
NiedielnitsevIvan Mar 22, 2024
c4db96e
feat: [AXM-53] add assertions for primary course (#2522)
NiedielnitsevIvan Apr 2, 2024
b348cb5
feat: [AXM-33] create enrollments filtering by course completion stat…
NiedielnitsevIvan Apr 8, 2024
b64a690
feat: [AXM-236] Add progress for other courses (#2536)
NiedielnitsevIvan Apr 10, 2024
aaa1b2d
fix: [AXM-277] Change _get_last_visited_block_path_and_unit_name meth…
KyryloKireiev Apr 16, 2024
9a9ce41
feat: [AXM-288] Change response to represent Future assignments (#2550)
KyryloKireiev Apr 29, 2024
8accb6d
feat: [AXM-297] Add progress to assignments in BlocksInfoInCourseView…
KyryloKireiev Apr 25, 2024
88d31ae
feat: [AXM-287,310,331] Change course progress calculation logic (#2553)
KyryloKireiev May 13, 2024
faa1fc4
refactor: [AXM-506] Move progress logic to core apps
KyryloKireiev Jun 21, 2024
0ff9f55
style: [AXM-506] Replace noqa to pylint disable
KyryloKireiev Jun 21, 2024
8fa5ef3
refactor: [AXM-506] Refactor get_course_assignments func
KyryloKireiev Jun 25, 2024
7aa27c2
style: [AXM-506] Remove unused import
KyryloKireiev Jun 25, 2024
7c53063
refactor: [AXM-506] Fix code review issues
KyryloKireiev Jul 1, 2024
eee5e7b
refactor: [AXM-506] Rename progress calculation functions
KyryloKireiev Jul 2, 2024
12a6866
refactor: [AXM-506] Refactor user's enrolments API
KyryloKireiev Jul 3, 2024
cfa816c
fix: [AXM-506] Try to fix pipelines
KyryloKireiev Jul 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions common/djangoapps/student/models/course_enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,71 @@ class UnenrollmentNotAllowed(CourseEnrollmentException):
pass


class CourseEnrollmentQuerySet(models.QuerySet):
"""
Custom queryset for CourseEnrollment with Table-level filter methods.
"""

def active(self):
"""
Returns a queryset of CourseEnrollment objects for courses that are currently active.
"""
return self.filter(is_active=True)

def without_certificates(self, user_username):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

username is enough.

"""
Returns a queryset of CourseEnrollment objects for courses that do not have a certificate.
"""
from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel
course_ids_with_certificates = GeneratedCertificate.objects.filter(
user__username=user_username
).values_list('course_id', flat=True)
return self.exclude(course_id__in=course_ids_with_certificates)

def with_certificates(self, user_username):
"""
Returns a queryset of CourseEnrollment objects for courses that have a certificate.
"""
from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel
course_ids_with_certificates = GeneratedCertificate.objects.filter(
user__username=user_username
).values_list('course_id', flat=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines are duplicated, can we write a method for user_course_ids_with_certificates?

return self.filter(course_id__in=course_ids_with_certificates)

def in_progress(self, user_username, time_zone=UTC):
"""
Returns a queryset of CourseEnrollment objects for courses that are currently in progress.
"""
now = datetime.now(time_zone)
return self.active().without_certificates(user_username).filter(
Q(course__start__lte=now, course__end__gte=now)
| Q(course__start__isnull=True, course__end__isnull=True)
| Q(course__start__isnull=True, course__end__gte=now)
| Q(course__start__lte=now, course__end__isnull=True),
)

def completed(self, user_username):
"""
Returns a queryset of CourseEnrollment objects for courses that have been completed.
"""
return self.active().with_certificates(user_username)

def expired(self, user_username, time_zone=UTC):
"""
Returns a queryset of CourseEnrollment objects for courses that have expired.
"""
now = datetime.now(time_zone)
return self.active().without_certificates(user_username).filter(course__end__lt=now)


class CourseEnrollmentManager(models.Manager):
"""
Custom manager for CourseEnrollment with Table-level filter methods.
"""

def get_queryset(self):
return CourseEnrollmentQuerySet(self.model, using=self._db)

def is_small_course(self, course_id):
"""
Returns false if the number of enrollments are one greater than 'max_enrollments' else true
Expand Down
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.courseware.courses.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.courseware.courses.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
108 changes: 105 additions & 3 deletions lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from crum import get_current_request
from dateutil.parser import parse as parse_date
from django.conf import settings
from django.core.cache import cache
from django.http import Http404, QueryDict
from django.urls import reverse
from django.utils.translation import gettext as _
Expand All @@ -35,6 +36,7 @@
)
from lms.djangoapps.courseware.access_utils import check_authentication, check_data_sharing_consent, check_enrollment, \
check_correct_active_enterprise_customer, is_priority_access_error
from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc
from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
from lms.djangoapps.courseware.date_summary import (
CertificateAvailableDate,
Expand All @@ -50,7 +52,9 @@
from lms.djangoapps.courseware.masquerade import check_content_start_date_for_masquerade_user
from lms.djangoapps.courseware.model_data import FieldDataCache
from lms.djangoapps.courseware.block_render import get_block
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.survey.utils import SurveyRequiredAccessError, check_survey_required_and_unanswered
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.enrollments.api import get_course_enrollment_details
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
Expand Down Expand Up @@ -587,7 +591,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,7 +611,8 @@ 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 due and graded:

if (due or include_without_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(
subsection_key, 'contains_gated_content', False)
Expand All @@ -624,7 +629,11 @@ def get_course_assignments(course_key, user, include_access=False): # lint-amne
else:
complete = False

past_due = not complete and due < now
if due:
past_due = not complete and due < now
else:
past_due = False
due = None
assignments.append(_Assignment(
subsection_key, title, url, due, contains_gated_content,
complete, past_due, assignment_type, None, first_component_block_id
Expand Down Expand Up @@ -764,6 +773,38 @@ def _ora_assessment_to_assignment(
)


def get_assignments_grades(user, course_id, cache_timeout):
"""
Calculate the progress of the assignment for the user in the course.

Arguments:
user (User): Django User object.
course_id (CourseLocator): The course key.
cache_timeout (int): Cache timeout in seconds
Returns:
list (ReadSubsectionGrade, ZeroSubsectionGrade): The list with assignments grades.
"""
is_staff = bool(has_access(user, 'staff', course_id))

try:
cache_key = f'course_block_structure_{str(course_id)}_{user.id}'
Copy link
Member

@GlugovGrGlib GlugovGrGlib Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should use course_version attribute to make sure that cache is invalidated with each new published course version
Please see https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/course_home_api/outline/views.py#L456 for similar caching approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, added cache invalidation with course version

collected_block_structure = cache.get(cache_key)
if not collected_block_structure:
collected_block_structure = get_block_structure_manager(course_id).get_collected()
cache.set(cache_key, collected_block_structure, cache_timeout)

course_grade = CourseGradeFactory().read(user, collected_block_structure=collected_block_structure)

# 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())
except Exception as err: # pylint: disable=broad-except
log.warning(f'Could not get grades for the course: {course_id}, error: {err}')
return []

return subsection_grades


def get_first_component_of_block(block_key, block_data):
"""
This function returns the first leaf block of a section(block_key)
Expand Down Expand Up @@ -1019,3 +1060,64 @@ def get_course_chapter_ids(course_key):
log.exception('Failed to retrieve course from modulestore.')
return []
return [str(chapter_key) for chapter_key in chapter_keys if chapter_key.block_type == 'chapter']


def get_past_and_future_course_assignments(request, user, course):
"""
Returns the future assignment data and past assignments data for given user and course.

Arguments:
request (Request): The HTTP GET request.
user (User): The user for whom the assignments are received.
course (Course): Course object for whom the assignments are received.
Returns:
tuple (list, list): Tuple of `past_assignments` list and `next_assignments` list.
`next_assignments` list contains only uncompleted assignments.
"""
assignments = get_course_assignment_date_blocks(course, user, request, include_past_dates=True)
past_assignments = []
future_assignments = []

timezone = get_user_timezone_or_last_seen_timezone_or_utc(user)
for assignment in sorted(assignments, key=lambda x: x.date):
if assignment.date < datetime.now(timezone):
past_assignments.append(assignment)
else:
if not assignment.complete:
future_assignments.append(assignment)

if future_assignments:
future_assignment_date = future_assignments[0].date.date()
next_assignments = [
assignment for assignment in future_assignments if assignment.date.date() == future_assignment_date
]
else:
next_assignments = []

return next_assignments, past_assignments


def get_assignments_completions(course_key, user):
"""
Calculate the progress of the user in the course by assignments.

Arguments:
course_key (CourseLocator): The Course for which course progress is requested.
user (User): The user for whom course progress is requested.
Returns:
dict (dict): Dictionary contains information about total assignments count
in the given course and how many assignments the user has completed.
"""
course_assignments = get_course_assignments(course_key, 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,
}
5 changes: 5 additions & 0 deletions lms/djangoapps/mobile_api/course_info/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""
Common constants for the `course_info` API.
"""

BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour
11 changes: 10 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_assignments_completions
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,12 @@ def get_course_modes(self, course_overview):
for mode in course_modes
]

def get_course_progress(self, obj: CourseOverview) -> Dict[str, int]:
"""
Gets course progress calculated by course assignments.
"""
return get_assignments_completions(obj.id, self.context.get('user'))


class MobileCourseEnrollmentSerializer(serializers.ModelSerializer):
"""
Expand Down
Loading
Loading