Openfoodnetwork: Shipment state indicates 'ready' and not 'shipped'

[tschumilas]

Description

Until the latest release (April 20, 2021), when an enterprise manager clicked the shipping icon on an order, the shipment status changed to 'shipped', this helps the manager keep track of shipped and not shipped orders.

Expected Behavior

when the manager selects the small shipping icon on the order listing page, the shipment status should change to "shipped"

Actual Behaviour

When the manager clicks the shipping icon on the order listing page, the shipment icon disappears, but the shipment state still reads 'ready' instead of 'shipped'

NOTE - the shipment confirmation email IS appropriately sent to the customer.

Steps to Reproduce

1.Place an order in a shop
2.Click the shipping icon on the orders listing page
3.
4.

Animated Gif/Screenshot

Workaround

Severity

s2? if the shipment status doesn't change to 'shipped' there is no way for an enterprise manager to search their orders listing to tell which orders have shipped and which have not. No report (that I can see) provides shipping status of orders.

bug-s1: a critical feature is broken: checkout, payments, signup, login
bug-s2: a non-critical feature is broken, no workaround
bug-s3: a feature is broken but there is a workaround
bug-s4: it's annoying, but you can use it
bug-s5: we can live with it, only a few users impacted

https://github.com/openfoodfoundation/openfoodnetwork/wiki/Bug-severity
-->

Your Environment

• Version used:
• Browser name and version:
• Operating System and version (desktop or mobile):

Possible Fix

[filipefurtad0]

Thanks for reporting @tschumilas , this is indeed a recent regression. I'd agree with S2, I don't see a workaround and it's indeed high impact... ping @RachL @lin-d-hop .

[filipefurtad0]

I think our test suite is not catching this case, because of this assertion, on spec/features/admin/orders_spec.rb, line 95:
expect(order.reload.shipments.any?(&:shipped?)).to be true

This asserts for the state of the shipment (a separate table), and not the shipping state of the order as in order.shipment_state which is still ready. Also it does so by looking at the DB, maybe we could add a line on the feature level here as well.

Adding any/both these here lines reproduce the bug on the spec:
expect(order.shipment_state).to eq("shipped")
expect(page).to have_content "SHIPPED"