bugfix: do not render things when no items are in cart (#2112)

* Move fragments out of components for reuse. Do not render adjustments or price summary when no items are in cart Signed-off-by: Stephen Rugh <[email protected]> * Fix useCartPage tests Signed-off-by: Stephen Rugh <[email protected]> * Compose fragment to top level cart page and reuse in mutation Signed-off-by: Stephen Rugh <[email protected]> * Reset fetch policies to default cache-first. Use ids in all graphql cart queries to match cache entries Signed-off-by: Stephen Rugh <[email protected]> * Move fragment out of product listing query Signed-off-by: Stephen Rugh <[email protected]> * Use id in fragments instead of query Signed-off-by: Stephen Rugh <[email protected]> * readd fetch policy since other mutations would have to be modified and that's out of scope... Signed-off-by: Stephen Rugh <[email protected]> * recompose cart query using child fragments Signed-off-by: Stephen Rugh <[email protected]> * do not render listing if no items. fix tests Signed-off-by: Stephen Rugh <[email protected]>
magento · Jan 23, 2020 · 0d72488 · 0d72488
1 parent cacde9c
commit 0d72488
Show file tree

Hide file tree

Showing 21 changed files with 338 additions and 103 deletions.
diff --git a/packages/peregrine/lib/talons/CartPage/ProductListing/useProduct.js b/packages/peregrine/lib/talons/CartPage/ProductListing/useProduct.js
@@ -4,12 +4,7 @@ import { useMutation } from '@apollo/react-hooks';
 import { useCartContext } from '../../../context/cart';
 
 export const useProduct = props => {
-    const {
-        item,
-        refetchCartQuery,
-        refetchPriceQuery,
-        removeItemMutation
-    } = props;
+    const { item, removeItemMutation } = props;
 
     const flatProduct = flattenProduct(item);
     const [removeItem] = useMutation(removeItemMutation);
@@ -33,24 +28,14 @@ export const useProduct = props => {
             variables: {
                 cartId,
                 itemId: item.id
-            },
-            refetchQueries: [
-                {
-                    query: refetchCartQuery,
-                    variables: { cartId }
-                },
-                {
-                    query: refetchPriceQuery,
-                    variables: { cartId }
-                }
-            ]
+            }
         });
 
         if (error) {
             setIsRemoving(false);
             console.error('Cart Item Removal Error', error);
         }
-    }, [cartId, item.id, refetchCartQuery, refetchPriceQuery, removeItem]);
+    }, [cartId, item.id, removeItem]);
 
     return {
         handleEditItem,

diff --git a/packages/peregrine/lib/talons/CartPage/__tests__/useCartPage.spec.js b/packages/peregrine/lib/talons/CartPage/__tests__/useCartPage.spec.js
@@ -3,7 +3,19 @@ import { createTestInstance } from '@magento/peregrine';
 
 import { useCartPage } from '../useCartPage';
 
-jest.mock('../../../context/app', () => {
+jest.mock('@apollo/react-hooks', () => {
+    const runQuery = jest.fn();
+    const queryResult = {
+        data: null,
+        error: null,
+        loading: false
+    };
+    const useLazyQuery = jest.fn(() => [runQuery, queryResult]);
+
+    return { useLazyQuery };
+});
+
+jest.mock('@magento/peregrine/lib/context/app', () => {
     const state = {};
     const api = {
         toggleDrawer: jest.fn()
@@ -12,7 +24,16 @@ jest.mock('../../../context/app', () => {
 
     return { useAppContext };
 });
-jest.mock('../../../context/user', () => {
+jest.mock('@magento/peregrine/lib/context/cart', () => {
+    const state = {
+        cartId: 'cart123'
+    };
+    const api = {};
+    const useCartContext = jest.fn(() => [state, api]);
+
+    return { useCartContext };
+});
+jest.mock('@magento/peregrine/lib/context/user', () => {
     const state = {
         isSignedIn: false
     };
@@ -24,7 +45,10 @@ jest.mock('../../../context/user', () => {
 
 const log = jest.fn();
 const Component = () => {
-    const talonProps = useCartPage();
+    const cartPageQuery = {};
+    const talonProps = useCartPage({
+        cartPageQuery
+    });
 
     useEffect(() => {
         log(talonProps);
@@ -39,6 +63,7 @@ test('it returns the proper shape', () => {
 
     // Assert.
     expect(log).toHaveBeenCalledWith({
+        hasItems: expect.any(Boolean),
         handleSignIn: expect.any(Function),
         isSignedIn: expect.any(Boolean)
     });

diff --git a/packages/peregrine/lib/talons/CartPage/useCartPage.js b/packages/peregrine/lib/talons/CartPage/useCartPage.js
@@ -1,19 +1,40 @@
-import { useCallback } from 'react';
+import { useCallback, useEffect } from 'react';
+import { useLazyQuery } from '@apollo/react-hooks';
 
-import { useAppContext } from '../../context/app';
-import { useUserContext } from '../../context/user';
+import { useAppContext } from '@magento/peregrine/lib/context/app';
+import { useUserContext } from '@magento/peregrine/lib/context/user';
+import { useCartContext } from '@magento/peregrine/lib/context/cart';
 
-export const useCartPage = () => {
-    const [, appApi] = useAppContext();
-    const { toggleDrawer } = appApi;
+export const useCartPage = props => {
+    const { cartPageQuery } = props;
+
+    const [, { toggleDrawer }] = useAppContext();
     const [{ isSignedIn }] = useUserContext();
+    const [{ cartId }] = useCartContext();
+
+    const [fetchCartData, { data }] = useLazyQuery(cartPageQuery, {
+        // TODO: Purposely overfetch and hit the network until all components
+        // are correctly updating the cache. Will be fixed by PWA-321.
+        fetchPolicy: 'cache-and-network'
+    });
 
     const handleSignIn = useCallback(() => {
         // TODO: set navigation state to "SIGN_IN". useNavigation:showSignIn doesn't work.
         toggleDrawer('nav');
     }, [toggleDrawer]);
 
+    useEffect(() => {
+        if (cartId) {
+            fetchCartData({
+                variables: {
+                    cartId
+                }
+            });
+        }
+    }, [cartId, fetchCartData]);
+
     return {
+        hasItems: !!(data && data.cart.total_quantity),
         handleSignIn,
         isSignedIn
     };

diff --git a/packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js b/packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js
@@ -6,10 +6,11 @@ import Button from '../../Button';
 import { mergeClasses } from '../../../classify';
 import defaultClasses from './priceSummary.css';
 
-import DiscountSummary, { DiscountSummaryFragment } from './discountSummary';
-import GiftCardSummary, { GiftCardSummaryFragment } from './giftCardSummary';
-import ShippingSummary, { ShippingSummaryFragment } from './shippingSummary';
-import TaxSummary, { TaxSummaryFragment } from './taxSummary';
+import DiscountSummary from './discountSummary';
+import GiftCardSummary from './giftCardSummary';
+import ShippingSummary from './shippingSummary';
+import TaxSummary from './taxSummary';
+import { PriceSummaryFragment } from './priceSummaryFragments';
 
 /**
  * A component that fetches and renders cart data including:
@@ -23,7 +24,7 @@ import TaxSummary, { TaxSummaryFragment } from './taxSummary';
 const PriceSummary = props => {
     const classes = mergeClasses(defaultClasses, props.classes);
     const talonProps = usePriceSummary({
-        query: PriceSummaryQuery
+        query: GET_PRICE_SUMMARY
     });
 
     const {
@@ -98,34 +99,14 @@ const PriceSummary = props => {
     );
 };
 
-// queries exported as static member to be used by refetchQueries.
-export const PriceSummaryQuery = gql`
+export const GET_PRICE_SUMMARY = gql`
     query getPriceSummary($cartId: String!) {
         cart(cart_id: $cartId) {
             id
-            items {
-                quantity
-            }
-            ...ShippingSummaryFragment
-            prices {
-                ...TaxSummaryFragment
-                ...DiscountSummaryFragment
-                grand_total {
-                    currency
-                    value
-                }
-                subtotal_excluding_tax {
-                    currency
-                    value
-                }
-            }
-            ...GiftCardSummaryFragment
+            ...PriceSummaryFragment
         }
     }
-    ${ShippingSummaryFragment}
-    ${TaxSummaryFragment}
-    ${DiscountSummaryFragment}
-    ${GiftCardSummaryFragment}
+    ${PriceSummaryFragment}
 `;
 
 export default PriceSummary;
diff --git a/packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummaryFragments.js b/packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummaryFragments.js
@@ -0,0 +1,33 @@
+import gql from 'graphql-tag';
+
+import { DiscountSummaryFragment } from './discountSummary';
+import { GiftCardSummaryFragment } from './giftCardSummary';
+import { ShippingSummaryFragment } from './shippingSummary';
+import { TaxSummaryFragment } from './taxSummary';
+
+export const PriceSummaryFragment = gql`
+    fragment PriceSummaryFragment on Cart {
+        id
+        items {
+            quantity
+        }
+        ...ShippingSummaryFragment
+        prices {
+            ...TaxSummaryFragment
+            ...DiscountSummaryFragment
+            grand_total {
+                currency
+                value
+            }
+            subtotal_excluding_tax {
+                currency
+                value
+            }
+        }
+        ...GiftCardSummaryFragment
+    }
+    ${DiscountSummaryFragment}
+    ${GiftCardSummaryFragment}
+    ${ShippingSummaryFragment}
+    ${TaxSummaryFragment}
+`;
diff --git a/...ib/components/CartPage/ProductListing/__tests__/__snapshots__/productListing.spec.js.snap b/...ib/components/CartPage/ProductListing/__tests__/__snapshots__/productListing.spec.js.snap
@@ -16,8 +16,4 @@ exports[`renders list of products with items in cart 1`] = `
 </ul>
 `;
 
-exports[`renders string with no items in cart 1`] = `
-<h3>
-  There are no items in your cart.
-</h3>
-`;
+exports[`renders null with no items in cart 1`] = `null`;
diff --git a/packages/venia-ui/lib/components/CartPage/ProductListing/__tests__/productListing.spec.js b/packages/venia-ui/lib/components/CartPage/ProductListing/__tests__/productListing.spec.js
@@ -32,7 +32,7 @@ test('renders loading indicator while data fetching', () => {
     expect(root.findByType(LoadingIndicator)).toBeDefined();
 });
 
-test('renders string with no items in cart', () => {
+test('renders null with no items in cart', () => {
     useLazyQuery.mockReturnValueOnce([
         () => {},
         {

diff --git a/packages/venia-ui/lib/components/CartPage/ProductListing/product.js b/packages/venia-ui/lib/components/CartPage/ProductListing/product.js
@@ -9,17 +9,14 @@ import ProductOptions from '../../MiniCart/productOptions';
 import Section from '../../MiniCart/section';
 import Image from '../../Image';
 import defaultClasses from './product.css';
-import { GET_PRODUCT_LISTING } from './productListing';
-import { PriceSummaryQuery } from '../PriceSummary/priceSummary';
+import { CartPageFragment } from '../cartPageFragments';
 
 const IMAGE_SIZE = 100;
 
 const Product = props => {
     const { item } = props;
     const talonProps = useProduct({
         item,
-        refetchPriceQuery: PriceSummaryQuery,
-        refetchCartQuery: GET_PRODUCT_LISTING,
         removeItemMutation: REMOVE_ITEM_MUTATION
     });
     const {
@@ -95,7 +92,9 @@ export const REMOVE_ITEM_MUTATION = gql`
         removeItemFromCart(input: { cart_id: $cartId, cart_item_id: $itemId }) {
             cart {
                 id
+                ...CartPageFragment
             }
         }
     }
+    ${CartPageFragment}
 `;
diff --git a/packages/venia-ui/lib/components/CartPage/ProductListing/productListing.js b/packages/venia-ui/lib/components/CartPage/ProductListing/productListing.js
@@ -6,6 +6,7 @@ import { mergeClasses } from '../../../classify';
 import LoadingIndicator from '../../LoadingIndicator';
 import defaultClasses from './productListing.css';
 import Product from './product';
+import { ProductListingFragment } from './productListingFragments';
 
 const ProductListing = props => {
     const talonProps = useProductListing({ query: GET_PRODUCT_LISTING });
@@ -23,46 +24,18 @@ const ProductListing = props => {
         ));
 
         return <ul className={classes.root}>{productComponents}</ul>;
-    } else {
-        return <h3>There are no items in your cart.</h3>;
     }
-};
-
-export default ProductListing;
 
-export const CartBody = gql`
-    fragment CartBody on Cart {
-        id
-        items {
-            id
-            product {
-                name
-                small_image {
-                    url
-                }
-            }
-            prices {
-                price {
-                    currency
-                    value
-                }
-            }
-            quantity
-            ... on ConfigurableCartItem {
-                configurable_options {
-                    option_label
-                    value_label
-                }
-            }
-        }
-    }
-`;
+    return null;
+};
 
 export const GET_PRODUCT_LISTING = gql`
     query getProductListing($cartId: String!) {
         cart(cart_id: $cartId) {
-            ...CartBody
+            ...ProductListingFragment
         }
-        ${CartBody}
     }
+    ${ProductListingFragment}
 `;
+
+export default ProductListing;
diff --git a/packages/venia-ui/lib/components/CartPage/ProductListing/productListingFragments.js b/packages/venia-ui/lib/components/CartPage/ProductListing/productListingFragments.js
@@ -0,0 +1,29 @@
+import gql from 'graphql-tag';
+
+export const ProductListingFragment = gql`
+    fragment ProductListingFragment on Cart {
+        id
+        items {
+            id
+            product {
+                name
+                small_image {
+                    url
+                }
+            }
+            prices {
+                price {
+                    currency
+                    value
+                }
+            }
+            quantity
+            ... on ConfigurableCartItem {
+                configurable_options {
+                    option_label
+                    value_label
+                }
+            }
+        }
+    }
+`;