diff --git a/enterprise_subsidy/apps/api/v1/serializers.py b/enterprise_subsidy/apps/api/v1/serializers.py index c102c4ce..aff4185f 100644 --- a/enterprise_subsidy/apps/api/v1/serializers.py +++ b/enterprise_subsidy/apps/api/v1/serializers.py @@ -28,7 +28,7 @@ class SubsidySerializer(serializers.ModelSerializer): """ current_balance = serializers.SerializerMethodField(help_text="The current (remaining) balance of this subsidy.") is_active = serializers.BooleanField(read_only=True, help_text="Whether this subsidy is currently active.") - total_deposits = serializers.SerializerMethodField( + total_deposits = serializers.IntegerField( help_text="The aggregate of the initial balance plus all adjustments made on the subsidy in usd cents" ) @@ -68,10 +68,6 @@ class Meta: def get_current_balance(self, obj) -> int: return obj.current_balance() - @extend_schema_field(serializers.IntegerField) - def get_total_deposits(self, obj) -> int: - return obj.total_deposits() - class ReversalSerializer(serializers.ModelSerializer): """ diff --git a/enterprise_subsidy/apps/api/v1/tests/test_views.py b/enterprise_subsidy/apps/api/v1/tests/test_views.py index 2d74898a..d1f534d7 100644 --- a/enterprise_subsidy/apps/api/v1/tests/test_views.py +++ b/enterprise_subsidy/apps/api/v1/tests/test_views.py @@ -274,7 +274,7 @@ def test_get_adjustments_related_subsidy(self): "internal_only": False, "revenue_category": RevenueCategoryChoices.BULK_ENROLLMENT_PREPAY, "is_active": True, - "total_deposits": self.subsidy_5.total_deposits(), + "total_deposits": self.subsidy_5.total_deposits, } total_deposits_including_positive_adjustment = sum( [self.subsidy_5.starting_balance, APITestBase.adjustment_quantity_1] @@ -312,7 +312,7 @@ def test_get_adjustments_related_subsidy(self): "internal_only": False, "revenue_category": RevenueCategoryChoices.BULK_ENROLLMENT_PREPAY, "is_active": True, - "total_deposits": self.subsidy_5.total_deposits(), + "total_deposits": self.subsidy_5.total_deposits, } total_deposits_including_negative_adjustment = sum( diff --git a/enterprise_subsidy/apps/subsidy/migration_utils.py b/enterprise_subsidy/apps/subsidy/migration_utils.py new file mode 100644 index 00000000..8f031ef3 --- /dev/null +++ b/enterprise_subsidy/apps/subsidy/migration_utils.py @@ -0,0 +1,40 @@ +""" +Helper functions for data migrations. +""" +from django.db.models import F, Window +from django.db.models.functions import FirstValue +# This utils module is imported by migrations, so never import any models directly here, nor any modules that import +# models in turn. +from openedx_ledger.constants import INITIAL_DEPOSIT_TRANSACTION_SLUG + + +def find_legacy_initial_transactions(transaction_cls): + """ + Heuristic to identify "legacy" initial transactions. + + An initial transaction is one that has the following traits: + * Is chronologically the first transaction for a Ledger. + * Contains a hint in its idempotency key which indicates that it is an initial deposit. + * Has a positive quantity. + + A legacy initial transaction is one that has the following additional traits: + * does not have a related Deposit. + """ + # All transactions which are chronologically the first in their respective ledgers. + first_transactions = transaction_cls.objects.annotate( + first_tx_uuid=Window( + expression=FirstValue('uuid'), + partition_by=['ledger'], + order_by=F('created').asc(), # "first chronologically" above means first created. + ), + ).filter(uuid=F('first_tx_uuid')) + + # Further filter first_transactions to find ones that qualify as _initial_ and _legacy_. + legacy_initial_transactions = first_transactions.filter( + # Traits of an _initial_ deposit: + idempotency_key__contains=INITIAL_DEPOSIT_TRANSACTION_SLUG, + quantity__gte=0, + # Traits of a _legacy_ initial deposit: + deposit__isnull=True, + ) + return legacy_initial_transactions diff --git a/enterprise_subsidy/apps/subsidy/migrations/0022_backfill_initial_deposits.py b/enterprise_subsidy/apps/subsidy/migrations/0022_backfill_initial_deposits.py new file mode 100644 index 00000000..121c0678 --- /dev/null +++ b/enterprise_subsidy/apps/subsidy/migrations/0022_backfill_initial_deposits.py @@ -0,0 +1,83 @@ +""" +Backfill initial deposits. + +Necessarily, this also backfills SalesContractReferenceProvider objects based on the values currently defined via +SubsidyReferenceChoices. + +Note this has no reverse migration logic. Attempts to rollback the deployment which includes this PR will not delete +(un-backfill) the deposits created during the forward migration. +""" +import django.utils.timezone +from django.db import migrations + +from enterprise_subsidy.apps.subsidy.migration_utils import find_legacy_initial_transactions +from enterprise_subsidy.apps.subsidy.models import SubsidyReferenceChoices + + +def forwards_func(apps, schema_editor): + """ + The core logic of this migration. + """ + # We get the models from the versioned app registry; if we directly import it, it'll be the wrong version. + Transaction = apps.get_model("openedx_ledger", "Transaction") + Deposit = apps.get_model("openedx_ledger", "Deposit") + HistoricalDeposit = apps.get_model("openedx_ledger", "HistoricalDeposit") + SalesContractReferenceProvider = apps.get_model("openedx_ledger", "SalesContractReferenceProvider") + + # Idempotently duplicate all SubsidyReferenceChoices into SalesContractReferenceProvider. + sales_contract_reference_providers = {} + for slug, name in SubsidyReferenceChoices.CHOICES: + sales_contract_reference_providers[slug], _ = SalesContractReferenceProvider.objects.get_or_create( + slug=slug, + defaults={"name": name}, + ) + + # Fetch all "legacy" initial transactions. + legacy_initial_transactions = find_legacy_initial_transactions(Transaction).select_related("ledger__subsidy") + + # Construct all missing Deposits and HistoricalDeposits to backfill, but do not save them yet. + # + # Note: The reason we need to manually create historical objects is that Django's bulk_create() built-in does not + # call post_save hooks, which is normally where history objects are created. Next you might ask why we don't just + # use django-simple-history's bulk_create_with_history() utility function: that's because it attempts to access the + # custom simple history model manager, but unfortunately custom model attributes are unavailable from migrations. + deposits_to_backfill = [] + historical_deposits_to_backfill = [] + for tx in legacy_initial_transactions: + deposit_fields = { + "ledger": tx.ledger, + "transaction": tx, + "desired_deposit_quantity": tx.quantity, + "sales_contract_reference_id": tx.ledger.subsidy.reference_id, + "sales_contract_reference_provider": sales_contract_reference_providers[tx.ledger.subsidy.reference_type], + } + deposit = Deposit(**deposit_fields) + historical_deposit = HistoricalDeposit( + uuid=deposit.uuid, + history_date=django.utils.timezone.now(), + history_type="+", + history_change_reason="Data migration to backfill initial deposits", + **deposit_fields, + ) + deposits_to_backfill.append(deposit) + historical_deposits_to_backfill.append(historical_deposit) + + # Finally, save the missing Deposits and HistoricalDeposits in bulk. + Deposit.objects.bulk_create(deposits_to_backfill, batch_size=50) + HistoricalDeposit.objects.bulk_create(historical_deposits_to_backfill, batch_size=50) + + +class Migration(migrations.Migration): + """ + Migration for backfilling initial deposits. + """ + dependencies = [ + ("subsidy", "0021_alter_historicalsubsidy_options"), + # This migration relies on Deposit.sales_contract_reference_id being an optional field. Django alone cannot + # possibly know about this dependency without our help. + ("openedx_ledger", "0012_optional_deposit_sales_contract_reference"), + ] + + operations = [ + migrations.RunPython(forwards_func), + ] diff --git a/enterprise_subsidy/apps/subsidy/models.py b/enterprise_subsidy/apps/subsidy/models.py index 3d56b651..4dea4241 100644 --- a/enterprise_subsidy/apps/subsidy/models.py +++ b/enterprise_subsidy/apps/subsidy/models.py @@ -21,7 +21,7 @@ from edx_rbac.utils import ALL_ACCESS_CONTEXT from model_utils.models import TimeStampedModel from openedx_ledger import api as ledger_api -from openedx_ledger.models import Adjustment, Ledger, TransactionStateChoices, UnitChoices +from openedx_ledger.models import Ledger, TransactionStateChoices, UnitChoices from openedx_ledger.utils import create_idempotency_key_for_transaction from requests.exceptions import HTTPError from rest_framework import status @@ -115,6 +115,8 @@ class Meta: Metaclass for Subsidy. """ ordering = ['-created'] + verbose_name = 'Subsidy' + verbose_name_plural = 'Subsidies' # Please reserve the "subsidy_type" field name for the future when we use it to distinguish between # LearnerCreditSubsidy vs. SubscriptionSubsidy. @@ -350,19 +352,18 @@ def price_for_content(self, content_key): def current_balance(self): return self.ledger.balance() + @property def total_deposits(self): """ - Returns the sum of all adjustments for the subsidy + the starting balance in USD cents. + Returns the sum of all value added to the subsidy. + + At the time of writing, this includes both deposits AND adjustments, as both are essentially meant to augment + the value added to the subsidy. Returns: - int: Sum of all adjustments and the starting balance in USD cents. - """ - adjustments_for_subsidy = Adjustment.objects.filter(ledger=self.ledger) - total_deposits = sum([ - adjustment.transaction.quantity - for adjustment in adjustments_for_subsidy - ], self.starting_balance) - return total_deposits + int: Sum of all value added to the subsidy, in USD cents. + """ + return self.ledger.total_deposits() def create_transaction( self, diff --git a/enterprise_subsidy/apps/subsidy/signals.py b/enterprise_subsidy/apps/subsidy/signals.py index 28eff8b1..22bad383 100644 --- a/enterprise_subsidy/apps/subsidy/signals.py +++ b/enterprise_subsidy/apps/subsidy/signals.py @@ -4,8 +4,9 @@ from django.db.models.signals import pre_save from django.dispatch import receiver from openedx_ledger.api import create_ledger +from openedx_ledger.models import SalesContractReferenceProvider -from .models import Subsidy +from .models import Subsidy, SubsidyReferenceChoices @receiver(pre_save, sender=Subsidy) @@ -21,6 +22,16 @@ def subsidy_pre_save(sender, instance, *args, **kwargs): # pylint: disable=unus if instance.ledger or not instance._state.adding: return + # In order to call create_ledger() later, we first need to get or create a SalesContractReferenceProvider. In order + # to avoid manual intervention, we mirror the SubsidyReferenceChoices selection into the + # SalesContractReferenceProvider table as needed. The normal steady-state is to always just re-use (get) an existing + # provider. + subsidy_reference_choices = dict((slug, name) for slug, name in SubsidyReferenceChoices.CHOICES) + sales_contract_reference_provider, _ = SalesContractReferenceProvider.objects.get_or_create( + slug=instance.reference_type, + defaults={"name": subsidy_reference_choices[instance.reference_type]}, + ) + # create_ledger() saves the ledger instance. # If a transaction for the starting_balance is created, # that transaction record is also saved during @@ -29,4 +40,6 @@ def subsidy_pre_save(sender, instance, *args, **kwargs): # pylint: disable=unus unit=instance.unit, subsidy_uuid=instance.uuid, initial_deposit=instance.starting_balance, + sales_contract_reference_id=instance.reference_id, + sales_contract_reference_provider=sales_contract_reference_provider, ) diff --git a/enterprise_subsidy/apps/subsidy/tests/test_migration_utils.py b/enterprise_subsidy/apps/subsidy/tests/test_migration_utils.py new file mode 100644 index 00000000..a9d2eb77 --- /dev/null +++ b/enterprise_subsidy/apps/subsidy/tests/test_migration_utils.py @@ -0,0 +1,53 @@ +""" +Tests for the migration_utils.py module. +""" +from django.test import TestCase +from openedx_ledger.constants import INITIAL_DEPOSIT_TRANSACTION_SLUG +from openedx_ledger.models import Transaction +from openedx_ledger.test_utils.factories import AdjustmentFactory, DepositFactory, LedgerFactory, TransactionFactory + +from enterprise_subsidy.apps.subsidy import migration_utils + + +class MigrationUtilsTests(TestCase): + """ + Tests for utils used for migrations. + """ + def test_find_legacy_initial_transactions(self): + """ + Test find_legacy_initial_transactions(), used for a data migration to backfill initial deposits. + """ + ledgers = [ + (LedgerFactory(), False), + (LedgerFactory(), False), + (LedgerFactory(), True), + (LedgerFactory(), False), + (LedgerFactory(), True), + ] + expected_legacy_initial_transactions = [] + for ledger, create_initial_deposit in ledgers: + # Simulate a legacy initial transaction (i.e. transaction WITHOUT a deposit). + initial_transaction = TransactionFactory( + ledger=ledger, + idempotency_key=INITIAL_DEPOSIT_TRANSACTION_SLUG, + quantity=100, + ) + if create_initial_deposit: + # Make it a modern initial deposit by creating a related Deposit. + DepositFactory( + ledger=ledger, + transaction=initial_transaction, + desired_deposit_quantity=initial_transaction.quantity, + ) + else: + # Keep it a legacy initial deposit by NOT creating a related Deposit. + expected_legacy_initial_transactions.append(initial_transaction) + # Throw in a few spend, deposit, and adjustment transactions for fun. + TransactionFactory(ledger=ledger, quantity=-10) + TransactionFactory(ledger=ledger, quantity=-10) + DepositFactory(ledger=ledger, desired_deposit_quantity=50) + tx_to_adjust = TransactionFactory(ledger=ledger, quantity=-5) + AdjustmentFactory(ledger=ledger, adjustment_quantity=5, transaction_of_interest=tx_to_adjust) + + actual_legacy_initial_transactions = migration_utils.find_legacy_initial_transactions(Transaction) + assert set(actual_legacy_initial_transactions) == set(expected_legacy_initial_transactions) diff --git a/enterprise_subsidy/apps/subsidy/tests/test_migrations.py b/enterprise_subsidy/apps/subsidy/tests/test_migrations.py new file mode 100644 index 00000000..6add9813 --- /dev/null +++ b/enterprise_subsidy/apps/subsidy/tests/test_migrations.py @@ -0,0 +1,96 @@ +""" +Test migrations. +""" +import uuid + +import django.utils.timezone +import pytest +from openedx_ledger.constants import INITIAL_DEPOSIT_TRANSACTION_SLUG +from openedx_ledger.models import TransactionStateChoices + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "initial_deposit_exists,subsidy_reference_id", + [ + (False, None), + (False, "abc123"), + (True, None), + (True, "abc123"), + ], +) +def test_migration_0022_backfill_initial_deposits(migrator, initial_deposit_exists, subsidy_reference_id): + """ + Test Backfilling initial deposits via data migration. + """ + old_state = migrator.apply_initial_migration([ + ("subsidy", "0021_alter_historicalsubsidy_options"), + ("openedx_ledger", "0012_optional_deposit_sales_contract_reference"), + ]) + + Subsidy = old_state.apps.get_model("subsidy", "Subsidy") + Ledger = old_state.apps.get_model("openedx_ledger", "Ledger") + Transaction = old_state.apps.get_model("openedx_ledger", "Transaction") + Deposit = old_state.apps.get_model("openedx_ledger", "Deposit") + HistoricalDeposit = old_state.apps.get_model("openedx_ledger", "HistoricalDeposit") + SalesContractReferenceProvider = old_state.apps.get_model("openedx_ledger", "SalesContractReferenceProvider") + + ledger = Ledger.objects.create() + subsidy = Subsidy.objects.create( + ledger=ledger, + starting_balance=100, + reference_id=subsidy_reference_id, + enterprise_customer_uuid=uuid.uuid4(), + ) + transaction = Transaction.objects.create( + ledger=ledger, + idempotency_key=INITIAL_DEPOSIT_TRANSACTION_SLUG, + quantity=subsidy.starting_balance, + state=TransactionStateChoices.COMMITTED + ) + if initial_deposit_exists: + sales_contract_reference_provider = SalesContractReferenceProvider.objects.create( + slug=subsidy.reference_type, + name="Foo Bar", + ) + Deposit.objects.create( + ledger=ledger, + desired_deposit_quantity=transaction.quantity, + transaction=transaction, + sales_contract_reference_id=subsidy_reference_id, # Sometimes this is None. + sales_contract_reference_provider=sales_contract_reference_provider, + ) + HistoricalDeposit.objects.create( + ledger=ledger, + desired_deposit_quantity=transaction.quantity, + transaction=transaction, + sales_contract_reference_id=subsidy_reference_id, + sales_contract_reference_provider=sales_contract_reference_provider, + history_date=django.utils.timezone.now(), + history_type="+", + history_change_reason="Data migration to backfill initial deposits", + ) + + new_state = migrator.apply_tested_migration( + ("subsidy", "0022_backfill_initial_deposits"), + ) + Deposit = new_state.apps.get_model("openedx_ledger", "Deposit") + HistoricalDeposit = new_state.apps.get_model("openedx_ledger", "HistoricalDeposit") + + # Make sure there is exactly one deposit, suggesting that if one already existed it is not re-created. + assert Deposit.objects.all().count() == 1 + assert HistoricalDeposit.objects.all().count() == 1 + + # Finally check that all the deposit values are correct. + assert Deposit.objects.first().ledger.uuid == ledger.uuid + assert Deposit.objects.first().desired_deposit_quantity == subsidy.starting_balance + assert Deposit.objects.first().transaction.uuid == transaction.uuid + assert Deposit.objects.first().sales_contract_reference_id == subsidy_reference_id + assert Deposit.objects.first().sales_contract_reference_provider.slug == subsidy.reference_type + assert HistoricalDeposit.objects.first().ledger.uuid == ledger.uuid + assert HistoricalDeposit.objects.first().desired_deposit_quantity == subsidy.starting_balance + assert HistoricalDeposit.objects.first().transaction.uuid == transaction.uuid + assert HistoricalDeposit.objects.first().sales_contract_reference_id == subsidy_reference_id + assert HistoricalDeposit.objects.first().sales_contract_reference_provider.slug == subsidy.reference_type + assert HistoricalDeposit.objects.first().history_type == "+" + assert HistoricalDeposit.objects.first().history_change_reason == "Data migration to backfill initial deposits" diff --git a/requirements/base.txt b/requirements/base.txt index becb2a09..b018e8a2 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -199,7 +199,7 @@ openedx-events==9.11.0 # -r requirements/base.in # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/base.in packaging==24.1 # via drf-yasg diff --git a/requirements/dev.txt b/requirements/dev.txt index 3686cdb7..0dbfb796 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -169,6 +169,8 @@ django-simple-history==3.4.0 # via # -r requirements/validation.txt # openedx-ledger +django-test-migrations==1.4.0 + # via -r requirements/validation.txt django-waffle==4.1.0 # via # -r requirements/validation.txt @@ -377,7 +379,7 @@ openedx-events==9.11.0 # -r requirements/validation.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/validation.txt packaging==24.1 # via diff --git a/requirements/doc.txt b/requirements/doc.txt index 77635198..9b1a88a6 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -358,7 +358,7 @@ openedx-events==9.11.0 # -r requirements/test.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/test.txt packaging==24.1 # via diff --git a/requirements/production.txt b/requirements/production.txt index 7acddc68..0095298e 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -244,7 +244,7 @@ openedx-events==9.11.0 # -r requirements/base.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/base.txt packaging==24.1 # via diff --git a/requirements/quality.txt b/requirements/quality.txt index ef033acf..b58ff340 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -339,7 +339,7 @@ openedx-events==9.11.0 # -r requirements/test.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/test.txt packaging==24.1 # via diff --git a/requirements/test.in b/requirements/test.in index ff279994..fd499473 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -7,6 +7,7 @@ code-annotations coverage ddt django-dynamic-fixture # library to create dynamic model instances for testing purposes +django-test-migrations # For unit testing data migrations edx-lint factory-boy mock diff --git a/requirements/test.txt b/requirements/test.txt index f696d1d4..d7b472b9 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -140,6 +140,8 @@ django-simple-history==3.4.0 # -c requirements/constraints.txt # -r requirements/base.txt # openedx-ledger +django-test-migrations==1.4.0 + # via -r requirements/test.in django-waffle==4.1.0 # via # -r requirements/base.txt @@ -285,7 +287,7 @@ openedx-events==9.11.0 # -r requirements/base.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via -r requirements/base.txt packaging==24.1 # via diff --git a/requirements/validation.txt b/requirements/validation.txt index b83dc1f8..99ce8c3f 100644 --- a/requirements/validation.txt +++ b/requirements/validation.txt @@ -435,7 +435,7 @@ openedx-events==9.11.0 # -r requirements/test.txt # edx-event-bus-kafka # openedx-ledger -openedx-ledger==1.5.1 +openedx-ledger==1.5.3 # via # -r requirements/quality.txt # -r requirements/test.txt