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: #5182 add subtotal display amount to dataForOrderEmail #5183

Merged
merged 2 commits into from May 21, 2019

Conversation

machikoyasuda
Copy link
Contributor

Resolves #5182
Impact: minor
Type: feature|bugfix

Issue

The Order Email's getDataForOrderEmail method's return object should also return subtotal's displayAmount.

Solution

Adds displayAmount to subtotal
Updates tests

Testing

  1. Run the test
  2. Use subtotal.displayAmount in an email

…to data for order email

Signed-off-by: Machiko Yasuda <machiko@reactioncommerce.com>
@machikoyasuda machikoyasuda force-pushed the fix-5182-machikoyasuda-dataforemail-subtotal branch from ad41145 to 145dfc0 Compare May 20, 2019 22:37
@machikoyasuda
Copy link
Contributor Author

machikoyasuda commented May 20, 2019

@aldeed Any idea why these specs are failing? The error message is hard to decipher: https://circleci.com/gh/reactioncommerce/reaction/43994

I tried adding these to the specs and these specs all pass:

  expect(data.combinedItems[0]).toHaveProperty("subtotal");
  expect(data.combinedItems[0].subtotal).toHaveProperty("displayAmount");
  expect(data.combinedItems[0].subtotal).toHaveProperty("amount");
  expect(data.combinedItems[0].subtotal).toHaveProperty("currencyCode");
  expect(data.combinedItems[0].subtotal.displayAmount).toBeTruthy();
  expect(data.combinedItems[0].subtotal.amount).toBeTruthy();

@aldeed
Copy link
Contributor

aldeed commented May 21, 2019

@machikoyasuda In the same way you added displayAmount: jasmine.any(String) in combinedItems, you also need to add it for the subtotal object in order.shipping.items further down. I think that's the only actual difference shown in the diff. (Annoyingly it shows the Anys as differences so you need to ignore those.)

@aldeed aldeed self-requested a review May 21, 2019 18:55
Signed-off-by: Machiko Yasuda <machiko@reactioncommerce.com>
@machikoyasuda machikoyasuda force-pushed the fix-5182-machikoyasuda-dataforemail-subtotal branch from e52b3d8 to ddd1eb0 Compare May 21, 2019 19:01
@machikoyasuda machikoyasuda merged commit 9fed2a9 into develop May 21, 2019
@machikoyasuda machikoyasuda deleted the fix-5182-machikoyasuda-dataforemail-subtotal branch May 21, 2019 19:36
@jeffcorpuz jeffcorpuz mentioned this pull request Jul 2, 2019
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.

None yet

2 participants