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: write reversal on LC enrollment revoked event #282

Merged
merged 3 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions enterprise_subsidy/apps/api_client/tests/test_enterprise.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def test_successful_fetching_of_recent_unenrollments(self, mock_oauth_client):
'enterprise_customer_user': 10,
'course_id': self.courserun_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': transaction_id,
'uuid': fulfillment_uuid,
Expand All @@ -236,7 +236,7 @@ def test_successful_fetching_of_recent_unenrollments(self, mock_oauth_client):
'enterprise_customer_user': 10,
'course_id': self.courserun_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': transaction_id,
'uuid': fulfillment_uuid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
Transaction Reversals where appropriate.
"""
import logging
from datetime import datetime, timedelta

from django.conf import settings
from django.contrib import auth
Expand All @@ -14,6 +13,7 @@
from enterprise_subsidy.apps.api_client.enterprise import EnterpriseApiClient
from enterprise_subsidy.apps.content_metadata.api import ContentMetadataApi
from enterprise_subsidy.apps.transaction.api import cancel_transaction_external_fulfillment, reverse_transaction
from enterprise_subsidy.apps.transaction.utils import unenrollment_can_be_refunded

logger = logging.getLogger(__name__)
User = auth.get_user_model()
Expand Down Expand Up @@ -52,67 +52,6 @@ def add_arguments(self, parser):
),
)

def convert_unenrollment_datetime_string(self, datetime_str):
"""
Helper method to strip microseconds from a datetime object
"""
try:
formatted_datetime = datetime.strptime(datetime_str, "%Y-%m-%dT%H:%M:%SZ")
except ValueError:
formatted_datetime = datetime.strptime(datetime_str, "%Y-%m-%dT%H:%M:%S.%fZ")
return formatted_datetime

def unenrollment_can_be_refunded(
self,
content_metadata,
enterprise_course_enrollment,
):
"""
helper method to determine if an unenrollment is refundable
"""
# Retrieve the course start date from the content metadata
enrollment_course_run_key = enterprise_course_enrollment.get("course_id")
enrollment_created_at = enterprise_course_enrollment.get("created")
course_start_date = None
if content_metadata.get('content_type') == 'courserun':
course_start_date = content_metadata.get('start')
else:
for run in content_metadata.get('course_runs', []):
if run.get('key') == enrollment_course_run_key:
course_start_date = run.get('start')
break

if not course_start_date:
logger.warning(
f"No course start date found for course run: {enrollment_course_run_key}. "
"Unable to determine refundability."
)
return False

# https://2u-internal.atlassian.net/browse/ENT-6825
# OCM course refundability is defined as True IFF:
# ie MAX(enterprise enrollment created at, course start date) + 14 days > unenrolled_at date
enrollment_created_at = enterprise_course_enrollment.get("created")
enrollment_unenrolled_at = enterprise_course_enrollment.get("unenrolled_at")

enrollment_created_datetime = self.convert_unenrollment_datetime_string(enrollment_created_at)
course_start_datetime = self.convert_unenrollment_datetime_string(course_start_date)
enrollment_unenrolled_at_datetime = self.convert_unenrollment_datetime_string(enrollment_unenrolled_at)
refund_cutoff_date = max(course_start_datetime, enrollment_created_datetime) + timedelta(days=14)

if refund_cutoff_date > enrollment_unenrolled_at_datetime:
logger.info(
f"Course run: {enrollment_course_run_key} is refundable for enterprise customer user: "
f"{enterprise_course_enrollment.get('enterprise_customer_user')}. Writing Reversal record."
)
return True
else:
logger.info(
f"Unenrollment from course: {enrollment_course_run_key} by user: "
f"{enterprise_course_enrollment.get('enterprise_customer_user')} is not refundable."
)
return False

def handle_reversing_enterprise_course_unenrollment(self, unenrollment):
"""
Helper method to determine refund eligibility of unenrollments and generating reversals for enterprise course
Expand Down Expand Up @@ -168,7 +107,7 @@ def handle_reversing_enterprise_course_unenrollment(self, unenrollment):
content_metadata = self.fetched_content_metadata.get(enrollment_course_run_key)

# Check if the OCM unenrollment is refundable
if not self.unenrollment_can_be_refunded(content_metadata, enterprise_course_enrollment):
if not unenrollment_can_be_refunded(content_metadata, enterprise_course_enrollment):
logger.info(
f"{self.dry_run_prefix}Unenrollment from course: {enrollment_course_run_key} by user: "
f"{enterprise_course_enrollment.get('enterprise_customer_user')} is not refundable."
Expand Down
145 changes: 141 additions & 4 deletions enterprise_subsidy/apps/transaction/signals/handlers.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,65 @@
"""
Subsidy Service signals handler.

The following two scenarios detail what happens when either ECS or a learner initiates unenrollment, and explains how
infinite loops are terminated.

1. When ECS invokes transaction reversal:
=========================================
* Reversal gets created.
↳ Emit TRANSACTION_REVERSED signal.
* TRANSACTION_REVERSED triggers the `listen_for_transaction_reversal()` handler.
↳ Revoke internal & external fulfillments.
↳ Emit LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED openedx event.
↳ Emit LEDGER_TRANSACTION_REVERSED openedx event.
* LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED triggers the `handle_lc_enrollment_revoked()` handler.
↳ Fail first base case (reversal already exists) and quit. <-------THIS TERMINATES THE INFINITE LOOP!
* LEDGER_TRANSACTION_REVERSED triggers the `update_assignment_status_for_reversed_transaction()` handler.
↳ Updates any assignments as needed.

2. When a learner invokes unenrollment:
=======================================
* Enterprise app will perform internal fulfillment revocation.
↳ Emit LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED openedx event.
* LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED triggers the `handle_lc_enrollment_revoked()` handler.
↳ Revoke external fulfillments.
↳ Create reversal.
↳ Emit TRANSACTION_REVERSED signal.
* TRANSACTION_REVERSED triggers the `listen_for_transaction_reversal()` handler.
↳ Attempt to idempotently revoke external enrollment (API no-op).
↳ Attempt to idempotently revoke internal enrollment (API no-op). <---THIS TERMINATES THE INFINITE LOOP!
↳ Emit LEDGER_TRANSACTION_REVERSED openedx event.
* LEDGER_TRANSACTION_REVERSED triggers the `update_assignment_status_for_reversed_transaction()` handler.
↳ Updates any assignments as needed.
"""
import logging

from django.conf import settings
from django.dispatch import receiver
from openedx_events.enterprise.signals import LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED
from openedx_ledger.models import Transaction, TransactionStateChoices
from openedx_ledger.signals.signals import TRANSACTION_REVERSED

from enterprise_subsidy.apps.content_metadata.api import ContentMetadataApi
from enterprise_subsidy.apps.core.event_bus import send_transaction_reversed_event

from ..api import cancel_transaction_external_fulfillment, cancel_transaction_fulfillment
from ..exceptions import TransactionFulfillmentCancelationException
from enterprise_subsidy.apps.transaction.api import (
cancel_transaction_external_fulfillment,
cancel_transaction_fulfillment,
reverse_transaction
)
from enterprise_subsidy.apps.transaction.exceptions import TransactionFulfillmentCancelationException
from enterprise_subsidy.apps.transaction.utils import unenrollment_can_be_refunded

logger = logging.getLogger(__name__)


@receiver(TRANSACTION_REVERSED)
def listen_for_transaction_reversal(sender, **kwargs):
"""
Listen for the TRANSACTION_REVERSED signals and issue an unenrollment request to platform.
Listen for the TRANSACTION_REVERSED signals and issue an unenrollment request to internal and external fulfillments.

This subsequently emits a LEDGER_TRANSACTION_REVERSED openedx event to signal to enterprise-access that any
assignents need to be reversed too.
"""
logger.info(
f"Received TRANSACTION_REVERSED signal from {sender}, attempting to unenroll platform enrollment object"
Expand All @@ -36,3 +78,98 @@
error_msg = f"Error canceling platform fulfillment {transaction.fulfillment_identifier}: {exc}"
logger.exception(error_msg)
raise exc


@receiver(LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED)
def handle_lc_enrollment_revoked(**kwargs):
"""
openedx event handler to respond to LearnerCreditEnterpriseCourseEnrollment revocations.

The critical bits of this handler's business logic can be summarized as follows:

* Receive LC fulfillment revocation event and run this handler.
* BASE CASE: If this fulfillment's transaction has already been reversed, quit.
* BASE CASE: If the refund deadline has passed, quit.
* Cancel/unenroll any external fulfillments related to the transaction.
* Reverse the transaction.

Args:
learner_credit_course_enrollment (dict-like):
An openedx-events serialized representation of LearnerCreditEnterpriseCourseEnrollment.
"""
if not settings.ENABLE_HANDLE_LC_ENROLLMENT_REVOKED:
logger.info(

Check warning on line 101 in enterprise_subsidy/apps/transaction/signals/handlers.py

View check run for this annotation

Codecov / codecov/patch

enterprise_subsidy/apps/transaction/signals/handlers.py#L101

Added line #L101 was not covered by tests
"Handling of LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED event has been disabled. "
"Skipping handle_lc_enrollment_revoked() handler."
)
return

Check warning on line 105 in enterprise_subsidy/apps/transaction/signals/handlers.py

View check run for this annotation

Codecov / codecov/patch

enterprise_subsidy/apps/transaction/signals/handlers.py#L105

Added line #L105 was not covered by tests
revoked_enrollment_data = kwargs.get('learner_credit_course_enrollment')
fulfillment_uuid = revoked_enrollment_data.get("uuid")
enterprise_course_enrollment = revoked_enrollment_data.get("enterprise_course_enrollment")
enrollment_course_run_key = enterprise_course_enrollment.get("course_id")
enrollment_unenrolled_at = enterprise_course_enrollment.get("unenrolled_at")

# Look for a transaction related to the unenrollment
related_transaction = Transaction.objects.filter(
uuid=revoked_enrollment_data.get('transaction_id')
).first()
if not related_transaction:
logger.info(
f"No Subsidy Transaction found for enterprise fulfillment: {fulfillment_uuid}"
)
return
# Fail early if the transaction is not committed, even though reverse_full_transaction()
# would throw an exception later anyway.
if related_transaction.state != TransactionStateChoices.COMMITTED:
logger.info(
f"Transaction: {related_transaction} is not in a committed state. "
f"Skipping Reversal creation."
)
return

# Look for a Reversal related to the unenrollment
existing_reversal = related_transaction.get_reversal()
if existing_reversal:
logger.info(
f"Found existing Reversal: {existing_reversal} for enterprise fulfillment: "
f"{fulfillment_uuid}. Skipping Reversal creation for Transaction: {related_transaction}."
)
return

# Continue on if no reversal found
logger.info(
f"No existing Reversal found for enterprise fulfillment: {fulfillment_uuid}. "
f"Writing Reversal for Transaction: {related_transaction}."
)

# NOTE: get_content_metadata() is backed by TieredCache, so this would be performant if a bunch learners unenroll
# from the same course at the same time. However, normally no two learners in the same course would unenroll within
# a single cache timeout period, so we'd expect this to normally always re-fetch from remote API. That's OK because
# unenrollment volumes are manageable.
content_metadata = ContentMetadataApi.get_content_metadata(
enrollment_course_run_key,
)

# Check if the OCM unenrollment is refundable
if not unenrollment_can_be_refunded(content_metadata, enterprise_course_enrollment):
logger.info(
f"Unenrollment from course: {enrollment_course_run_key} by user: "
f"{enterprise_course_enrollment.get('enterprise_customer_user')} is not refundable."
)
return

logger.info(
f"Course run: {enrollment_course_run_key} is refundable for enterprise "
f"customer user: {enterprise_course_enrollment.get('enterprise_customer_user')}. Writing "
"Reversal record."
)

successfully_canceled = cancel_transaction_external_fulfillment(related_transaction)
if not successfully_canceled:
logger.warning(
'Could not cancel external fulfillment for transaction %s, no reversal written',
related_transaction.uuid,
)
return

reverse_transaction(related_transaction, unenroll_time=enrollment_unenrolled_at)
20 changes: 10 additions & 10 deletions enterprise_subsidy/apps/transaction/tests/test_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ def test_write_reversals_from_enterprise_unenrollment_does_not_rerequest_metadat
'enterprise_customer_user': 10,
'course_id': self.transaction.content_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': self.transaction.uuid,
'uuid': str(self.transaction.fulfillment_identifier),
Expand All @@ -298,7 +298,7 @@ def test_write_reversals_from_enterprise_unenrollment_does_not_rerequest_metadat
'enterprise_customer_user': 11,
'course_id': self.transaction.content_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': transaction_uuid_2,
'uuid': str(uuid.uuid4()),
Expand Down Expand Up @@ -385,7 +385,7 @@ def test_write_reversals_from_enterprise_unenrollment_transaction_does_not_exist
'enterprise_customer_user': 10,
'course_id': self.courserun_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': uuid.uuid4(),
'uuid': self.fulfillment_identifier,
Expand Down Expand Up @@ -413,7 +413,7 @@ def test_write_reversals_from_enterprise_unenrollment_with_uncommitted_transacti
'enterprise_customer_user': 10,
'course_id': self.courserun_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': self.transaction.uuid,
'uuid': self.fulfillment_identifier,
Expand Down Expand Up @@ -441,8 +441,8 @@ def test_write_reversals_from_enterprise_unenrollment_with_uncommitted_transacti
'enterprise_subsidy.apps.transaction.api.EnterpriseApiClient'
)
@ddt.data(
('2023-05-25T19:27:29Z', '2023-06-1T19:27:29Z'),
('2023-06-1T19:27:29Z', '2023-05-25T19:27:29Z'),
('2023-05-25T19:27:29Z', '2023-06-01T19:27:29Z'),
('2023-06-01T19:27:29Z', '2023-05-25T19:27:29Z'),
)
@ddt.unpack
def test_write_reversals_from_enterprise_unenrollment_refund_period_ended(
Expand Down Expand Up @@ -574,7 +574,7 @@ def test_write_reversals_from_enterprise_unenrollments(
'enterprise_customer_user': 10,
'course_id': self.transaction.content_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': self.transaction.uuid,
'uuid': str(self.transaction.fulfillment_identifier),
Expand Down Expand Up @@ -646,7 +646,7 @@ def test_write_reversals_from_enterprise_unenrollments(
reversal = Reversal.objects.first()
assert reversal.transaction == self.transaction
assert reversal.idempotency_key == (
f'unenrollment-reversal-{self.transaction.fulfillment_identifier}-2023-06-1T19:27:29Z'
f'unenrollment-reversal-{self.transaction.fulfillment_identifier}-2023-06-01T19:27:29Z'
)
mock_send_event_bus_reversed.assert_called_once_with(self.transaction)
else:
Expand Down Expand Up @@ -692,7 +692,7 @@ def test_write_reversals_from_geag_enterprise_unenrollments_enabled_setting(
'enterprise_customer_user': 10,
'course_id': self.geag_transaction.content_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': self.geag_transaction.uuid,
'uuid': str(self.geag_transaction.fulfillment_identifier),
Expand Down Expand Up @@ -797,7 +797,7 @@ def test_write_reversals_from_geag_enterprise_unenrollments_unknown_provider(
'enterprise_customer_user': 10,
'course_id': self.unknown_transaction.content_key,
'created': '2023-05-25T19:27:29Z',
'unenrolled_at': '2023-06-1T19:27:29Z',
'unenrolled_at': '2023-06-01T19:27:29Z',
},
'transaction_id': self.unknown_transaction.uuid,
'uuid': str(self.unknown_transaction.fulfillment_identifier),
Expand Down
Loading
Loading