Address QA feedback

magento · Nov 5, 2020 · 1c1a478 · 1c1a478
1 parent 115c3ac
commit 1c1a478
Show file tree

Hide file tree

Showing 12 changed files with 99 additions and 80 deletions.
diff --git a/...es/peregrine/lib/talons/OrderHistoryPage/__tests__/__snapshots__/useOrderRow.spec.js.snap b/...es/peregrine/lib/talons/OrderHistoryPage/__tests__/__snapshots__/useOrderRow.spec.js.snap
@@ -8,11 +8,13 @@ Object {
       "thumbnail": Object {
         "url": "sku1 thumbnail url",
       },
+      "url_key": "sku1",
     },
     Object {
       "thumbnail": Object {
         "url": "sku2 thumbnail url",
       },
+      "url_key": "sku2",
     },
   ],
   "isOpen": false,

diff --git a/packages/peregrine/lib/talons/OrderHistoryPage/__tests__/useOrderRow.spec.js b/packages/peregrine/lib/talons/OrderHistoryPage/__tests__/useOrderRow.spec.js
@@ -25,36 +25,58 @@ const Component = props => {
     return null;
 };
 
+const defaultProps = {
+    queries: { getProductThumbnailsQuery: 'getProductThumbnailsQuery' },
+    items: [{ product_url_key: 'sku1' }, { product_url_key: 'sku2' }]
+};
+const items = [
+    { thumbnail: { url: 'sku1 thumbnail url' }, url_key: 'sku1' },
+    { thumbnail: { url: 'sku2 thumbnail url' }, url_key: 'sku2' }
+];
+const dataResponse = {
+    data: {
+        products: {
+            items
+        }
+    },
+    loading: false
+};
+
 test('returns correct shape', () => {
-    const items = [
-        { thumbnail: { url: 'sku1 thumbnail url' } },
-        { thumbnail: { url: 'sku2 thumbnail url' } }
-    ];
-    useQuery.mockReturnValue({ data: { products: { items } }, loading: false });
-    createTestInstance(
-        <Component
-            queries={{ getProductThumbnailsQuery: 'getProductThumbnailsQuery' }}
-            items={[{ product_sku: 'sku1' }, { product_sku: 'sku2' }]}
-        />
-    );
+    useQuery.mockReturnValue(dataResponse);
+    createTestInstance(<Component {...defaultProps} />);
 
     const talonProps = log.mock.calls[0][0];
 
     expect(talonProps).toMatchSnapshot();
 });
 
+test('filters out items not in the request', () => {
+    useQuery.mockReturnValue({
+        data: {
+            products: {
+                items: [
+                    ...items,
+                    {
+                        thumbnail: { url: 'bundle-sku thumbnail url' },
+                        url_key: 'bundle-sku'
+                    }
+                ]
+            }
+        },
+        loading: false
+    });
+
+    createTestInstance(<Component {...defaultProps} />);
+
+    const talonProps = log.mock.calls[0][0];
+
+    expect(talonProps.imagesData).toHaveLength(2);
+});
+
 test('callback toggles open state', () => {
-    const items = [
-        { thumbnail: { url: 'sku1 thumbnail url' } },
-        { thumbnail: { url: 'sku2 thumbnail url' } }
-    ];
-    useQuery.mockReturnValue({ data: { products: { items } }, loading: false });
-    createTestInstance(
-        <Component
-            queries={{ getProductThumbnailsQuery: 'getProductThumbnailsQuery' }}
-            items={[{ product_sku: 'sku1' }, { product_sku: 'sku2' }]}
-        />
-    );
+    useQuery.mockReturnValue(dataResponse);
+    createTestInstance(<Component {...defaultProps} />);
 
     const talonProps = log.mock.calls[0][0];
 

diff --git a/packages/peregrine/lib/talons/OrderHistoryPage/orderHistoryPage.gql.js b/packages/peregrine/lib/talons/OrderHistoryPage/orderHistoryPage.gql.js
@@ -48,7 +48,6 @@ export const GET_CUSTOMER_ORDERS = gql`
                     shipments {
                         id
                         tracking {
-                            carrier
                             number
                         }
                     }

diff --git a/packages/peregrine/lib/talons/OrderHistoryPage/useOrderRow.js b/packages/peregrine/lib/talons/OrderHistoryPage/useOrderRow.js
@@ -31,11 +31,15 @@ export const useOrderRow = props => {
     });
     const imagesData = useMemo(() => {
         if (data) {
-            return data.products.items;
+            // filter out items returned that we didn't query for
+            const filteredItems = data.products.items.filter(item =>
+                urlKeys.includes(item.url_key)
+            );
+            return filteredItems;
         } else {
             return [];
         }
-    }, [data]);
+    }, [data, urlKeys]);
 
     const [isOpen, setIsOpen] = useState(false);
 

diff --git a/packages/venia-ui/i18n/en_US.json b/packages/venia-ui/i18n/en_US.json
@@ -230,7 +230,7 @@
     "orderDetails.subtotal": "Subtotal",
     "orderDetails.tax": "Tax",
     "orderDetails.total": "Total",
-    "orderDetails.trackingInformation": "{carrier} Tracking: {number}",
+    "orderDetails.trackingInformation": "<strong>Tracking number:</strong> {number}",
     "orderHistoryPage.emptyDataMessage": "You don't have any orders yet.",
     "orderHistoryPage.pageTitleText": "Order History",
     "orderProgressBar.deliveredText": "Delivered",

diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/billingInformation.css b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/billingInformation.css
@@ -4,19 +4,6 @@
 }
 
 .heading {
-    grid-row: 1 / span 1;
     font-weight: var(--venia-global-fontWeight-bold);
     padding-bottom: 0.375rem;
 }
-
-.name {
-    grid-row: 2 / span 1;
-}
-
-.addressLine1 {
-    grid-row: 3 / span 1;
-}
-
-.addressLine2 {
-    grid-row: 4 / span 1;
-}
diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/billingInformation.js b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/billingInformation.js
@@ -21,6 +21,13 @@ const BillingInformation = props => {
 
     const additionalAddressString = `${city}, ${region} ${postcode} ${country_code}`;
     const fullName = `${firstname} ${lastname}`;
+    const streetRows = street.map((row, index) => {
+        return (
+            <span className={classes.streetRow} key={index}>
+                {row}
+            </span>
+        );
+    });
 
     return (
         <div className={classes.root}>
@@ -31,8 +38,8 @@ const BillingInformation = props => {
                 />
             </div>
             <span className={classes.name}>{fullName}</span>
-            <div className={classes.addressLine1}>{street}</div>
-            <div className={classes.addressLine2}>
+            {streetRows}
+            <div className={classes.additionalAddress}>
                 {additionalAddressString}
             </div>
         </div>
@@ -46,16 +53,16 @@ BillingInformation.propTypes = {
         root: string,
         heading: string,
         name: string,
-        addressLine1: string,
-        addressLine2: string
+        streetRow: string,
+        additionalAddress: string
     }),
     data: shape({
         city: string,
         country_code: string,
         firstname: string,
         lastname: string,
         postcode: string,
-        region_id: string,
+        region: string,
         street: arrayOf(string)
     })
 };
diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingInformation.css b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingInformation.css
@@ -4,19 +4,6 @@
 }
 
 .heading {
-    grid-row: 1 / span 1;
     font-weight: var(--venia-global-fontWeight-bold);
     padding-bottom: 0.375rem;
 }
-
-.name {
-    grid-row: 2 / span 1;
-}
-
-.addressLine1 {
-    grid-row: 3 / span 1;
-}
-
-.addressLine2 {
-    grid-row: 4 / span 1;
-}
diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingInformation.js b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingInformation.js
@@ -21,6 +21,13 @@ const ShippingInformation = props => {
 
     const additionalAddressString = `${city}, ${region} ${postcode} ${country_code}`;
     const fullName = `${firstname} ${lastname}`;
+    const streetRows = street.map((row, index) => {
+        return (
+            <span className={classes.streetRow} key={index}>
+                {row}
+            </span>
+        );
+    });
 
     return (
         <div className={classes.root}>
@@ -31,8 +38,8 @@ const ShippingInformation = props => {
                 />
             </div>
             <span className={classes.name}>{fullName}</span>
-            <div className={classes.addressLine1}>{street}</div>
-            <div className={classes.addressLine2}>
+            {streetRows}
+            <div className={classes.additionalAddress}>
                 {additionalAddressString}
             </div>
         </div>
@@ -46,16 +53,16 @@ ShippingInformation.propTypes = {
         root: string,
         heading: string,
         name: string,
-        addressLine1: string,
-        addressLine2: string
+        streetRow: string,
+        additionalAddress: string
     }),
     data: shape({
         city: string,
         country_code: string,
         firstname: string,
         lastname: string,
         postcode: string,
-        region_id: string,
+        region: string,
         street: arrayOf(string),
         telephone: string
     })

diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingMethod.css b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingMethod.css
@@ -14,5 +14,7 @@
 }
 
 .tracking {
+    display: grid;
     grid-row: 3 / span 1;
+    row-gap: 0.375rem;
 }
diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingMethod.js b/packages/venia-ui/lib/components/OrderHistoryPage/OrderDetails/shippingMethod.js
@@ -17,21 +17,24 @@ const ShippingMethod = props => {
      */
     let trackingElement;
     if (shipments.length) {
-        const [{ tracking }] = shipments;
+        trackingElement = shipments.map(shipment => {
+            const { tracking } = shipment;
+            if (tracking.length) {
+                const [{ number }] = tracking;
 
-        if (tracking.length) {
-            const [{ carrier, number }] = tracking;
-
-            trackingElement = (
-                <FormattedMessage
-                    id="orderDetails.trackingInformation"
-                    values={{
-                        carrier,
-                        number
-                    }}
-                />
-            );
-        }
+                return (
+                    <span className={classes.trackingRow}>
+                        <FormattedMessage
+                            id="orderDetails.trackingInformation"
+                            values={{
+                                number,
+                                strong: chunks => <strong>{chunks}</strong>
+                            }}
+                        />
+                    </span>
+                );
+            }
+        });
     } else {
         trackingElement = (
             <FormattedMessage
@@ -62,7 +65,8 @@ ShippingMethod.propTypes = {
         root: string,
         heading: string,
         method: string,
-        tracking: string
+        tracking: string,
+        trackingRow: string
     }),
     data: shape({
         shippingMethod: string,
@@ -71,7 +75,6 @@ ShippingMethod.propTypes = {
                 id: string,
                 tracking: arrayOf(
                     shape({
-                        carrier: string,
                         number: string
                     })
                 )

diff --git a/packages/venia-ui/lib/components/OrderHistoryPage/orderRow.js b/packages/venia-ui/lib/components/OrderHistoryPage/orderRow.js
@@ -189,7 +189,7 @@ OrderRow.propTypes = {
         ),
         invoices: arrayOf(
             shape({
-                id: number
+                id: string
             })
         ),
         number: string,
@@ -221,7 +221,6 @@ OrderRow.propTypes = {
                 id: string,
                 tracking: arrayOf(
                     shape({
-                        carrier: string,
                         number: string
                     })
                 )
-Original file line number
+Diff line change
@@ Expand Up / @@ -48,7 +48,6 @@ export const GET_CUSTOMER_ORDERS = gql` @@
                         shipments {
                             id
                             tracking {
-                                carrier
                                 number
                             }
                         }
@@ Expand Down @@