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 duplicate shipping methods in first checkout #2880

Conversation

foladipo
Copy link
Contributor

@foladipo foladipo commented Sep 17, 2017

Resolves #2866.

This PR fixes a bug in the checkout process. Previously, the very first time you checkout (after a reaction reset or a fresh installation of Reaction), you get duplicate instances of flat rate shipping methods. This PR fixes that.

Test Instructions

Part 1:

  1. Do this: reaction reset
  2. Login as admin.
  3. Enable flat rate shipping methods.
  4. Add a product to your cart and proceed to checkout.
  5. Enter your address and save it.
  6. Observe that the flat rate shipping methods are not duplicated.

Part 2:

  1. Ensure you left flat rate shipping enabled from part 1 above.
  2. Open an incognito window.
  3. Add items to your cart and checkout.
  4. Add an address and save it.
  5. Observe that flat rate shipping methods are not duplicated.
  6. Repeat this process two or more times and observe that flat rate shipping methods are still not duplicated.

@foladipo foladipo changed the title [WIP] Fix duplicate shipping methods in first checkout Fix duplicate shipping methods in first checkout Sep 17, 2017
@brent-hoover
Copy link
Collaborator

This works and I am going to merge it but maybe you can help me understand the problem and why this fixes it better.

@brent-hoover brent-hoover merged commit 670ed43 into marketplace Sep 18, 2017
@brent-hoover brent-hoover deleted the folusho-fix-duplicate-shipping-methods-after-reset-2866 branch September 18, 2017 00:09
@@ -34,6 +34,23 @@ function getShippingRates(previousQueryResults, cart) {
return previousQueryResults;
}
}
if (!(cart.shipping && cart.shipping[0] && cart.shipping[0].address)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

One of our goals was to eliminate these hard-coded 0th indexes - I'm a bit concerned about introducing these here as it seems like adding technical debt immediately

@foladipo
Copy link
Contributor Author

The reason why this fixes the bugs is that, on the first ever checkout, cart.shipping will be undefined/empty. So, a shipping plugin should not attempt, in that case, to add shipping methods to the cart because, since cart.shipping is undefined/empty, there won't be cart.shipping[0].address, which is necessary for any shipping activity. The reaction-shippo plugin took care of this, which was why it wasn't duplicated on the first checkout. So, I add to handle this case for the flat rate shipping pluggin too.

Much of the same argument also applies to when cart.items is undefined or empty.

@brent-hoover
Copy link
Collaborator

Unfortunately I don't think that assumption is always correct. If a user has checked out and has selected a default address before it immediately attaches the address to the cart as soon as it is created.

@foladipo
Copy link
Contributor Author

Are you saying a user can have a saved address, and thus have a non-empty cart.shipping BEFORE they make their first ever purchase?

@spencern spencern mentioned this pull request Oct 11, 2017
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.

3 participants