Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENT-8079 - Make DSC optional based on enterprise config #190

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions enterprise_subsidy/apps/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
from datetime import datetime

from django.conf import settings
from edx_django_utils.cache import RequestCache
from pytz import UTC

from enterprise_subsidy import __version__ as code_version

CACHE_KEY_SEP = ':'
DEFAULT_NAMESPACE = 'enterprise-subsidy-default'


def versioned_cache_key(*args):
Expand All @@ -29,3 +31,10 @@ def versioned_cache_key(*args):
def localized_utcnow():
"""Helper function to return localized utcnow()."""
return UTC.localize(datetime.utcnow()) # pylint: disable=no-value-for-parameter


def request_cache(namespace=DEFAULT_NAMESPACE):
"""
Helper that returns a namespaced RequestCache instance.
"""
return RequestCache(namespace=namespace)
40 changes: 37 additions & 3 deletions enterprise_subsidy/apps/fulfillment/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Python API for interacting with fulfillment operations
related to subsidy redemptions.
"""
import logging

from django.conf import settings
from getsmarter_api_clients.geag import GetSmarterEnterpriseApiClient
from openedx_ledger.models import ExternalFulfillmentProvider, ExternalTransactionReference
Expand All @@ -10,11 +12,15 @@
# pylint: disable=unused-import
from enterprise_subsidy.apps.content_metadata import api as content_metadata_api
from enterprise_subsidy.apps.content_metadata.api import ContentMetadataApi
from enterprise_subsidy.apps.core.utils import request_cache, versioned_cache_key
from enterprise_subsidy.apps.subsidy.constants import CENTS_PER_DOLLAR

from .constants import EXEC_ED_2U_COURSE_TYPES, OPEN_COURSES_COURSE_TYPES
from .exceptions import FulfillmentException, InvalidFulfillmentMetadataException

REQUEST_CACHE_NAMESPACE = 'enterprise_data'
logger = logging.getLogger(__name__)


def create_fulfillment(subsidy_uuid, lms_user_id, content_key, **metadata):
"""
Expand Down Expand Up @@ -81,11 +87,35 @@ def _get_geag_variant_id(self, transaction):
ent_uuid = self._get_enterprise_customer_uuid(transaction)
return ContentMetadataApi().get_geag_variant_id(ent_uuid, transaction.content_key)

def _get_auth_org_id(self, transaction):
def _get_enterprise_customer_data(self, transaction):
"""
Fetches and caches enterprise customer data based on a transaction.
"""
cache_key = versioned_cache_key(
'get_enterprise_customer_data',
self._get_enterprise_customer_uuid(transaction),
transaction.uuid,
)
# Check if data is already cached
cached_response = request_cache(namespace=REQUEST_CACHE_NAMESPACE).get_cached_response(cache_key)
if cached_response.is_found:
logger.info(
'subsidy_record cache hit '
f'enterprise_customer_uuid={self._get_enterprise_customer_uuid(transaction)}, '
f'subsidy_uuid={transaction.uuid}'
)
return cached_response.value
# If data is not cached, fetch and cache it
enterprise_customer_uuid = str(self._get_enterprise_customer_uuid(transaction))
ent_client = self.get_enterprise_client(transaction)
ent_data = ent_client.get_enterprise_customer_data(enterprise_customer_uuid)
return ent_data.get('auth_org_id')
enterprise_data = ent_client.get_enterprise_customer_data(enterprise_customer_uuid)

request_cache(namespace=REQUEST_CACHE_NAMESPACE).set(cache_key, enterprise_data)

return enterprise_data

def _get_auth_org_id(self, transaction):
return self._get_enterprise_customer_data(transaction).get('auth_org_id')

def _create_allocation_payload(self, transaction, currency='USD'):
# TODO: come back an un-hack this once GEAG validation is
Expand Down Expand Up @@ -121,7 +151,11 @@ def _validate(self, transaction):

Raises an exception when the transaction is missing required information
"""
enterprise_customer_data = self._get_enterprise_customer_data(transaction)
enable_data_sharing_consent = enterprise_customer_data.get('enable_data_sharing_consent', False)
for field in self.REQUIRED_METADATA_FIELDS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[curious] I'm wondering if it'd make sense to conditionally push geag_data_share_consent onto self.REQUIRED_METADATA_FIELDS when enable_data_sharing_consent = True, so that the fields listed in self.REQUIRED_METADATA_FIELDS are truly only the required fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is related to the tests since they share they same object adding more removing into the REQUIRED_METADATA_FIELDS array in causes other tests to fail which where support to pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I suppose the other concern is that I'm not sure we'd be able to get at the appropriate enterprise customer within a class constructor, as we are relying on a specific transaction to identify the enterprise customer. I think how you have it is reasonable.

if field == 'geag_data_share_consent' and not enable_data_sharing_consent:
continue
if not transaction.metadata.get(field):
raise InvalidFulfillmentMetadataException(f'missing {field} transaction metadata')
return True
Expand Down
15 changes: 12 additions & 3 deletions enterprise_subsidy/apps/fulfillment/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ def test_create_allocation_payload(self, mock_get_enterprise_customer_data, mock
else:
assert geag_payload.get(geag_field) == tx_metadata.get(payload_field)

def test_validate_pass(self):
@mock.patch("enterprise_subsidy.apps.api_client.enterprise.EnterpriseApiClient.get_enterprise_customer_data")
def test_validate_pass(self, mock_get_enterprise_customer_data):
"""
Ensure `_validate` method passes
"""
Expand All @@ -208,7 +209,10 @@ def test_validate_pass(self):
'geag_email': '[email protected]',
'geag_date_of_birth': '1900-01-01',
'geag_terms_accepted_at': '2021-05-21T17:32:28Z',
'geag_data_share_consent': True,
}
mock_get_enterprise_customer_data.return_value = {
'auth_org_id': 'asde23eas',
'enable_data_sharing_consent': False,
}
transaction = TransactionFactory.create(
state=TransactionStateChoices.PENDING,
Expand All @@ -221,7 +225,8 @@ def test_validate_pass(self):
# pylint: disable=protected-access
assert self.geag_fulfillment_handler._validate(transaction)

def test_validate_fail(self):
@mock.patch("enterprise_subsidy.apps.api_client.enterprise.EnterpriseApiClient.get_enterprise_customer_data")
def test_validate_fail(self, mock_get_enterprise_customer_data):
"""
Ensure `_validate` method raises with a missing `geag_terms_accepted_at`
"""
Expand All @@ -233,6 +238,10 @@ def test_validate_fail(self):
'geag_terms_accepted_at': '2021-05-21T17:32:28Z',
'geag_data_share_consent': True,
}
mock_get_enterprise_customer_data.return_value = {
'auth_org_id': 'asde23eas',
'enable_data_sharing_consent': True
}
transaction = TransactionFactory.create(
state=TransactionStateChoices.PENDING,
quantity=-19998,
Expand Down
Loading