When deleting a product, all orders dealing with that product show an error 500

[RachL]

Description

When I delete a product, I cannot see orders that were dealing with this product anymore. Even if those orders contained other products as well

Steps to Reproduce

1. Check an order that has a particular product
2. Delete the product
3. If you try to edit the order again you will have an error 500

Bugsnag details on Katuma staging (the order contain only this product):

https://app.bugsnag.com/katuma/katuma/errors/5cf942f2d45903001a72f17f?event_id=5cf942f20041e9c719f30000&i=sk&m=nw

On Katuma production (the order contained other products):

https://app.bugsnag.com/katuma/katuma/errors/5cf8e37a020d19001a50ffda?event_id=5cf8e37a0041e1a0a46c0000&i=sk&m=nw

Animated Gif/Screenshot

Severity

bug-s3: a feature is broken but there is a workaround

Your Environment

• Version used: v2.0.2
• Browser name and version: Firefox 66
• Operating System and version (desktop or mobile): Desktop

[RachL]

On French production (v1) this is working. The product is still shown in the order, even it was deleted.

[sauloperez]

Root cause

I finally found out that the root cause of the issue is the soft-delete implementation on variant. When fetching data from DB to render the view, among others, we perform a

SELECT "spree_variants".* FROM "spree_variants" WHERE "spree_variants"."id" IN (6) AND ("spree_variants".deleted_at IS NULL)

obviously the result set won't contain the variant of the deleted product and thus in the method below adds a nil variant into the package.

def to_package
  package = Spree::Config.package_factory.new(stock_location, order)
  inventory_units.includes(:variant).each do |inventory_unit|
    package.add inventory_unit.variant, 1, inventory_unit.state_name
  end
  package
end

https://github.com/openfoodfoundation/spree/blob/46d6f8f5fd434105b0c69958276d1727a3066a97/core/app/models/spree/shipment.rb#L227

As a result, the following gets raised

NoMethodError (undefined method `shipping_category' for nil:NilClass):
  app/models/stock/package.rb:17:in `shipping_methods'

Similar errors will pop up wherever we display variants of a deleted product like in orders/:order_number.

Paranoia

That ("spree_variants".deleted_at IS NULL) is what the Paranoia gem adds as AR's default scope. The only way I managed to render the page was by removing the call to acts_as_paranoid from the variant model. Other mechanisms such as without_default_scope: true did not work.

Sof-deletable variants have been introduced in v2 so I wonder how we managed in v1 to delete products but still list their variants in the order details.

On the other hand, what I don't understand is how could this work in Spree itself. How didn't they handle this scenario of they implemented this feature? I'll see if I can find a fix somewhere in their git history.

[sauloperez]

It's also worth checking https://github.com/rubysherpas/paranoia#about-indexes

[Matt-Yorkley]

Isn't that information in the line items...?

[sauloperez]

which one you mean @Matt-Yorkley ? we always have to fetch the variant as well because of the definition of line items: line_items(order_id, variant_id, quantity, ...)

[luisramos0]

🎉