When auth token is invalid reset cart and refetch details #2379

sirugh · 2020-05-04T15:48:11Z

Description

The retry logic was not functioning when a guest user tried to perform an operation on an "authed" cart. This situation could arise if a user signs in and then the auth token expires naturally (1 hour ttl). The old, authed, cart id persisted and was used but since there was no more token in store the request failed.

Related Issue

Closes #2378 .

Acceptance

Verification Stakeholders

Specification

Verification Steps

Log in and add an item to your cart.
In local storage, change the M2_VENIA_BROWSER_PERSISTENCE__signin_token entry to have a timeStored of some low number like 1.
Add another item to your cart. The operation should succeed but the cart should only have the item you just added. The cart trigger should be updated to show 1 item in the cart.

Rerun the above steps but on step 3 refresh the page and then add an item.

Screenshots / Screen Captures (if appropriate)

Checklist

I have updated the documentation accordingly, if necessary.
I have added tests to cover my changes, if necessary.

PWAStudioBot · 2020-05-04T16:01:51Z

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against 4778e15

Signed-off-by: sirugh <rugh@adobe.com>

sirugh · 2020-05-04T16:34:23Z

packages/peregrine/lib/store/actions/cart/asyncActions.js

+            err =>
+                err.message.includes('Could not find a cart') ||
+                err.message.includes(
+                    'The current user cannot perform operations on cart'


This addition is all that is necessary for the user flow to "work". The other changes in this PR are to make the UI (cart trigger item qty) update properly without having to refresh the page or add another item

…fresh

tjwiebell · 2020-05-13T21:46:07Z

packages/peregrine/lib/talons/Header/useCartTrigger.js

    const [, { toggleDrawer }] = useAppContext();
    const [{ cartId }, { getCartDetails }] = useCartContext();

-    const [getItemCount, { data }] = useLazyQuery(getItemCountQuery);
+    const [getItemCount, { data }] = useLazyQuery(getItemCountQuery, {
+        fetchPolicy: 'cache-and-network'


@sirugh - I couldn't get the CartTrigger to update without a refreshing unless I added this fetch policy. I suspect because the manual cache deletions don't trigger lifecycle events, it was still pulling the value from the in-memory cache.

supernova-at

Code looks good.

Running through the verification steps though,

Log in and add an item to your cart.

In local storage, change the M2_VENIA_BROWSER_PERSISTENCE__signin_token entry to have a > timeStored of some low number like 1.

Add another item to your cart. The operation should succeed but the cart should only have the > item you just added. The cart trigger should be updated to show 1 item in the cart.

Then I sign out and sign back in again. The item I added in step 1 is in my cart, not the item I added in step 3. This doesn't seem correct.

Edit: Well, I guess that does make sense.
But it's confusing to a user (in this case me) that my authed cart got swapped out for a guest cart. When I add that second item I have no idea that my auth expired. Everywhere else in the app (left drawer) says I'm signed in (authed).

supernova-at · 2020-05-14T19:13:36Z

packages/peregrine/lib/store/actions/cart/asyncActions.js

+                    await dispatch(signOut());
+                } else {
+                    // Delete the cached ID from local storage.
+                    await dispatch(removeCart());


I was a little surprised to see this as an else. I'm assuming both removeCart and signOut delete the auth token from local storage?

Ok nevermind, just read user/asyncActions. Might be useful to add that removeCart is called as part of signOut to the comment here.

I'm going to investigate this more; this change should be resetting customer global state back to initial guest values, so if there are still components rendering like you're logged in, there's a bug somewhere.

Will add that comment and resolve the one area I've observed stale data (left drawer is still pinned like you're logged in). Did you see any other components still rendering with customer data?

That was the one I noticed too. No others I think.

Fixed in ae8fe42

supernova-at

Verification steps

supernova-at · 2020-05-19T16:50:53Z

packages/peregrine/lib/talons/AuthModal/useAuthModal.js

@@ -62,9 +71,10 @@ export const useAuthModal = props => {
    }, [showMyAccount]);

    const handleSignOut = useCallback(async () => {
+        setIsSigningOut(true);


Does this ever go back to false?

Or do we rely on unmount for that?

Correct, this callback will trigger a page refresh, performing the reset for us.

supernova-at · 2020-05-19T16:56:26Z

After adding the second item in the verification steps I'm still appearing as signed in when I open the left drawer.

So I think I'm signed in with item (2) as the only thing in my cart.
But if I log out and log back in, item (1) is the only thing in my cart.

tjwiebell · 2020-05-19T21:32:33Z

@supernova-at - I didn't even run through those verification steps 🤦 this has been resolved in e41d29f.

supernova-at · 2020-05-20T16:24:38Z

This is much better - thanks!

I still think there's some awkwardness here, but that may just be the nature of the issue. Off the top of my head I was thinking a toast or something to inform the user that they've been signed out, or better yet a prompt asking if they'd like to "stay signed in".

Since we don't have UX for any of that though, I think there's enough value here to get this in.

devops-pwa-codebuild · 2020-05-20T17:56:03Z

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2379.pwa-venia.com : LH Performance Expected 0.85 Actual 0.57, LH Best Practices Expected 1 Actual 0.92
https://pr-2379.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.37, LH Best Practices Expected 1 Actual 0.92
https://pr-2379.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.51, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.92

m2-community-project bot added this to Ready for Review in Pull Request Progress May 4, 2020

When auth token is invalid reset cart in redux

8455fda

sirugh force-pushed the rugh/fix-mini-cart-auth-expiry branch from 37b26df to 8455fda Compare May 4, 2020 15:59

PWAStudioBot added the pkg:peregrine label May 4, 2020

sirugh mentioned this pull request May 4, 2020

[bug]: Expired token results in graphql errors in mini cart #2378

Closed

8 tasks

sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label May 4, 2020

Manually clear cart from apollo cache on getCartDetails retry

ff5f47c

Signed-off-by: sirugh <rugh@adobe.com>

sirugh requested review from tjwiebell and jimbo May 4, 2020 16:32

sirugh commented May 4, 2020

View reviewed changes

m2-community-project bot assigned sirugh May 4, 2020

m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress May 4, 2020

m2-community-project bot added the Progress: review label May 4, 2020

Handle use case where cart permission error happens without a page re…

703f03a

…fresh

tjwiebell reviewed May 13, 2020

View reviewed changes

m2-community-project bot assigned tjwiebell May 13, 2020

tjwiebell marked this pull request as ready for review May 13, 2020 21:46

tjwiebell added 2 commits May 13, 2020 16:58

Fix tests

39f809d

Merge branch 'develop' into rugh/fix-mini-cart-auth-expiry

212e6c0

PWAStudioBot added the pkg:venia-ui label May 13, 2020

supernova-at reviewed May 14, 2020

View reviewed changes

m2-community-project bot assigned supernova-at May 14, 2020

supernova-at requested changes May 14, 2020

View reviewed changes

Handle view reset and add detail to comment

ae8fe42

supernova-at reviewed May 19, 2020

View reviewed changes

Also reset user state on add to cart failures

e41d29f

supernova-at approved these changes May 20, 2020

View reviewed changes

Merge branch 'develop' into rugh/fix-mini-cart-auth-expiry

4778e15

dpatil-magento merged commit 2089baa into develop May 20, 2020

dpatil-magento deleted the rugh/fix-mini-cart-auth-expiry branch May 20, 2020 22:34

m2-community-project bot moved this from Review in Progress to Done in Pull Request Progress May 20, 2020

m2-community-project bot removed the Progress: review label May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

When auth token is invalid reset cart and refetch details #2379

When auth token is invalid reset cart and refetch details #2379

sirugh commented May 4, 2020 •

edited

PWAStudioBot commented May 4, 2020 •

edited

sirugh May 4, 2020

tjwiebell May 13, 2020 •

edited

supernova-at left a comment •

edited

supernova-at May 14, 2020

supernova-at May 14, 2020

tjwiebell May 18, 2020

supernova-at May 18, 2020

tjwiebell May 18, 2020

supernova-at left a comment

supernova-at May 19, 2020

supernova-at May 19, 2020

tjwiebell May 19, 2020

supernova-at commented May 19, 2020 •

edited

tjwiebell commented May 19, 2020

supernova-at commented May 20, 2020

devops-pwa-codebuild commented May 20, 2020

When auth token is invalid reset cart and refetch details #2379

When auth token is invalid reset cart and refetch details #2379

Conversation

sirugh commented May 4, 2020 • edited

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented May 4, 2020 • edited

Choose a reason for hiding this comment

tjwiebell May 13, 2020 • edited

Choose a reason for hiding this comment

supernova-at left a comment • 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

supernova-at 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

supernova-at commented May 19, 2020 • edited

tjwiebell commented May 19, 2020

supernova-at commented May 20, 2020

devops-pwa-codebuild commented May 20, 2020

sirugh commented May 4, 2020 •

edited

PWAStudioBot commented May 4, 2020 •

edited

tjwiebell May 13, 2020 •

edited

supernova-at left a comment •

edited

supernova-at commented May 19, 2020 •

edited