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 for console errors on server #3064

Merged
merged 8 commits into from
Oct 10, 2017
Merged

Conversation

brent-hoover
Copy link
Collaborator

@brent-hoover brent-hoover commented Oct 9, 2017

Resolves #3024

To Test

  1. Create a second shop and create a product in it
  2. Go to the main shop and place an order with only that product in it
  3. Observe there are no errors in the console
  4. In order confirmation email, observe that the information there is correct

Additional testing

  1. Add item from main shop and newly created item in step 1 above to cart and checkout
  2. Go through with steps 3 and 4 above

@brent-hoover brent-hoover changed the title Fix for console errors on server [WIP] Fix for console errors on server Oct 9, 2017
@kieha kieha changed the title [WIP] Fix for console errors on server Fix for console errors on server Oct 9, 2017
@kieha kieha requested a review from spencern October 9, 2017 23:44
@kieha
Copy link
Contributor

kieha commented Oct 9, 2017

@spencern @zenweasel question on this: is the shipping applied per shop or is it only once? I.e. with 2 items from different shops, will the shipping charge be applied per shop or just once for the two items?

@brent-hoover
Copy link
Collaborator Author

brent-hoover commented Oct 10, 2017

@kieha When you have a shipping charge it's applied twice e.g. if you have a $2.99 charge it's $2.99 for one shop and $2.99 for the other for a total of $5.98.

You should be basically able to sum the shipping charges from the shipping records to the final charge.

@kieha
Copy link
Contributor

kieha commented Oct 10, 2017

@zenweasel if you could also review this, would be super. Couldn't add you as a reviewer to your own PR.

@brent-hoover
Copy link
Collaborator Author

@kieha It's ready to go or?

@kieha
Copy link
Contributor

kieha commented Oct 10, 2017

@zenweasel yup.

@brent-hoover
Copy link
Collaborator Author

@kieha @spencern I made a couple of little changes but this seems to be working for me

Copy link
Contributor

@spencern spencern left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Email issues that I had before are fixed.
image

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