Event Source pt. 2: Inventory Discrepancies #3917
Conversation
| belongs_to :user, optional: true | ||
|
|
||
| before_create do | ||
| self.user_id = PaperTrail.request&.whodunnit |
There was a problem hiding this comment.
Ah, as a mechanism to get the current user, I see. Seems fine.
There was a problem hiding this comment.
I wonder if there are other things to tie to, like a request id that goes into the logs. Other stuff to make tracing of what went wrong easier
There was a problem hiding this comment.
I was looking into that... I can try to add more metadata to PaperTrail, perhaps. Otherwise you need to mess with thread variables and it gets messy. The truth is that the operation itself should give us most if not everything that we need - there are very few ways (usually one) that any individual event can be created. If we find some pain dealing with the current way, we can always add more info later.
| # Used to indicate that a storage location exists in one source but not the other. | ||
| class LocationDiff < Dry::Struct | ||
| attribute :storage_location_id, Types::Integer | ||
| attribute :database, Types::Bool |
There was a problem hiding this comment.
I'm wondering if there is a different or more explicit name for this. Like... in_transactional_model or in_stateful_model or something. Food for thought
There was a problem hiding this comment.
I'm definitely down with brainstorming better names... the most explicit thing I could think of is "this is the thing that lives in the database" and "this is the thing that gets built via an aggregate". I'm not sure about something like "stateful" (an aggregate is state, it's just derived state) or transactional (things can live in the database without relying on transactions).
| class LocationDiff < Dry::Struct | ||
| attribute :storage_location_id, Types::Integer | ||
| attribute :database, Types::Bool | ||
| attribute :aggregate, Types::Bool |
There was a problem hiding this comment.
Similar for these, maybe in_event_aggregate
| self.user_id = PaperTrail.request&.whodunnit | ||
| end | ||
|
|
||
| after_create_commit do |
There was a problem hiding this comment.
I think it is appropriate for this to immediately identify and record issues when they happen, but flagging they we have to beware of the resulting slowdown. For individual actions I don't think we'd ever notice, but we should be on the lookout for batch operations. Also once there is a diff then we'll always have one (for the org/location) and this'll find things every time unless the problems cancel each other out (a finance friend of mine calls this "The God of Offsets").
There was a problem hiding this comment.
Yep, we definitely should monitor if there's a huge drop in efficiency. I do want to put in a followup PR that relies more heavily on snapshots (which should be done more often) which should speed this up.
And yeah I know that the diffs will keep piling on. The goal is not for this to be a permanent feature, but for us to monitor over time and verify that the events are the accurate interpretation (and to do fixes until they are). Once we're confident in that, we switch over to using them as the source of truth and the diffs no longer need to happen.
|
@dorner: Your PR/Issue |
See #3911 (this needs to be split up into additional issues).
Description
This adds a new model, InventoryDiscrepancy. Whenever an event is published, the calculated event sourced inventory is checked against the current database-backed inventory. If there are any differences, these are saved in an InventoryDiscrepancy object. For now this will live in a table, which we can pull down and inspect manually. We might want to add a UI in the superadmin area of the app to inspect this.
As part of this PR, we are also adding user_ids to events, piggybacking off of PaperTrail to fetch this information.