feat: create ios products on appstore for given course key #4090
Conversation
| } | ||
|
|
||
| private_key = configuration['private_key'] | ||
| logger.error(private_key) |
There was a problem hiding this comment.
We should remove this logger statement
| "reviewNote": 'This in-app purchase will unlock all the content of the course {course_name}\n\n' | ||
| 'For testing the end-to-end payment flow, please follow the following steps:\n1. ' | ||
| 'Go to the Discover tab\n2. Search for "{course_name}"\n3. Enroll in the course' | ||
| ' "{course_name}"\n4. Hit \"Upgrade to access more features\", it will open a ' | ||
| 'detail unlock features page\n5. Hit "Upgrade now for ${course_price}" from the' | ||
| ' detail page'.format(course_name=course['name'], course_price=course['price']), |
There was a problem hiding this comment.
Can we move the reviewNote string to ecommerce/extensions/iap/api/v1/constants.py?
| logger.error(response.content) | ||
| logger.error(response.status_code) |
| data = { | ||
| "data": { | ||
| "type": "inAppPurchasePriceSchedules", | ||
| "attributes": {}, | ||
| "relationships": { | ||
| "inAppPurchase": { | ||
| "data": { | ||
| "type": "inAppPurchases", | ||
| "id": in_app_purchase_id | ||
| } | ||
| }, | ||
| "manualPrices": { | ||
| "data": [ | ||
| { | ||
| "type": "inAppPurchasePrices", | ||
| "id": "${price}" | ||
| } | ||
| ] | ||
| }, | ||
| "baseTerritory": { | ||
| "data": { | ||
| "type": "territories", | ||
| "id": "USA" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "included": [ | ||
| { | ||
| "id": "${price}", | ||
| "relationships": { | ||
| "inAppPurchasePricePoint": { | ||
| "data": { | ||
| "type": "inAppPurchasePricePoints", | ||
| "id": nearest_low_price_id | ||
| } | ||
| } | ||
| }, | ||
| "type": "inAppPurchasePrices", | ||
| "attributes": { | ||
| "startDate": None | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we move this hardcoded dict to constants and only set variables here?
This would clean up a lot of code.
| @@ -17,3 +24,271 @@ def products_in_basket_already_purchased(user, basket, site): | |||
| UserAlreadyPlacedOrder.user_already_placed_order(user=user, product=product, site=site): | |||
| return True | |||
| return False | |||
|
|
|||
| def create_ios_skus(course, ios_sku, configuration): | |||
There was a problem hiding this comment.
Can we rename this to something like create_ios_products? Since it conflicts with creating skus in Ecommerce itself, where as this method is about creating products on the Appstore. Also adding a docstring would be really helpful.
| with staticfiles_storage.open('images/mobile_ios_product_screenshot.png', 'rb') as image: | ||
| img_headers = headers.copy() | ||
| img_headers['Content-Type'] = 'image/png' | ||
| response = request_connect_store(url, headers=img_headers, data=image.read(), method='put') | ||
|
|
||
| if not response.status_code == 200: | ||
| raise AppStoreRequestException("Couldn't upload screenshot") |
There was a problem hiding this comment.
Can we add breif description in a comment or a docstring about what is happening with the screenshot image here from the staticfiles? Where does the screenshot images/mobile_ios_product_screenshot.png come from in the first place?
| @@ -385,6 +385,7 @@ def post(self, request): | |||
| missing_course_runs = [] | |||
| failed_course_runs = [] | |||
| created_skus = {} | |||
| failed_ios_skus = [] | |||
There was a problem hiding this comment.
failed_ios_products in favor of failed_ios_skus?
| http = Session() | ||
| retries = Retry( | ||
| total=3, | ||
| backoff_factor=3, |
There was a problem hiding this comment.
total and backoff_factor, would they ever have a different value? Should they be hard coded? Or can you add a comment on what this is/how this could change for the future
| if response.status_code == 201: | ||
| return response.json()["data"]["id"] | ||
|
|
||
| raise AppStoreRequestException("Couldn't create inapp purchase id") |
There was a problem hiding this comment.
You could add the request call on a try/except, unless assuming if status_code is anything other than 201 it will always be the same exception and it won't error out before it gets to the raise
There was a problem hiding this comment.
Can you explain it a little?
| } | ||
|
|
||
| response = request_connect_store(url, headers, data=data) | ||
| if not response.status_code == 201: |
There was a problem hiding this comment.
I see sometimes you use not ... == and sometimes !=, for readability the later is easier
I am just throwing the actual exception in parent function(create_ios_product) where we are adding more context to the actual error. |
⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH
⛔️ 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
(^ We can remove that manual check once REV-2624 is done and the corresponding e2e test runs again)
Description
We need to increase IAP exposure to the edX mobile app user base. In order to do so we need to create course products within AppStore Connect to support purchases within the app.
Jira ticket: https://2u-internal.atlassian.net/browse/LEARNER-9758
Useful information to include:
Supporting information
Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.
Testing instructions
Please provide detailed step-by-step instructions for testing this change; how did YOU test this change?
Other information
Include anything else that will help reviewers and consumers understand the change.