Skip to content

Commit

Permalink
feat: get_list_price refactoring for assigned price.
Browse files Browse the repository at this point in the history 
ENT-8040 | Computes the list price dict returned by
the `can-redeem` view from the value of the `LearnerContentAssignment.quantity`
for assignment-based policies.
  • Loading branch information
@iloveagent57
iloveagent57 committed Dec 7, 2023
1 parent a787b3f commit f9c5115
Show file tree
Hide file tree
Showing 6 changed files with 263 additions and 73 deletions.
127 changes: 108 additions & 19 deletions enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1486,12 +1486,11 @@ def test_credits_available_endpoint_with_content_assignments(
self.assertEqual(response_json[0]['learner_content_assignments'][0], expected_learner_content_assignment)


@ddt.ddt
class TestSubsidyAccessPolicyCanRedeemView(APITestWithMocks):
class BaseCanRedeemTestMixin:
"""
Tests for the can-redeem view
Mixin to help with customer data, JWT cookies, and mock setup
for testing can-redeem view.
"""

def setUp(self):
super().setUp()

Expand All @@ -1501,13 +1500,6 @@ def setUp(self):
'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE,
'context': self.enterprise_uuid,
}])

self.redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory(
enterprise_customer_uuid=self.enterprise_uuid,
spend_limit=500000,
)
self.non_redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory()

self.subsidy_access_policy_can_redeem_endpoint = reverse(
"api:v1:policy-redemption-can-redeem",
kwargs={"enterprise_customer_uuid": self.enterprise_uuid},
Expand Down Expand Up @@ -1563,6 +1555,22 @@ def setup_mocks(self):
self.addCleanup(get_content_metadata_patcher.stop)
self.addCleanup(transactions_for_learner_patcher.stop)


@ddt.ddt
class TestSubsidyAccessPolicyCanRedeemView(BaseCanRedeemTestMixin, APITestWithMocks):
"""
Tests for the can-redeem view
"""

def setUp(self):
super().setUp()

self.redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory(
enterprise_customer_uuid=self.enterprise_uuid,
spend_limit=500000,
)
self.non_redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory()

def test_can_redeem_policy_missing_params(self):
"""
Test that the can_redeem endpoint returns an access policy when one is redeemable.
Expand Down Expand Up @@ -1619,7 +1627,7 @@ def mock_get_subsidy_content_data(*args):
self.mock_get_content_metadata.side_effect = mock_get_subsidy_content_data

with mock.patch(
'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
side_effect=mock_get_subsidy_content_data,
):
query_params = {'content_key': [test_content_key_1, test_content_key_2]}
Expand Down Expand Up @@ -1714,7 +1722,7 @@ def mock_get_subsidy_content_data(*args, **kwargs):
self.mock_get_content_metadata.side_effect = mock_get_subsidy_content_data

with mock.patch(
'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
side_effect=mock_get_subsidy_content_data,
):
query_params = {'content_key': [test_content_key_1, test_content_key_2]}
Expand Down Expand Up @@ -1811,8 +1819,9 @@ def test_can_redeem_policy_existing_redemptions(self, mock_transactions_cache_fo
"content_price": 19900,
}

metadata_api_path = 'enterprise_access.apps.subsidy_access_policy.content_metadata_api'
with mock.patch(
'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
f'{metadata_api_path}.get_and_cache_content_metadata',
return_value=mocked_content_data_from_view,
):
query_params = {'content_key': 'course-v1:demox+1234+2T2023'}
Expand All @@ -1838,9 +1847,9 @@ def test_can_redeem_policy_existing_redemptions(self, mock_transactions_cache_fo
self.assertIsNone(response_list[0]["redeemable_subsidy_access_policy"])
self.assertFalse(response_list[0]["can_redeem"])
self.assertEqual(response_list[0]["reasons"], [])
# the subsidy.can_redeem check returns false, so we don't make
# it to the point of fetching subsidy content data
self.assertFalse(self.mock_get_content_metadata.called)

# We call this to fetch the list_price
self.mock_get_content_metadata.assert_called_once_with("course-v1:demox+1234+2T2023")

@mock.patch('enterprise_access.apps.subsidy_access_policy.subsidy_api.get_and_cache_transactions_for_learner')
def test_can_redeem_policy_existing_reversed_redemptions(self, mock_transactions_cache_for_learner):
Expand Down Expand Up @@ -1886,7 +1895,7 @@ def test_can_redeem_policy_existing_reversed_redemptions(self, mock_transactions
}

with mock.patch(
'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
return_value=mocked_content_data_from_view,
):
query_params = {'content_key': 'course-v1:demox+1234+2T2023'}
Expand Down Expand Up @@ -1944,7 +1953,7 @@ def test_can_redeem_policy_no_price(self, mock_lms_client, mock_transactions_cac
}

with mock.patch(
'enterprise_access.apps.api.v1.views.subsidy_access_policy.get_and_cache_content_metadata',
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
return_value=mocked_content_data_from_view,
):
query_params = {'content_key': test_content_key}
Expand Down Expand Up @@ -1974,3 +1983,83 @@ def test_can_redeem_subsidy_client_http_error(self, mock_get_client):
assert response.json() == {
'detail': 'Subsidy Transaction API error: HTTPError occurred in Subsidy API request.',
}


@ddt.ddt
class TestAssignedSubsidyAccessPolicyCanRedeemView(BaseCanRedeemTestMixin, APITestWithMocks):
"""
Tests for the can-redeem view for assignment-based policies.
"""

def setUp(self):
super().setUp()
self.assignment_configuration = AssignmentConfigurationFactory(
enterprise_customer_uuid=self.enterprise_uuid,
)
self.assigned_learner_credit_policy = AssignedLearnerCreditAccessPolicyFactory(
display_name='An assigned learner credit policy, for the test customer.',
enterprise_customer_uuid=self.enterprise_uuid,
active=True,
assignment_configuration=self.assignment_configuration,
spend_limit=1000000,
)
self.content_key = 'edX+demoX'
self.content_title = 'edx: Demo 101'
self.assigned_price_cents = 25000
self.assignment = LearnerContentAssignmentFactory.create(
assignment_configuration=self.assignment_configuration,
learner_email='alice@foo.com',
lms_user_id=self.user.lms_user_id,
content_key=self.content_key,
content_title=self.content_title,
content_quantity=-self.assigned_price_cents,
state=LearnerContentAssignmentStateChoices.ALLOCATED,
)

@mock.patch('enterprise_access.apps.subsidy_access_policy.subsidy_api.get_and_cache_transactions_for_learner')
def test_can_redeem_assigned_policy(self, mock_transactions_cache_for_learner):
"""
Test that the can_redeem endpoint returns an assigned access policy when one is redeemable.
"""
mock_transactions_cache_for_learner.return_value = {
'transactions': [],
'aggregates': {
'total_quantity': 0,
},
}
test_content_key_1 = f"course-v1:{self.content_key}+3T2020"
test_content_key_1_metadata_price = 29900

mock_get_subsidy_content_data = {
"content_uuid": str(uuid4()),
"content_key": test_content_key_1,
"source": "edX",
"content_price": test_content_key_1_metadata_price,
}

self.mock_get_content_metadata.return_value = mock_get_subsidy_content_data

with mock.patch(
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
side_effect=mock_get_subsidy_content_data,
):
query_params = {'content_key': [test_content_key_1]}
response = self.client.get(self.subsidy_access_policy_can_redeem_endpoint, query_params)

assert response.status_code == status.HTTP_200_OK
response_list = response.json()

assert len(response_list) == 1

# Check the response for the first content_key given.
assert response_list[0]["content_key"] == test_content_key_1
assert response_list[0]["list_price"] == {
"usd": float(self.assigned_price_cents / 100),
"usd_cents": self.assigned_price_cents,
}
assert len(response_list[0]["redemptions"]) == 0
assert response_list[0]["has_successful_redemption"] is False
assert response_list[0]["redeemable_subsidy_access_policy"]["uuid"] == \
str(self.assigned_learner_credit_policy.uuid)
assert response_list[0]["can_redeem"] is True
assert len(response_list[0]["reasons"]) == 0
59 changes: 25 additions & 34 deletions enterprise_access/apps/api/v1/views/subsidy_access_policy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from collections import defaultdict
from contextlib import suppress

import requests
from django.conf import settings
from django.core.exceptions import ValidationError
from django.utils.functional import cached_property
Expand Down Expand Up @@ -45,7 +44,6 @@
MissingSubsidyAccessReasonUserMessages,
TransactionStateChoices
)
from enterprise_access.apps.subsidy_access_policy.content_metadata_api import get_and_cache_content_metadata
from enterprise_access.apps.subsidy_access_policy.exceptions import (
ContentPriceNullException,
MissingAssignment,
Expand Down Expand Up @@ -664,18 +662,18 @@ def can_redeem(self, request, enterprise_customer_uuid):
# Determine if the learner has already redeemed the requested content_key. Just because a transaction has
# state='committed' doesn't mean it counts as a successful redemption; it must also NOT have a committed
# reversal.
has_successful_redemption = any(
redemption['state'] == TransactionStateChoices.COMMITTED and (
successful_redemptions = [
redemption for redemption in redemptions
if redemption['state'] == TransactionStateChoices.COMMITTED and (
not redemption['reversal'] or
redemption['reversal'].get('state') != TransactionStateChoices.COMMITTED
)
for redemption in redemptions
)
]

# Of all policies for this customer, determine which are redeemable and which are not.
# But, only do this if there are no existing successful redemptions,
# so we don't unnecessarily call `can_redeem()` on every policy.
if not has_successful_redemption:
if not successful_redemptions:
redeemable_policies, non_redeemable_policies = self.evaluate_policies(
enterprise_customer_uuid, lms_user_id, content_key
)
Expand All @@ -689,14 +687,31 @@ def can_redeem(self, request, enterprise_customer_uuid):
if redeemable_policies:
resolved_policy = SubsidyAccessPolicy.resolve_policy(redeemable_policies)

if resolved_policy or has_successful_redemption:
list_price = self._get_list_price(enterprise_customer_uuid, content_key)
try:
if resolved_policy:
list_price = resolved_policy.get_list_price(lms_user_id, content_key)
elif successful_redemptions:
# Get the policy record used at time of successful redemption.
# [2023-12-05] TODO: consider cleaning this up.
# This is kind of silly, b/c we only need this policy to compute the
# list price, and it's really only *necessary* to fetch that price
# from within the context of a *policy record* for cases where that successful
# policy was assignment-based (because the list price for assignments might
# slightly different from the current list price in the canonical content metadata).
successfully_redeemed_policy = self.get_queryset().filter(
uuid=successful_redemptions[0]['subsidy_access_policy_uuid'],
).first()
list_price = successfully_redeemed_policy.get_list_price(lms_user_id, content_key)
except ContentPriceNullException as exc:
raise RedemptionRequestException(
detail=f'Could not determine list price for content_key: {content_key}',
) from exc

element_response = {
"content_key": content_key,
"list_price": list_price,
"redemptions": redemptions,
"has_successful_redemption": has_successful_redemption,
"has_successful_redemption": bool(successful_redemptions),
"redeemable_subsidy_access_policy": resolved_policy,
"can_redeem": bool(resolved_policy),
"reasons": reasons,
Expand All @@ -713,30 +728,6 @@ def can_redeem(self, request, enterprise_customer_uuid):
)
return Response(response_serializer.data, status=status.HTTP_200_OK)

def _get_list_price(self, enterprise_customer_uuid, content_key):
"""
Determine the price for content for display purposes only.
"""
try:
content_metadata = get_and_cache_content_metadata(enterprise_customer_uuid, content_key)
# Note that the "content_price" key is guaranteed to exist, but the value may be None.
list_price_integer_cents = content_metadata["content_price"]
# TODO: simplify this function by consolidating this conversion logic into the response serializer:
if list_price_integer_cents is not None:
list_price_decimal_dollars = float(list_price_integer_cents) / 100
else:
list_price_decimal_dollars = None
except requests.exceptions.HTTPError as exc:
logger.warning(f'{exc} when checking content metadata for {enterprise_customer_uuid} and {content_key}')
raise RedemptionRequestException(
detail=f'Could not determine price for content_key: {content_key}',
) from exc

return {
"usd": list_price_decimal_dollars,
"usd_cents": list_price_integer_cents,
}


class SubsidyAccessPolicyAllocateViewset(UserDetailsFromJwtMixin, PermissionRequiredMixin, viewsets.GenericViewSet):
"""
Expand Down
46 changes: 46 additions & 0 deletions enterprise_access/apps/subsidy_access_policy/content_metadata_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@
for use in the domain of SubsidyAccessPolicies.
"""
import logging
from decimal import Decimal

import requests
from django.conf import settings
from edx_django_utils.cache import TieredCache
from requests.exceptions import HTTPError

from enterprise_access.cache_utils import versioned_cache_key

from ..api_client.enterprise_catalog_client import EnterpriseCatalogApiClient
from .exceptions import ContentPriceNullException
from .utils import get_versioned_subsidy_client

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -79,3 +82,46 @@ def get_and_cache_catalog_contains_content(enterprise_catalog_uuid, content_key,
)
TieredCache.set_all_tiers(cache_key, result, timeout or DEFAULT_CACHE_TIMEOUT)
return result


def get_list_price_for_content(enterprise_customer_uuid, content_key, content_metadata=None):
"""
Given a customer and content identifier, fetch content metadata and return a list price
dictionary. If the caller already has a dictionary of ``content_metadata`` in scope, this
function computes its return value from that.
Returns:
A dictionary of the form
```
{
"usd": 149.50, # the list price in US Dollars as a float
"usd_cents": 14950 # the list price in USD Cents as an int
}
Raises:
A ``ContentPriceNullException`` if we encountered an HTTPError fetch content metadata.
"""
if not content_metadata:
try:
content_metadata = get_and_cache_content_metadata(enterprise_customer_uuid, content_key)
except requests.exceptions.HTTPError as exc:
logger.warning(
f'{exc} when checking content metadata for {enterprise_customer_uuid} and {content_key}'
)
raise ContentPriceNullException(f'Could not determine list price for {content_key}') from exc

# Note that the "content_price" key is guaranteed to exist, but the value may be None.
return list_price_dict_from_usd_cents(content_metadata['content_price'])


def list_price_dict_from_usd_cents(list_price_integer_cents):
"""
Helper to compute a list price dictionary given the non-negative price of the content in USD cents.
"""
list_price_decimal_dollars = None
if list_price_integer_cents is not None:
list_price_decimal_dollars = Decimal(list_price_integer_cents) / 100

return {
"usd": list_price_decimal_dollars,
"usd_cents": list_price_integer_cents,
}
Loading

0 comments on commit f9c5115

Please sign in to comment.