Fixes Checkout Shipping Methods Inadvertent Error Message #2371

supernova-at · 2020-04-30T17:13:38Z

Description

This PR updates the way the Shipping Methods component on the Checkout page loads its data which solves two problems:

It was showing an error state when it didn't need to
It was inefficient

1. Incorrectly Showing Error State

The talon determines the proper displayState for the component to be in by looking at cart.shipping_addresses[0].selected_shipping_method.

But it was also inadvertently waiting for the results of fetching all shipping methods before setting a selectedShippingMethod.

Therefore the CompletedView was being rendered before it had a selectedShippingMethod, which caused it to show an error message; it didn't have any data to show.

This PR updates how the talon sets selectedShippingMethod - it no longer waits for all shipping methods. This keeps the talon internally consistent and fixes the inadvertent display of an error message by the CompletedView.

2. Inefficiency

Previously it was only querying for carrier_code and method_code of the cart.shipping_addresses.selected_shipping_method.
Therefore in order to display what it needed to in the CompletedView, it had to wait for the query that fetched all shipping methods to return, and then .find the selected_shipping_method in the list of all available methods.

Instead, this PR modifies the query for selected_shipping_method to include all the fields it needs to display. This means the CompletedView can render immediately without having to wait for all the shipping methods to load first.

Related Issue

Closes PWA-550.

Acceptance

Verification Stakeholders

Specification

Verification Steps

Bug Resolution

Add an item to your cart
Go to the /checkout page
Select a shipping address
Select a shipping method
Refresh the page
See that the Shipping Method card no longer shows an error before displaying the selected shipping method

Shipping Method Full Regression

Because this PR changes how the selectedShippingMethod is determined, the Shipping Method component should get a full regression, including:

Changing the shipping address (to an international country) after selecting a shipping method. Ensure you are presented with the shipping radios again to re-select a shipping method. Ensure the order summary stays up to date.
Update the selected shipping method via the update modal by clicking "Edit". Verify that the method updates correctly.

Screenshots / Screen Captures (if appropriate)

Checklist

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

…_methods

instead of a serialized string value

PWAStudioBot · 2020-04-30T17:27:03Z

	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-550.

Generated by 🚫 dangerJS against 8bbae51

sirugh

A few minor changes but otherwise functional and works!

sirugh · 2020-05-01T16:09:32Z

packages/peregrine/lib/talons/CheckoutPage/useShippingMethod.js

+                    .selected_shipping_method;
+        } catch (err) {
+            // We don't have data from our specific query to fetch the selected shipping method.
+            // Intentionally swallow this error.


Since we hide logs in production we can and should log the error just in case. Swallowing errors can make future us sad.

if (process.env.NODE_ENV === 'development') { console.log(err); }

Fixed in 7318d2b.

This was never really an error worth showing anyone: I was using that try / catch as a way not to have to check each object in chosenShippingMethodData.cart.shipping_addresses[0].selected_shipping_method for existence but it turns out the only one we really have to check for is chosenShippingMethodData itself, so I just went ahead and did that.

sirugh · 2020-05-01T16:14:12Z

packages/peregrine/lib/talons/CheckoutPage/useShippingMethod.js

+                console.log(
+                    'selected method from list of methods',
+                    selectedMethod
+                ); // has all display data needed


Suggested change

console.log(

'selected method from list of methods',

selectedMethod

); // has all display data needed

Oops, thanks!

Fixed in 7318d2b.

sirugh · 2020-05-01T16:18:38Z

packages/venia-ui/lib/components/CheckoutPage/ShippingMethod/shippingRadios.js

@@ -46,11 +46,15 @@ const ShippingRadios = props => {
        return { label, value };
    });

+    // Match the serialization of method objects from the useShippingMethod talon.
+    const { carrier_code, method_code } = selectedShippingMethod;
+    const selectedShippingMethodSerializedValue = `${carrier_code}|${method_code}`;


This logic should probably move to useShippingRadios and then be handled by some utility function that is shared between useShippingMethod and useShippingRadios talons.

👍 I had the same thought and was on the fence about it.

Fixed in 7318d2b.

…tion

sirugh

A+

revanth0212

Awesome 👍

devops-pwa-codebuild · 2020-05-01T22:01:55Z

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-2371.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.66

dpatil-magento · 2020-05-04T15:19:32Z

@supernova-at After selecting Shipping method if we change address to other country then > Previously selected shipping methods loads for a sec and then new available shipping method loads.

dpatil-magento · 2020-05-04T19:16:52Z

Above mentioned will be handled separately.

supernova-at added 4 commits April 30, 2020 11:04

Fixes bug in shipping methods where error was incorrectly shown

a1708f0

Merge remote-tracking branch 'origin/develop' into supernova/550_load…

dd6889e

…_methods

Removes console logs

2e461a6

Update selectedShippingMethod to be an object

20e5e1a

instead of a serialized string value

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

supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Apr 30, 2020

supernova-at changed the title ~~Supernova/550 load methods~~ Fixes Checkout Shipping Methods Inadvertent Error Message Apr 30, 2020

Runs prettier and lint

9299a20

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Apr 30, 2020

supernova-at and others added 3 commits April 30, 2020 12:40

Updates unit tests

1547668

Runs prettier

f53ea79

Merge branch 'develop' into supernova/550_load_methods

cbb3460

sirugh requested changes May 1, 2020

View reviewed changes

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

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

m2-community-project bot added the Progress: changes requested label May 1, 2020

Splits the serialization of shipping methods out into a reusable func…

7318d2b

…tion

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

m2-community-project bot moved this from Changes Requested to Review in Progress in Pull Request Progress May 1, 2020

m2-community-project bot added Progress: review and removed Progress: changes requested labels May 1, 2020

Merge branch 'develop' into supernova/550_load_methods

605ab30

sirugh approved these changes May 1, 2020

View reviewed changes

revanth0212 approved these changes May 1, 2020

View reviewed changes

m2-community-project bot assigned revanth0212 May 1, 2020

m2-community-project bot moved this from Review in Progress to Reviewer Approved in Pull Request Progress May 1, 2020

m2-community-project bot added Progress: approved and removed Progress: review labels May 1, 2020

Merge branch 'develop' into supernova/550_load_methods

8bbae51

dpatil-magento merged commit fb1d118 into develop May 4, 2020

dpatil-magento deleted the supernova/550_load_methods branch May 4, 2020 19:17

m2-community-project bot moved this from Reviewer Approved to Done in Pull Request Progress May 4, 2020

m2-community-project bot removed the Progress: approved label May 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes Checkout Shipping Methods Inadvertent Error Message #2371

Fixes Checkout Shipping Methods Inadvertent Error Message #2371

supernova-at commented Apr 30, 2020

PWAStudioBot commented Apr 30, 2020 •

edited

sirugh left a comment

sirugh May 1, 2020

supernova-at May 1, 2020 •

edited

sirugh May 1, 2020

supernova-at May 1, 2020

sirugh May 1, 2020

supernova-at May 1, 2020

sirugh left a comment

revanth0212 left a comment

devops-pwa-codebuild commented May 1, 2020

dpatil-magento commented May 4, 2020 •

edited

dpatil-magento commented May 4, 2020

Fixes Checkout Shipping Methods Inadvertent Error Message #2371

Fixes Checkout Shipping Methods Inadvertent Error Message #2371

Conversation

supernova-at commented Apr 30, 2020

Description

1. Incorrectly Showing Error State

2. Inefficiency

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Bug Resolution

Shipping Method Full Regression

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Apr 30, 2020 • edited

sirugh left a comment

Choose a reason for hiding this comment

sirugh May 1, 2020

Choose a reason for hiding this comment

supernova-at May 1, 2020 • edited

Choose a reason for hiding this comment

sirugh May 1, 2020

Choose a reason for hiding this comment

supernova-at May 1, 2020

Choose a reason for hiding this comment

sirugh May 1, 2020

Choose a reason for hiding this comment

supernova-at May 1, 2020

Choose a reason for hiding this comment

sirugh left a comment

Choose a reason for hiding this comment

revanth0212 left a comment

Choose a reason for hiding this comment

devops-pwa-codebuild commented May 1, 2020

dpatil-magento commented May 4, 2020 • edited

dpatil-magento commented May 4, 2020

PWAStudioBot commented Apr 30, 2020 •

edited

supernova-at May 1, 2020 •

edited

dpatil-magento commented May 4, 2020 •

edited