[BUG]: Editing stuff from before a snapshot doesn't work correctly
Is there an existing issue for this?
- [X] I have searched the existing issues
Current Behavior
When a snapshot is taken and we enable an organization for event reading, editing or deleting a previous event does not do what we expect (i.e. adding or removing inventory).
Need to figure out what to do here - add events for all itemizables in the last X number of months? Lock down editing once a snapshot is made? Make a temporary "fake event" when handling these changes?
I'm not sure we can reasonably impose locking down the editing once a snapshot is made -- so let's put that at the bottom of the list of possibles. Let's pull info about updated versus created on the itemizables and see how often, if ever, we get edits past, say, a quarter? That'll inform our answer, yes?
This definitely looks like something that does happen, particularly with distributions.
diaper_dev_public_distributions.csv diaper_dev_public_donations.csv
Huh. The extent of that really surprises me. I don't know what the business story around doing that would be.
I'm wondering if there are updates that aren't inventory changes (like comments or something). Will dig a bit further into the data. I'm concerned if our assumption that people "never" make corrections on previous itemizables wasn't true. Might need to do some customer communication if so.
Yes there are updates that aren't inventory changes -- potentially comments and the purchase prices / monetary donations-- but that there are updates more than a quarter ago still seems strange.
It does look like there are several cases where an org has gone in and changed a slew of them in one day. (Edit it might be worth going in and looking at some of those -- because what????!?)
Wellll, Looking at organization 3, from the comments, it looks like they actually were changing the partners (WOT?!). A case where they had multiple similarly named partners and/ or were consolidating them or something.
I gotta say, our users do not cease to amaze.
We only started keeping track of versions in July of 2023. (But I think we can ignore anything earlier than that anyway as irrelevant.)
Unfortunately, because of how we save line items, there is no way to tell if line items were updated or created at that time (line items are always blown away and recreated on update). But there are definitely some inventory changes happening.
@cielf it might be worth it to reach out to some of the orgs that did changes after many months and find out why and how their changes interacted with inventory. I think it is almost a certainty that changing a 6-month-old distribution shouldn't be assumed that the changes are happening right now (and in fact that might be yet another cause of some of our errors).
This might go back to the idea of "make a correction" to put the event time at the right point.
This seems to indicate that the whole idea of snapshots might not work for our use case. If people can legitimately edit things that old, there will never be a good time to make a snapshot that's guaranteed to be correct.
In that case, we should scrap the idea of using snapshots except as "this is when event sourcing started" and optionally think about using view models / projections instead for performance purposes (if/when we need to).
And it also implies that for this particular problem of editing something that existed before we turned on event sourcing, all we need to do is:
- Check to see if there are any events at all for this thing
- If not, create a special "UpdateExisting" event (attached to the same thing) rather than the "normal" event
- This would have diffs using the old and new values in the database, rather than absolute values like we'd usually have, and we could use that to manage the inventory.
Probably worth a chat on Sunday @cielf @awwaiid @scooter-dangle
The UpdateExisting approach sounds plausible to me, FWIW.
This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.
Addressed by #4287