Merge pull request #513 from openedx/pwnage101/ENT-9133

feat: Add policy django action to deposit funds

docs/decisions/0022-deposit-creation-ux.rst

@@ -0,0 +1,83 @@
+0022 Deposit Creation UX
+************************
+
+Status
+======
+**Adopted** (Jul 2024)
+
+Context
+=======
+
+Support for "topping up" leaner credit plans (a.k.a. adding value to a Subsidy) has been added to
+openedx-ledger and enterprise-subsidy via Deposit and related models via these PRs:
+
+1. `feat: Deposit model and supporting functionality <https://github.com/openedx/openedx-ledger/pull/91>`_
+2. `feat: Ledger creation is now capable of initial Deposit creation <https://github.com/openedx/openedx-ledger/pull/96>`_
+3. `fix: deposit sales references should be optional <https://github.com/openedx/openedx-ledger/pull/97>`_
+4. `feat: Add data migration to backfill legacy initial deposits <https://github.com/openedx/enterprise-subsidy/pull/269>`_
+5. `fix: make initial deposit backfill defensive against subsidy-less ledgers <https://github.com/openedx/enterprise-subsidy/pull/271>`_
+6. `fix: make deposit backfill migration defensive against bogus subsidy reference type <https://github.com/openedx/enterprise-subsidy/pull/272>`_
+
+However, the UX for creating a deposit and making the additional value available to learners involves multiple
+potentially error-prone steps across multiple services:
+
+1. [enterprise-access django admin] Lookup the Subsidy UUID for the policy to which you want to add redeemable value.
+2. [enterprise-subsidy django admin] Lookup the Ledger UUID for the Subsidy.
+3. [enterprise-subsidy django admin] Create a Deposit against that Ledger.
+4. [enterprise-access django admin] Increase the spend_limit for the Policy.
+
+Decision
+========
+
+Create a new Django Admin Action for the SubsidyAccessPolicy edit page called "Deposit Funds" perform all 4 steps
+automatically. In order to support this from enterprise-access, we'd also need to expose a Deposit creation API endpoint
+from enterprise-subsidy.
+
+In terms of the form fields, we will request only the smallest possible subset of information to avoid errors:
+
+- The deposit amount (in USD)
+- The Salesforce Opportunity Line Item ID
+
+Note that until now, the enterprise-access codebase was not aware of any types of sales CRMs. This decision will result
+in needing to hard-code a selection of the specific type of sales contract reference provider (the Salesforce
+Opportunity Line Item ID).
+
+Demonstration UX
+----------------
+
+Here's what the action button on the Policy edit page may look like:
+
+|deposit_action_button|
+
+And here's what the action form may look like:
+
+|deposit_action_form|
+
+Consequences
+============
+
+The enterprise-access codebase will become more coupled to enterprise-subsidy/openedx-ledger models, especially
+pertaining to the sales contract reference provider slug & name. If we ever change the default sales contract identifier
+in the enterprise-subsidy codebase, we'd need to remember update the provider slug & name in enterprise-access code to
+match.
+
+This consequence can be somewhat mitigated by consolidating all cross-service couplings into django settings, for
+example (enterprise_access/settings/base.py)::
+
+  SALES_CONTRACT_REFERENCE_PROVIDER_NAME = 'Salesforce OpportunityLineItem'
+  SALES_CONTRACT_REFERENCE_PROVIDER_SLUG = 'salesforce_opportunity_line_item'
+
+These values can then be referenced in custom admin forms and underlying view logic.
+
+Alternatives Considered
+=======================
+
+We could instead have added deposit creation support to the existing "learner credit provisioning tool" feature in
+support-tools. However, at the time of writing, further investment into the provisioning tool has been halted.
+
+We could have added a brand new tool to support-tools under the Learner Credit section, which arguably would be more
+intuitive and closer to where ECS support staff typically spend time, but that would have required a larger investment,
+as django admin actions provide a significant portion of the feature out-of-the-box.
+
+.. |deposit_action_button|  image:: ../images/0022-deposit-action-button.png
+.. |deposit_action_form|  image:: ../images/0022-deposit-action-form.png

enterprise_access/apps/subsidy_access_policy/admin/__init__.py

@@ -19,7 +19,10 @@
 from enterprise_access.apps.api.serializers.subsidy_access_policy import SubsidyAccessPolicyResponseSerializer
 from enterprise_access.apps.subsidy_access_policy import constants, models
 from enterprise_access.apps.subsidy_access_policy.admin.utils import UrlNames
-from enterprise_access.apps.subsidy_access_policy.admin.views import SubsidyAccessPolicySetLateRedemptionView
+from enterprise_access.apps.subsidy_access_policy.admin.views import (
+    SubsidyAccessPolicyDepositFundsView,
+    SubsidyAccessPolicySetLateRedemptionView
+)
 
 logger = logging.getLogger(__name__)
 
@@ -81,6 +84,7 @@ class BaseSubsidyAccessPolicyMixin(DjangoObjectActions, SimpleHistoryAdmin):
 
     change_actions = (
         'set_late_redemption',
+        'deposit_funds',
     )
 
     @action(
@@ -95,6 +99,18 @@ def set_late_redemption(self, request, obj):
         set_late_redemption_url = reverse('admin:' + UrlNames.SET_LATE_REDEMPTION, args=(obj.uuid,))
         return HttpResponseRedirect(set_late_redemption_url)
 
+    @action(
+        label='Deposit Funds',
+        description='Top-up the subsidy and spend_limit associated with this policy'
+    )
+    def deposit_funds(self, request, obj):
+        """
+        Object tool handler method - redirects to deposit_funds view.
+        """
+        # url names coming from get_urls are prefixed with 'admin' namespace
+        deposit_funds_url = reverse('admin:' + UrlNames.DEPOSIT_FUNDS, args=(obj.uuid,))
+        return HttpResponseRedirect(deposit_funds_url)
+
     def get_urls(self):
         """
         Returns the additional urls used by the custom object tools.
@@ -105,6 +121,11 @@ def get_urls(self):
                 self.admin_site.admin_view(SubsidyAccessPolicySetLateRedemptionView.as_view()),
                 name=UrlNames.SET_LATE_REDEMPTION,
             ),
+            re_path(
+                r"^([^/]+)/deposit_funds",
+                self.admin_site.admin_view(SubsidyAccessPolicyDepositFundsView.as_view()),
+                name=UrlNames.DEPOSIT_FUNDS,
+            ),
         ]
         return additional_urls + super().get_urls()
 

enterprise_access/apps/subsidy_access_policy/admin/forms.py

@@ -2,6 +2,7 @@
 Forms to be used for subsidy_access_policy django admin.
 """
 from django import forms
+from django.conf import settings
 from django.utils.translation import gettext as _
 
 
@@ -33,3 +34,30 @@ class SetLateRedemptionForm(forms.Form):
         help_text=_("Unless disabled now, late redemptions will be disabled at midnight UTC of the selected day."),
         required=True,
     )
+
+
+class DepositFundsForm(forms.Form):
+    """
+    Form to deposit funds into a subsidy from a policy.
+    """
+    desired_quantity_usd = forms.FloatField(
+        label=_("Top-up quantity in USD"),
+        help_text=_(
+            "Amount of funds to add to the associated Subsidy, and also by which to increase this Policy's spend_limit."
+        ),
+        required=True,
+        min_value=1,
+    )
+    sales_contract_reference_id = forms.RegexField(
+        label=settings.SALES_CONTRACT_REFERENCE_PROVIDER_NAME,
+        help_text=_(
+            "Reference the original sales object that originated this deposit. "
+            "This must be the 18-character case-insensitive version of the ID."
+        ),
+        required=True,
+        regex="^00[kK][0-9a-zA-Z]{15}$",
+        error_messages={
+            "invalid": 'Salesforce Opportunity Line Item ID is invalid. Must be 18 characters and start with "00k".',
+            "required": 'Salesforce Opportunity Line Item ID is required.',
+        }
+    )

enterprise_access/apps/subsidy_access_policy/admin/utils.py

@@ -8,3 +8,4 @@ class UrlNames:
     Collection on URL names used in admin
     """
     SET_LATE_REDEMPTION = "set_late_redemption"
+    DEPOSIT_FUNDS = "deposit_funds"

enterprise_access/apps/subsidy_access_policy/admin/views.py

@@ -11,7 +11,9 @@
 from django.utils.translation import gettext as _
 from django.views.generic import View
 
+from enterprise_access.apps.subsidy_access_policy import constants
 from enterprise_access.apps.subsidy_access_policy.admin.forms import (
+    DepositFundsForm,
     LateRedemptionDaysFromNowChoices,
     SetLateRedemptionForm
 )
@@ -91,3 +93,71 @@ def post(self, request, policy_uuid):
             'ENTERPRISE_LEARNER_PORTAL_URL': settings.ENTERPRISE_LEARNER_PORTAL_URL
         }
         return render(request, self.template, context)
+
+
+class SubsidyAccessPolicyDepositFundsView(View):
+    """
+    View which allows admins to deposit funds into a Subsidy from a Policy.
+    """
+    template = "subsidy_access_policy/admin/deposit_funds.html"
+
+    def get(self, request, policy_uuid):
+        """
+        Handle GET request - render "Deposit Funds" form.
+
+        Args:
+            request (django.http.request.HttpRequest): Request instance
+            policy_uuid (str): Subsidy Access Policy UUID
+
+        Returns:
+            django.http.response.HttpResponse: HttpResponse
+        """
+        policy = SubsidyAccessPolicy.objects.get(uuid=policy_uuid)
+        opts = policy._meta
+        context = {
+            'deposit_funds_form': DepositFundsForm(),
+            'subsidy_access_policy': policy,
+            'opts': opts,
+        }
+        return render(request, self.template, context)
+
+    def post(self, request, policy_uuid):
+        """
+        Handle POST request - handle form submissions.
+
+        Arguments:
+            request (django.http.request.HttpRequest): Request instance
+            policy_uuid (str): Subsidy Access Policy UUID
+
+        Returns:
+            django.http.response.HttpResponse: HttpResponse
+        """
+        policy = SubsidyAccessPolicy.objects.get(uuid=policy_uuid)
+        opts = policy._meta
+        deposit_funds_form = DepositFundsForm(request.POST)
+
+        if deposit_funds_form.is_valid():
+            desired_deposit_quantity_usd = deposit_funds_form.cleaned_data.get('desired_quantity_usd')
+            sales_contract_reference_id = deposit_funds_form.cleaned_data.get('sales_contract_reference_id')
+
+            policy.create_deposit(
+                desired_deposit_quantity=desired_deposit_quantity_usd * constants.CENTS_PER_DOLLAR,
+                sales_contract_reference_id=sales_contract_reference_id,
+                sales_contract_reference_provider=settings.SALES_CONTRACT_REFERENCE_PROVIDER_SLUG,
+            )
+
+            messages.success(request, _("Successfully deposited funds."))
+
+            # Redirect to form GET if everything went smooth.
+            deposit_funds_url = reverse("admin:" + UrlNames.DEPOSIT_FUNDS, args=(policy_uuid,))
+            return HttpResponseRedirect(deposit_funds_url)
+        else:
+            messages.error(request, _("One or more invalid inputs."))
+
+        # Form validation failed. Re-render form.
+        context = {
+            'deposit_funds_form': deposit_funds_form,
+            'subsidy_access_policy': policy,
+            'opts': opts,
+        }
+        return render(request, self.template, context)

enterprise_access/apps/subsidy_access_policy/models.py

@@ -1038,6 +1038,37 @@ def resolve_policy(cls, redeemable_policies):
         logger.info('resolve_policy multiple policies resolved')
         return sorted_policies[0]
 
+    def create_deposit(
+        self,
+        desired_deposit_quantity,
+        sales_contract_reference_id,
+        sales_contract_reference_provider,
+        metadata=None,
+    ):
+        """
+        Create a Deposit for the associated Subsidy and update this Policy's spend_limit.
+
+        Alternatively, this is referred to as a "Top-Up".
+
+        Raises:
+            SubsidyAPIHTTPError if the Subsidy API request failed.
+        """
+        deposit_kwargs = {
+            "subsidy_uuid": self.subsidy_uuid,
+            "desired_deposit_quantity": desired_deposit_quantity,
+            "sales_contract_reference_id": sales_contract_reference_id,
+            "sales_contract_reference_provider": sales_contract_reference_provider,
+            "metadata": metadata,
+        }
+        logger.info("Attempting deposit creation with arguments %s", deposit_kwargs)
+        try:
+            self.subsidy_client.create_subsidy_deposit(**deposit_kwargs)
+        except requests.exceptions.HTTPError as exc:
+            logger.exception("Deposit creation request failed, skipping updating policy spend_limit.")
+            raise SubsidyAPIHTTPError() from exc
+        self.spend_limit += desired_deposit_quantity
+        self.save()
+
     def delete(self, *args, **kwargs):
         """
         Perform a soft-delete, overriding the standard delete() method to prevent hard-deletes.

enterprise_access/apps/subsidy_access_policy/tests/test_models.py

@@ -1,6 +1,7 @@
 """
 Tests for subsidy_access_policy models.
 """
+import contextlib
 from datetime import datetime, timedelta
 from unittest.mock import ANY, PropertyMock, patch
 from uuid import uuid4
@@ -734,6 +735,64 @@ def test_redeem_pass_late_enrollment(
         assert self.mock_subsidy_client.create_subsidy_transaction.call_args.kwargs['metadata'] \
             == expected_metadata_sent_to_subsidy
 
+    @ddt.data(
+        {
+            'old_spend_limit': 100,
+            'deposit_quantity': 50,
+            'api_side_effect': None,
+            'expected_spend_limit': 150,
+        },
+        {
+            'old_spend_limit': 100,
+            'deposit_quantity': 50,
+            'api_side_effect': requests.exceptions.HTTPError,
+            'expected_spend_limit': 100,
+        },
+    )
+    @ddt.unpack
+    def test_create_deposit(
+        self,
+        old_spend_limit,
+        deposit_quantity,
+        api_side_effect,
+        expected_spend_limit,
+    ):
+        """
+        Test the Policy.create_deposit() function.
+        """
+        policy = PerLearnerSpendCapLearnerCreditAccessPolicyFactory(
+            spend_limit=old_spend_limit,
+            per_learner_spend_limit=None,
+        )
+        subsidy_record_patcher = patch.object(policy, 'subsidy_record')
+        mock_subsidy_record = subsidy_record_patcher.start()
+        mock_subsidy_record.return_value = {
+            'id': 1,
+            'active_datetime': datetime.utcnow() - timedelta(days=1),
+            'expiration_datetime': datetime.utcnow() + timedelta(days=1),
+            'is_active': True,
+            'current_balance': 9999,
+            'total_deposits': 9999,
+        }
+        self.mock_subsidy_client.create_subsidy_deposit.side_effect = api_side_effect
+        assert_raises_or_not = self.assertRaises(SubsidyAPIHTTPError) if api_side_effect else contextlib.nullcontext()
+        with assert_raises_or_not:
+            policy.create_deposit(
+                desired_deposit_quantity=deposit_quantity,
+                sales_contract_reference_id='test-ref-id',
+                sales_contract_reference_provider='test-slug',
+                metadata={'foo': 'bar'},
+            )
+        self.mock_subsidy_client.create_subsidy_deposit.assert_called_once_with(
+            subsidy_uuid=policy.subsidy_uuid,
+            desired_deposit_quantity=deposit_quantity,
+            sales_contract_reference_id='test-ref-id',
+            sales_contract_reference_provider='test-slug',
+            metadata={'foo': 'bar'},
+        )
+        policy.refresh_from_db()
+        assert policy.spend_limit == expected_spend_limit
+
 
 class SubsidyAccessPolicyResolverTests(TestCase):
     """ SubsidyAccessPolicy.resolve_policy() tests. """

enterprise_access/settings/base.py

@@ -509,3 +509,11 @@ def root(*path_fragments):
 BRAZE_GROUPS_EMAIL_AUTO_REMINDER_DAY_50_CAMPAIGN = ''
 BRAZE_GROUPS_EMAIL_AUTO_REMINDER_DAY_65_CAMPAIGN = ''
 BRAZE_GROUPS_EMAIL_AUTO_REMINDER_DAY_85_CAMPAIGN = ''
+
+# The "Desposit Funds" button (custom django object action) triggers an API call which needs to pass a sales contract
+# reference provider slug matching one SalesContractReferenceProvider in the enterprise-subsidy database. Since these
+# slugs are operator-defined at runtime, this codebase cannot hard-code the value. However, the least we can do is
+# inherit the same default:
+# https://github.com/openedx/enterprise-subsidy/blob/70e1a13f9f9b1be6a09a2c2f1a02e7a46315eaa6/enterprise_subsidy/apps/subsidy/models.py#L67
+SALES_CONTRACT_REFERENCE_PROVIDER_NAME = 'Salesforce OpportunityLineItem'
+SALES_CONTRACT_REFERENCE_PROVIDER_SLUG = 'salesforce_opportunity_line_item'