Skip to content

Commit

Permalink
feat: allow staff users to override lms_user_id in can-redeem API (
Browse files Browse the repository at this point in the history
  • Loading branch information
adamstankiewicz committed Aug 20, 2024
1 parent 61dc1f9 commit 3112bba
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,10 @@ class SubsidyAccessPolicyCanRedeemRequestSerializer(serializers.Serializer):
allow_empty=False,
help_text='Content keys about which redeemability will be queried.',
)
lms_user_id = serializers.IntegerField(
required=False,
help_text='The user identifier for which redeemability will be queried (only applicable to staff users).',
)


# pylint: disable=abstract-method
Expand Down Expand Up @@ -664,7 +668,7 @@ class SubsidyAccessPolicyCanRedeemElementResponseSerializer(serializers.Serializ
"""
Response serializer representing a single element of the response list for the can_redeem endpoint.
"""
content_key = ContentKeyField(help_text="requested content_key to which the rest of this element pertains.")
content_key = ContentKeyField(help_text="Requested content_key to which the rest of this element pertains.")
list_price = ListPriceResponseSerializer(help_text="List price for content.")
redemptions = serializers.ListField(
# TODO: figure out a way to import TransactionSerializer from enterprise-subsidy. Until then, the output docs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
REASON_LEARNER_ASSIGNMENT_CANCELLED,
REASON_LEARNER_ASSIGNMENT_FAILED,
REASON_LEARNER_NOT_ASSIGNED_CONTENT,
REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP,
REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
AccessMethods,
MissingSubsidyAccessReasonUserMessages,
Expand Down Expand Up @@ -1109,7 +1110,7 @@ def test_policy_redemption_forbidden_requests(self, role_context_dict, expected_
response = self.client.get(reverse('api:v1:policy-redemption-credits-available'), query_params)
self.assertEqual(response.status_code, expected_response_code)

# The can_redeem endpoint
# The can-redeem endpoint
url = reverse(
"api:v1:policy-redemption-can-redeem",
kwargs={"enterprise_customer_uuid": self.enterprise_uuid},
Expand Down Expand Up @@ -1789,12 +1790,18 @@ def setUp(self):
)
self.non_redeemable_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory()

group_uuid = uuid4()
PolicyGroupAssociationFactory(
enterprise_group_uuid=group_uuid,
enterprise_group_uuid=TEST_ENTERPRISE_GROUP_UUID,
subsidy_access_policy=self.redeemable_policy
)

enterprise_user_record_patcher = patch.object(
SubsidyAccessPolicy, 'enterprise_user_record'
)
self.mock_enterprise_user_record = enterprise_user_record_patcher.start()
self.mock_enterprise_user_record.return_value = TEST_USER_RECORD
self.addCleanup(enterprise_user_record_patcher.stop)

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 @@ -2283,6 +2290,96 @@ def test_can_redeem_subsidy_client_http_error(self, mock_get_client):
'subsidy_status_code': str(status.HTTP_503_SERVICE_UNAVAILABLE),
}

@ddt.data(
{'is_staff': True, 'lms_user_id_override': 1234, 'expected_can_redeem': False},
{'is_staff': True, 'lms_user_id_override': None, 'expected_can_redeem': True},
{'is_staff': False, 'lms_user_id_override': 5678, 'expected_can_redeem': True},
{'is_staff': False, 'lms_user_id_override': None, 'expected_can_redeem': True}
)
@mock.patch('enterprise_access.apps.subsidy_access_policy.subsidy_api.get_and_cache_transactions_for_learner')
@mock.patch('enterprise_access.apps.api.v1.views.subsidy_access_policy.LmsApiClient')
@ddt.unpack
def test_can_redeem_lms_user_id_override_for_staff(
self,
mock_lms_client,
mock_transactions_cache_for_learner,
is_staff,
lms_user_id_override,
expected_can_redeem,
):
"""
Test that the can_redeem endpoint allows staff to override the LMS user ID.
"""
self.user.lms_user_id = TEST_USER_RECORD['user']['id']
# Authenticate as a staff user
if is_staff:
self.user.is_staff = True
else:
self.user.is_staff = False
self.user.save()
self.set_jwt_cookie([{
'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE,
'context': self.enterprise_uuid,
}])

# Setup mocks
mock_enterprise_customer_data = {
'uuid': self.enterprise_uuid,
'slug': 'sluggy',
'admin_users': [{'email': '[email protected]'}],
}
mock_lms_client.return_value.get_enterprise_customer_data.return_value = mock_enterprise_customer_data

if is_staff and lms_user_id_override:
test_other_user_record = copy.deepcopy(TEST_USER_RECORD)
test_other_user_record['user']['id'] = lms_user_id_override
test_other_user_record['enterprise_group'] = [uuid4()] # different group membership
self.mock_enterprise_user_record.return_value = test_other_user_record
else:
self.mock_enterprise_user_record.return_value = TEST_USER_RECORD

mock_transactions_cache_for_learner.return_value = {
'transactions': [],
'aggregates': {
'total_quantity': 0,
},
}
test_content_key = 'course-v1:demox+1234+2T2023'
mock_subsidy_content_data = {
'content_uuid': str(uuid4()),
'content_key': test_content_key,
'source': 'edX',
'content_price': 19900,
}
self.mock_get_content_metadata.return_value = mock_subsidy_content_data
query_params = {'content_key': test_content_key}
if lms_user_id_override:
query_params['lms_user_id'] = lms_user_id_override
with mock.patch(
'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata',
return_value=mock_subsidy_content_data,
):
response = self.client.get(self.subsidy_access_policy_can_redeem_endpoint, query_params)

assert response.status_code == status.HTTP_200_OK
response_json = response.json()
assert len(response_json) == 1
assert response_json[0]['content_key'] == test_content_key
assert response_json[0]['can_redeem'] == expected_can_redeem
learner_not_in_group_reason = {
'reason': REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP,
'user_message': MissingSubsidyAccessReasonUserMessages.LEARNER_NOT_IN_ENTERPRISE,
'metadata': {
'enterprise_administrators': mock_enterprise_customer_data['admin_users'],
},
'policy_uuids': [str(self.redeemable_policy.uuid)],
}
assert response_json[0]['reasons'] == [] if expected_can_redeem else [learner_not_in_group_reason]

# Reset current user to be non-staff
self.user.is_staff = False
self.user.save()


@ddt.ddt
class TestSubsidyAccessPolicyGroupViewset(CRUDViewTestMixin, APITestWithMocks):
Expand Down
14 changes: 10 additions & 4 deletions enterprise_access/apps/api/v1/views/subsidy_access_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ def get_queryset(self):
enterprise_customer_uuid=self.enterprise_customer_uuid,
).order_by('-created')

def evaluate_policies(self, enterprise_customer_uuid, lms_user_id, content_key):
def evaluate_policies(self, enterprise_customer_uuid, lms_user_id, content_key, skip_customer_user_check=False):
"""
Evaluate all policies for the given enterprise customer to check if it can be redeemed against the given learner
and content.
Expand All @@ -499,7 +499,9 @@ def evaluate_policies(self, enterprise_customer_uuid, lms_user_id, content_key):
all_policies_for_enterprise = self.get_queryset()
for policy in all_policies_for_enterprise:
try:
redeemable, reason, _ = policy.can_redeem(lms_user_id, content_key, skip_customer_user_check=True)
redeemable, reason, _ = policy.can_redeem(
lms_user_id, content_key, skip_customer_user_check=skip_customer_user_check
)
logger.info(
f'[can_redeem] {policy} inputs: (lms_user_id={lms_user_id}, content_key={content_key}) results: '
f'redeemable={redeemable}, reason={reason}.'
Expand Down Expand Up @@ -718,7 +720,8 @@ def can_redeem(self, request, enterprise_customer_uuid):
serializer.is_valid(raise_exception=True)

content_keys = serializer.data['content_key']
lms_user_id = self.lms_user_id or request.user.lms_user_id
lms_user_id_override = serializer.data.get('lms_user_id') if request.user.is_staff else None
lms_user_id = lms_user_id_override or self.lms_user_id or request.user.lms_user_id
if not lms_user_id:
logger.warning(
f'No lms_user_id found when checking if we can redeem {content_keys} '
Expand Down Expand Up @@ -767,7 +770,10 @@ def can_redeem(self, request, enterprise_customer_uuid):
# so we don't unnecessarily call `can_redeem()` on every policy.
if not successful_redemptions:
redeemable_policies, non_redeemable_policies = self.evaluate_policies(
enterprise_customer_uuid, lms_user_id, content_key
enterprise_customer_uuid,
lms_user_id, content_key,
# don't skip the customer user check if we're using an override lms_user_id
skip_customer_user_check=not bool(lms_user_id_override),
)

if not successful_redemptions and not redeemable_policies:
Expand Down

0 comments on commit 3112bba

Please sign in to comment.