Support non-promotion line-level adjustments #2188
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jordan-brough
force-pushed
the
tax-fix-line-level-adjustments
branch
from
aa265a3 to
ec8bc02
Compare
jhawthorn
reviewed
Aug 29, 2017
core/app/models/spree/line_item.rb
Outdated
| # @return [BigDecimal] the amount of this item, taking into consideration | ||
| # all non-tax adjustments. | ||
| def final_amount_without_additional_tax | ||
| amount + adjustments.reject(&:tax?).sum(&:amount) |
There was a problem hiding this comment.
This probably needs to check eligible? (and a spec catching that would be great)
There was a problem hiding this comment.
👍 thanks. Added a spec as well.
|
Update: With some feedback from @mamhoff I've updated with what seem like some better names all around. |
jordan-brough
force-pushed
the
tax-fix-line-level-adjustments
branch
4 times, most recently
from
2c8a4dc to
5d8834d
Compare
mamhoff
approved these changes
Sep 11, 2017
gmacdougall
reviewed
Sep 13, 2017
…ments In PR 1933 we starting allowing allow non-promotion adjustments on line items and shipments. However, we didn't update some other important code (including the DefaultTax calculator) to consider these adjustments. This commit: - Adds a `total_before_tax` method on LineItem, Shipment, and ShippingRate. - Updates Calculator::DefaultTax to use this method when calculating taxes. - Updates all other code inside Solidus to use this method. - Deprecates the old `discounted_amount` methods.
Aiming to provide more consistent and obvious naming for these fields. Shipment: `final_price` -> `total` (and the display_ method) `pre_tax_amount` -> `total_excluding_vat` `final_price_with_items` -> `total_with_items` LineItem: `final_amount` -> `total` (and the display_ method) `pre_tax_amount` -> `total_excluding_vat` (and the display_ method) Order: `pre_tax_item_amount` -> `item_total_excluding_vat` ReturnAuthorization: `pre_tax_total` -> `total_excluding_vat` (and the display_ method) ReturnItem: `pre_tax_amount` -> `total_excluding_vat` (and the display_ method) CustomerReturn: `pre_tax_total` -> `total_excluding_vat` (and the display_ method)
jordan-brough
force-pushed
the
tax-fix-line-level-adjustments
branch
from
4a4a677 to
dd24466
Compare
gmacdougall
approved these changes
Sep 20, 2017
jhawthorn
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #1933 we starting allowing non-promotion adjustments on line items
and shipments. However, we didn't update some other important code (including
the DefaultTax calculator) to consider these adjustments.
This PR fixes the above issue in its first commit and renames several existing methods in its second commit.
Please see individual commits.