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
23 stars 14 forks source link

Move reasons to reason_options table #5082

Open roxy-dao opened 3 weeks ago

roxy-dao commented 3 weeks ago

The suggestion

Will need #5081 implemented first before this issue can be done.

Currently, we are creating a new table every time we implement a new option type from OG. This causes a lot of overhead whenever a new option is added in Open mSupply.

Migrate the data in the existing Inventory Adjustment Reason and Return Reason table to the new reason_options table and make sure any foreign keys in invoice line/stocktake line for these two tables are pointing to the corresponding reason in the new table.

Why should we invest time in this?

Makes it faster to add new options and keeps all the options in one place.

Are there any risks associated with this change?

Will need extra testing to ensure that the constraints in invoice lines and stocktake lines are still pointing to the correct 'adjustment/return reason'.

How much effort is required?

Maybe a day include adding tests for the migrations

Agreed Solution

Chris-Petty commented 2 weeks ago

May need some UI changes too as I believe the Return Reasons are configured on OMS central rather than OG Central. Migrating might also create headaches as OG Central won't have the "Options" unless OMS central translates/pushes Options back to OG central? We can make that work if syncv5 doesn't currently allow it though.

roxy-dao commented 2 weeks ago

No it's configured in OG Central. Preferences -> Options -> Return reason

Chris-Petty commented 2 weeks ago

So some reason_options will be configured on OMS and some will be on OG? How will OG know? It already has return reasons: image

andreievg commented 1 day ago

Good to have long term, not an issue atm

lache-melvin commented 23 hours ago

Does this make #5230 a non-start?

roxy-dao commented 23 hours ago

Does this make #5230 a non-start?

Can do 5230 first cause don't know when we'll get around to refactoring... 5230 should be a quick one....