-
Notifications
You must be signed in to change notification settings - Fork 4
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
SONIC-704: Add CT discount availed check for outline tab #303
base: main
Are you sure you want to change the base?
Changes from 7 commits
8de2d2c
e4f2f55
2f4e823
37ee470
f653ed8
959a1cf
184427c
b56f77e
f60b48c
c8a1720
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -603,3 +603,36 @@ def retire_customer_anonymize_fields(self, customer_id: str, customer_version: i | |
f"with ID: {customer_id}, after LMS retirement with " | ||
f"error correlation id {err.correlation_id} and error/s: {err.errors}") | ||
raise err | ||
|
||
def is_first_time_discount_eligible(self, email: str) -> bool: | ||
""" | ||
Check if a user is eligible for a first time discount | ||
Args: | ||
email (str): Email of the user | ||
Returns (bool): True if the user is eligible for a first time discount | ||
""" | ||
try: | ||
discounts = self.base_client.discount_codes.query( | ||
where="code in :discountCodes", | ||
predicate_var={'discountCodes': settings.COMMERCETOOLS_FIRST_TIME_DISCOUNTS} | ||
) | ||
discount_ids = [discount.id for discount in discounts.results] | ||
|
||
discounted_orders = self.base_client.orders.query( | ||
where=[ | ||
"customerEmail=:email", | ||
"orderState=:orderState", | ||
"discountCodes(discountCode(id in :discountIds))" | ||
], | ||
predicate_var={'email': email, 'discountIds': discount_ids, 'orderState': 'Complete'} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just making sure that we don't want to cater for refunded orders here right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refunded orders are not handled by CT itself, and if I remember correctly, we had to check for their first usage only. Correct me if I am wrong |
||
) | ||
|
||
if discounted_orders.total > 0: | ||
return False | ||
|
||
return True | ||
except CommercetoolsError as err: # pragma no cover | ||
# Logs & ignores version conflict errors due to duplicate Commercetools messages | ||
handle_commercetools_error(err, f"Unable to check if user {email} is eligible for a " | ||
f"first time discount", True) | ||
return True |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -803,6 +803,99 @@ def test_update_customer_with_anonymized_fields_exception(self): | |
|
||
log_mock.assert_called_once_with(expected_message) | ||
|
||
def test_is_first_time_discount_eligible_success(self): | ||
base_url = self.client_set.get_base_url_from_client() | ||
email = '[email protected]' | ||
|
||
mock_discount_codes = { | ||
"results": [ | ||
{"id": "discount-id-1"}, | ||
{"id": "discount-id-2"} | ||
] | ||
} | ||
|
||
mock_orders = { | ||
"total": 0 | ||
} | ||
|
||
with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: | ||
mocker.get( | ||
f"{base_url}discount-codes", | ||
json=mock_discount_codes, | ||
status_code=200 | ||
) | ||
|
||
mocker.get( | ||
f"{base_url}orders", | ||
json=mock_orders, | ||
status_code=200 | ||
) | ||
|
||
result = self.client_set.client.is_first_time_discount_eligible(email) | ||
self.assertTrue(result) | ||
|
||
def test_is_first_time_discount_eligible_not_eligible(self): | ||
base_url = self.client_set.get_base_url_from_client() | ||
email = '[email protected]' | ||
|
||
mock_discount_codes = { | ||
"results": [ | ||
{"id": "discount-id-1"}, | ||
{"id": "discount-id-2"} | ||
] | ||
} | ||
|
||
mock_orders = { | ||
"total": 1 | ||
} | ||
|
||
with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: | ||
mocker.get( | ||
f"{base_url}discount-codes", | ||
json=mock_discount_codes, | ||
status_code=200 | ||
) | ||
|
||
mocker.get( | ||
f"{base_url}orders", | ||
json=mock_orders, | ||
status_code=200 | ||
) | ||
|
||
result = self.client_set.client.is_first_time_discount_eligible(email) | ||
self.assertFalse(result) | ||
|
||
def test_is_first_time_discount_eligible_invalid_email(self): | ||
invalid_email = "[email protected]" | ||
base_url = self.client_set.get_base_url_from_client() | ||
|
||
mock_discount_codes = { | ||
"results": [ | ||
{"id": "discount-id-1"}, | ||
{"id": "discount-id-2"} | ||
] | ||
} | ||
|
||
mock_orders = { | ||
"total": 0 | ||
} | ||
|
||
with requests_mock.Mocker(real_http=True, case_sensitive=False) as mocker: | ||
mocker.get( | ||
f"{base_url}discount-codes", | ||
json=mock_discount_codes, | ||
status_code=200 | ||
) | ||
|
||
mocker.get( | ||
f"{base_url}orders", | ||
json=mock_orders, | ||
status_code=200 | ||
) | ||
|
||
result = self.client_set.client.is_first_time_discount_eligible(invalid_email) | ||
self.assertTrue(result) | ||
|
||
|
||
class PaginatedResultsTest(TestCase): | ||
"""Tests for the simple logic in our Paginated Results Class""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
from commerce_coordinator.apps.commercetools.constants import COMMERCETOOLS_ORDER_MANAGEMENT_SYSTEM | ||
from commerce_coordinator.apps.commercetools.pipeline import ( | ||
AnonymizeRetiredUser, | ||
CheckCommercetoolsDiscountEligibility, | ||
CreateReturnForCommercetoolsOrder, | ||
CreateReturnPaymentTransaction, | ||
GetCommercetoolsOrders, | ||
|
@@ -264,3 +265,33 @@ def test_pipeline(self, mock_anonymize_fields, mock_customer_by_lms_id, mock_ano | |
ret = pipe.run_filter(lms_user_id=self.mock_lms_user_id) | ||
result_data = ret['returned_customer'] | ||
self.assertEqual(result_data, self.update_customer_response) | ||
|
||
|
||
class CommercetoolsDiscountEligibilityPipelineTests(TestCase): | ||
"""Commercetools pipeline testcase for checking discount eligibility in CT""" | ||
def setUp(self) -> None: | ||
self.mock_email = "[email protected]" | ||
self.mock_eligible_result = True | ||
self.mock_ineligible_result = False | ||
|
||
@patch( | ||
'commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient' | ||
'.is_first_time_discount_eligible' | ||
) | ||
def test_pipeline_eligible(self, mock_is_eligible): | ||
pipe = CheckCommercetoolsDiscountEligibility("test_pipe", None) | ||
mock_is_eligible.return_value = self.mock_eligible_result | ||
ret = pipe.run_filter(self.mock_email) | ||
result_data = ret['is_eligible'] | ||
self.assertEqual(result_data, self.mock_eligible_result) | ||
|
||
@patch( | ||
'commerce_coordinator.apps.commercetools.clients.CommercetoolsAPIClient' | ||
'.is_first_time_discount_eligible' | ||
) | ||
def test_pipeline_ineligible(self, mock_is_eligible): | ||
pipe = CheckCommercetoolsDiscountEligibility("test_pipe", None) | ||
mock_is_eligible.return_value = self.mock_ineligible_result | ||
ret = pipe.run_filter(self.mock_email) | ||
result_data = ret['is_eligible'] | ||
self.assertEqual(result_data, self.mock_ineligible_result) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,3 +382,72 @@ def test_post_with_unexpected_exception_fails(self, mock_filter): | |
response = self.client.post(self.url, self.valid_payload, format='json') | ||
|
||
self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) | ||
|
||
|
||
@ddt.ddt | ||
class FirstTimeDiscountEligibleViewTests(APITestCase): | ||
""" | ||
Tests for the FirstTimeDiscountEligibleView to check if a user is eligible for a first-time discount. | ||
""" | ||
|
||
test_user_username = 'test' | ||
test_user_email = '[email protected]' | ||
test_user_password = 'secret' | ||
|
||
url = reverse('lms:first_time_discount_eligible') | ||
|
||
def setUp(self): | ||
super().setUp() | ||
self.user = User.objects.create_user( | ||
self.test_user_username, | ||
self.test_user_email, | ||
self.test_user_password, | ||
is_staff=True, | ||
) | ||
|
||
def tearDown(self): | ||
super().tearDown() | ||
self.client.logout() | ||
|
||
def authenticate_user(self): | ||
self.client.login(username=self.test_user_username, password=self.test_user_password) | ||
self.client.force_authenticate(user=self.user) | ||
|
||
@patch('commerce_coordinator.apps.lms.views.CheckFirstTimeDiscountEligibility.run_filter') | ||
def test_get_with_valid_email_eligibility_true(self, mock_filter): | ||
""" | ||
Test case where the email is eligible for a first-time discount. | ||
""" | ||
self.authenticate_user() | ||
mock_filter.return_value = {'is_eligible': True} | ||
|
||
response = self.client.get(self.url, {'email': self.test_user_email}) | ||
|
||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertEqual(response.data, {"is_eligible": True}) | ||
mock_filter.assert_called_once_with(email=self.test_user_email) | ||
|
||
@patch('commerce_coordinator.apps.lms.views.CheckFirstTimeDiscountEligibility.run_filter') | ||
def test_get_with_valid_email_eligibility_false(self, mock_filter): | ||
""" | ||
Test case where the email is not eligible for a first-time discount. | ||
""" | ||
self.authenticate_user() | ||
mock_filter.return_value = {'is_eligible': False} | ||
|
||
response = self.client.get(self.url, {'email': self.test_user_email}) | ||
|
||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertEqual(response.data, {"is_eligible": False}) | ||
mock_filter.assert_called_once_with(email=self.test_user_email) | ||
|
||
def test_get_with_missing_email_fails(self): | ||
""" | ||
Test case where the email is not provided in the request query params. | ||
""" | ||
self.authenticate_user() | ||
|
||
response = self.client.get(self.url) | ||
|
||
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) | ||
self.assertEqual(response.data, {'detail': 'Could not detect user email.'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can maybe save two API calls from being made here.
First thought on my mind was to use reference expansion which I couldn't find any for discounts. I think you have also checked this.
The second option here is to store the discount IDs themselves in settings/edx-internal.
We are already following this pattern for line_item_transition: Reference
It is not normally suggested to store the database identifiers like this but as long as the discounts remain the same, the ids/keys are going to be the same so we can minimize the extra API call here which makes more sense to me.
@shafqatfarhan to weigh in on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the first point, we cannot query expanded objects. For the second option, I wanted the identifiers to be consistent across all environments, which is why I suggested using discount codes, as storing IDs didn’t seem ideal. However, if we're okay with this approach, we can certainly save an API call here