@W-19448788 Tightening the returning shopper flow#3467
@W-19448788 Tightening the returning shopper flow#3467syadupathi-sf merged 6 commits intofeature/1cc_paymentsfrom
Conversation
syadupathi-sf
added
the
skip changelog
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| // Make sure UI reflects merged state before proceeding | ||
| await currentBasketQuery.refetch() | ||
| const mergedBasketId = merged?.basketId || merged?.basket_id || merged?.id | ||
| const refreshedBasketId = await currentBasketQuery.refetch() |
There was a problem hiding this comment.
Do we need to refetch here? Or just using the mergedBasketId is sufficient already for the call below?
From our testing seems like this refetch does not actually give us the right basket currently - or no?
There was a problem hiding this comment.
The right basket is fetched intermittently. Sometimes it is not. But the situation is that we don't know which is the right basket until we make the updateCustomerForBasket call, which fails when it is not the right basket. Either we completely rely on the mergedBasketId or we have to update the updateCustomerForBasket logic.
| body: {email: email} | ||
| }) | ||
| try { | ||
| await updateCustomerForBasket.mutateAsync({ |
There was a problem hiding this comment.
Would there be a scenario where email is empty here? Prolly not but maybe it's worth doing a check here?
On the other hand, what if we're merged basket already has an email, are we overriding - but then I'm assuming the email would be the same since it's unique per customer?
There was a problem hiding this comment.
Great questions. I would also just add this bit from the API doc.
This endpoint doesn't merge Personally Identifiable Information (PII).
I think it might be technically possible to have a different email because while email will be unique for a login that doesn't necessarily mean that a basket can't contain a different email than the user's login. Pretty edgy scenario but might be technically possible.
There was a problem hiding this comment.
The email cannot be null because we can't move ahead in checkout without entering an email address. And how can the merge basket have a different email address? Aren't we determining the user login based on the email address?
There was a problem hiding this comment.
- User logs in to storefront using their email
- Creates a basket and edits the email
- They leave the storefront without placing an order and subsequently they get logged out
- User comes back to storefront as guest and creates a basket
- They login via OTP using their users email
- We merge baskets. Emails differ
It's an unlikely scenario but I believe it's technically possible.
There was a problem hiding this comment.
At step 5, if they are using the changed email, then would they still be associated with the basket created in step 2? We can test this scenario and see what happens.
| }) | ||
| // Make sure UI reflects merged state before proceeding | ||
| await currentBasketQuery.refetch() | ||
| const mergedBasketId = merged?.basketId || merged?.basket_id || merged?.id |
There was a problem hiding this comment.
I dont think the response shape will ever be different in this case will it? If so we need test cases for each otherwise coverage will be affected
There was a problem hiding this comment.
Yes, I will remove the ORs
Signed-off-by: syadupathi-sf <66088780+syadupathi-sf@users.noreply.github.com>
4 participants
Description
As a returning user, intermittently, the updateCustomerForBasket call is being made with the guest basket and not the merged basket. The response is a Basket Not Found error. The UI hangs. This is the sequence of events:
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization