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

Support applepay validation #193

Merged
merged 8 commits into from
Jan 28, 2021
Merged

Support applepay validation #193

merged 8 commits into from
Jan 28, 2021

Conversation

hobadams
Copy link
Contributor

I've tried to add support for the ApplePay session method - https://docs.mollie.com/reference/v2/wallets-api/request-apple-pay-payment-session

I've not been able to test because this method is not available in test mode.

I'd love some feedback if you're able to provide it

@hobadams
Copy link
Contributor Author

@Pimm - Are you the best person to request a review from?

@Pimm Pimm self-assigned this Jan 25, 2021
@hobadams hobadams marked this pull request as ready for review January 27, 2021 12:29
@hobadams
Copy link
Contributor Author

@Pimm - Thanks so much for looking at this so quickly.

@Pimm
Copy link
Collaborator

Pimm commented Jan 27, 2021

Thank you for the contribution!

I changed your PR in two ways:

  • I renamed the method to requestPaymentSession to match the documentation, and
  • I made ApplePaySessionData not inherit from Model, as the object does not actually have an id and resource property like most objects returned by the API do.

It is unfortunate that we can't test this in the way we test the other endpoints.

@hobadams
Copy link
Contributor Author

Awesome, thanks so much @Pimm. Do you think this will make it's way into your next release?

@Pimm
Copy link
Collaborator

Pimm commented Jan 27, 2021

Yes!

Copy link
Collaborator

@edorivai edorivai left a comment

Choose a reason for hiding this comment

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

Changes generally look good to me. I would advise to issue a prerelease that @hobadams can start using in their app, given that we don't have the ability to test applepay in a test environment.

@hobadams would you be willing to test this out and report back with results after a while?

@hobadams
Copy link
Contributor Author

@edorivai - good idea. I'll be using it with our Mollie / Deity (https://deity.io/) integration so it should get heavily tested.

@Pimm Pimm merged commit 487f2ef into mollie:master Jan 28, 2021
@Pimm Pimm mentioned this pull request Jan 28, 2021
@hobadams
Copy link
Contributor Author

hobadams commented Feb 1, 2021

@Pimm @edorivai - I've just tested the merged code using production Mollie credentials and it's returning the data I expected. I've still got a little way before I have a full integration working in my app to show you but this works well.

Thanks!

@Pimm
Copy link
Collaborator

Pimm commented Feb 1, 2021

To hopefully make your job slightly easier, you can now use the version with the beta dist tag (using npm install @mollie/api-client@beta). It reflects master at this point in time, and includes your contribution.

Should you run into any issues while integrating Mollie ‒ including anything not Apple Pay related ‒ please let me know.

@hobadams
Copy link
Contributor Author

hobadams commented Feb 8, 2021

@Pimm I've put live ApplePay transactions through Mollie using the beta branch so I can confirm it works :-)

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.

3 participants