diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 7c52648..c9cf143 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -25,6 +25,8 @@ openedx_ledger.HistoricalReversal: ".. no_pii:": "This model has no PII" openedx_ledger.HistoricalTransaction: ".. no_pii:": "This model has no PII" +openedx_ledger.HistoricalAdjustment: + ".. no_pii:": "This model has no PII" sessions.Session: ".. no_pii:": "This model has no PII" social_django.Association: diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a63ce47..57c9eda 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ Unreleased ********** * Nothing unreleased +[1.2.0] +******* +* Add an ``Adjustment`` model + [1.1.0] ******* * Add support for Django 4.2 diff --git a/docs/decisions/0007-adjustments.rst b/docs/decisions/0007-adjustments.rst new file mode 100644 index 0000000..a1b1208 --- /dev/null +++ b/docs/decisions/0007-adjustments.rst @@ -0,0 +1,55 @@ +0007 Ledger Adjustments +####################### + +Status +****** +**Accepted** (October 2023) + +Context +******* +We'd like to have the ability to manually adjust the balance of ledgers +in a somewhat unstructured way - that is, in a way that doesn't reverse +an existing transaction. For example, the manager of a customer account +may need to manually "credit" a ledger when: + +* a learner manages to redeem an enrollment via a ledger in a course that our + system should not have allowed them to access at the time of redemption. +* a learner had unforseen technical challenges in taking the course, but + is not able to officially unenroll/reverse the redemption. +* some client-relationship building needs to take place. + +Decision +******** +We'll introduce an ``Adjustment`` model: + +* This is somewhat different from our existing pattern of modeling any modification + to a ledger via models that inherit from ``BaseTransaction`` (like we + do with ``Reversal``). +* Creating an ``Adjustment`` will cause a new ``Transaction`` to be written + to the adjustment's ledger object, with the same quantity as the adjustment record. + This transaction record is the thing that fundamentally changes the + balance of the related ledger. The transaction will be referred to from a foreign key of the ``Adjustment`` + record. +* An adjustment must define some enumerated ``reason`` for being created. The ``reason`` should + be from a fixed set of choices, and those choices should generally **not** overlap + with the strict notion of reversing a transaction. +* An ``Adjustment`` *may* refer to a ``transaction_of_interest`` - this is a foreign + key to another transaction of note that is relevant to the reason for the + creation of the adjustment. It is not required. +* An adjustment *may* include some free-text ``notes`` that help to further + explain why the adjustment exists. + +Consequences +************ +* Adjustments work in kind of the opposite way of reversals: instead of using an + existing ledger-transaction to instantiate a reversal, we'll have a situation + where the action of creating an adjustment happens, which has a side-effect + of creating a transaction to adjust the ledger balance. +* Care must be taken to ensure that adjustments are not over-used. For one, using them + to transfer balance from a customer to a ledger upon the expiration of our business + contracts (i.e. to "renew" a contract) could be inappropriate or destructive from + a back-office recordkeeping perspective. And they should never be used in place + of ``Reversals`` when the latter are applicable. +* It would be beneficial to observe and track the usage of ``Adjustments`` + over time within the system, and perhaps to restrict their usage + via Django Admin tools or "hard" thresholds. diff --git a/openedx_ledger/__init__.py b/openedx_ledger/__init__.py index e676514..912eef9 100644 --- a/openedx_ledger/__init__.py +++ b/openedx_ledger/__init__.py @@ -1,4 +1,4 @@ """ A library that records transactions against a ledger, denominated in units of value. """ -__version__ = "1.1.0" +__version__ = "1.2.0" diff --git a/openedx_ledger/admin.py b/openedx_ledger/admin.py index 94f8377..b190d15 100644 --- a/openedx_ledger/admin.py +++ b/openedx_ledger/admin.py @@ -1,6 +1,7 @@ """ Admin configuration for openedx_ledger models. """ +from django import forms from django.conf import settings from django.contrib import admin from django.http import HttpResponseRedirect @@ -8,7 +9,7 @@ from django_object_actions import DjangoObjectActions from simple_history.admin import SimpleHistoryAdmin -from openedx_ledger import constants, models, views +from openedx_ledger import api, constants, models, views def can_modify(): @@ -36,6 +37,12 @@ class Meta: model = models.Ledger fields = ('uuid', 'idempotency_key', 'unit', 'balance_usd', 'metadata') + # The autocomplete_fields of AdjustmentAdmin include the ledger field, + # which in turn requires that we define here that ledgers are + # searchable by uuid. + search_fields = [ + 'uuid', + ] if can_modify(): readonly_fields = ('uuid', 'balance_usd') else: @@ -75,6 +82,29 @@ class ExternalTransactionReferenceInlineAdmin(admin.TabularInline): model = models.ExternalTransactionReference +class AdjustmentInlineAdmin(admin.TabularInline): + """ + Inline admin configuration for the Adjustment model. + """ + model = models.Adjustment + fk_name = 'transaction' + fields = [ + 'uuid', + 'get_quantity_usd', + 'reason', + 'created', + 'modified', + ] + readonly_fields = fields + show_change_link = True + + @admin.display(description='Amount in U.S. Dollars') + def get_quantity_usd(self, obj): + if not obj._state.adding: # pylint: disable=protected-access + return cents_to_usd_string(obj.adjustment_quantity) + return None + + @admin.register(models.Transaction) class TransactionAdmin(DjangoObjectActions, SimpleHistoryAdmin): """ @@ -98,7 +128,7 @@ class Meta: ) _all_fields = [ field.name for field in models.Transaction._meta.get_fields() - if field.name != 'external_reference' + if field.name not in {'external_reference', 'adjustment', 'adjustment_of_interest'} ] _writable_fields = [ 'fulfillment_identifier', @@ -124,7 +154,11 @@ class Meta: ] else: readonly_fields = _all_fields - inlines = [ExternalTransactionReferenceInlineAdmin] + + def get_inlines(self, request, obj): + if obj and not obj._state.adding: # pylint: disable=protected-access + return [AdjustmentInlineAdmin, ExternalTransactionReferenceInlineAdmin] + return [ExternalTransactionReferenceInlineAdmin] @admin.display(ordering='reversal', description='Has a reversal') def has_reversal(self, obj): @@ -176,3 +210,133 @@ class Meta: model = models.Reversal fields = '__all__' + + +class AdjustmentAdminCreateForm(forms.ModelForm): + """ + Form that allows users to enter adjustment quantities in dollars + instead of cents. + """ + class Meta: + model = models.Adjustment + fields = [ + 'ledger', + 'quantity_usd_input', + 'reason', + 'notes', + 'transaction_of_interest', + 'transaction', + 'adjustment_quantity', + ] + + quantity_usd_input = forms.FloatField( + required=True, + help_text='Amount of adjustment in US Dollars.', + ) + + +class AdjustmentAdminChangeForm(forms.ModelForm): + """ + Form for reading and changing only the allowed fields of an existing adjustment record. + """ + class Meta: + model = models.Adjustment + fields = [ + 'ledger', + 'reason', + 'notes', + 'transaction_of_interest', + 'transaction', + 'adjustment_quantity', + ] + + +@admin.register(models.Adjustment) +class AdjustmentAdmin(SimpleHistoryAdmin): + """ + Admin configuration for the Adjustment model. + """ + form = AdjustmentAdminCreateForm + + _readonly_fields = [ + 'get_quantity_usd', + 'uuid', + 'transaction', + 'adjustment_quantity', + 'created', + 'modified', + ] + + list_display = ( + 'uuid', + 'get_ledger_uuid', + 'get_quantity_usd', + 'reason', + 'created', + 'modified', + ) + list_filter = ( + 'reason', + ) + autocomplete_fields = [ + 'ledger', + 'transaction_of_interest', + ] + + def get_readonly_fields(self, request, obj=None): + """ + Don't allow changing the ledger if we've already saved the adjustment record. + """ + if obj and not obj._state.adding: # pylint: disable=protected-access + return ['ledger'] + self._readonly_fields + return self._readonly_fields + + def get_fields(self, request, obj=None): + """ + Don't include the ``quantity_usd_input`` field unless we're creating a new adjustment. + """ + # When we're adding a new adjustment, use default fields + if not obj: + return super().get_fields(request, obj) + else: + # Don't show the USD amount input field on read/change + return [ + field for field in super().get_fields(request, obj) + if field != 'quantity_usd_input' + ] + + def get_form(self, request, obj=None, **kwargs): # pylint: disable=arguments-differ + """ + Don't worry about validating the ``quantity_usd_input`` unless we're creating a new adjustment. + """ + if obj and not obj._state.adding: # pylint: disable=protected-access + kwargs['form'] = AdjustmentAdminChangeForm + return super().get_form(request, obj, **kwargs) + + @admin.display(description='Amount in U.S. Dollars') + def get_quantity_usd(self, obj): + if not obj._state.adding: # pylint: disable=protected-access + return cents_to_usd_string(obj.adjustment_quantity) + return None + + @admin.display(ordering='uuid', description='Ledger uuid') + def get_ledger_uuid(self, obj): + return obj.ledger.uuid + + def save_model(self, request, obj, form, change): + if change: + super().save_model(request, obj, form, change) + else: + raw_usd_input = form.cleaned_data.get('quantity_usd_input') + quantity_usd_cents = raw_usd_input * constants.CENTS_PER_US_DOLLAR + # AED 2023-10-16: Use the auto-generated "stub" UUID for the Adjustment record + # to persist the Adjustment record, so that Django Admin doesn't get lost + # when a user clicks "Save and Continue Editing". + api.create_adjustment( + adjustment_uuid=obj.uuid, + ledger=obj.ledger, + quantity=quantity_usd_cents, + reason=obj.reason, + notes=obj.notes, + transaction_of_interest=obj.transaction_of_interest, + ) diff --git a/openedx_ledger/api.py b/openedx_ledger/api.py index a08f93a..8f1bd1f 100644 --- a/openedx_ledger/api.py +++ b/openedx_ledger/api.py @@ -1,11 +1,16 @@ """ The openedx_ledger python API. """ +import logging +import uuid + from django.db.transaction import atomic, get_connection from openedx_ledger import models, utils from openedx_ledger.signals.signals import TRANSACTION_REVERSED +logger = logging.getLogger(__name__) + class LedgerBalanceExceeded(Exception): """ @@ -19,6 +24,19 @@ class NonCommittedTransactionError(Exception): """ +class CannotReverseAdjustmentError(Exception): + """ + Raised when a caller attempts to reverse the transaction that comprises + an ``Adjustment`` record. + """ + + +class AdjustmentCreationError(Exception): + """ + Raised when, for whatever reason, an adjustment could not be created. + """ + + def create_transaction( ledger, quantity, @@ -89,6 +107,12 @@ def reverse_full_transaction(transaction, idempotency_key, **metadata): openedx_ledger.api.NonCommittedTransactionError: Raises this if the transaction is not in a COMMITTED state. """ + # Do not allow the reversal of transactions that comprise an Adjustment + if transaction.get_adjustment(): + raise CannotReverseAdjustmentError( + f"Transaction {transaction.uuid} comprises an Adjustment, can't reverse." + ) + with atomic(durable=True): # select the transaction and any reversals # if there is a reversal: return, no work to do here @@ -97,7 +121,8 @@ def reverse_full_transaction(transaction, idempotency_key, **metadata): if transaction.state != models.TransactionStateChoices.COMMITTED: raise NonCommittedTransactionError( - "Cannot reverse transaction because it is not in a committed state." + f"Cannot reverse transaction {transaction.uuid} " + "because it is not in a committed state." ) reversal, _ = models.Reversal.objects.get_or_create( @@ -147,3 +172,51 @@ def create_ledger(unit=None, idempotency_key=None, subsidy_uuid=None, initial_de ) return ledger + + +def create_adjustment( + ledger, + quantity, + adjustment_uuid=None, + reason=models.AdjustmentReasonChoices.TECHNICAL_CHALLENGES, + notes=None, + idempotency_key=None, + transaction_of_interest=None, + **metadata, +): + """ + Creates a new Transaction and related Adjustment record + to adjust the balance of the given ledger. + """ + if idempotency_key is None: + tx_idempotency_key = f'{ledger.uuid}-adjustment-{quantity}-reason-{uuid.uuid4()}' + else: + tx_idempotency_key = idempotency_key + + try: + with atomic(): + transaction = create_transaction( + ledger, + quantity, + idempotency_key=tx_idempotency_key, + state=models.TransactionStateChoices.COMMITTED, + **metadata, + ) + kwargs = {} + if adjustment_uuid: + kwargs['uuid'] = adjustment_uuid + adjustment = models.Adjustment.objects.create( + ledger=ledger, + adjustment_quantity=quantity, + transaction=transaction, + transaction_of_interest=transaction_of_interest, + reason=reason, + notes=notes, + **kwargs, + ) + except Exception as exc: + message = f'Failed to create adjustment in ledger {ledger.uuid} for amount {quantity}' + logger.exception(message) + raise AdjustmentCreationError(str(exc)) from exc + + return adjustment diff --git a/openedx_ledger/migrations/0008_adjustment_model.py b/openedx_ledger/migrations/0008_adjustment_model.py new file mode 100644 index 0000000..56305fc --- /dev/null +++ b/openedx_ledger/migrations/0008_adjustment_model.py @@ -0,0 +1,62 @@ +# Generated by Django 3.2.19 on 2023-10-16 16:26 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +import simple_history.models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('openedx_ledger', '0007_alter_externalfulfillmentprovider_name'), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalAdjustment', + fields=[ + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('uuid', models.UUIDField(db_index=True, default=uuid.uuid4, editable=False)), + ('adjustment_quantity', models.BigIntegerField(help_text='How many units to adjust for in the related transaction.')), + ('reason', models.CharField(choices=[('unauthorized_enrollment', 'Unauthorized enrollment'), ('poor_content_fit', 'Poor content fit'), ('technical_challenges', 'Technical challenges'), ('missed_refund_or_date', 'Missed refund or date'), ('good_faith', 'Good faith/Relationship building')], db_index=True, default='technical_challenges', help_text='The primary reason for the existence of this adjustment.', max_length=255)), + ('notes', models.TextField(blank=True, help_text='Any additional context you have for the existence of this adjustment.', null=True)), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('ledger', models.ForeignKey(blank=True, db_constraint=False, help_text='The Ledger instance with which this Adjustment is associated.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_ledger.ledger')), + ('transaction', models.ForeignKey(blank=True, db_constraint=False, help_text='The Transaction instance which adjusts the balance of the relevant ledger.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_ledger.transaction')), + ('transaction_of_interest', models.ForeignKey(blank=True, db_constraint=False, help_text='Any transaction of interest w.r.t. the reason for being of this adjustment. For example, the transaction of interest may point to some transaction record for which the enrolling user is unsatisfied, but for which we cannot issue a reversal due to business rules.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_ledger.transaction')), + ], + options={ + 'verbose_name': 'historical adjustment', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': 'history_date', + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='Adjustment', + fields=[ + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('uuid', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True)), + ('adjustment_quantity', models.BigIntegerField(help_text='How many units to adjust for in the related transaction.')), + ('reason', models.CharField(choices=[('unauthorized_enrollment', 'Unauthorized enrollment'), ('poor_content_fit', 'Poor content fit'), ('technical_challenges', 'Technical challenges'), ('missed_refund_or_date', 'Missed refund or date'), ('good_faith', 'Good faith/Relationship building')], db_index=True, default='technical_challenges', help_text='The primary reason for the existence of this adjustment.', max_length=255)), + ('notes', models.TextField(blank=True, help_text='Any additional context you have for the existence of this adjustment.', null=True)), + ('ledger', models.ForeignKey(help_text='The Ledger instance with which this Adjustment is associated.', on_delete=django.db.models.deletion.CASCADE, related_name='adjustments', to='openedx_ledger.ledger')), + ('transaction', models.OneToOneField(help_text='The Transaction instance which adjusts the balance of the relevant ledger.', on_delete=django.db.models.deletion.CASCADE, related_name='adjustment', to='openedx_ledger.transaction')), + ('transaction_of_interest', models.OneToOneField(blank=True, help_text='Any transaction of interest w.r.t. the reason for being of this adjustment. For example, the transaction of interest may point to some transaction record for which the enrolling user is unsatisfied, but for which we cannot issue a reversal due to business rules.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='adjustment_of_interest', to='openedx_ledger.transaction')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/openedx_ledger/models.py b/openedx_ledger/models.py index 5f0bd27..b515fdd 100644 --- a/openedx_ledger/models.py +++ b/openedx_ledger/models.py @@ -54,6 +54,24 @@ class TransactionStateChoices: ) +class AdjustmentReasonChoices: + """ + Allowed choices for the ``Adjustment.reason`` field. + """ + UNAUTHROZIED_ENROLLMENT = 'unauthorized_enrollment' + POOR_CONTENT_FIT = 'poor_content_fit' + TECHNICAL_CHALLENGES = 'technical_challenges' + MISSED_REFUND_OR_DATE = 'missed_refund_or_date' + GOOD_FAITH = 'good_faith' + CHOICES = ( + (UNAUTHROZIED_ENROLLMENT, 'Unauthorized enrollment'), + (POOR_CONTENT_FIT, 'Poor content fit'), + (TECHNICAL_CHALLENGES, 'Technical challenges'), + (MISSED_REFUND_OR_DATE, 'Missed refund or date'), + (GOOD_FAITH, 'Good faith/Relationship building'), + ) + + class LedgerLockAttemptFailed(Exception): """ Raise when attempt to lock Ledger failed due to an already existing lock. @@ -362,6 +380,17 @@ def get_reversal(self): except Reversal.DoesNotExist: return None + def get_adjustment(self): + """ + Convenience method for fetching the ``Adjustment`` which + created this transaction, if one exists. If one does not exist, + returns ``None``. + """ + try: + return self.adjustment # pylint: disable=no-member + except Adjustment.DoesNotExist: + return None + class ExternalFulfillmentProvider(TimeStampedModel): """ @@ -491,3 +520,70 @@ class Meta: history = HistoricalRecords() # Reversal quantities should always have the opposite sign of the transaction (i.e. negative) # We have to enforce this somehow... + + +class Adjustment(TimeStampedModelWithUuid): + """ + Represents some adjustment to the balance of a ledger via + a transaction (with quantity > 0) and some audit fields. + + .. no_pii: + """ + ledger = models.ForeignKey( + Ledger, + related_name='adjustments', + null=False, + on_delete=models.CASCADE, + help_text=( + "The Ledger instance with which this Adjustment is associated." + ) + ) + adjustment_quantity = models.BigIntegerField( + null=False, + blank=False, + help_text=( + "How many units to adjust for in the related transaction." + ), + ) + transaction = models.OneToOneField( + Transaction, + related_name='adjustment', + unique=True, + null=False, + on_delete=models.CASCADE, + help_text=( + "The Transaction instance which adjusts the balance of the relevant ledger." + ), + ) + transaction_of_interest = models.OneToOneField( + Transaction, + related_name='adjustment_of_interest', + null=True, + blank=True, + on_delete=models.SET_NULL, + help_text=( + "Any transaction of interest w.r.t. the reason for being of this adjustment. " + "For example, the transaction of interest may point to some transaction record " + "for which the enrolling user is unsatisfied, but for which we cannot issue a reversal " + "due to business rules." + ), + ) + reason = models.CharField( + max_length=255, + blank=False, + null=False, + choices=AdjustmentReasonChoices.CHOICES, + default=AdjustmentReasonChoices.TECHNICAL_CHALLENGES, + db_index=True, + help_text=( + 'The primary reason for the existence of this adjustment.' + ), + ) + notes = models.TextField( + blank=True, + null=True, + help_text=( + 'Any additional context you have for the existence of this adjustment.' + ), + ) + history = HistoricalRecords() diff --git a/openedx_ledger/test_utils/factories.py b/openedx_ledger/test_utils/factories.py index 1ff37cc..564e2d7 100644 --- a/openedx_ledger/test_utils/factories.py +++ b/openedx_ledger/test_utils/factories.py @@ -6,6 +6,7 @@ import factory from openedx_ledger.models import ( + Adjustment, ExternalFulfillmentProvider, ExternalTransactionReference, Ledger, @@ -80,3 +81,15 @@ class Meta: idempotency_key = factory.LazyFunction(uuid4) state = TransactionStateChoices.COMMITTED quantity = factory.Faker("random_int", min=100, max=10000) + + +class AdjustmentFactory(factory.django.DjangoModelFactory): + """ + Test factory for the `Adjustment` model. + """ + class Meta: + model = Adjustment + + ledger = factory.SubFactory(LedgerFactory) + transaction = factory.SubFactory(TransactionFactory) + adjustment_quantity = factory.Faker("random_int", min=100, max=10000) diff --git a/openedx_ledger/tests/test_views.py b/openedx_ledger/tests/test_views.py index de41fd9..07906e4 100644 --- a/openedx_ledger/tests/test_views.py +++ b/openedx_ledger/tests/test_views.py @@ -11,7 +11,7 @@ from openedx_ledger.models import Reversal, TransactionStateChoices from openedx_ledger.signals.signals import TRANSACTION_REVERSED -from openedx_ledger.test_utils.factories import LedgerFactory, ReversalFactory, TransactionFactory +from openedx_ledger.test_utils.factories import AdjustmentFactory, LedgerFactory, ReversalFactory, TransactionFactory @pytest.mark.django_db @@ -130,11 +130,41 @@ def test_reverse_transaction_view_post_with_non_committed_transaction(self): response = self.client.post(url) self.assertEqual(response.status_code, 400) + expected_content = ( + 'Transaction Reversal failed: ' + f'Cannot reverse transaction {self.transaction.uuid} ' + 'because it is not in a committed state.' + ) self.assertEqual( - response.content, - b'Transaction Reversal failed: ' - b'Cannot reverse transaction because it is not in a committed state.' + response.content.decode(), + expected_content, ) signal_received.assert_not_called() assert Reversal.objects.count() == 0 + + def test_reverse_transaction_view_post_with_related_adjustment(self): + """ + Tests that you can't reverse the transaction of an adjustment. + """ + signal_received = MagicMock() + TRANSACTION_REVERSED.connect(signal_received) + + assert Reversal.objects.count() == 0 + + adjustment = AdjustmentFactory() + + url = self.get_reverse_transaction_url(adjustment.transaction.uuid) + response = self.client.post(url) + + # Assert that the post request returns a 400 status code + self.assertEqual(response.status_code, 400) + # Assert that the response contains the expected error message + expected_content = ( + "Transaction Reversal failed: " + f"Transaction {adjustment.transaction.uuid} comprises an Adjustment, can't reverse." + ) + self.assertEqual(response.content.decode(), expected_content) + + # Assert that the transaction reversal signal was not sent or received + signal_received.assert_not_called() diff --git a/openedx_ledger/views.py b/openedx_ledger/views.py index ab60275..8fd1861 100644 --- a/openedx_ledger/views.py +++ b/openedx_ledger/views.py @@ -10,7 +10,7 @@ from django.urls import reverse from django.views.generic import View -from openedx_ledger.api import NonCommittedTransactionError, reverse_full_transaction +from openedx_ledger.api import CannotReverseAdjustmentError, NonCommittedTransactionError, reverse_full_transaction from openedx_ledger.models import Transaction logger = logging.getLogger(__name__) @@ -74,5 +74,10 @@ def post(self, request, transaction_id): f"ReverseTransactionView Error: transaction is not in a committed state: {transaction_id}" ) return HttpResponseBadRequest(f'Transaction Reversal failed: {error}') + except CannotReverseAdjustmentError as error: + logger.exception( + f"ReverseTransactionView Error: cannot reverse the transaction of an adjustment: {transaction_id}" + ) + return HttpResponseBadRequest(f'Transaction Reversal failed: {error}') url = reverse("admin:openedx_ledger_transaction_change", args=(transaction_id,)) return HttpResponseRedirect(url) diff --git a/pytest.local.ini b/pytest.local.ini new file mode 100644 index 0000000..a7b6a68 --- /dev/null +++ b/pytest.local.ini @@ -0,0 +1,4 @@ +[pytest] +DJANGO_SETTINGS_MODULE = test_settings +addopts = --cov openedx_ledger --cov-report term-missing --cov-report xml -W ignore +norecursedirs = .* docs requirements site-packages diff --git a/tests/test_api.py b/tests/test_api.py index af7417e..db31171 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,10 +1,12 @@ """ Tests for the openedx_ledger Python API. """ +import uuid + import pytest from openedx_ledger import api -from openedx_ledger.models import TransactionStateChoices, UnitChoices +from openedx_ledger.models import AdjustmentReasonChoices, TransactionStateChoices, UnitChoices @pytest.mark.django_db @@ -67,3 +69,72 @@ def test_multiple_reversals(): third_reversal = api.reverse_full_transaction(tx_1, idempotency_key='reversal-1') assert ledger.balance() == 0 assert reversal == third_reversal + + +@pytest.mark.django_db +def test_adjustment_creation_happy_path(): + ledger = api.create_ledger(unit=UnitChoices.USD_CENTS, idempotency_key='my-happy-ledger') + assert ledger.balance() == 0 + + tx_of_interest = api.create_transaction( + ledger, + quantity=5000, + idempotency_key='tx_of_interest', + state=TransactionStateChoices.COMMITTED, + ) + + assert ledger.balance() == 5000 + + test_uuid = uuid.uuid4() + + adjustment = api.create_adjustment( + ledger, + adjustment_uuid=test_uuid, + quantity=-100, + idempotency_key='unique-string-for-transaction', + reason=AdjustmentReasonChoices.POOR_CONTENT_FIT, + notes='Long form notes about this record', + transaction_of_interest=tx_of_interest, + some_key='some_value', # tests that metadata is recorded on the adjustment's transaction + ) + + assert adjustment.uuid == test_uuid + assert adjustment.transaction.state == TransactionStateChoices.COMMITTED + assert adjustment.transaction.uuid != tx_of_interest.uuid + assert adjustment.transaction.idempotency_key == 'unique-string-for-transaction' + assert adjustment.adjustment_quantity == -100 + assert adjustment.reason == AdjustmentReasonChoices.POOR_CONTENT_FIT + assert adjustment.notes == 'Long form notes about this record' + assert adjustment.transaction_of_interest == tx_of_interest + assert adjustment.transaction.metadata == { + 'some_key': 'some_value', + } + assert ledger.balance() == 4900 + + +@pytest.mark.django_db +def test_adjustment_creation_balance_exceeded(): + ledger = api.create_ledger(unit=UnitChoices.USD_CENTS, idempotency_key='my-happy-ledger') + + # Add a little bit of adjustment balance first + first_adjustment = api.create_adjustment( + ledger, + quantity=50, + reason=AdjustmentReasonChoices.POOR_CONTENT_FIT, + notes='Long form notes about this record', + ) + assert ledger.balance() == 50 + assert f'{ledger.uuid}' in str(first_adjustment.transaction.idempotency_key) + + with pytest.raises( + api.AdjustmentCreationError, + match="A Transaction was not created because it would exceed the ledger balance." + ): + api.create_adjustment( + ledger, + quantity=-100, + reason=AdjustmentReasonChoices.POOR_CONTENT_FIT, + notes='Long form notes about this record', + ) + assert ledger.balance() == 50 + assert ledger.transactions.all().count() == 1