search

msupply-foundation / open-msupply

Open mSupply represents our most recent advancement in the Logistics Management Information System (LMIS), expanding on more than two decades of development inherited from the well-established legacy of the original mSupply.
https://msupply.foundation/open-msupply/
Other
20 stars 12 forks source link

Revisit location movements #1981

Open andreievg opened 1 year ago

andreievg commented 1 year ago

Original issue: https://github.com/openmsupply/open-msupply/issues/730, that was implemented

Via stocktake or invoice - when stock is added, a movement is created with an entry datetime

When stock is added 'to a location' ?

Via stocktake or invoice - if the stock level is reduced to zero, an exit datetime is recorded

  1. How useful is this ? i.e. how useful is it to know what the last vial out of say 1000 has exited the location ?

if we looking at say stock value exposed to a temperature breach in a location, we need to look at how much stock out of the batch was exposed, which means looking at stock on date vs location movements, the exit movement doesn't actually help)

  1. If it does help calculate location movement, does it justify logic scattered in a lot of places to update this movement ?

In order to know that stock did fully exit the location we do something like this (in outbound shipment update status to picked)

https://github.com/openmsupply/open-msupply/blob/2525d325fb8edb18ec60b1ec9d2a7f97f17e3a2c/server/service/src/invoice/outbound_shipment/update/generate.rs#L69

https://github.com/openmsupply/open-msupply/blob/2525d325fb8edb18ec60b1ec9d2a7f97f17e3a2c/server/service/src/invoice/outbound_shipment/update/generate.rs#L257

But we can still edit outbound shipment in picked status, and reduce stock being issued, or add new stock etc... This would required removing location_movement if not all stock was issued ? Also we would need to put this logic when invoice line is inserted/updated/deleted (not just when invoice is changed to picked status)

Also I am not sure that inbound shipment location_movement logic is quite accurate, we create movements for all lines when invoice status is changed to Delivered:

https://github.com/openmsupply/open-msupply/blob/2525d325fb8edb18ec60b1ec9d2a7f97f17e3a2c/server/service/src/invoice/inbound_shipment/update/generate.rs#L78

https://github.com/openmsupply/open-msupply/blob/2525d325fb8edb18ec60b1ec9d2a7f97f17e3a2c/server/service/src/invoice/inbound_shipment/update/generate.rs#L106-L112

But delivered inbound shipment is still editable (similar issue as per above), I think we should only generate location_movements when inbound shipment is verified ? Or not make location movements at all when they are already recorded with invoice_line (only when they are not recorded with invoice_line, like when changing location in stock view or in stocktake when there is no stock movements).

Btw i was originally arguing that we should record location movement with invoice_line, even if there is no stock movements (we could also record things like batch/expiry changes without stock movement in say stocktake), there was pushback on this, because invoice_line should only exist when there is stock reduction/addition (even though we use invoicelines for pricing, when there is no stock reduction/addition) ¯_(ツ)/¯

andreievg commented 1 year ago

Priority may need to be bumped when we do temperature association with stock