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

Adopt ceil method for ios inapp prices #4

Merged
merged 6 commits into from
Aug 6, 2024
Merged

Conversation

jawad-khan
Copy link

⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH

  • I have checked the branch to which I would like to merge.

⛔️ DEPRECATION WARNING

This repository is deprecated and in maintainence-only operation while we work on a replacement, please see this announcement for more information.

Although we have stopped integrating new contributions, we always appreciate security disclosures and patches sent to [email protected]

Anyone internally merging to this repository is expected to release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.

Required Testing

  • Before deploying this change, complete a purchase in the stage environment.
    (^ We can remove that manual check once REV-2624 is done and the corresponding e2e test runs again)

Description

Adopt ceil method for ios inapp prices i.e. pick closest higher price point for an inapp purchase. And if course price is > 1000 then don't apply price for this product.
Previously for android purchases we were not storing transaction ids in paymentresponseextension table, this change will also fix this issue.
For user cancelled payments return user cancelled error in execute api view.

Supporting information

Jira Ticket: https://2u-internal.atlassian.net/browse/LEARNER-10095

@@ -34,6 +34,9 @@ def create_ios_product(course, ios_product, configuration):
Create in app ios product on connect store.
return error message in case of failure.
"""
if course['price'] > 1000:
return 'Error: Appstore does not allow price> 1000'

Choose a reason for hiding this comment

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

nit: price> should be price >

if nearest_low_price < customer_price <= price:
nearest_low_price = customer_price
nearest_low_price_id = price_point['id']
if nearest_high_price > customer_price >= price:

Choose a reason for hiding this comment

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

question: what is the scenario where price is greater than customer_price?

Copy link
Author

Choose a reason for hiding this comment

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

customer price range is 0-1000 and we are already filtering > 1000 cases. Therefore there is no chance for this scenario.
Even if it occurs this function will return exception and we will not apply price for that product and will investigate issue afterwards.

'price': 1001
}
error_msg = create_ios_product(course, self.ios_seat, self.configuration)
self.assertEqual(error_msg, 'Error: Appstore does not allow price> 1000')

Choose a reason for hiding this comment

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

price> 1000 should be price > 1000

@@ -212,6 +212,7 @@ def record_processor_response(self, response, transaction_id=None, basket=None,
basket=basket)

meta_data = self._get_metadata(currency_code=currency_code, price=price)
original_transaction_id = original_transaction_id or transaction_id

Choose a reason for hiding this comment

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

question: how does this change relate to the inapp price change?

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't. I noticed that nit and will be sending as a separate commit in this PR.

Copy link

@christopappas christopappas left a comment

Choose a reason for hiding this comment

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

good once CI goes ✅ 👍

@jawad-khan jawad-khan merged commit 5ea69a6 into 2u/main Aug 6, 2024
7 checks passed
@jawad-khan jawad-khan deleted the LEARNER-10095 branch August 6, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants