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.

Sign up for GitHub

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

fix: unenrolled defaults are now not enrollable #2301

Merged
merged 1 commit into from
Dec 10, 2024
Merged

fix: unenrolled defaults are now not enrollable #2301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ Unreleased
----------
* nothing unreleased

[5.4.1]
--------
* fix: The default enrollment ``learner_status`` view now considers intended courses
from which the requested user has unenrolled as no-longer realizable.

[5.4.0]
--------
* chore: Update python requirements.
Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Your project description goes here.
"""

__version__ = "5.4.0"
__version__ = "5.4.1"
11 changes: 11 additions & 0 deletions enterprise/api/v1/views/default_enterprise_enrollments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,17 @@ def _partition_default_enrollment_intentions_by_enrollment_status(
already_enrolled.append((intention, non_audit_enrollment))
continue

if non_audit_enrollment and non_audit_enrollment.unenrolled:
Copy link
Member

Choose a reason for hiding this comment

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

[sanity check] If the existing enrollment is audit (vs. the non_audit_enrollment here), the learners unenrolls, and there's non-audit modes that exist, would we also want to prevent automatic re-enrollment, too?

For example, would the conditions below the NOTE comment also need to take into account audit_enrollment.unenrolled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that but decided against it, I can leave a comment in the code. My reasoning is that default enrollments should be "exactly once" per (user, enterprise), if possible. If they enrolled in audit and unenrolled, it means they likely never had a default intention realized for them. This isn't a strongly-held position though.

Copy link
Member

Choose a reason for hiding this comment

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

If they enrolled in audit and unenrolled, it means they likely never had a default intention realized for them.

Ah yeah, this probably represents the source of most audit enrollments. The exception would be for courses that are audit-only, which likely rarely happens in production; it does happen somewhat frequently on stage, though we shouldn't optimize for bad stage data 😉

Let's leave it as is for now, with a comment!

# Learner has un-enrolled in non-audit mode for this course run,
# so don't consider this as an enrollable intention.
# Note that that we don't consider the case of an unenrolled *audit* enrollment,
# because default enrollments should be "exactly once" per (user, enterprise), if possible.
# If only an (unenrolled) audit enrollment exists, it means the user likely
# never had a default intention realized for them in the given course,
# so we'd still like to consider it as enrollable.
needs_enrollment_not_enrollable.append((intention, non_audit_enrollment))
continue

if not intention.is_course_run_enrollable:
# Course run is not enrollable
needs_enrollment_not_enrollable.append((intention, audit_enrollment))
Expand Down
Loading
Loading