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

Inject siteId #13

Merged
merged 6 commits into from Aug 24, 2021
Merged

Inject siteId #13

merged 6 commits into from Aug 24, 2021

Conversation

momo-ozawa
Copy link
Contributor

Description

This PR injects the site id into MobilePayKit.

Notes

Apply patch to test the new createOrders endpoint (D65254-code)

For sandbox info, follow instructions in these posts:

  • pbMoDN-mH-p2
  • pbArwn-24D-p2

How to test

Running tests

  1. Run MobilePayKitTests test target
  2. ✅ Tests should pass

Running the app

  1. Configure the bearer token and site id in ProductListViewModel

MobilePayKit.configure(
oAuthToken: "token",
bundleId: Bundle.main.bundleIdentifier,
siteId: "siteID"
)

  1. Run the iOS app
  2. Purchase the product by tapping on it
  3. Tap Confirm
  4. Tap OK
  5. On Xcode debugger menu, click on the Manage StoreKit Transactions menu item
  6. ✅ There should be a record of a product being purchased (See example below)

Screen Shot 2021-07-19 at 16 32 32

@momo-ozawa momo-ozawa self-assigned this Aug 9, 2021
@momo-ozawa momo-ozawa marked this pull request as ready for review August 10, 2021 15:23
Copy link
Member

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

@momo-ozawa works as described!

I'd just change siteId to be optional, given it's not a mandatory field.


public init(oAuthToken: String, bundleId: String?) {
public init(oAuthToken: String, bundleId: String?, siteId: String) {
Copy link
Member

Choose a reason for hiding this comment

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

siteId is optional, I think we can make it optional and assign a default nil value, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! 033e9f5

Copy link
Member

Choose a reason for hiding this comment

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

There's a build error after this change, you need to update the remaining places too. :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oof! Fixed... updated site_id to Int? as well, per D65254-code

Copy link
Member

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

Backend is currently failing if no site_id is given, this will be addressed in a diff.

@momo-ozawa momo-ozawa merged commit 374c12c into develop Aug 24, 2021
@momo-ozawa momo-ozawa deleted the task/inject-site-id branch August 27, 2021 14:33
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.

None yet

2 participants