Closed (fixed)
Project:
Commerce Core
Version:
8.x-2.x-dev
Component:
Cart
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
3 Jan 2017 at 23:14 UTC
Updated:
27 Jan 2017 at 00:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
bojanz commentedWe need an OrderSummary service that will build the totals and supply them to the order total formatter for display.
The OrderSummary needs to inject the adjustment type manager to get the relevant weights.
Then in the buildTotals($order) method it calculates the subtotals the adjustment totals, the end total, returns all of that.
We summarize adjustments by adding together all of them which have the same type and source id. If the source id is NULL, we always leave that adjustment alone. Ideally we'd have $order->collectAdjustments() that gave you an array of all adjustments (order item + order ones).
(I guess the logic could live in the formatter itself but in our initial discussion having a service seemed cleaner.
Comment #3
steveoliver commentedSee https://github.com/drupalcommerce/commerce/pull/594.
Comment #4
bojanz commentedWe're close on this one. Primary tasks remaining are a unit test for the service and styling for the totals (something with two-columns and minimal styling).
Comment #5
ransomweaver commented@steveoliver, I'm testing and I have a problem. I add a tax using an order process service, like this: https://github.com/mglaman/commerce_demo/blob/master/src/OrderProcessor/.... Before patching with your code, this works correctly, the displayed order amount in checkout is adjusted and saved with the order, the adjustment is blobbed in the db for the order. After, my tax order process runs before your order total summary (i put a dsm in each), but $collected_adjustments = $order->collectAdjustments(); is empty, and in the db the order amount isn't adjusted. However, the adjustment, with the correct adjustment amount, DOES appear in commerce_order__adjustments. Any insight on this?
Comment #6
steveoliver commentedI'm not sure why, but I notice the following two issues outstanding that are blocking this issue:
1. nothing but 'custom' adjustment types show up in the OrderTotalSummary::buildTotals 'adjustments' element.
2. however, all adjustments are included in the total calculation
3. except ... nothing but 'custom' adjustment types are included in the order total calculation.
I'm unassigning myself in the hopes that mglaman, bojanz or someone else picks the issue up and finds out what I'm doing wrong.
@ransomweaver - I'm not sure why you're having that issue. Are you defining the adjustment type before trying to create an adjustment of that type? See (edit for development) commerce_order.commerce_adjustment_types.yml.
Comment #7
steveoliver commentedWe should also include the order total summaries in 1. the order admin view and 2. the order receipt.
Comment #8
steveoliver commentedAdding related issue.
Comment #9
steveoliver commentedComment #10
ransomweaver commented@steveoliver You are correct, I didn't define a "tax" adjustment type in a module yml file, to match the type I was creating in the OrderProcessor. Now that I have done that, it works. Thanks!
Comment #11
steveoliver commentedComment #12
steveoliver commentedComment #13
steveoliver commentedUpdate issue summary.
Comment #14
bojanz commentedWrapping this up.
Comment #15
bojanz commentedComment #17
bojanz commentedBoom!
Comment #18
ransomweaver commentedThe change from using @steveoliver's pull request as a patch to this commit in the lastest 2.x-dev has caused the rendering of my Tax order adjustment to break.
It throws an error in commerce/modules/price/src/TwigExtension/PriceTwigExtension.php, InvalidArgumentException, because param price is null.
However, the summary DOES total the price with tax correctly (if I insert "return null" before throw new \InvalidArgumentException so I can see the page load).
Subtotal $1,815.00
Sales tax << where the problem is
Total $1,964.74
Comment #19
bojanz commented@ransomweaver
One of my changes was tweaking the output format of the totals.
You might have a custom template that's still using the old structure / variable names?
Comment #20
steveoliver commentedNope, @ransomweaver is right -- adjusments have adjustment.amount, not adustment.total -- we need to apply this patch.
Comment #21
steveoliver commented...this one fixes issues with both the theme callback template and the order email template.
Comment #22
ransomweaver commented#21 works great, thanks @steveoliver
Comment #24
bojanz commentedI wanted to rename amount to total, but the rename was incomplete. Now fixed. Sorry for the disturbance.