human-essentials icon indicating copy to clipboard operation
human-essentials copied to clipboard

Allow editing of records pre-snapshot

Open dorner opened this issue 1 year ago • 9 comments

This will create a new UpdateExisting event when a donation, purchase or distribution is updated if there isn't an existing event to compare against.

Ultimately we're going to want to remove this logic and lock down inventory line items from being edited more than X period of time from when they were created.

Need to let tests run, not sure if this causes failures. Will address next week if so (have to go now)

dorner avatar Apr 19 '24 20:04 dorner

I should, however, check that when you edit a from-before itemizable a second time, it works properly. First thing on my next work session (probably tomorrow)

cielf avatar Apr 30 '24 20:04 cielf

I just ran into a very strange problem while editing a pre-snapshot on production data. Investigating further - but it came up complaining about an item that isn't in the distribution (though it looks like it was in the request). So tenatively withdrawing my ok on this one until we get that sussed out. Will put details in the spreadsheet.

cielf avatar May 01 '24 16:05 cielf

Never mind. it's an error re-running events issue. Which means I have to find someone that hasn't dropped down below zero on any inventory since the end of March to test this.

cielf avatar May 01 '24 16:05 cielf

@dorner Now I've got a real issue though. If you edit a pre-snapshot distribution, then edit it again, unexpected things happen. This was using prod data -- See recreation instructions in the event sourcing testing spreadsheet - currently line 111. When changing the second time, the inventory is dropping by the amount entered in the quantity rather than by the difference. I haven't checked the analagous on donation or purchase yet, but I would expect the same problem.

cielf avatar May 01 '24 17:05 cielf

For clarification -- that's dropping the quantity entered from the initial inventory level, not the inventory level after the initial edit. Which I suspect is a clue.

cielf avatar May 01 '24 17:05 cielf

OK - I have a test verifying this. Looks like I need to continue with the UpdateExisting flow if there's any existing UpdateExisting events. :( I'm going to be happy when we can finally kill this flow because it's a lot of unnecessary complexity.

dorner avatar May 03 '24 20:05 dorner

Pushed the fix.

dorner avatar May 03 '24 20:05 dorner

Seems to work now! @awwaiid Can you take a look from a technical pov?

cielf avatar May 06 '24 18:05 cielf

Looks good but I did fix a merge conflict; will approve/merge if that passes (or investigate otherwise).

awwaiid avatar May 10 '24 19:05 awwaiid

Also Hurrah!

cielf avatar May 12 '24 17:05 cielf

@dorner: Your PR Allow editing of records pre-snapshot is part of today's Human Essentials production release: 2024.05.26. Thank you very much for your contribution!

github-actions[bot] avatar May 26 '24 14:05 github-actions[bot]