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

Multi Unit Item #2078

Closed andreievg closed 6 months ago

andreievg commented 1 year ago

From KDD: https://github.com/openmsupply/open-msupply/pull/1918

As Pack Lens - Option 3

Allow item base unit to be viewed as one of centrally configured unit variants, quantities will be displayed based on the selected unit variant and display pack size and unit information based on those configured variants. Below diagram describes it well, also in google docs:

pack_lens (1)

Tasks

Then use above in

Back End

Central

Extra

Documentation

Break down

From end consumer perspective, we have three types of fields that would need to be adjusted by the unit/pack lens. We can use these methods from a hook to address consumer requirements:

Pack/Unit column

Screenshot 2023-08-08 at 3 22 58 PM

const { asPackUnit } = useUnitVariant("itemIdHere");
// .. 

// For this item there is a unit variant of packSize 10 with name = 'blist of 10 tab'
const packSize = 10;
const packUnit = asPackUnit(packSize); // i.e. blist of 10 tab

// For this item there is no variant matching packSize 10. Unit name is tablet
const packSize = 10;
const packUnit = asPackUnit(packSize); // i.e. tablet x 10

Quantity column

Screenshot 2023-08-08 at 3 59 25 PM

For all quantity fields above

const { numberOfPacksFromQuantity } = useUnitVariant("itemIdHere");
// ..
const totalQuantity = 1000;
const packUnit = numberOfPacksFromQuantity(totalQuantity);

Variant Selector

Screenshot 2023-08-08 at 4 02 20 PM

To build Pack/Unit drop down we would need

const { options, setDefaultOption, defaultOption } = useUnitVariant("itemIdHere");
// ..
<>
  <dropdown options={options} default={defaultOption} onChange={(newOption) => setDefaultOption(newOption)}/>
</>

NOTE: setDefaultOption is most likely a bad name in this context, user is choosing their preferred option no the default option

The hook and context

For the easy of use, we would define a top level provider/context that fetches multi unit variants and default variant (based on stock lines) from API and combines default with app data (if user select variant other then calculated default variant from back end, it is saved in app data as default), and then a hook is exposed

// I think having the "itemId" as parameter will allow for easier memorisation 
const { asPack, numberOfPacksFromQuantity, options, setDefaultOption, defaultOption } = useItemUnitVariants("itemId")

A data format that would be easy for hook to use would be like this:

const items = [
  {
    itemId: 'uuidhere',
    unit: {},
    // Back end determines (base on stock lines in stock ? Or based on number of stock lines)
    mostUsed: 'idHere',
    // Use can overwrite and store in app data
    userSet: 'idHere',
    variants: [
      {
        shortName: 'blist of 10',
        longName: 'blister of 10 tablets',
        id: 'idHere',
        packSize: 10,
      },
    ],
  },
];

By memorisation I was mean that we reduce number of re-renders when one unit variant default/selection is changed (making sure we only update fields for the one that was changed), this should be simple enough.

API Result

I think API should reply with data that is very close to the consumer data:

const items = [
  {
    itemId: 'uuidhere',
    unit: {},
    // Back end determines (base on stock lines in stock ? Or based on number of stock lines)
    mostUsed: 'idHere',
    //  userSet: 'idHere', -> this is an extension that is populated by the provider/hook/context
    variants: [
      {
        shortName: 'blist of 10',
        longName: 'blister of 10 tablets',
        id: 'idHere',
        packSize: 10,
      },
    ],
  },
];

App data

Storage of user picked unit variant should be pretty simple, just key value, key = itemId and value = unitVariantId

Schema and configurations

I was hoping we could do configurations in parallel omSupply Central server, I estimate the base functionality would be 3 or so days of work.

We can of course do the configuration as global pref in JSON form on mSupply server for now.

Btw schema would just need to be able to store this:

Screenshot 2023-08-08 at 4 54 36 PM

MW job code

OMS:NEST

andreievg commented 1 year ago

Question 1:

How do we figure out what is the default (if not chosen by user):

1 - Number of stock lines (all together, including 0 stock lines) ? 2 - Most quantity of stock lines (non zero ones of course)

2 and then 1 ?

andreievg commented 1 year ago

Question 2:

For internal order, we need to adjust all of the graphs, should this be done:

1 - On front end 2 - Provide unit variant to API

andreievg commented 1 year ago

Question 3:

In the KDD i went a little bit over the board with outbound shipment modifications:

Screenshot 2023-08-08 at 5 15 49 PM

I think for now maybe we shouldn't relocate placeholder etc, but I think having the grouping of pack and unit could help (and unit variant selector for placeholder).

Was wondering what your thoughts are on above view in general vs what we have now

andreievg commented 1 year ago

Question 4:

Do we go with current mSupply and json config or omSupply central server in parallel and do UI configurations there, as per this issue , I think it's about 3 days of work. I need to summarise my finding from RnD day a bit further, but yeah central data config should be pretty simple.

mark-prins commented 1 year ago

thanks @andreievg! that's a very comprehensive write up. an epic epic. nicely covered - can't speak for anyone else of course, but I think that you've captured what's required / wanted. I do like the kdd option 3 (tho will go back and re-read the latest version to double check)

had some questions after reading through:

for this example - wouldn't the result be tablet x 10? 

const { asPackUnit } = useUnitVariant("itemIdHere");
// ..
const packSize = 10;
const packUnit = asPackUnit(packSize); // i.e. blist of 12 tab

If so, the comment is potentially confusing things. If not - what happens when you have a packSize which doesn't match any of the available variants?

I'm concerned about being able to specify a placeholder quantity in a random pack size, which would result in non-integer numbers of placeholder quantities. Are we to allow part-packs of placeholders?

with const { options, setDefaultOption, defaultOption } = useUnitVariant("itemIdHere"); I think setDefaultOption is a bad idea: the default option is just that, the 'default' to display before a user touches the control. typically there is a 'default' and a 'selected'. The user isn't changing the default option, they are changing the selected option.

for the hook, are you suggesting that we fetch all items from the server and keep those in the context? tbh I can't see how else we could do it, as we need to have all items available and fetching each item when requested is going to be too slow I think.

..and some answers to your questions:

Question 1: yes, I like 2 then 1 Q2: front end should be ok - it's not a lot of data. I'd be concerned at the speed of a round trip to the server every time the pack selection changes Q3: I am a little biased, having spent a long time reworking that modal already. I like the placeholder at the bottom, don't have a problem with 'Any' and was confused with the 'unit' section - since the price shown is per unit, but the quantity is a total, but expressed in units. Also not sure if there's enough space on screen for everything.

basically - yes, we can improve. Would prefer to leave that with Anisha and some PM type people to work through the process this time.

Q4: I rather like the idea to have a new central server implementation 😉

🙏

andreievg commented 1 year ago

@mark-prins thanks for your feedback.

for this example - wouldn't the result be tablet x 10? If so, the comment is potentially confusing things. If not - what happens when you have a packSize which doesn't match any of the available variants?

I could have done better in clarifying this:

If there is a matching variant (matched by pack size), then we show the short name for that variant, if there is no matching variants, then it's {unit} x {pack size} (or just {unit} if pack size is 1 and no variants with packs size 1 are defined), i refined that example, btw this is kind of what it would look like. Screenshot 2023-08-09 at 11 51 27 AM

Does that make sense ?

I'm concerned about being able to specify a placeholder quantity in a random pack size, which would result in non-integer numbers of placeholder quantities. Are we to allow part-packs of placeholders?

The lens for placeholder line is just for display purpose really. Behind the scene placeholder would still be saved with packSize 1 for now (so should always be whole number). Although I agree that that view might need a bit more thought, I've made a sub task "Refine outbound shipment issue modal UI (meeting with team to work it out)"

with const { options, setDefaultOption, defaultOption } = useUnitVariant("itemIdHere"); I think setDefaultOption is a bad idea: the default option is just that, the 'default' to display before a user touches the control. typically there is a 'default' and a 'selected'. The user isn't changing the default option, they are changing the selected option.

Nice, agree, added a comment in main description that it's not default that user is choosing but their preferred, implemented should adjust accordingly.

This brings another question, will be added below.

for the hook, are you suggesting that we fetch all items from the server and keep those in the context? tbh I can't see how else we could do it, as we need to have all items available and fetching each item when requested is going to be too slow I think.

It would be like a global configuration, that we don't actually need to 'wait' for, the query should be pretty quick and the multi unit variant configuration is not compulsory (don't have to use it and if there is no config it should be pretty fast.

Q3: I am a little biased, having spent a long time reworking that modal already. I like the placeholder at the bottom, don't have a problem with 'Any' and was confused with the 'unit' section - since the price shown is per unit, but the quantity is a total, but expressed in units. Also not sure if there's enough space on screen for everything.

Oki, i think another discussion is needed there, as per the created sub task, one thing i've noticed with omSupply UI (and of course even more so with mSupply), is that columns appear in different order in different places, i think the entry box for say pack qty to issue should be quite close to the pack size, and typically a column that's auto updated or column that is total (in this case it's both, total qty) should appear after the data entry column (pack qty), that was one of the reasons for re-arrangment

basically - yes, we can improve. Would prefer to leave that with Anisha and some PM type people to work through the process this time.

Great idea to get Anisha on board, I've changed the sub task slightly to mention this.

Thanks Mark!

andreievg commented 1 year ago

Question 5

Since the preferred option chose by user will trump the default option, do we need to have a way to clear the preferred option chosen by user ? I think it should be fine for user to choose the option and for it to persist for ever (or until app data is cleared, etc)

mark-prins commented 1 year ago

Does that make sense ?

Yep - the explanation next the example is clearer now, and it helps having the extra example 🙏 ( though in the revised example, a pack size 10 matches to blist of 12 tab which would probably be 12 tablets and not 10.. I understand what you mean though! )

( that comment about the outbound shipment 'add item' modal and the columns and such.. )

totally! I had a mental note to review all the column names, you're right about the inconsistent order of columns too. We need to have a bit of a tidy

andreievg commented 1 year ago

10 matches to blist of 12 tab

🤦‍♂️

andreievg commented 1 year ago

If unit and pack size columns are combined, we may loose info about unit of item (if no variants exists so nothing is given by the server), this can be dealt with by making sure that server return unit/item or by updating ItemVariantPackUnit to also have unitName extractor and passing that as back up to asPackUnit

andreievg commented 1 year ago

What about a use case where there is a pack size of 100. But there are actually two variants, one is box of 10 blisters of 10 and box of 4 blisters of 25. We can't really capture this in any way, unless new fields are added. I think this use case needs to be communicated to the users, just noting it here to be captured in the docs.

roxy-dao commented 6 months ago

Issues have been made for remaining points.

marthakeezy commented 5 days ago

This has been tested as a part of V2.0.0 Release, so not sure why it's still in our QA board... Moving this and related issues to Done ✅