Skip to content
133 changes: 121 additions & 12 deletions edx_when/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from django.core.exceptions import ValidationError
from django.db import transaction
from django.db.models import DateTimeField, ExpressionWrapper, F, ObjectDoesNotExist, Q
from django.db.models import DateTimeField, ExpressionWrapper, F, ObjectDoesNotExist, Prefetch, Q
from edx_django_utils.cache.utils import TieredCache
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
Expand Down Expand Up @@ -137,6 +137,41 @@ def _get_end_dates_from_content_dates(qset):
return end_datetime, cutoff_datetime


def _resolve_policy_dates(
content_dates, schedule=None, end_datetime=None, cutoff_datetime=None, course_id=None
):
"""
Resolve ContentDate objects to their policy-derived datetimes.

Passes schedule/end/cutoff so relative dates (self-paced courses) are included.
Silently skips entries that require a schedule when none is available.

Arguments:
content_dates: iterable of ContentDate objects to resolve.
schedule: Schedule obj (optional), used for relative date calculations.
end_datetime: course end datetime (optional), caps relative dates.
cutoff_datetime: cutoff datetime (optional) for self-paced late starters.
course_id: CourseKey (optional). When provided, each location is normalized
with ``location.map_into_course(course_id)``.

Returns:
Tuple ``(dates, policies)`` where ``dates`` maps ``(location, field)`` tuples to
resolved datetimes and ``policies`` maps ``content_date.id`` to the same key
(used to apply user overrides).
"""
dates = {}
policies = {}
for cdate in content_dates:
location = cdate.location.map_into_course(course_id) if course_id else cdate.location
key = (location, cdate.field)
try:
dates[key] = cdate.policy.actual_date(schedule, end_datetime, cutoff_datetime)
except models.MissingScheduleError:
pass
Comment on lines +169 to +170

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kyrylo-kh these are the lines to be tested, this will increase the coverage and help to test the code better.

policies[cdate.id] = key
return dates, policies


def _processed_results_cache_key(
course_id, user_id, schedule, allow_relative_dates,
subsection_and_higher_only, published_version
Expand Down Expand Up @@ -241,19 +276,11 @@ def get_dates_for_course(
)
TieredCache.set_all_tiers(raw_results_cache_key, qset)

dates = {}
policies = {}
end_datetime, cutoff_datetime = _get_end_dates_from_content_dates(qset)

for cdate in qset:
key = (cdate.location.map_into_course(course_id), cdate.field)
try:
dates[key] = cdate.policy.actual_date(schedule, end_datetime, cutoff_datetime)
except models.MissingScheduleError:
# We had a relative date but no schedule. This is permissible in some cases (staff users viewing a course
# they are not enrolled in, for example). Just let it go by.
pass
policies[cdate.id] = key
dates, policies = _resolve_policy_dates(
qset, schedule, end_datetime, cutoff_datetime, course_id=course_id
)

if user_id:
for userdate in models.UserDate.objects.filter(
Expand Down Expand Up @@ -548,6 +575,88 @@ def get_schedules_with_due_date(course_id, assignment_date):
return schedules


def get_user_dates(course_id, user_id, block_types=None, block_keys=None, date_types=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm trying to grok how this function and [get_dates_for_course()](https://github.com/openedx/edx-when/blob/ab2ee71af64ee6ef720083586eb26e1f58a748d9/edx_when/api.py#L163) relate. Given that there is some repeated code, is there a cleaner refactoring that's possible?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

get_user_dates is a narrow, filterable API for one user. get_dates_for_cours is the full path with caching, subsection filtering, and relative dates. The shared part is that loop that turns content dates into policy dates and applies user overrides. A cleaner approach would be to extract that into a small helper and have both call it (and pass schedule/end/cutoff from the helper so self-paced works in both). Would you like to have such helper function in this PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that makes sense.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

"""
Get all user dates for a course with optional filters.

Arguments:
course_id: either a CourseKey or string representation of same
user_id: User ID
block_types: optional list of block types to filter by (e.g., ['sequential', 'vertical'])
block_keys: optional list of UsageKey objects or strings to filter by
date_types: optional list of date field types to filter by (e.g., ['due', 'start', 'end'])

Returns:
dict with keys as (location, field) tuples and values as date objects
User overrides take priority over content defaults
"""
course_id = _ensure_key(CourseKey, course_id)
allow_relative_dates = _are_relative_dates_enabled(course_id)

content_dates_query = models.ContentDate.objects.filter(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about when edx_when.api._are_relative_dates_enabled is true/false, are we expected to return relative deadlines? I was looking into (relative_dates tests)[https://github.com/openedx/edx-when/blob/master/tests/test_api.py#L363], probably we should add this function there as well?

course_id=course_id,
active=True,
).select_related('policy')

if not allow_relative_dates:
content_dates_query = content_dates_query.filter(policy__rel_date__isnull=True)

if block_types:
content_dates_query = content_dates_query.filter(block_type__in=block_types)

if block_keys:
normalized_keys = [_ensure_key(UsageKey, key) for key in block_keys]
content_dates_query = content_dates_query.filter(location__in=normalized_keys)

if date_types:
content_dates_query = content_dates_query.filter(field__in=date_types)

content_dates = list(
content_dates_query.prefetch_related(
Prefetch(
'userdate_set',
queryset=models.UserDate.objects.filter(user_id=user_id).order_by('-modified'),
to_attr='user_overrides'
)
)
)

schedule = get_schedule_for_user(user_id, course_id)

course_dates_for_bounds = list(
models.ContentDate.objects.filter(course_id=course_id, active=True).select_related('policy')
)
end_datetime, cutoff_datetime = _get_end_dates_from_content_dates(course_dates_for_bounds)
dates, _ = _resolve_policy_dates(content_dates, schedule, end_datetime, cutoff_datetime)

for content_date in content_dates:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Guard against no schedule/ MissingScheduleError as in _resolve_policy_dates above

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added 5285469

if content_date.user_overrides:
try:
actual_date = content_date.user_overrides[0].actual_date
except models.MissingScheduleError:
continue
dates[(content_date.location, content_date.field)] = actual_date

return dates


def get_locations_with_due_dates(course_key):
"""
Return the set of block locations that already have an active 'due' ContentDate for the given course.

Arguments:
course_key: either a CourseKey or string representation of same

Returns:
set of UsageKey objects representing blocks with existing due dates
"""
course_key = _ensure_key(CourseKey, course_key)
return set(
models.ContentDate.objects.filter(course_id=course_key, field='due', active=True)
.values_list('location', flat=True)
)


class BaseWhenException(Exception):
pass

Expand Down
Loading
Loading