solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
4.96k stars 1.28k forks source link

Shipping Calculators still calculate for cancelled items #1837

Open ericsaupe opened 7 years ago

ericsaupe commented 7 years ago

Steps to reproduce

  1. Complete order with many items and flexirate shipping calculator
  2. Cancel items in shipment
  3. Adjustment created does not take into account shipping cost
  4. Cost on shipping was not changed even though less items will be shipped

Expected behavior

After an item is cancelled the calculator should only calculate cost based on what is actually going to ship.

Actual behavior

Shipping calculators are calculating cost on all items even if they are cancelled:

System configuration

Solidus Version: 2.1 (seems to be in all)

Extensions in use: core, frontend, backend

jacobherrington commented 5 years ago

This is still present in Solidus 2.7.

kennyadsl commented 5 years ago

@ericsaupe should be closed with #3059, can you please check?

ericsaupe commented 5 years ago

It still seems to be a little bit off. Maybe it's just the taxes? Anyway so the setup here is

Shipping Rate Calculator - Flexible Rate by item with first item $1.5 and addition items $1

screen shot 2019-02-05 at 2 56 26 pm

This was done against the current 2.9.0.alpha

Correct me if I'm misunderstanding it all here but I think it should adjust the cost of shipping by the canceled item change, adjust the line item by the cost of the item, and maybe additional adjustments to remove taxes? Just thinking about it from an accounting standpoint where adjustments were created to charge money from a customer and then negative adjustments were created to restore money to the customer against what they paid for.

mdesantis commented 5 years ago

@kennyadsl, #3059 isn't related to this issue, since it doesn't mess with shipment adjustments.

I worked recently on Spree::UnitCancel and I can't remember any attempt there to create adjustment for shipment costs, so I suspect to be that way by design. If instead we all agree that when we go on 'Cancel Items' tab and we cancel line items the related shipment costs should be adjusted accordingly, we should work on that as it is a "feature", from a developer point of view

mdesantis commented 5 years ago

@ericsaupe, by my checks, cancelling items doesn't take in consideration shipment costs using any shipment method: does it match with your observations?

cesartalves commented 1 year ago

Since @jacobherrington has said it was still present on 2.7, I verified how the code was since that version and it seems it hasn't changed much in regards to the described method. https://github.com/solidusio/solidus/blob/v2.7/core/app/models/spree/calculator/shipping/flexi_rate.rb

My guess would be that the problem still lingers since no code was changed, but I will need to attempt to reproduce.

A solution would be to add a new shipped_contents to the Calculators, however this involves package building (specifically build_package on the splitter and Stock::Package class) so I will need to try and reproduce the problem on supported versions.

seand7565 commented 11 months ago

Can confirm this is still an issue. To reproduce (on the demo store if you have no vanilla environment):

  1. Update one of the shipping methods to Flexible Rate Per Package Item (I set first item to $1 and consecutive items to $2) image
  2. Create a new order and add 3 items
  3. Add a random taxable NA address, ensure the flexible shipping method is picked, pay for the order via check and complete it. Shipping should be $5 if using the configuration above ($1 for first item, $2 for each of the next two items, for $5 total)
  4. Cancel two of the items on the order after completion using the "Cancel Items" tab.
  5. Refresh. Shipping is still $5, when it should be $1, as only one item is actually shipping. image

Usually you'd want to avoid recalculating orders post-completion, but this situation kind of demands it. I think a simple solve - assuming this doesn't introduce other issues - could be to toss an order.recalculate in the cancellations_controller after this line