Requesting price summary fragment in shipping info mutation. #2445

revanth0212 · 2020-06-01T21:05:41Z

Description

The price summary is not updated on submitting shipping information on the checkout page. This is because the price summary is not being fetched in the mutation result.

Solved by adding price summary fragment to shipping information mutation.

Related Issue

Closes PWA-614

Verification Stakeholders

@tjwiebell

Verification Steps

Go to the deployed app's checkout page.
Update the shipping address to a state that has no tax (Texas). There should be no tax section in the price summary.
Update the shipping address to a state that has tax (California). There should be a tax section in the price summary.

@fooman mentioned there is a setting in the admin panel to perform tax calculation based on billing address instead. @dpatil-magento when you are testing, try to check for that option as well. The code has logic to render tax changes on billing address change as well.

Screenshots / Screen Captures (if appropriate)

Lone Star State (No Tax)

Golden State (😮 Tax)

Checklist

No tests needed. It is a GQL update.

PWAStudioBot · 2020-06-01T21:18:21Z

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

Generated by 🚫 dangerJS against 06608bf

devops-pwa-codebuild · 2020-06-01T21:21:20Z

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-2445.pwa-venia.com : LH Performance Expected 0.85 Actual 0.58, LH Best Practices Expected 1 Actual 0.92
https://pr-2445.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.36, LH Best Practices Expected 1 Actual 0.92
https://pr-2445.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.51, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.92

fooman · 2020-06-01T22:26:38Z

It is possible to configure the back-end to calculate tax based on billing address. Would it make sense to add the same PriceSummaryFragment to the set billing address mutation?

supernova-at

It makes sense that we want the PriceSummaryFragment included in the SET_GUEST_SHIPPING_MUTATION mutation but not in the GET_SHIPPING_INFORMATION query.

packages/venia-ui/lib/components/CheckoutPage/ShippingInformation/shippingInformation.gql.js

supernova-at · 2020-06-02T16:39:52Z

It is possible to configure the back-end to calculate tax based on billing address. Would it make sense to add the same PriceSummaryFragment to the set billing address mutation?

Thanks for pointing this out; I would say yes. If we find that we're overfetching and that is problematic, we can find a way to conditionally include the fragment based on the back-end configuration.

jimbo · 2020-06-02T17:33:48Z

It is possible to configure the back-end to calculate tax based on billing address. Would it make sense to add the same PriceSummaryFragment to the set billing address mutation?

Good callout. I could see how it would make more sense to have it configured that way, too.

revanth0212 · 2020-06-02T17:41:58Z

It is possible to configure the back-end to calculate tax based on billing address. Would it make sense to add the same PriceSummaryFragment to the set billing address mutation?

Nice point @fooman. I have included the fragment in the setBillingAddressMutation as well and try to set the billing address state like Texas to remove the taxes but unfortunately, the backend didn't recognize that. I will include the fragment so if the backend supports it, you should see the tax update on billing address change.

@jimbo @supernova-at any thoughts?

jimbo

Looks good to me now. Let's let QA have a shot at it. 👍

dpatil-magento · 2020-06-03T15:56:46Z

@revanth0212 Zero checkout flow breaks!

Add Emilia Cropped Lace Top product to cart .
Go to checkout and enter Texas address.
Select Free Shipping
Apply gift card 0BPAZOGI82B8 , no payment required as order total is < gift card amount.
Now edit shipping address to California.

Issue - Even though order total is greater than gift card amount, payement module still stale.
(and vice-versa, if CA address entered first then changed to TX.)

revanth0212 · 2020-06-03T16:48:08Z

@revanth0212 Zero checkout flow breaks!

Add Emilia Cropped Lace Top product to cart .

Go to checkout and enter Texas address.

Select Free Shipping

Apply gift card 0BPAZOGI82B8 , no payment required as order total is < gift card amount.

Now edit shipping address to California.

Issue - Even though order total is greater than gift card amount, payement module still stale.
(and vice-versa, if CA address entered first then changed to TX.)

Nice catch @dpatil-magento.

There are 2 things we have to note here.

We have to fetch available payment methods when the shipping address change, which I will add in the next commit. This brings me to another question, should we re-fetch available shipping methods as well, because shipping methods can change when the shipping address has changed. @jimbo @supernova-at should I fetch shipping methods as well along with payment methods in the mutation response?
When I added the GQL fragment to fetch the payment methods on shipping address change, you can see that the Free payment method is still available even though the total is not 0, but when I refresh the page, the GQL query on the same cart id returns a diff response for available payment methods (does not return free). That seems like a bug in GQL.

On shipping address submit:

On refresh:

…ange.

…ub.com/magento/pwa-studio into revanth/checkout_page_price_update_fix

supernova-at · 2020-06-03T20:52:22Z

@jimbo @supernova-at should I fetch shipping methods as well along with payment methods in the mutation response?

Just to update here, that fragment should exist already and the list of available shipping methods should already update on shipping address change.

revanth0212 · 2020-06-03T20:58:16Z

@jimbo @supernova-at should I fetch shipping methods as well along with payment methods in the mutation response?

Just to update here, that fragment should exist already and the list of available shipping methods should already update on shipping address change.

Yeah, you are right. Found it. Thanks Andy.

tjwiebell

Approving, but spotted an odd linter line that should be corrected. Please take a look.

tjwiebell · 2020-06-03T21:09:58Z

packages/venia-ui/lib/components/CheckoutPage/PaymentInformation/paymentInformation.gql.js

        }
    }
+    ${AvailablePaymentMethodsFragment}
 `;

 /* eslint-disable graphql/required-fields */


Not related to this PR, but couldn't help but notice this rule get disabled, and never re-enabled. This is probably preventing any file after this from being properly linted. Consider fixing in this scope, or please create a followup task to resolve this.

Good catch Tommy. Havn't noticed that. Also looks like my linter is not working 😓 . Fixed it in 06608bf

tjwiebell · 2020-06-03T21:21:18Z

packages/venia-ui/lib/components/CheckoutPage/PaymentInformation/creditCard.gql.js

@@ -1,5 +1,8 @@
 import gql from 'graphql-tag';

+import { PriceSummaryFragment } from '../../CartPage/PriceSummary/priceSummaryFragments';


On line 28 in this file too, I think we intended to turn the lint rule back on.

Requesting price summary fragment in shipping info mutation.

2141979

revanth0212 added the version: Patch This changeset includes backwards compatible bug fixes. label Jun 1, 2020

PWAStudioBot added the pkg:venia-ui label Jun 1, 2020

supernova-at requested changes Jun 2, 2020

View reviewed changes

packages/venia-ui/lib/components/CheckoutPage/ShippingInformation/shippingInformation.gql.js Show resolved Hide resolved

m2-community-project bot assigned supernova-at Jun 2, 2020

m2-community-project bot added the Progress: changes requested label Jun 2, 2020

m2-community-project bot assigned revanth0212 Jun 2, 2020

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

Fetching order summary on billing address change.

0b6e452

jimbo previously approved these changes Jun 2, 2020

View reviewed changes

m2-community-project bot assigned jimbo Jun 2, 2020

m2-community-project bot added Progress: approved and removed Progress: review labels Jun 2, 2020

supernova-at previously approved these changes Jun 2, 2020

View reviewed changes

Merge branch 'develop' into revanth/checkout_page_price_update_fix

9104dbf

dpatil-magento and others added 2 commits June 3, 2020 11:50

Merge branch 'develop' into revanth/checkout_page_price_update_fix

d901eae

Fetching available payment methods on shipping and billing address ch…

a2b0cb4

…ange.

Merge branch 'revanth/checkout_page_price_update_fix' of https://gith…

96555dd

…ub.com/magento/pwa-studio into revanth/checkout_page_price_update_fix

revanth0212 dismissed stale reviews from supernova-at and jimbo via 96555dd June 3, 2020 17:59

revanth0212 and others added 2 commits June 3, 2020 14:32

Added id in available payment methods fragment.

553e312

Merge branch 'develop' into revanth/checkout_page_price_update_fix

52e90f9

supernova-at previously approved these changes Jun 3, 2020

View reviewed changes

tjwiebell previously approved these changes Jun 3, 2020

View reviewed changes

m2-community-project bot assigned tjwiebell Jun 3, 2020

tjwiebell reviewed Jun 3, 2020

View reviewed changes

Minor.

06608bf

revanth0212 dismissed stale reviews from tjwiebell and supernova-at via 06608bf June 3, 2020 21:36

jimbo approved these changes Jun 3, 2020

View reviewed changes

dpatil-magento self-requested a review June 3, 2020 22:13

dpatil-magento approved these changes Jun 3, 2020

View reviewed changes

m2-community-project bot assigned dpatil-magento Jun 3, 2020

dpatil-magento merged commit e7d2c68 into develop Jun 3, 2020

dpatil-magento deleted the revanth/checkout_page_price_update_fix branch June 3, 2020 22:13

m2-community-project bot removed the Progress: approved label Jun 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Requesting price summary fragment in shipping info mutation. #2445

Requesting price summary fragment in shipping info mutation. #2445

revanth0212 commented Jun 1, 2020 •

edited

Loading

PWAStudioBot commented Jun 1, 2020 •

edited

Loading

devops-pwa-codebuild commented Jun 1, 2020 •

edited

Loading

fooman commented Jun 1, 2020

supernova-at left a comment

supernova-at commented Jun 2, 2020

jimbo commented Jun 2, 2020

revanth0212 commented Jun 2, 2020 •

edited

Loading

jimbo left a comment

dpatil-magento commented Jun 3, 2020

revanth0212 commented Jun 3, 2020

supernova-at commented Jun 3, 2020

revanth0212 commented Jun 3, 2020

tjwiebell left a comment

tjwiebell Jun 3, 2020

revanth0212 Jun 3, 2020

tjwiebell Jun 3, 2020

		@@ -1,5 +1,8 @@
		import gql from 'graphql-tag';

		import { PriceSummaryFragment } from '../../CartPage/PriceSummary/priceSummaryFragments';

Requesting price summary fragment in shipping info mutation. #2445

Requesting price summary fragment in shipping info mutation. #2445

Conversation

revanth0212 commented Jun 1, 2020 • edited Loading

Description

Related Issue

Verification Stakeholders

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Jun 1, 2020 • edited Loading

devops-pwa-codebuild commented Jun 1, 2020 • edited Loading

fooman commented Jun 1, 2020

supernova-at left a comment

Choose a reason for hiding this comment

supernova-at commented Jun 2, 2020

jimbo commented Jun 2, 2020

revanth0212 commented Jun 2, 2020 • edited Loading

jimbo left a comment

Choose a reason for hiding this comment

dpatil-magento commented Jun 3, 2020

revanth0212 commented Jun 3, 2020

supernova-at commented Jun 3, 2020

revanth0212 commented Jun 3, 2020

tjwiebell left a comment

Choose a reason for hiding this comment

tjwiebell Jun 3, 2020

Choose a reason for hiding this comment

revanth0212 Jun 3, 2020

Choose a reason for hiding this comment

tjwiebell Jun 3, 2020

Choose a reason for hiding this comment

revanth0212 commented Jun 1, 2020 •

edited

Loading

PWAStudioBot commented Jun 1, 2020 •

edited

Loading

devops-pwa-codebuild commented Jun 1, 2020 •

edited

Loading

revanth0212 commented Jun 2, 2020 •

edited

Loading