feat(payment): BOLT-19 added BoltCheckout implementation for customer… #1163

serhii-tkachenko · 2021-06-24T06:36:55Z

…s with bolt account

What?

Added BoltCheckout implementation for customers with bolt account.

Why?

We need to add custom flow for continue as guest, sign in and sign up events, to show BoltCheckout modal if the customer has BoltAccount

Task: https://jira.bigcommerce.com/browse/BOLT-19

Sibling pull-request

checkout-js: bigcommerce/checkout-js#619

Testing / Proof

Some gifs to figure out how it looks like:

There how it will looks like if the user have default flow or customer hasn't bolt account:

@bigcommerce/checkout @bigcommerce/payments

BC-ACherednichenko

Nicely done! I like this idea more than than the first revision #1158, we re-use the existing strategy without creating additional one that behaves pretty much in the same way

@davidchin @icatalina can you take a look at these two possible options and let us know your thoughts. The possible way that we will choose more likely correspond to the philosophy of the existing customer strategy, hence your input will be invaluable

BC-ACherednichenko

Looks good to me! There is only one minor concern

@davidchin can take a look?

BC-ACherednichenko · 2021-07-01T14:27:41Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+                }
+            } catch (error) {
+                resolve();
+                throw error;


per our discussion, it worth to just log an error in order to not interrupt the default flow (will check if throw fully interrupts the default flow). continueWith strategy is not as vital for checkout as a strategy of a payment provider. we can remain the default flow if we have any issue

I checked it and it is working as expected. If we somehow get an error, we will continue with default flow without any problem

I think the intention is good. But I think what we should do, from SDK library's perspective, is to return an error and let the client to catch and decide what to do next as a recovery. The client may choose to silently ignore the error and log it behind the scene. Or it may choose to display a message informing that the shopper that something went wrong but they can still progress.

vitalii-koshovyi

LGTM, except some tests names. I've sent my propositions to @Sireks in slack message.

vitalii-koshovyi · 2021-07-01T14:50:35Z

src/customer/customer-strategy-reducer.spec.ts

+            isContinuingAsGuest: false,
+        });
+
+        expect(customerStrategyReducer(initialState, action).statuses).toEqual({


is that expect useful?

Yes, it is.

Already fixed all those propositions that you sent me in slack.

davidchin

Thank you @Sireks. It's looking good no major issue with the code itself. However, as I mentioned in your other PR, I'm blocking this for now until we get some clarification from product.

davidchin · 2021-07-06T06:58:49Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+        const { methodId } = options;
+
+        if (!methodId) {
+            throw new MissingDataError(MissingDataErrorType.MissingPaymentMethod);


InvalidArgument is a better error to throw (i.e.: the required argument is not passed).

davidchin · 2021-07-06T06:59:28Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+        const { developerConfig, publishableKey } = paymentMethod.initializationData;
+
+        return this._boltScriptLoader.load(publishableKey, paymentMethod.config.testMode, developerConfig)
+            .then(boltCheckout => this._boltClient = boltCheckout)


Prefer async/await for any new files that we create.

davidchin · 2021-07-06T07:00:53Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+            .then(() => this._store.getState());
+    }
+
+    private openCustomBoltCheckoutWidget(email: string, methodId: string): Promise<InternalCheckoutSelectors> {


We have a convention of prefixing private members, including methods, with an underscore. This is because when TS is compiled to JS, people know that they shouldn't be accessing those members.

davidchin · 2021-07-06T07:02:21Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+                    };
+
+                    this._boltClient.openCheckout(email, callbacks)
+                        .then(result => !result && resolve());


Can openCheckout return an error / rejected promise. If so, we need to handle it accordingly,.

It is already wrapped in try...catch.
I will remove that then chaining, because it is unnecessary anymore

The try/catch block has no effect because you're using the then syntax. It only works if you use the async/await syntax.

davidchin · 2021-07-06T07:04:08Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+    }
+
+    private openCustomBoltCheckoutWidget(email: string, methodId: string): Promise<InternalCheckoutSelectors> {
+        return new Promise(resolve => {


I don't think you need to manually construct a Promise as openCheckout already returns one. So you can just chain to it and return a Promise that way.

I will check this one

I will change the method to make it more readable, but I need to keep new Promise(resolve => ...) to make all the code that left run only if the modal was closed.

If Bolt Checkout modal was closed, it's ok, we will run default customer flow. But if shopper will continue with Bolt Checkout Modal, he/she will be redirected to Success page in the end of the flow.

davidchin · 2021-07-06T07:07:02Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+                }
+            } catch (error) {
+                resolve();
+                throw error;


I think the intention is good. But I think what we should do, from SDK library's perspective, is to return an error and let the client to catch and decide what to do next as a recovery. The client may choose to silently ignore the error and log it behind the scene. Or it may choose to display a message informing that the shopper that something went wrong but they can still progress.

davidchin · 2021-07-06T07:09:58Z

src/customer/strategies/bolt/bolt-customer-strategy.ts

+            throw new MissingDataError(MissingDataErrorType.MissingPaymentMethod);
+        }
+
+        return this._store.dispatch(this._customerActionCreator.signInCustomer(credentials, options))


Doesn't this mean that if a shopper chooses to sign into their BC account, and then refresh the page before they get a chance to complete whatever that's needed in the Bolt widget, they won't be prompted to complete the Bolt widget again (because the shopper is already signed in so they will be prompted to complete the next/shipping step instead)?

I am working on the implementation right now. We need to check if there any customContinueFlowEnabled in Checkout component and then handle to open customer step to open the BoltCheckout modal again.

bigcommerce/checkout-js#627

May be you can give me some advice how to do it right

The logged in shopper will have Bolt checkout modal open each time when open Checkout page and if the shopper has Bolt Account

I think we have to be careful on this because you're assuming that the shopper wants to use Bolt. It might be that they intentionally close the Bolt modal. But they are prompted again everytime they reopen checkout or go back to the customer step.

But we don't know why the user closed Bolt Checkout modal, maybe shopper want to get more stuff to make the order or so. We will open the modal only for those customer that has their own Bolt Account. Otherwise the modal will not be showed.

We discussed this case a couple times... And we made the decision to make it like this.

I understand your team has made a decision. And I'm sorry if you feel like we're revisiting that decision. But considering that decision was made without consulting with our team, I think it's reasonable for us to question it and perhaps make adjustments to it if we both agree they are appropriate.

Anyway the reason why I brought this up is because you're making an assumption that shoppers with an existing Bolt account really want to use Bolt. Having a Bolt account does not mean the shopper wants to use it. It just means they have an account which they may choose to use it. Anyway, I guess you can say the shopper can just close it again if they really don't want use Bolt. That's fair enough. But what if the store has PayPal enabled and the shopper (with an existing Bolt account) clicks on the PayPal button on the cart page, authorises the payment on PayPal side and then get redirected to checkout, would they see the Bolt modal open automatically? If that's the case, I think we have a problem. At least, we have to make sure that if there's a payment in progress, don't try to encourage the shopper to abandon it and switch to use Bolt.

Thanks @davidchin!

It's ok. We will discuss the issue and I or someone from my team will tell you about what we will doing next.

But it's a question for another task and the case has not to be a blocker for current task.

I understand your team has made a decision. And I'm sorry if you feel like we're revisiting that decision. But considering that decision was made without consulting with our team, I think it's reasonable for us to question it and perhaps make adjustments to it if we both agree they are appropriate.

@davidchin this is what these 2 PRs about, as long as the flow is a bit tricky, we made some decisions and started conversation with these 2 POCs

I think we have to be careful on this because you're assuming that the shopper wants to use Bolt. It might be that they intentionally close the Bolt modal. But they are prompted again everytime they reopen checkout or go back to the customer step.

To my knowdledge, we have decided to introduce some flag in checkout session (the research on this feature is in the progress), that will be set whenever a buyer mananges popup, so in the scope of single quote they will not be bombarded with the popup over and overagain

davidchin · 2021-07-06T07:11:32Z

tslint.json

@@ -4,6 +4,7 @@
        "max-line-length": false,
        "no-empty": false,
        "no-shadowed-variable": false,
-        "object-literal-sort-keys": false
+        "object-literal-sort-keys": false,
+        "cyclomatic-complexity": false


You should try to resolve the cyclomatic complexity by refactoring the code rather than disabling the rule. If the refactoring takes too much effort and you'd rather tackle it separately, then you can disable the rule locally for the file only rather than disabling it globally for all files.

…s with bolt account

serhii-tkachenko · 2021-07-08T09:48:40Z

@davidchin
I added some fixed. Can you check if everything all right?

serhii-tkachenko requested a review from a team as a code owner June 24, 2021 06:36

serhii-tkachenko mentioned this pull request Jun 24, 2021

feat(payment): BOLT-19 added BoltCheckout implementation for customer… bigcommerce/checkout-js#619

Closed

serhii-tkachenko changed the title ~~feat(payment): BOLT-19 added BoltCheckout implementation for customer…~~ [draft-2, wip] feat(payment): BOLT-19 added BoltCheckout implementation for customer… Jun 24, 2021

serhii-tkachenko requested review from vitalii-koshovyi, davidchin, icatalina and BC-ACherednichenko June 24, 2021 06:48

serhii-tkachenko marked this pull request as draft June 24, 2021 08:45

BC-ACherednichenko reviewed Jun 25, 2021

View reviewed changes

serhii-tkachenko force-pushed the BOLT-19-2 branch 4 times, most recently from e390ee6 to 1dd651f Compare June 30, 2021 12:33

serhii-tkachenko marked this pull request as ready for review June 30, 2021 12:47

serhii-tkachenko changed the title ~~[draft-2, wip] feat(payment): BOLT-19 added BoltCheckout implementation for customer…~~ feat(payment): BOLT-19 added BoltCheckout implementation for customer… Jun 30, 2021

serhii-tkachenko mentioned this pull request Jun 30, 2021

[draft, wip] feat(payment): BOLT-19 added new BoltCheckout implementation to custo… bigcommerce/checkout-js#616

Closed

serhii-tkachenko added the sydney label Jul 1, 2021

BC-ACherednichenko approved these changes Jul 1, 2021

View reviewed changes

vitalii-koshovyi approved these changes Jul 1, 2021

View reviewed changes

serhii-tkachenko force-pushed the BOLT-19-2 branch from 1dd651f to a243be0 Compare July 2, 2021 10:21

davidchin requested changes Jul 6, 2021

View reviewed changes

serhii-tkachenko force-pushed the BOLT-19-2 branch 2 times, most recently from 0ffb8f5 to 60ca262 Compare July 8, 2021 06:46

feat(payment): BOLT-19 added BoltCheckout implementation for customer…

ea1248f

…s with bolt account

serhii-tkachenko force-pushed the BOLT-19-2 branch from 60ca262 to ea1248f Compare July 8, 2021 09:40

serhii-tkachenko removed the sydney label Jul 28, 2021

serhii-tkachenko marked this pull request as draft July 28, 2021 11:09

serhii-tkachenko closed this Aug 4, 2021

serhii-tkachenko deleted the BOLT-19-2 branch October 20, 2022 08:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(payment): BOLT-19 added BoltCheckout implementation for customer… #1163

feat(payment): BOLT-19 added BoltCheckout implementation for customer… #1163

serhii-tkachenko commented Jun 24, 2021 •

edited

BC-ACherednichenko left a comment

BC-ACherednichenko left a comment

BC-ACherednichenko Jul 1, 2021

serhii-tkachenko Jul 2, 2021 •

edited

davidchin Jul 6, 2021

vitalii-koshovyi left a comment

vitalii-koshovyi Jul 1, 2021

serhii-tkachenko Jul 2, 2021 •

edited

davidchin left a comment

davidchin Jul 6, 2021

serhii-tkachenko Jul 8, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 8, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 8, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 7, 2021

davidchin Jul 8, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 6, 2021

serhii-tkachenko Jul 7, 2021

davidchin Jul 6, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 6, 2021 •

edited

serhii-tkachenko Jul 6, 2021

davidchin Jul 7, 2021

serhii-tkachenko Jul 7, 2021

davidchin Jul 8, 2021

serhii-tkachenko Jul 8, 2021

BC-ACherednichenko Jul 8, 2021

davidchin Jul 6, 2021

serhii-tkachenko Jul 8, 2021

serhii-tkachenko commented Jul 8, 2021

feat(payment): BOLT-19 added BoltCheckout implementation for customer… #1163

feat(payment): BOLT-19 added BoltCheckout implementation for customer… #1163

Conversation

serhii-tkachenko commented Jun 24, 2021 • edited

What?

Why?

Sibling pull-request

Testing / Proof

BC-ACherednichenko left a comment

Choose a reason for hiding this comment

BC-ACherednichenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii-tkachenko Jul 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitalii-koshovyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii-tkachenko Jul 2, 2021 • edited

Choose a reason for hiding this comment

davidchin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii-tkachenko Jul 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhii-tkachenko commented Jul 8, 2021

serhii-tkachenko commented Jun 24, 2021 •

edited

serhii-tkachenko Jul 2, 2021 •

edited

serhii-tkachenko Jul 2, 2021 •

edited

serhii-tkachenko Jul 6, 2021 •

edited