Close adjustments so that updates are effective #3666
Conversation
Spree 2.0 keeps shipping fee adjustments open unless they are manually closed. But open adjustments cannot be edited. To preserve updates, like changing the amount of the shipping fee we close the adjustment first.
| @@ -38,6 +38,17 @@ def set_included_tax | |||
| params[:adjustment][:included_tax] = 0 | |||
| end | |||
| end | |||
|
|
|||
| # Spree 2.0 keeps shipping fee adjustments open unless they are manually | |||
| # closed. But open adjustments cannot be edited. | |||
There was a problem hiding this comment.
How did you get to this conclusion? our #update_adjustment! still opens the adjustment to update it so it feels contradictory.
There was a problem hiding this comment.
Good question. I simply observed that adjustment.update_attributes didn't have any effect on an open shipping fee adjustment. But after I closed it, it did update the attributes. I guess that you need to open adjustments to reset them to their calculated value and you have to close them to set a custom value. Do you think we should dig deeper in the code to verify that theory?
There was a problem hiding this comment.
I checked vanilla Spree and an adjustment can be updated either open or closed. But it triggers the after_save callback update_adjustable which calls update! on the order and that eventually triggers a reset of the adjustment. I guessed that before but here is the proper trace:
adjustment.update_attributestriggersadjustable.update!.Order#update!triggers theSpree::OrderUpdater.- The updater updates all shipments and adjustments.
Adjustment#update!doesn't do anything if the adjustment is closed.- When the adjustment is open,
originator.update_adjustmentis called. ShippingMethod#Adjustmentrecalculates the amount and luckily avoids callbacks:adjustment.update_attribute_without_callbacks(:amount, compute_amount(calculable)). Otherwise we would have an infinite loop.
All this seems Spree code. It's not something we accidentally caused with OFN overrides. So Spree defines this way:
- Open shipping adjustments are always recalculated -> can't be changed to a custom value.
- Closed shipping adjustments are not recalculated -> can be changed to a custom value.
This research presents another option though. We could try to call adjustment.update_attribute_without_callbacks to update the amount. I guess though that it wouldn't last long. The adjustment would probably be recalculated when anything in the callback jungle is triggered, e.g. order.next. I conclude that closing the adjustment is the "Spree thing" to do if we want to set a custom amount.
Do you think that's reasonable?
There was a problem hiding this comment.
ah, yes, I followed this track in the past. Makes sense.
When you edit an adjustment, you need to close it, otherwise it gets recalculated. That's the spree logic.
finalized means you can't, you shouldnt change the value in any way.
There was a problem hiding this comment.
Could you take the time to paste that amazing investigation in the Spree upgrade wiki section? It will save us time in the future. I specially like your final open/closed summary 👏
There was a problem hiding this comment.
|
This is good to go https://docs.google.com/document/d/1Ks1lcvGxUaAiDHINnNcGnBaBfsQr-AfoUhbQkVrKKDA/edit# I've noticed something weird on the order total, but I will test this further and see if a new issue needs to be open. |
|
nice nice nice!!! 👏 |
What? Why?
Closes #1830
Spree 2.0 keeps shipping fee adjustments open unless they are manually closed. But open adjustments cannot be edited. To preserve updates, like changing the amount of the shipping fee we close the adjustment first.
What should we test?
How is this related to the Spree upgrade?
Part of the Spree Upgrade.
Documentation updates
Deleting shipping fees
If you delete a shipping fee and look at the order details again, the shipping fee will re-appear. You can use this behaviour if you want to re-calculate the shipping fee. If you don't want to charge a shipping fee you can set the amount of the shipping fee to
0.