Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stocktake, adjusts based on snapshot quantity difference not on real quantity difference #638

Closed
andreievg opened this issue Mar 26, 2018 · 30 comments

Comments

@andreievg
Copy link
Contributor

Stock-takes by their nature need to be done synchronously to other actions like receiving/issuing stock. This is explained in training but is not restricted in the app. Sound like we need to handle the cases where users issues/receives stock while stocktake is in progress, to minimise a chance of ledger issues.
Currently when stocktake is finalise we create an inventory adjustment based on difference between snapshot quantity, not actual quantity. This can result in ledger discrepancy.
i.e.

Item A -> Starting Quantity: 1000

  • Stocktake is created with snapshot quantity = 1000
  • Then a customer invoice is created, issuing 100 of stock (Item A Quantity = 900, Transaction Line A = -100)
  • User goes back to stocktake and enters Actual quantity = 900
  • Stocktake is finalised (Item A Quantity = 900, and stock take adjustment, based on Snapshot Quantity = 1000 - Real Quantity = 900, Transaction Line B = -100)

So our actual quantity = 900 but our ledger reads:
Starting Quantity = 1000
Transaction Line A -100
Transaction Line B -100
Expected Quantity = 800

Even though Quantity is correct, ledger is not.

This is how this example looks in mSupply Desktop Item->Stock->Ledger:

screen shot 2018-03-26 at 4 42 40 pm

Prior to #634 with some trickery it was possible to finalise an a stocktake that show a negative ledger.

Solution:

  • As per mSupply desktop, when creating adjustment, make sure to use real quantity not snapshot quantity
  • When opening a stocktake, update the snapshot quantity = real quantity. (makes more sense from data integrity point of view, i.e. adjustment = difference between snapshot quantity and counted/actual quantity).
  • Disable issuing and receiving items that are in un-finalised stocktake (a bit of a mission to implement + restricts users from some tasks they may wish to do)

@GaryWilletts @ujwalSussol @Chris-Petty @craigdrown, which one do you prefer ?

  • Can we adjust possible ledger problems with migration code ?
    • Will have to implement forward/backwards ledger calculations, and adjust inventory adjustment lines accordingly.
@andreievg
Copy link
Contributor Author

A correction (while talking to @ujwalSussol), he explained, and I tested. mSupply Desktop adjusts stocktake based on 'difference' between snapshot and counted quantity. (and as explained above mobile sets real quantity = counted quantity).
Scenario:

  • Stock adjusted in stocktake, -100
  • Customer invoice is created, -100
  • Stocktake is finalised, total reduction of real quantity = -200

This is makes sense if stocktake counted/actual quantity is entered before the customer invoice (i.e. most places when doing stocktake, a printout is given to the store-man, who does a 'snapshot' stocktake, and data entry may 'lag' behind stocktake's finalisation).

Makes a little less sense when we assume that at the time of finalisation, counted quantity is the actual, real quantity of the stock. (which maybe what happens in mobile ?)

I am happy with replicating mSupply logic of adjustments.

@ujwalSussol
Copy link
Contributor

ujwalSussol commented Mar 26, 2018 via email

@Chris-Petty
Copy link
Contributor

Chris-Petty commented Mar 26, 2018

It's a notorious problem with stocktake. When using paper you have to make sure to include anything that has been picked for outgoing in your counted quantity (if picked before you counted).

I think places get around this by doing a massive full stocktake outside of normal work hours (at least in my experience).

Applying the difference from counted - actual stock on hand isn't any good either, the ledger problems are just positive values rather than negative.

  • Snapshot was 100
  • Counted quantity set to 100
  • Actual SoH becomes 0 through CI
  • Finalise stocktake: 100 - 0 = +100 difference to add
  • Real quantity is 0, mSupply quantity is 100.

Updating snapshot quantities to real quantity suffers the same issue.

For the record, mobile does apply the difference as you describe desktop does. Desktop even throws the same sort of error if you try to make reducing inventory adjustments when a customer invoice has reduced stock to 0 already. Problem with mobile was that you could wiggle around it by abusing a bug.
image

The code in mobile is interesting though.
StocktakeBatch.js

  finalise(database) {
    const isAddition = this.countedTotalQuantity > this.snapshotTotalQuantity;
    const inventoryAdjustment = isAddition ? this.stocktake.getAdditions(database)
                                            : this.stocktake.getReductions(database);
    // Adjust inventory
    this.itemBatch.batch = this.batch;
    // Review: The item batch is edited directly, changing the number of packs
    this.itemBatch.numberOfPacks = this.countedNumberOfPacks;
    this.itemBatch.expiryDate = this.expiryDate;
    // Create TransactionItem, TransactionBatch to store inventory adjustment in this Stocktake
    const item = this.itemBatch.item;
    const transactionItem = createRecord(database, 'TransactionItem', inventoryAdjustment, item);
    const transactionBatch = createRecord(database, 'TransactionBatch',
                                          transactionItem, this.itemBatch);
    // Review: The corresponding transaction batch is made using difference, but the
    // transaction (inventory adjustment) is created with confirmed state. This is means
    // that the difference isn't applied to stock, that was directly done above. I am not
    // sure why it's done this way. This is why ledger gets a negative value but 
    // mobile sets it's stock to the counted quantity
    transactionBatch.numberOfPacks = Math.abs(this.snapshotTotalQuantity
                                              - this.countedTotalQuantity);

    database.save('ItemBatch', this.itemBatch);
    database.save('TransactionBatch', transactionBatch);
  }

If you haven't, read the "Review:" comments I made in the code above.
Desktop does appear to do it similarly. The inventory adjustments are never moved to status "finalised". They can be deleted separately from the stocktake which seems really naff to me. Why not have the inventory adjustment as "suggested" when starting finalisation of the stocktake and once the stocktake finalising code has completed populating the adjustment, confirm or finalise it to apply adjustments? That way in mobile we can stop editing the stock value directly (setting it to counted). At least then we'll actually show negative stock when when a bug allowed it!

As implied that doesn't actually fix the fact that the applied difference could give a negative stock value (if you wiggle round the check for changes into negative stock like you could in mobile).

For the current logic the only half decent way I can think is for customer invoices, supplier invoices and inventory adjustments to adjust snapshot quantity (making it no longer a snapshot...) or adjust the counted quantity of unfinalised stocktakes. A customer invoice would have to decrease the former or increase the latter. Old unfinalised stocktakes will still become a full blown mess at any rate. The meaning of counted quantity vs snapshot quantity for a given start date is lost, but for the final date is correct. That said, you can add items after you created the stocktake in mobile, so it already wasn't solid for the start date.

@Chris-Petty
Copy link
Contributor

@GaryWilletts I detect has 2 cents given a comment on cerb.

@sussol/msupplyadmin in general probably have some wisdom on the matter.

@GaryWilletts
Copy link

Aye, I do! Changing the snapshot is a bad idea. The assumed way of working in mSupply desktop is that you stop all normal warehouse activity after the snapshot is taken, make the physical count, finalise the stocktake then resume normal warehouse activities. On average, this will be easier in mobile stores because they're smaller. But whichever way you work it, you have to assume the operators are behaving one way or another, you can't read their minds to know what's been counted. So best to keep it simple and prescribe what most people expect and understand - no transactions while the stock is being counted after the snapshot is taken.

@andreievg
Copy link
Contributor Author

@Chris-Petty I think I confused you re how mSupply desktop applies the adjustments, it doesn't actually look at the counted quantity vs actual quantity, it looks at counted quantity vs snapshot quantity and tried to apply the difference to actual quantity.

@andreievg
Copy link
Contributor Author

I am quite keen to reduce user error / confusion that results from stocktake (not by telling user not to do something, but by removing the possibility of that something being done).

For desktop I can only see one issue, when stocktake is finalised, the counted quantity is not necessary what the quantity will be adjusted to.. This maybe confusing for the user

Mobile, as outlined above will always adjust quantity to be counted quantity on finalisation, but mobile may create inventory adjustments that lead to ledger discrepancies (even thought the counted/actual quantity should stay correct)

@Chris-Petty
Copy link
Contributor

Are you still arguing your case for in your original comment?

In mobile it can only make ledger issues if you abuse that bug we know about right? I agree it'd be better if mobile and desktop did the same by applying difference. Going negative stock should be blocked, but if we do (through a bug that allows it) I think mobile should show it.

Not sure if silly idea: What if stocktake had a "Refresh" button. It'd have a modal that comes up that warns/confirms. It'd look at past transactions since the creation date of the stocktake and adjust snapshot and counted quantities (don't touch "Not counted") by the same amount. Bring the creation date forward. Though if you've left your stocktake floating around like that to begin with, normal practice would be just to delete it as far as I know.

@GaryWilletts
Copy link

@Chris-Petty be sure, the idea is silly ;-) I foresee a world of pain if you allow stocktakes to be updated like that. You're right, normal procedure is to delete the stocktake and start again.

@andreievg when you say:

For desktop I can only see one issue, when stocktake is finalised, the counted quantity is not necessary what the quantity will be adjusted to.. This maybe confusing for the user

under what circumstances will the quantity the stock has been adjusted to not match the counted quantity?

@andreievg
Copy link
Contributor Author

@GaryWilletts
If there is a reduction or addition transactions while the stocktake is in progress, when the stocktake is 'finalised' -> adjusted quantity != counted quantity. (for an example please read the first note of this issue)

@Chris-Petty I like the idea of refresh button, I think the solution we discussed yesterday is similar 'auto-refresh'.

Anytime a stocktake is opened, we check snapshot quantity vs actual quantity for every stocktake line, if snapshot quantity is different, then we readjust it and set counted quantity field to 'not counted'. Also a modal will pop up, listing items which have been 'refreshed', maybe even a message warning that while stocktake is in progress, one should not make/edit transactions with items in the stocktake.

Benefits:

  • No ledger problem
  • Counted Quantity = Actual Quantity (this is a benefit of doing it this way, rather then the desktop way)

@Chris-Petty
Copy link
Contributor

Chris-Petty commented Apr 11, 2018

Ok I've been testing in mobile and have come to some conclusions. Little bit weird.

Just to be clear "Stock on Hand"/"Actual Stock" will be referred to as SOH.

Case one Make a stocktake for itemA (increasing) and then customer invoice

  1. Stocktake for itemA, snapshot 1000, counted 1500, diff +500
  2. Issue 500 in customer invoice
  3. finalise stocktake
    Once synced, desktop reconciles to say 1000 on ledger (definitely the correct value), SOH is 1500 (uh oh).
    image

Case two - Make a stocktake for itemA (decreasing) and then customer invoice (this is the one Andrei originally described)

  1. Stocktake for itemA, snapshot 1500, counted 1000, diff -500
  2. Issue 500 in customer invoice
  3. finalise stocktake
    Once synced, desktop reconciles to say 500 on ledger (definitely the correct value), SOH is 1000 (uh oh).
    image

Case three - Make a stocktake for itemA and then supplier invoice
Short story is this works fine. No ledger problems. The first 3 transaction in above picture resulted correctly in 1500. Doesn't matter if the stocktake increased or reduced stock for itemA, works fine.

Mobile is doing a bad between customer invoice and stocktake. Pending me investigating exactly what happens on the mobile side:
It should match the ledger by applying the difference (like desktop). This is ideal because it means previous counts in a stocktake are mostly fine except the one exception...
If the difference applied to SOH will result in negative ledger, it currently just blocks the user on finalising. It's a major headache to correct these. Throwing out a whole stocktake to correct a few lines would be a waste.
We discussed yesterday either blocking making new customer invoices all together while there is an unfinalised stocktake, but that's gross.
"Refreshing" or resetting lines which snapshot no longer matches SOH means that this is more spread out, but unnecessary if we correctly apply the difference.
TL;DR
Resetting lines that aren't counted yet and the lines that will have a difference applied that goes to negative stock will conserve the most amount of work, and keep the ledger and SOH as correct as possible. @andreievg did I miss something

@GaryWilletts, @craigdrown @adrianwb didn't complain when we talked about refreshing/resetting lines yesterday. I will note that mobile does give us a bit of flexibility to do this because there is only ever one point of data entry for , the tablet. This wouldn't work with desktop due to the multi client environment and tendency to print out stocktake sheets.

Actions

  1. I'm going to poke around customer invoices and stocktake. Probably make stocktake correctly apply difference. Will have to test that supplier invoices keep playing ball.
  2. When opening a unfinalised stocktake, quickly check all lines won't cause negative stock. If they do, reset them and inform user. Update snapshot for all uncounted lines. (Will do this one when all debate is settled)

@GaryWilletts
Copy link

@andreievg thanks, just checking I was understanding what you were saying - I was :-)

@Chris-Petty I'm coming round to the idea of refreshing the snapshot in mobile but I'm still not convinced. Let's see what your investigations show. Someone has to stand up for tradition ;-)

@andreievg
Copy link
Contributor Author

Sorry to be adding comments to this discussion this late.
But while doing a review of #655 I've realised that our discussion didn't fully end, and we still have to agree on something.

My suggestion was to reset snapshot and counted quantity (without an option, but with a message) for all items where snapshot quantity != actual quantity (current available quantity), and do this every time a stock take is opened, with a message saying something along those lines: Snapshot quantities for these items have changed (due to stock issued or received while this stocktake is in progress), both Counted Quantity and Snapshot quantity will be reset: Item A, Item B, Item C..

This way we are completely locking the user out of making 'error' or something they will perceive as errors. There is a scenario where there might be a question, will give an example at the very bottom of this comment.

@Chris-Petty, I am happy with replicating the adjustment logic as per mSupply.
But I am still worried about these scenarios:

  • If we allow a cancel button in the reset snapshot modal (as per #638 reset out of date stocktake items #655 currently), we might get these question from the user:

    • I did a stocktake for Item A, which had snapshot quantity of 100. Before counting, we issued 20 of Item A, then we went back to Stocktake (and pressed cancel as we didn't quite understand the message), then we set Item A to our desired quantity = 80. It seems mSupply adjusted the quantity to 60. Please help us fix it. (Explain why, ask them to do readjustment etc.. at this stage they will most likely want to do a recount for that item anyways)
    • Consider the same example but in more complex form, Item A has two batches, Batch A (first expiry) snapshot = 25 and Batch B (later expiry) snapshot = 75. We issued 20 using custom invoice, then went back to stocktake, pressed cancel again. Now we adjusted the quantity to what it currently is, Batch A = 5, Batch B = 75. Now we are getting an error saying we cannot reduce stock quantity below minimum. Please help us fix it (Again have to explain and again will need to recount)
  • If we do not adjust items with already counted quantity

    • Hi we did a stocktake for Item A, snapshot quantity was 100, and we set it to 80. Right after the stocktake (before we finalised it), we had to issue 20 worth of stock via customer invoice. Before finalising our stocktake we realised that Actual Quantity for Item A is not 80 anymore but is 60, so we set it to 60. After finalising the stocktake, the quantity has changed to 40! Can you please explain how this happened that help us fix this issue (most likely a recount will be needed)
    • Similar can have even more complicated issue when talking about batches.

Would it not be better if the user does not ask us questions, and or thinks that our system is erroneously adjusting the stock ?

There is a potential confusion for the user as per my suggestion. They can ask a question

  • I've created a stocktake for Item A, adjusted it to from 100 to 80. I had to make an urgent stock issue, and issued 20 of Item A to a customer. When I went back to stocktake I've received a message saying Item A will be reset as the snapshot quantity for the item has changed... Why did this happen, do I really need to recount this item now ? (Answer will be generic, we can point the user at our doc page where the answer lives, i.e. don't issue/receive stock while stocktake is in progress. Can even have a link in mobile, in the modal re why these items are being reset ?).

I am still holding strong to resetting all snapshot quantities / counted quantities. Thus all the scenarios.
I think the question for us is, what do we think is right ? And what customer thinks is right ? I think with the solution I am suggestion, the confusion between the two is minimised.

@Chris-Petty
Copy link
Contributor

Your first 2 examples assume that they press the cancel button, not heeding the warning given to them. The blunt way to resolve that would be to not allow it to be cancelled. It's up to us if we want the new functionality/behaviour as optional. Counted items that are fine have their work preserved but the risk of error on other lines is resolved.

If we do not adjust items with already counted quantity...

Your suggestion of resetting any lines of snapshot quantity != stock on hand does mitigate this, but I argue that slightly better training on stocktake would enable them to do less rework (only recounting lines that would apply into negative stock). Common practice is rolling stocktake or full stocktake, diverging from that is training anyway. That said, mobile docs are suuuuper basic - assuming user has knowledge of stocktaking kinda basic.

Just a note: The current implementation in #655 will prompt every time they look at the stocktake until it's finalised. Could also prompt on editing every line (don't think it'll be much overhead).

@andreievg
Copy link
Contributor Author

@Chris-Petty

I argue that slightly better training on stocktake would enable them to do less rework (only recounting lines that would apply into negative stock). Common practice is rolling stocktake or full stocktake, diverging from that is training anyway. That said, mobile docs are suuuuper basic - assuming user has knowledge of stocktaking kinda basic.

So far training has not helped (I'll telegram you examples shortly), imagine 100+ mobile sites, doing stocktake every month, with possibility of erroneous adjustments. That's a lot of support emails, we can solve this issue for mobile once and for all by reseting as i've suggested, why not do it ?
By the way with this suggestion the app teaches the user, if they have done something they shouldn't have (stock movements) while a stocktake is in progress, they will need to recount those items, now they have an incentive, not to issue/receive stock when stocktake is in progress.
Mobile sites have small warehouses, and recounting a couple of items is not a big task.

Why allow a possibility of erroneous adjustments ?

Just a note: The current implementation in #655 will prompt every time they look at the stocktake until it's finalised. Could also prompt on editing every line (don't think it'll be much overhead).

With no cancel button and resetting snapshot and counted quantity for all items that had stock movements since stocktake started, we don't need to prompt on every line edit.

If we are in disagreement here still by the end of the day, we should ask for some mSupply guru opinions. (it's too much of a blocker atm)

@Chris-Petty
Copy link
Contributor

I agree with making it non optional. I can get on board with resetting all stocktake lines that have had movement (i.e. including those that wouldn't result in negative stock). It should lead to less support for us and possibly a little more rework for them (though they should learn to avoid doing invoices while a stocktake is in progress as you've reiterated).

@sussol/managers can interject, but I'll move forward with that until then.

andreievg added a commit that referenced this issue Apr 26, 2018
@GaryWilletts
Copy link

The same issue applies to mSupply desktop and we should keep the two consistent. Whichever way it's done, the user has to know because it makes a difference to what they count in a stocktake.

Other options are to disallow confirming an invoice with an item included in an unfinalised stocktake or to warn the user if they confirm an invoice with an item in an unfinalised stocktake.

@craigdrown @craig you have an opinion?

@Chris-Petty
Copy link
Contributor

Mobile customer invoices are confirmed by default, so that's a bit of a paradigm shift on that platform @GaryWilletts . An equivalent would be not allowing confirmed invoices to be edited, though.

@GaryWilletts
Copy link

Ah, I'd forgotten that. We just need to make a decision about the way to go and it's not entirely clear which way is best, hence our long discussions. Personally I'd rather prevent someone editing a confirmed customer invoice in mobile if there's an unfinalised stocktake with the edited item on it (or putting the item on a new customer invoice if it's in an unfinalised stocktake). It prevents stock issues and trains the user to use correct stocktaking practice at the same time. Is the check expensive in mobile?

@andreievg
Copy link
Contributor Author

@GaryWilletts, this type of check is expensive in terms of keeping the code clean. If we do this type of check then it will have to be for Custom Invoices, Supplier Invoices, Customer Requisition (as it generates CI automatically) and for Other Stocktakes.

Can you please describe which part of the described solution you don't like or disagree with. Described solution being reseting the items in stocktake that have changed quantity (stock issued or received while stocktake is in progress)

Please keep in mind that this solution is aimed at mobile not desktop, where the scale of everything is smaller.

@GaryWilletts
Copy link

I'm not in disagreement, just trying to think round the wider issues. It's a pretty radical change and mobile and desktop will be working differently - important we do it the right way.

The expensive argument doesn't work I think because your proposed change is more expensive in all ways - you have to have the same checking code as if you were warning/stopping the user but you additionally have to check and update all suggested stocktakes.

I'm OK with updating the snapshot, just making sure it's properly thought through. Think of me as the grit that leads to the pearl ;-)

@ujwalSussol
Copy link
Contributor

Current mSupply desktop logic :

example 1 :

Snapshot = 600
stock take = 250 [ diff 600-250 = 350 ]

user issues = 100
So actual stock is : 600 - 100 = 500

Now stock take is finalised : 500 [current stock level] - 350 [stocktake difference] = 150

example 2:

Snapshot = 600
stock take = 250 [ diff 600-250 = 350 ]

user issues = 400
So actual stock is : 600 - 400 = 200

Now stock take is finalised : 200 [current stock level] - 350 [stocktake difference] = -150
Here the calculation shows that the stock will go negative if allowed... so mSupply disallows...

Instead it used to do : 200 [current stock level] - 200 [stocktake difference] = 0 .... Now we show the excel sheet which has created complication to the user.... Previously the inventory adjustment would be adjusted to bring the stock to zero.... If absolutely no stock was available, mSupply even adjusted the stock take to zero. This was before the excel option..... Now we are having to bring back this feature as our users can't handle the excel sheet.

@andreievg
Copy link
Contributor Author

@GaryWilletts

The expensive argument doesn't work I think because your proposed change is more expensive in all ways - you have to have the same checking code as if you were warning/stopping the user but you additionally have to check and update all suggested stocktakes.

In mobile there is only ever a single page interface (no multi window) so we can quite easily adjust snapshot quantity for items where snapshot quantity differs from available quantity. This is done when stocktake is opened, so not expensive at all

I'm not in disagreement, just trying to think round the wider issues. It's a pretty radical change and mobile and desktop will be working differently - important we do it the right way.

Yeah, there will be differences in how mobile and desktop handles stock-takes. Mobile already differs in a few ways from desktop, I think the differences were driven by trying to simplify Mobiles interface and operations. I believe this new change to stocktake reset is the simplest way to avoid user error, user confusion (leading to support tickets).

On a side note, it look like one good difference, the requisition modal, is being applied to desktop now.

@andreievg
Copy link
Contributor Author

andreievg commented May 1, 2018

@ujwalSussol @GaryWilletts @craigdrown
For mSupply Desktop, in my view the ideal solution will be to change our stocktake table + stock take line edit.

For stocktake table, change Enter Quantity to Snapshot Counted Quantity and add Finalised Quantity (the quantity stock level will be adjusted to after stocktake is finalised). Make the row red if Finalised Quantity is below zero.

And when stocktake row is double clicked, can add extra information. Finalised Quantity, Current Quantity (current actual or available quantity of the item, may be different to snapshot quantity) and Adjustment

We can even include a little formula here (maybe our interface can be in the shape of this formula)

  • Finalised Quantity = Current Quantity + Adjustment
  • Adjustment = Snapshot Counted Quantity - Snapshot Quantity

This way the user will know what's actually going on with stocktake adjustments.

@Chris-Petty
Copy link
Contributor

Obligatory "If we had a proper UX team and user testing/feedback system... "

@Chris-Petty
Copy link
Contributor

@ujwalSussol can I ask why there was the move to the excel option to start with?

The approach that you're bringing back is pretty simple but... is it gross?

What I've implemented on mobile

@andreievg
Copy link
Contributor Author

I've talked to Chris a little bit, it seems Snapshot Counted Quantity is not the best name for a column. I guess Counted Quantity will be sufficient (I was trying to say that 'this column is what you've counted, but may differ to actual quantity at the warehouse, as there could have been some stock movements since you've counted it'). And again there may be better suggestions for Finalised Quantity -> quantity that mSupply will adjust the stock to once inventory adjustment is created.

@andreievg
Copy link
Contributor Author

I've moved mSupply desktop related discussion to: https://bugs.msupply.org.nz/view.php?id=8811

@ujwalSussol
Copy link
Contributor

@Chris-Petty : I think we had one client that noticed mSupply making stock take adjustment and ledger adjustment due to lack of stock.... So the excel approach was to stop the process and to allow the users to logically do the maths.... As we have noticed lately this was asking too much of the users.....

So the easy option for desktop is to use bring the previous approach back but to have logs mention lack of stock for stock take, so mSupply made stock take and ledger adjustment.

For mobile : What every way you have done is fine for now.... as long as we don't have ledger problems.

@Chris-Petty
Copy link
Contributor

Apparently I sleepily closed this issue and commented a half comment haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants