ActiveRecord::RecordInvalid in checkout#update

[mkllnk]

This is not a current bug. It appeared after deploying a pull request which has been reverted since:

• Allow negative stock levels for on-demand products #12536

Error in OpenFoodNetwork Aus

ActiveRecord::RecordInvalid in checkout#update
Validation failed: Count on hand must be greater than or equal to 0

View on Bugsnag

Stacktrace

app/models/spree/stock_item.rb:26 - block in adjust_count_on_hand
app/models/spree/stock_item.rb:22 - adjust_count_on_hand
app/models/spree/stock_movement.rb:17 - update_stock_item_quantity
app/models/concerns/variant_stock.rb:124 - move
lib/open_food_network/scope_variant_to_hub.rb:50 - move
app/models/spree/stock_location.rb:49 - move
app/models/spree/stock_location.rb:45 - unstock
app/models/spree/shipment.rb:316 - manifest_unstock
app/models/spree/shipment.rb:208 - block in finalize!
app/models/spree/shipment.rb:208 - each
app/models/spree/shipment.rb:208 - finalize!
app/models/spree/order.rb:382 - block in finalize!
app/models/spree/order.rb:380 - finalize!
app/controllers/checkout_controller.rb:82 - confirm_order
app/controllers/checkout_controller.rb:39 - update
app/services/current_order_locker.rb:26 - block in lock_order_and_variants
app/services/current_order_locker.rb:24 - lock_order_and_variants
app/services/current_order_locker.rb:13 - around

View full stacktrace

Created by Maikel via Bugsnag

[dacook]

If this resulted in payments not being captured, we should list each one of these cases so we can let the affected enterprises know. And the same for all other bugsnag instances.. 😬

[mkllnk]

If this resulted in payments not being captured, we should list each one of these cases so we can let the affected enterprises know.

I went through Australian Bugsnag and send all affected order ids to @amidaOFN. But we should check others as well. Oh, and some subscriptions were affected bug I can't see the ids in Bugsnag.

[mkllnk]

Potential causing change reverted.

Comment added in Bugsnag by Maikel
ActiveRecord::RecordInvalid in checkout#update

[mkllnk]

An error linked to this issue has been marked as fixed in Bugsnag
ActiveRecord::RecordInvalid in checkout#update

[mkllnk]

Looking at the code, I have a suspicion:

• Stock items can have negative stock if they are backorderable (on demand).
• But variant overrides can set on_demand while not overriding the stock level (count_on_hand == nil).

A distributor with inventory can set a variant to on demand while not overriding the stock level. In the old model, orders would then not affect the stock level because on demand would disable stock tracking. But the offending pull request starting tracking stock for on demand products as well. I didn't consider this and thought that tracking stock for in this case was hidden to the user but it's actually a big change in logic. Now a sale of a distributor can reduce stock levels of the producer even though the distributor chose on demand.

I think that the solution is to enforce stock tracking on variant overrides when on demand is set. So the sales don't affect the producers stock level and the distributor has its own hidden stock tracking.

[mkllnk]

I reproduced the error in a spec:

• master...mkllnk:openfoodnetwork:order-stock-spec

There are pending specs in that branch to fix. Next steps is to change the validation of VariantOverride to allow stock tracking when on demand. Then the variant override should do the stock check instead of deferring to the stock item which would be invalid.

[mkllnk]

A new version of the pull request has been in production for over a week and I haven't this hasn't occurred again. Closing.