Concurrent update not prevented in Grid View

Description

Optimistic locking of invoices failed to prevent a user from updating DateInvoice and DateAcct on 4 invoices after they had been processed.

Copied from mattermost

pbowden
3:20 PM
hi, we've encountered a situation where a user managed to update DateAcct + DateInvoiced on 4 invoices after they had been completed and posted.

It looks like they figured out a way to bypass the concurrent update check implemented in GridTab.hasChangedCurrentTabAndParents()

norbertbede
3:26 PM
hi. steps to reproduce ?
ia should ellaborate on testing

pbowden
3:47 PM

Not sure how the user did it – but I managed to reproduce same result by:
1) Open invoice vendor window (window 1)
2) Create new invoice, and populate but do not complete
3) Open Invoice vendor window again (window 2)
4) Switch window 2 to Grid view, select the new invoice
5) Return to window 1, complete the invoice
6) Go back to window 2, edit the date invoiced (in grid view mode), then click on another row
We are working with version 5.1 but I was just able to reproduce in test.idempiere.org 7.1
I think the best fix would be something along the lines proposed by hengsin in
@hengsin was any work done on above already?

hengsin
3:52 PM
no, not yet.
should be pretty straightforward to implement though
not a high priority item for me. still struggling with the cache api (depressing stuff ...)

pbowden
4:02 PM
Call path when the save is triggered without checking for updates.

Selection_699.png

4:04 PM
I will create a patch to add check to GridTab.navigate() unless anyone has a better suggestion. Will leave for now as that is quite a significant change.

hengsin
4:05 PM
yeah, that should be patched

Environment

None

Activity

Show:
Paul Bowden
August 21, 2020, 7:57 AM

Added a patch that moves the check for the current record from GridTab.dataSave() down to GridTable.dataSave() to catch saves triggered by navigation etc.

Note however that this just prevent the save (and hence navigation) without notifying the user of the cause.

Paul Bowden
August 21, 2020, 7:59 AM

Sorry, also the patch is against 5.1 so might not be directly applicable to latest.

Assignee

Heng Sin Low

Reporter

Paul Bowden

Labels

Tested By

None

Fix versions

Affects versions

Priority

Critical
Configure