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

Fixes Cart page sign out error when cart does not exist in cache #2189

Merged
merged 3 commits into from
Feb 24, 2020

Conversation

supernova-at
Copy link
Contributor

@supernova-at supernova-at commented Feb 24, 2020

Description

This PR provides a fallback empty object ({}) to avoid a destructuring exception thrown when the cart does not exist in our Apollo cache.

This surfaced on the /cart page after the user signed out: the cache was cleared out using Apollo client's resetStore. But resetStore also causes all active queries to be refetched. GiftOptions has one such query that was querying for the cart in the cache. Because it was just cleared out via resetStore, the query got back undefined, which the code then attempted to destructure.

Providing the fallback empty object in the case of undefined returns the expected result (undefined values for include_gift_receipt, include_printed_card, and gift_message), and results in the ability to sign out from the cart page without error.

Related Issue

Closes PWA-368.

Acceptance

Verification Stakeholders

@revanth0212

Specification

Verification Steps

  1. Sign in
  2. Ensure an item is added to your cart
  3. Navigate to the /cart page
  4. Verify that you can sign out successfully without error
  5. Sign in
  6. Verify that your cart is restored to its previous state

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
Copy link
Contributor

PWAStudioBot commented Feb 24, 2020

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).
📖

Associated JIRA tickets: PWA-368.

Generated by 🚫 dangerJS against e512d61

gift_message
} = cache.data.data[cartIdInCache];
const { include_gift_receipt, include_printed_card, gift_message } =
cache.data.data[cartIdInCache] || {};
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what GraphQL suggests for client side caching/resolvers for this situation. We will likely run into it elsewhere - I wonder if there is some sort of "reset callback" that can reset the store to an initial state. After all, we call resetStore on sign out which was effectively deleting the cache, causing this to throw. Maybe resetStore takes a callback?

This is fine, by the way - just postulating.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I tried that a while ago when I was developing gift options. The problem is our gift options are name spaced to a certain cart id. Reset store, at that time, would not know which cart id to use to reset the store with empty gift options.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah @revanth0212 I forgot about that. So instead of doing it on store reset we would have to know to set that initial state whenever a cart is created. Hmm interesting! I don't think we can do that with the way that carts are created right now.

@m2-community-project m2-community-project bot moved this from Ready for Review to Reviewer Approved in Pull Request Progress Feb 24, 2020
@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label Feb 24, 2020
@m2-community-project m2-community-project bot moved this from Reviewer Approved to Review in Progress in Pull Request Progress Feb 24, 2020
@dpatil-magento dpatil-magento merged commit 4bc217a into develop Feb 24, 2020
@dpatil-magento dpatil-magento deleted the supernova/368_sign_out branch February 24, 2020 19:31
@m2-community-project m2-community-project bot moved this from Review in Progress to Done in Pull Request Progress Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:venia-ui version: Patch This changeset includes backwards compatible bug fixes.
Development

Successfully merging this pull request may close these issues.

None yet

5 participants