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

cart: enhance shipping cost information and discount calculation #328

Merged
merged 5 commits into from
Jul 15, 2021

Conversation

tessig
Copy link
Member

@tessig tessig commented Jul 13, 2021

No description provided.

Copy link
Member

@carstendietrich carstendietrich left a comment

Choose a reason for hiding this comment

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

Code looks fine, mostly stuff in readme/changelog that can be improved.

cart/Readme.md Outdated
|-----------------------------------|------------------------------------------------------------------------------|-------------------------------------------------------------------|
| SubTotalGross() | Sum of items RowPriceGross | |
| SumRowTaxes() | List of the sum of the different RowTaxes | |
| SumTotalTaxAmount() | Sum of all applied item taxes | |
Copy link
Member

Choose a reason for hiding this comment

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

guess we should mention that this includes shipping taxes

cart/Readme.md Outdated
| Key | Desc | Math |
|-----------------------------------|------------------------------------------------------------------------------|-------------------------------------------------------------------|
| SubTotalGross() | Sum of items RowPriceGross | |
| SumRowTaxes() | List of the sum of the different RowTaxes | |
Copy link
Member

Choose a reason for hiding this comment

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

maybe mention here that this is only for cart items

cart/Readme.md Outdated
| SumGrandTotalWithGiftCards() | The final amount with the applied gift cards subtracted. If there are no gift cards, equal to GrandTotal() | GrandTotal() - SumAppliedGiftCards() |
| Key | Desc | Math |
|-----------------------------------|----------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------|
| GrandTotal() | The final amount that need to be paid by the customer (Gross) | SubTotalGross() + SumTotalDiscountAmount() + Sum(Totalitems) |
Copy link
Member

Choose a reason for hiding this comment

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

The math here is wrong I guess or at least not matching to the code

cart/Readme.md Outdated
| SumNonItemRelatedDiscountAmount() | | Sum of items NonItemRelatedDiscountAmount |
| SumItemRelatedDiscountAmount() | | Sum of items ItemRelatedDiscountAmount |
| Key | Desc | Math |
|-----------------------------------|------------------------------------------------------------------------------|-------------------------------------------------------------------|
Copy link
Member

Choose a reason for hiding this comment

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

delivery.GrandTotal() missing

CHANGELOG.md Outdated
@@ -8,13 +8,17 @@
* Add new endpoint `DELETE /api/v1/cart/deliveries/items` to be able to remove all cart items from all deliveries but keeping delivery info and other cart data untouched
* Add new method `SumShippingGrossWithDiscounts` to the cart domain which returns gross shipping costs for the cart
* When using the `ItemSplitter` to split items in items with single qty (`SplitInSingleQtyItems`) the split discounts are reversed to make splitting the row total stable.
* **Breaking**: Delivery discount sum calculations `SumNonItemRelatedDiscountAmount`,`SumItemRelatedDiscountAmount` now take discount on shipping costs into account
Copy link
Member

Choose a reason for hiding this comment

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

SumTotalTaxAmount breaking as well now also with shipping

Copy link
Member

Choose a reason for hiding this comment

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

SumTotalDiscountAmount breaking since also shipping discounts involved

Copy link
Member

Choose a reason for hiding this comment

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

shippingItem.PriceGross() can be added as new, currently only in the GraphQL changelog

Copy link
Member

@carstendietrich carstendietrich left a comment

Choose a reason for hiding this comment

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

Looks good.

@tessig tessig merged commit 53ee719 into master Jul 15, 2021
@carstendietrich carstendietrich deleted the shipping-discounts branch September 28, 2021 09:25
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.

2 participants