feat: [COSMO2-846] Audit Expiry Urgency enrollment-time assignment with persisted expiry#197
Conversation
…ment and persisted expiry
There was a problem hiding this comment.
Pull request overview
Implements the “Audit Expiry Urgency (v1)” experiment by assigning an Optimizely variant at enrollment time, persisting the chosen variant + audit_expiry_at on the enrollment, and updating Course Duration Limits (CDL) reads to prefer the persisted expiry behind a waffle-flag kill switch.
Changes:
- Add enrollment
post_savesignal handling to persist experiment attributes (variant,audit_expiry_at) viaCourseEnrollmentAttribute. - Add
AUDIT_EXPIRY_URGENCY_V1_ENABLEDwaffle flag and update CDL expiration-date reads to use persistedaudit_expiry_atwhen enabled. - Add test coverage for both persistence/stickiness behavior and the CDL read-path behavior with flag on/off.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| openedx/features/course_duration_limits/access.py | Reads persisted audit_expiry_at when the waffle flag is enabled, otherwise falls back to computed CDL logic. |
| openedx/features/course_duration_limits/tests/test_access.py | Adds tests asserting persisted expiry is used only when the kill switch is enabled. |
| lms/djangoapps/experiments/audit_expiry_urgency.py | Implements eligibility checks, Optimizely activation/reuse logic, expiry computation, and persistence via enrollment attributes. |
| lms/djangoapps/experiments/signals.py | Hooks enrollment post_save to run persistence logic safely (never blocking enrollment). |
| lms/djangoapps/experiments/apps.py | Ensures signals are imported during app initialization. |
| lms/djangoapps/experiments/flags.py | Introduces the AUDIT_EXPIRY_URGENCY_V1_ENABLED waffle flag with annotations. |
| lms/djangoapps/experiments/tests/test_audit_expiry_urgency.py | Adds tests for allowlisting, Optimizely failure fallback, cross-course stickiness, idempotency, and forced-variant behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| access_duration = get_user_course_duration(user, course) | ||
| if access_duration is None: | ||
| return None | ||
|
|
||
| enrollment = enrollment or CourseEnrollment.get_enrollment(user, course.id) | ||
| if enrollment is None or enrollment.mode != CourseMode.AUDIT: | ||
| return None | ||
|
|
||
| # Audit Expiry Urgency (v1) experiment: if an explicit persisted expiry exists, | ||
| # it is the single source of truth for UI/API reads. | ||
| # Kill switch behavior: when disabled, ignore any persisted experiment values | ||
| # and fall back to the default computed CDL logic. | ||
| if AUDIT_EXPIRY_URGENCY_V1_ENABLED.is_enabled(): | ||
| persisted_expiry_attr = ( | ||
| CourseEnrollmentAttribute.objects.filter( |
There was a problem hiding this comment.
get_user_course_expiration_date always calls get_user_course_duration() before checking for a persisted audit_expiry_at. That means even when a persisted expiry exists (and the flag is on), we still do the CDL duration computation (including a potential Catalog API lookup via get_expected_duration). Consider reordering so the persisted-expiry lookup happens first (after verifying audit enrollment), and only fall back to get_user_course_duration() when no persisted expiry is present.
| from openedx.features.course_duration_limits.models import CourseDurationLimitConfig | ||
| from common.djangoapps.student.models import CourseEnrollmentAttribute | ||
| from edx_toggles.toggles.testutils import override_waffle_flag | ||
| from lms.djangoapps.experiments.flags import AUDIT_EXPIRY_URGENCY_V1_ENABLED |
There was a problem hiding this comment.
This import section is likely not isort-compliant: CourseEnrollmentAttribute (from common.djangoapps...) is inserted after openedx.features... imports, which will typically fail import-order linting. Consider regrouping/sorting imports so all common.djangoapps.* imports stay together with the other common.* imports at the top of the local-import block.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert computed is not None | ||
| assert computed != persisted |
There was a problem hiding this comment.
This test asserts computed != persisted, which can become brittle if the default CDL-computed duration ever happens to equal the chosen persisted value (e.g., if discovery returns 6 weeks). To make the intent deterministic, assert that the returned value matches the expected computed CDL expiration (recompute it the same way as the function does) rather than relying on inequality with an arbitrary timestamp.
| assert computed is not None | |
| assert computed != persisted | |
| # Recompute the expected CDL-based expiration date deterministically, | |
| # to verify that the persisted audit_expiry_at value is ignored. | |
| content_availability_date = max(enrollment.created, overview.start) | |
| access_duration = get_user_course_duration(user, overview) | |
| expected_course_expiration_date = content_availability_date + access_duration | |
| assert computed is not None | |
| assert computed == expected_course_expiration_date |
| user = None | ||
| enrollment_1 = CourseEnrollmentFactory.create(course=self.course_1, mode=CourseMode.AUDIT, is_active=True) | ||
| user = enrollment_1.user | ||
| enrollment_2 = CourseEnrollmentFactory.create(user=user, course=self.course_2, mode=CourseMode.AUDIT, is_active=True) |
There was a problem hiding this comment.
This line exceeds the repository’s configured max line length (120) and will trigger line-too-long in pylint (see pylintrc max-line-length). Please wrap the CourseEnrollmentFactory.create(...) call across multiple lines.
| enrollment_2 = CourseEnrollmentFactory.create(user=user, course=self.course_2, mode=CourseMode.AUDIT, is_active=True) | |
| enrollment_2 = CourseEnrollmentFactory.create( | |
| user=user, | |
| course=self.course_2, | |
| mode=CourseMode.AUDIT, | |
| is_active=True, | |
| ) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactor code for better readability by formatting the creation of CourseEnrollment objects.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removed unnecessary blank lines in test_audit_expiry_urgency.py.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactor eligibility checks for audit expiry urgency.
Removed comments to simplify the code and address pylint warnings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactor enrollment processing logic to use a dedicated function for eligibility checks, improving code clarity and maintainability.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if AUDIT_EXPIRY_URGENCY_V1_ENABLED.is_enabled(): | ||
| experiment_attributes = { | ||
| attr.name: attr.value | ||
| for attr in enrollment.attributes.filter(namespace='audit_expiry_experiment') |
There was a problem hiding this comment.
experiment_attributes is built from an unordered queryset, so if there are multiple CourseEnrollmentAttribute rows for the same name (which this PR explicitly notes can happen), the chosen value can be nondeterministic and may not reflect the latest persisted assignment. Consider ordering by id (or -id) and/or fetching the latest value per attribute name (e.g., order the queryset and let later rows overwrite earlier ones, or query last() per name) so reads are stable and consistent.
| for attr in enrollment.attributes.filter(namespace='audit_expiry_experiment') | |
| for attr in enrollment.attributes.filter( | |
| namespace='audit_expiry_experiment' | |
| ).order_by('id') |
| configured = [] | ||
| if isinstance(configured, str): | ||
| configured = [configured] | ||
| if not isinstance(configured, list): |
There was a problem hiding this comment.
_get_configured_target_course_id_strings() only accepts list for the allowlist value. If AUDIT_EXPIRY_EXPERIMENT_COURSES is configured as a tuple (a common pattern in Django settings) or any other sequence type, this code will silently treat it as invalid and behave as if no courses are allowlisted. Consider accepting tuple (and possibly other iterables) in addition to list (e.g., isinstance(configured, (list, tuple))).
| if not isinstance(configured, list): | |
| if not isinstance(configured, (list, tuple)): |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| experiment_attributes = { | ||
| attr.name: attr.value | ||
| for attr in enrollment.attributes.filter(namespace='audit_expiry_experiment') | ||
| } |
There was a problem hiding this comment.
get_access_expiration_data builds a dict from enrollment.attributes.filter(namespace='audit_expiry_experiment') without an explicit ordering. Since CourseEnrollmentAttribute has no uniqueness constraint (duplicates are possible), this can yield nondeterministic/stale values for experiment_key/variant/expiry_days depending on DB ordering. Consider ordering by id and ensuring the latest attribute per name wins (or fetching the latest row per attribute name explicitly).
| experiment_attributes = { | |
| attr.name: attr.value | |
| for attr in enrollment.attributes.filter(namespace='audit_expiry_experiment') | |
| } | |
| experiment_attributes = {} | |
| for attr in enrollment.attributes.filter( | |
| namespace='audit_expiry_experiment' | |
| ).order_by('id'): | |
| experiment_attributes[attr.name] = attr.value |
| if any(( | ||
| not enrollment.is_active, | ||
| enrollment.mode != CourseMode.AUDIT, | ||
| not is_target_course(enrollment.course_id), | ||
| )): | ||
| return | ||
|
|
||
| # Idempotency: do not overwrite once set. | ||
| if get_persisted_audit_expiry_at(enrollment): | ||
| return | ||
|
|
||
| access_duration = get_user_course_duration(enrollment.user, enrollment.course_overview) | ||
| if access_duration is None: | ||
| # Only apply if Course Duration Limits would normally apply. | ||
| return | ||
|
|
||
| target_course_ids = _get_configured_target_course_id_strings() | ||
| variant, decision_source = choose_variant(enrollment.user, target_course_ids) |
There was a problem hiding this comment.
maybe_persist_audit_expiry_urgency_attributes calls is_target_course(enrollment.course_id) (which reads site config) and then later calls _get_configured_target_course_id_strings() again to choose/reuse a variant. This results in duplicate config lookups per save when the flag is on; consider loading the configured target course IDs once and reusing it for both the eligibility check and choose_variant().
Summary
This PR implements the Audit Expiry Urgency experiment in edx-platform.
The experiment assigns eligible audit learners to one of two variants at enrollment time and persists the backend decision so that access expiration behavior remains stable across sessions and configured target courses.
Variants:
control_5_7_weeksexpiry_7_daysThe backend remains the source of truth for:
What This PR Does
Enrollment-time assignment and persistence
When an eligible learner enrolls in a target audit course, the backend:
Persisted experiment attributes:
experiment_keyvariantexpiry_daysassigned_atdecision_sourceaudit_expiry_atAll metadata is stored in
CourseEnrollmentAttributeunder theaudit_expiry_experimentnamespace.Read-side access expiration support
The existing
access_expirationresponse is extended to include experiment context:experiment_keyvariantexpiry_daysThis allows frontend consumers to use the same backend response for both expiration behavior and experiment-specific display or analytics needs.
Backend analytics
The backend emits Optimizely events for:
audit_expiry_urgency_exposedaudit_expiry_urgency_upgraded_to_verifiedSticky-assignment behavior
Assignment is learner-level and sticky across the configured target courses.
If a learner already has a persisted experiment assignment, that assignment is reused instead of creating a new one.
Rollback and fallback behavior
audit_expiry_atremains the source of truth for access enforcementcontrol_5_7_weeksConfiguration and Selection Flow
Course target list selection
The experiment uses
AUDIT_EXPIRY_EXPERIMENT_COURSESto determine which course runs are eligible.Course list resolution happens in this order:
So if
AUDIT_EXPIRY_EXPERIMENT_COURSESis configured in Site Configuration, that value is used. If it is not configured there, the backend falls back to the locally configured settings value.Variant selection order
Once an enrollment is eligible for the experiment, backend variant selection happens in this order:
AUDIT_EXPIRY_FORCE_VARIANTcontrol_5_7_weeksThis means Site Configuration controls course eligibility, while learner variant selection is handled separately by the experiment helper logic.
Post-Deployment Steps
Enable waffle flag
Enable:
experiments.audit_expiry_urgency_v1.enabledAdd Site Configuration
Configure
AUDIT_EXPIRY_EXPERIMENT_COURSESwith the target course IDs:{ "AUDIT_EXPIRY_EXPERIMENT_COURSES": [ "<target-course-run-1>", "<target-course-run-2>", "<target-course-run-3>" ] }