Finalise stocktake making invalid stock adjustments (into negative on ledger)

[Chris-Petty]

Build Number: 2.1.0-rc8

Description:
Through a fun sequence of editing you can make stocktakes that show a negative ledger total for an item but show's "fine" on mobile. Both are wrong. Fortunately it's not totally easy to achieve.

Reproducible: Yes

Reproduction Steps:

1. Make a new stocktake with an item
2. Make sure it has some quantity for snapshot (say 100)
3. Make a customer invoice for the same item and issue some amount (say 100)
4. Go back to the stocktake and set the actual quantity to 0. You cannot finalise because it'd make negative stock.
5. Set actual quantity >= 100
6. Edit actual quantity to 0 AND hit finalise before anything else
7. Confirm and viola, you have messed the ledger.

In mSupply mobile, it'll say there is 0 stock. In mSupply Desktop for that store it'll say 0 stock too, but by the ledger it should have -100 stock.

(I've done it a couple times here, so -250)

Comments: 🙈

[anildahalsussol]

Yah I also faced this problem in medical action myanmar with negative stock which is creating ledger problem. Sorry i didnot made the issue but i informed to andrei about it.

[andreievg]

@Chris-Petty
I think the really issue is in the callback nature of database.write(()=>{}).. We can also finalise CustomInvoice/Stocktake with 0 quantities, i.e. set quantity to something other then zero, then back to zero then immediately finalise...

I like your fix: #634 , I think it can be extended, after checking 'checkForFinaliseError' and before record.finalise.. call checkForFinalisationError again.

[andreievg]

I had a further look into this issue, first assumption was incorrect, it's nothing to do with realm.. It seems that onEndEditing is called after the button click, it's called just after the modal is shown. If you create a simple react native app with just a button and a text input, when button is pressed the text is still in focus. We can call Keyboard.dismiss() to un-focus the text input, and only after that try to open the finalisation modal. I've tested the code below, it works, but the timeout of 1 second might not be sufficient in all cases:

// mSupplyMobileApp
  renderFinaliseButton = () => (
    <FinaliseButton
      isFinalised={this.props.finaliseItem.record.isFinalised}
      onPress={() => {
        Keyboard.dismiss();
        setTimeout(() => this.setState({ confirmFinalise: true }), 1000);
      }}
    />
  )

[Chris-Petty]

Yea I also tested keyboard.dismiss to try and get onEndEditing to run before the modal did it's thing, but it wasn't quick enough. The setTimeout is a bit of a hack/dirty I thought and potentially flawed as you mentioned.

I did try to think if there was a way to use database.isInTransaction and async/promise to do it a bit smarter but was in a bit of a hurry so made a PR of what I had done.

[Chris-Petty]

Fixed by #634

Further discussions at #638