From 05d820f17812acbf88ec99f57f709ddcc2a0e63d Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Mon, 5 Aug 2024 14:22:09 -0700 Subject: [PATCH] feat: write reversal on LC enrollment revoked event Handle the following event bus event: org.openedx.enterprise.learner_credit_course_enrollment.revoked.v1 under the following openedx-signal: LEARNER_CREDIT_COURSE_ENROLLMENT_REVOKED This will perform the same duties as the `write_reversals_from_enterprise_unenrollments` management command, except it operates on only one unenrollment at a time, and no longer calls the "recent unenrollments" API located at: {LMS_BASE_URL}/enterprise/api/v1/operator/enterprise-subsidy-fulfillment/unenrolled/ ENT-9213 --- .../apps/transaction/signals/handlers.py | 198 +++++++++++++++++- .../transaction/tests/test_signal_handlers.py | 90 +++++++- requirements/base.txt | 2 +- requirements/constraints.txt | 2 + requirements/dev.txt | 2 +- requirements/doc.txt | 2 +- requirements/production.txt | 2 +- requirements/quality.txt | 2 +- requirements/test.txt | 2 +- requirements/validation.txt | 2 +- 10 files changed, 292 insertions(+), 12 deletions(-) diff --git a/enterprise_subsidy/apps/transaction/signals/handlers.py b/enterprise_subsidy/apps/transaction/signals/handlers.py index 0ceb9e8c..e36f959d 100644 --- a/enterprise_subsidy/apps/transaction/signals/handlers.py +++ b/enterprise_subsidy/apps/transaction/signals/handlers.py @@ -1,15 +1,54 @@ """ 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 (no-op). + ↳ Attempt to idempotently revoke internal enrollment (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 datetime import datetime, timedelta +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 logger = logging.getLogger(__name__) @@ -17,7 +56,10 @@ @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" @@ -36,3 +78,151 @@ def listen_for_transaction_reversal(sender, **kwargs): error_msg = f"Error canceling platform fulfillment {transaction.fulfillment_identifier}: {exc}" logger.exception(error_msg) raise exc + + +def unenrollment_can_be_refunded( + 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") + 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_datetime = enterprise_course_enrollment.get("created") + enrollment_unenrolled_at_datetime = enterprise_course_enrollment.get("unenrolled_at") + course_start_datetime = datetime.fromisoformat(course_start_date) + 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 + + +@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. + """ + 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}." + ) + + # On initial release we are only supporting learner initiated unenrollments for OCM courses. + # OCM courses are identified by the lack of an external_reference on the Transaction object. + # Externally referenced transactions can be unenrolled through the Django admin actions related to the + # Transaction model. + automatic_external_cancellation = getattr(settings, "ENTERPRISE_SUBSIDY_AUTOMATIC_EXTERNAL_CANCELLATION", False) + if related_transaction.external_reference.exists() and not automatic_external_cancellation: + logger.info( + f"Found unenrolled enterprise fulfillment: {fulfillment_uuid} related to " + f"an externally referenced transaction: {related_transaction.external_reference.first()}. " + f"Skipping ENTERPRISE_SUBSIDY_AUTOMATIC_EXTERNAL_CANCELLATION={automatic_external_cancellation}." + ) + return + + # 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) diff --git a/enterprise_subsidy/apps/transaction/tests/test_signal_handlers.py b/enterprise_subsidy/apps/transaction/tests/test_signal_handlers.py index 98b4f745..ae57255c 100644 --- a/enterprise_subsidy/apps/transaction/tests/test_signal_handlers.py +++ b/enterprise_subsidy/apps/transaction/tests/test_signal_handlers.py @@ -1,8 +1,14 @@ """ Tests for the subsidy service transaction app signal handlers """ +import re +from uuid import uuid4 from unittest import mock +from datetime import datetime +import ddt +from django.test.utils import override_settings +from openedx_ledger.models import TransactionStateChoices import pytest from django.test import TestCase from openedx_ledger.signals.signals import TRANSACTION_REVERSED @@ -13,12 +19,13 @@ ReversalFactory, TransactionFactory ) - from enterprise_subsidy.apps.api_client.enterprise import EnterpriseApiClient from enterprise_subsidy.apps.fulfillment.api import GEAGFulfillmentHandler +from enterprise_subsidy.apps.transaction.signals.handlers import handle_lc_enrollment_revoked from test_utils.utils import MockResponse +@ddt.ddt class TransactionSignalHandlerTestCase(TestCase): """ Tests for the transaction signal handlers @@ -92,3 +99,84 @@ def test_transaction_reversed_signal_without_fulfillment_identifier( assert mock_oauth_client.return_value.post.call_count == 0 self.assertFalse(mock_send_event_bus_reversed.called) + + + @ddt.data( + # Happy path. + {}, + # Sad paths: + { + "transaction_state": None, + "expected_log_regex": "No Subsidy Transaction found", + "expected_reverse_transaction_called": False, + }, + { + "transaction_state": TransactionStateChoices.PENDING, + "expected_log_regex": "not in a committed state", + "expected_reverse_transaction_called": False, + }, + { + "reversal_exists": True, + "expected_log_regex": "Found existing Reversal", + "expected_reverse_transaction_called": False, + }, + { + "refundable": False, + "expected_log_regex": "not refundable", + "expected_reverse_transaction_called": False, + }, + { + "external_fulfillment_will_succeed": False, + "expected_log_regex": "no reversal written", + "expected_reverse_transaction_called": False, + }, + ) + @ddt.unpack + @mock.patch('enterprise_subsidy.apps.transaction.signals.handlers.cancel_transaction_external_fulfillment') + @mock.patch('enterprise_subsidy.apps.transaction.signals.handlers.reverse_transaction') + @mock.patch('enterprise_subsidy.apps.transaction.signals.handlers.unenrollment_can_be_refunded') + @mock.patch('enterprise_subsidy.apps.transaction.signals.handlers.ContentMetadataApi.get_content_metadata') + @override_settings(ENTERPRISE_SUBSIDY_AUTOMATIC_EXTERNAL_CANCELLATION=True) + def test_handle_lc_enrollment_revoked( + self, + mock_get_content_metadata, + mock_unenrollment_can_be_refunded, + mock_reverse_transaction, + mock_cancel_transaction_external_fulfillment, + transaction_state=TransactionStateChoices.COMMITTED, + reversal_exists=False, + refundable=True, + external_fulfillment_will_succeed=True, + expected_log_regex=None, + expected_reverse_transaction_called=True, + ): + mock_get_content_metadata.return_value = {"unused": "unused"} + mock_unenrollment_can_be_refunded.return_value = refundable + mock_cancel_transaction_external_fulfillment.return_value = external_fulfillment_will_succeed + ledger = LedgerFactory() + transaction = None + if transaction_state: + transaction = TransactionFactory(ledger=ledger, state=transaction_state) + if reversal_exists: + ReversalFactory( + transaction=transaction, + quantity=-transaction.quantity, + ) + enrollment_unenrolled_at = datetime(2020, 1, 1) + test_lc_course_enrollment = { + "uuid": uuid4(), + "transaction_id": transaction.uuid if transaction else uuid4(), + "enterprise_course_enrollment": { + "course_id": "course-v1:bin+bar+baz", + "unenrolled_at": enrollment_unenrolled_at, + "enterprise_customer_user": { + "unused": "unused", + }, + } + } + with self.assertLogs(level='INFO') as logs: + handle_lc_enrollment_revoked(learner_credit_course_enrollment=test_lc_course_enrollment) + if expected_log_regex: + assert any(re.search(expected_log_regex, log) for log in logs.output) + if expected_reverse_transaction_called: + mock_reverse_transaction.assert_called_once_with(transaction, unenroll_time=enrollment_unenrolled_at) diff --git a/requirements/base.txt b/requirements/base.txt index 915b9d1a..1c02c0d4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -194,7 +194,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/base.in # edx-event-bus-kafka diff --git a/requirements/constraints.txt b/requirements/constraints.txt index b10592da..6a4e09bf 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -18,3 +18,5 @@ django-simple-history>=3.4.0,<3.5.0 # around default model fields vs. the serializer field defs, # and the discontinued use of OrderedDict in serializers. djangorestframework<3.15 + +git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d#egg=openedx_events diff --git a/requirements/dev.txt b/requirements/dev.txt index 2e59b022..fcce0b41 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -370,7 +370,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/validation.txt # edx-event-bus-kafka diff --git a/requirements/doc.txt b/requirements/doc.txt index 6cc9e3e7..02acbcc7 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -355,7 +355,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/test.txt # edx-event-bus-kafka diff --git a/requirements/production.txt b/requirements/production.txt index 6064c52e..666301cf 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -239,7 +239,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/base.txt # edx-event-bus-kafka diff --git a/requirements/quality.txt b/requirements/quality.txt index ca94203a..d2d15944 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -336,7 +336,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/test.txt # edx-event-bus-kafka diff --git a/requirements/test.txt b/requirements/test.txt index b918cdaf..653d2373 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -282,7 +282,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/base.txt # edx-event-bus-kafka diff --git a/requirements/validation.txt b/requirements/validation.txt index 91e2f45a..832e598e 100644 --- a/requirements/validation.txt +++ b/requirements/validation.txt @@ -433,7 +433,7 @@ oauthlib==3.2.2 # getsmarter-api-clients # requests-oauthlib # social-auth-core -openedx-events==9.11.0 +openedx-events @ git+https://github.com/pwnage101/openedx-events.git@f00926ae93e84a114779a0e9c98c152e8927706d # via # -r requirements/quality.txt # -r requirements/test.txt