Stocktake finalise error message unclear and incorrect highlighting of error lines when lines don't have a reason selected

[Chris-Petty]

What went wrong? 😲

Displaying unclear error (no reason selected for lines with discrepancies) and highlighting the rows that have no problem, rather than the rows that have a problem (no reason selected)

https://github.com/msupply-foundation/msupply/assets/7684221/23b6f8e2-75fc-4854-a4c0-331494687928

To Reproduce ⬇️

Steps to reproduce the behaviour:

1. Take the ol' hogwarts themed test datafile
2. Add General store to a remote site (IDK if this is strictly necessary. Any old store desktop/mobile that has unfinalised stocktakes will probably do)
3. Sync OMS with that site
4. I added some reasons for stocktakes in OG > Prefs > Options
5. Try finalise
6. It says that snapshot quantity doesn't match total quantity in error toast? What is total quantity? Does it mean counted quantity?
7. I believe it is actually highlighting all the lines that DON'T have a problem. The real problem is the lines that have a difference don't have a reason selected.

Expected behaviour 🤔

• It should highlight the lines that have a discrepancy and no reason selected
• The error toast at the bottom should actually say the relevant problem: "All lines with discrepancies must have a reason selected"

Your environment 🌱

• Version: develop 2.0.0 (likely in 1.6.X / main / production)
• mSupply version: develop 7.13 (likely irrelevant)
• Platform: macOS firefox
• Database type: sqlite3

[clemens-msupply]

Its actually a SnapshotCountCurrentCountMismatch error, i.e. the snapshot count of the stock at the time when the stocktake was started differs from the current stock count.

For example, in the HP dataset for "030453 Amoxicillin 250mg tabs" batch "K568672" snapshot count is 50 and stock count is 40.

@mark-prins I believe there is no way to currently resolve this? (other than deleting the stocktake line?). Any suggestion how to handle it?

Anyway this error can still be improved in the UI:

• [x] change main error message "One or more lines have a discrepancy between the current stock level and the snapshot count" (https://github.com/msupply-foundation/open-msupply/pull/3548)
• [x] Improve column highlight, i.e. don't highlight the "counted" column: (https://github.com/msupply-foundation/open-msupply/pull/3548)
• [x] Add error tooltip on error cells (list view and edit view) (https://github.com/msupply-foundation/open-msupply/pull/3746)

[clemens-msupply]

Should we have an option to update the snapshot count for selected lines?

[Chris-Petty]

@clemens-msupply hmmm that's a tricky one. The point of snapshot is to help alleviate that SoH might change between when the stock and is counted and the stocktake actually being finalised and changing SoH. I believe Legacy Mobile and Desktop will let you finalise a stocktake and it'll apply the diff of count - snapshot so long as it doesn't result in negative stock.

There are warnings about best practice: The omSupply equivalent would be a stern warning to make sure all outbound invoices are "Picked" before making the stocktake.

When stock will go negative... Choosing the top option appears to make an inventory adjustment that reduces available to 0, rather than negative. The stocktake keeps the counted quantity though so if you compared the stocktake to it's inv. adj. it'd not match up.

[clemens-msupply]

Remaining main problem: The stocktake snapshot count is != current stock count, e.g. stock has been shipped during the stocktake.

Some possible solutions:

• Suggest deleting/re-adding the stocktake line. Maybe in a tooltip?
• Leave as is and add a comment to the docs how this can be resolved
• Add a validateStocktake endpoint to highlight errors early on when opening the page (think this would be good in any case)
• Add endpoint to "resolve" these problems automatically

@andreievg

[clemens-msupply]

Summary from chat with Mark:

• Relative uncommon problem, so "hidden" workaround to delete line is ok
• Improve error message by adding an error tooltip on error cells which show the mismatch

[Chris-Petty]

Why not replicate the behaviour of Legacy? I think mobile does it as well, the snapshot quantity is fine to be different from SOH so long as the outcome of making adjustments isn't less than zero.

Might be unclear to users what deleting the line means - if they delete the line they must recount it. If they reuse their old count against a new snapshot, it's an out of date value and will create stock inaccuracy.

[andreievg]

@Chris-Petty

The omSupply equivalent would be a stern warning to make sure all outbound invoices are "Picked" before making the stocktake.

And disallowing any stock movements while stocktake is in progress ?

Summary from chat with Mark:

Relative uncommon problem, so "hidden" workaround to delete line is ok

Hmm I've faced it a few times, kind of agree that it's not common, but it is a problem seen in the real world.

Improve error message by adding an error tooltip on error cells which show the mismatch

I can't quite remember, was wondering what our strategy for tooltip was for mobile, did we agree it's ok but need a concrete indication that there is a tooltip, and easier way to access it ?

Why not replicate the behaviour of Legacy? I think mobile does it as well, the snapshot quantity is fine to be different from SOH so long as the outcome of making adjustments isn't less than zero.

There was one time, a huge stocktake was done by a customer, and they really wanted to finalise it, but couldn't because there was a lot of reduction below zero. I think the customer wanted the stock to be adjusted to the counted quantity, and not adjusted based on starting total quantity (snapshot) and counted quantity.

We had a couple more big discussions about this actually, i can't quite find them now. Tbh I think we need a way to refresh snapshot quantity, but do it in a way where customer can see the effect. i.e. (here is the snapshot quantities there will be adjusted, this is what they will be adjusted too, this is the previous adjustment for that line, here is what the new adjustment for the line would be)

[clemens-msupply]

I can't quite remember, was wondering what our strategy for tooltip was for mobile, did we agree it's ok but need a concrete indication that there is a tooltip, and easier way to access it ?

Just had a look at the Android docs and it says: "A tooltip is a small descriptive message that appears near a view when users long press the view or hover their mouse over it" Can't test right now but think this is how it works in MUI right now? Having a small visual indication that there is a tooltip sounds like a good idea (if it doesn't look horrible).

[Chris-Petty]

@andreievg @mark-prins I am positive that many places start stocktakes with unfinalised. There are warnings in OG/Mobile recommending to finalise all work first, but in reality users often don't.

Many places will do one giant stocktake where they close all other operations for a couple days, ideally they should finalise everything but we can't force them. If this means stock might go negative after finalising, mSupply just 0s it with big warnings.

Some places will do rolling stocktakes without stopping regular operations. There are techniques to make this smoother, like creating a stocktake for a particular location and having the counter be present at the location and keep tabs on what pickers take out and make sure those items are still in the count while counting things in the location. I think many of these places will want it to still appear like a "big" stock take in one transaction though and that just starts getting hairy. Resetting a snapshot helps, but not without resetting the count too (least adjusting it makes it go negative...). Some places will complain about the system rather than their bad processes, IDK what feature will fix that 🤷 and devs certainly can't decide it.

As for adding a tooltip... I reported that the toast is misleading and that the wrong cells appears to be highlighted. This is based on half the point of the snapshot quantity being to allow it to deviate from counted quantity, so IMO blocking the stocktake because snapshot != SoH broken UX and inconsistent with our other products.

I'm going to close the related PR and label this issue for triage as clearly this needs to go back to PMs/PO.

[andreievg]

IMO blocking the stocktake because snapshot != SoH broken UX and inconsistent with our other products.

Sure. Btw the "Continue and allow .. " option was not present in my example (at that time could only vie in excel).

I would still suggest to update snapshot based on current levels and inform user about the changes in adjustment. gg

[Chris-Petty]

IMO blocking the stocktake because snapshot != SoH broken UX and inconsistent with our other products.

Sure. Btw the "Continue and allow .. " option was not present in my example (at that time could only vie in excel).

I would still suggest to update snapshot based on current levels and inform user about the changes in adjustment. gg

Only if:

• We also clear the counts as to not accidentally make an incorrect adjustment (many lines that may have had a matching count previously may become additions or reductions if the snapshot changes).
• OR we also adjust the counts based on the difference of original snapshot vs new snapshot, in which case they may adjust to negative stock or down to 0. (I think this might be what you mean as "change in adjustment")

In either case, it will not match any documents if users have any process around printing and counting on paper and then entering into the system. (I can think of one major customer that does print the stocktake for record keeping but I'm 80% sure it's after finalising)

[mark-prins]

Shifted over to a discussion Side note, in response to the tooltip question: it requires a long-press on mobile, which I do not expect that users will know about. We cannot rely on them doing that, anything that is important to tell a user needs to be in text, toast, prompt/alert or badge

[Chris-Petty]

Retested seems things have changed a bit but my initial problem is still there @clemens-msupply :

In my original issue my confusion was because I know lines don't have a reason, and this error seemed like the wrong error all together to me. I understand now that we currently disallow snapshot being different from current stock. I am also aware now that we don't actually retroactively require reasons on stock lines created before reasons were added.

Here I just got confused by the gaps initially because when I clicked a row with no error, it's show me a dialog with an error. This is sorta expected though on second inspection! The error row is correctly highlighted in the main list, it's just a different order of batches in the main list when compared to the modal and I was caught out thinking there were differences in highlighting.

So will close as all round working as intended.

The rest of the discussion is just whether or not blocking based on snapshot not matching SoH is desired behavior. Currently a lot of red alerts here while mSupply desktop will happily finalise the same stocktake with no warnings (and not breaking ledger or anything...).

[regotaina]

So will close as all round working as intended.

PASSED THE TEST 👍

Based on Chris' comments on what's expected for now, we can confirm it works well.

OBS: noted with @lache-melvin that if you click on the field and type 0, while the text is highlighted in blue, like this:

https://github.com/msupply-foundation/open-msupply/assets/132425096/a5132a60-ce88-42f7-97ce-78cc492d18a5

the system does not recognise the input. it stays like "not counted. but this is being addressed on this PR

so no problems!