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

Implement googlepay button #173

Merged
merged 9 commits into from
Mar 7, 2022

Conversation

elizalucas
Copy link
Contributor

@elizalucas elizalucas commented Mar 3, 2022

This PR implements the following:

  1. Displays the googlepay button for stg/prod to get payment data from the Google Pay API
  2. Calls the convert token endpoint in wallets-api
  3. Adds a 'tokens' option to the POST payments page

Note: this is only initial implementation of the google pay button - there are some hard coded values in the googlePay lib in this PR that will be removed/moved to SSM in subsequent PRs

Sandbox/Smokebox:
Screen Shot 2022-03-04 at 5 40 23 PM

Stg/Prod:
Screen Shot 2022-03-04 at 5 40 56 PM

@huaweigu huaweigu requested review from kristinfritsch and antiv0 and removed request for kristinfritsch March 3, 2022 18:33
onClick: this.onGooglePayButtonClicked,
allowedPaymentMethods: [
{
type: 'CARD',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: (to my memory)
when i did these hardcoded config values i extracted it in separate config object, so later you have it one place

like you don't allow amex below, maybe you want to change that, maybe you want to change auth methods, having in one place

antiv0
antiv0 previously approved these changes Mar 3, 2022
Copy link
Contributor

@antiv0 antiv0 left a comment

Choose a reason for hiding this comment

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

lgtm

apiVersion: 2,
apiVersionMinor: 0,
merchantInfo: {
merchantId: '12345678901234567890',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we would want it to be real merchant id and name for stg/prod, will we store them in ssm in next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will store the merchant values and Checkout details in SSM for next PR

Copy link
Contributor

@antiv0 antiv0 Mar 6, 2022

Choose a reason for hiding this comment

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

// just for Eliza, just in case when doing next steps, secrets go to kubernetes db (which is likely etcd), not in aws ssm

Copy link
Contributor

Choose a reason for hiding this comment

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

curious on where/what the local secret is stored?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is public repo, in private repo deployment is defined

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wrong, it should live in K8S secrets for sample apple private

type: 'PAYMENT_GATEWAY',
parameters: {
gateway: 'checkoutltd',
gatewayMerchantId: 'YOUR_PUBLIC_KEY',
Copy link
Contributor

Choose a reason for hiding this comment

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

okay I think we can put it in SSM as well

currencyCode: 'USD',
countryCode: 'US',
totalPriceStatus: 'FINAL',
totalPrice: '12.00',
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, will we remove this hardcoded value in next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will be removing or moving all hardcoded values in the next PR, just had them here for testing purposes for now

Copy link
Contributor

@antiv0 antiv0 left a comment

Choose a reason for hiding this comment

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

lgtm to get code out

.then(function (response: IsReadyToPayResponse) {
if (response.result) {
const button = paymentsClient.createButton(buttonOptions)
document.getElementById('google-pay-button')?.append(button)
Copy link
Contributor

@antiv0 antiv0 Mar 6, 2022

Choose a reason for hiding this comment

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

nit: nowhere else in this codebase is DOM manipulated
somebody with good experience in our frontend stack could likely propose a way to do this that is in line with rest of codebase

like usage of Vue's v-if/v-on.. (i haven't dealt here with rendering of elements once promises resolve to know, but it seems to me somebody frontend should check it out)

const payload = {
type,
tokenData,
idempotencyKey: '394dffd9-e992-4b86-b3f3-0a242a44db48',
Copy link
Contributor

Choose a reason for hiding this comment

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

why not have this be randomly generated in this PR ?

@huaweigu huaweigu merged commit 728a1c4 into circlefin:master Mar 7, 2022
@huaweigu
Copy link
Contributor

huaweigu commented Mar 7, 2022

Do you mind linking the story in the PR title? Thanks!
e.g. [BRAVV-123]

@antiv0
Copy link
Contributor

antiv0 commented Mar 7, 2022

This is public repo, no internal info gets associated with this repo

@elizalucas elizalucas deleted the implement_googlepay_button branch March 8, 2022 18:52
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.

5 participants