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

fix: don't execute unnecessary requests for anonymous cart on initial load #705

Merged
merged 5 commits into from
Jul 20, 2020

Conversation

willopez
Copy link
Member

@willopez willopez commented Jul 15, 2020

Resolves #701
Impact: minor
Type: bugfix

Issue

  1. Unnecessary requests for anonymous cart are executed when they should not. The anonymous cart query should only executed after a user has added an item to their cart.
  2. An error was being returned by an api route when the user was anonymous, it should not be an error, just a flag that says that the user is anonymous.

Solution

  1. Use lazyQuerys to control when cart requests are executed.
  2. Return a boolean flag anonymous to denote when a user is anonymous

Breaking changes

Testing

  1. Load product grid and verify no anonymousCartByCartIdQuery request are executed.
  2. Add and remove items to and from the cart and verify they are add/removed as expected.
  3. Complete a checkout flow and place an order
  4. Verify there are no error for the api/token route

@willopez willopez marked this pull request as ready for review July 15, 2020 23:15
Copy link
Member

@manueldelreal manueldelreal left a comment

Choose a reason for hiding this comment

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

LGTM! are we including this in the pending release branch?

Copy link
Collaborator

@janus-reith janus-reith left a comment

Choose a reason for hiding this comment

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

I think using Lazy Query currently is the best solution.
Although the related errors in the Apollo-client repo were closed with the release of 3.0.1, multiple people still observed the described error.
I also tried upgrading apollo-client to the latest v3.0.2 and as that didnt work, also upgraded the remaining apollo packages that we're using to their latest version, also without any success.

After looking at your changes I checked out the PR locally and can confirm that the error is fixed.
Tried out anonymous carts, account carts and reconciliation, all seem to work fine.

The only minor change I'd like to suggest is the wording for refetchAccountCart / refetchCartAnonymous.

hooks/cart/useCart.js Outdated Show resolved Hide resolved
hooks/cart/useCart.js Outdated Show resolved Hide resolved
hooks/cart/useCart.js Outdated Show resolved Hide resolved
@willopez
Copy link
Member Author

@manueldelreal Yes, we will include this PR in the upcoming release.

@willopez
Copy link
Member Author

@janus-reith Thanks for the suggestions and testing your suggestion has been implemented.

@willopez willopez merged commit 8b295f3 into trunk Jul 20, 2020
@willopez willopez deleted the willopez-fix-bugs branch July 20, 2020 18:23
@janus-reith
Copy link
Collaborator

@willopez Looks like the upcoming v3.1.2 might fix this, so we could remove that workaround again: apollographql/apollo-client#6752

@kieckhafer kieckhafer mentioned this pull request Sep 25, 2020
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.

Unnecessary request for anonymous cart and account cart
3 participants