Fix event timing #3954
Fix event timing #3954
Conversation
| events = Event.for_organization(organization_id) | ||
| if first_event | ||
| events = events.where("id >= ?", first_event) |
There was a problem hiding this comment.
Should these go off of one of the timestamps instead of id? It will probably work using id most of the time.
There was a problem hiding this comment.
Honestly I don't have a good use for this just yet, it's for the eventual "start from last snapshot" behavior. We can double check if once that's in.
| inventory = EventTypes::Inventory.from(organization_id) | ||
| events.group_by { |e| [e.eventable_type, e.eventable_id] }.each do |_, event_batch| | ||
| last_event = event_batch.max_by(&:event_time) | ||
| handle(last_event, inventory) | ||
| last_grouped_event = event_batch.max_by(&:updated_at) |
There was a problem hiding this comment.
Conceptually, will there ever be events that have a created_at != updated_at?
| @@ -4,7 +4,7 @@ def self.publish(adjustment) | |||
| create( | |||
| eventable: adjustment, | |||
| organization_id: adjustment.organization_id, | |||
| event_time: Time.zone.now, | |||
| event_time: adjustment.created_at, | |||
There was a problem hiding this comment.
The passed in adjustment from AdjustmentCreateService, weirdly, could be an exisiting Adjustment rather than a freshly created one. Maybe so this gets a bit weird with that case; I'm not sure if adjustment.updated_at would be better or not.
There was a problem hiding this comment.
Adjustments are never updated, just created.
There was a problem hiding this comment.
OK for this one, I guess AdjustmentCreateService initialize we should sometime take out the first bit where it can be handed an Adjustment -- I verified that nothing does call it with one.
| @@ -6,7 +6,7 @@ def self.publish(audit) | |||
| create( | |||
| eventable: audit, | |||
| organization_id: audit.organization_id, | |||
| event_time: Time.zone.now, | |||
| event_time: audit.updated_at, | |||
There was a problem hiding this comment.
Yeah -- this one uses updated_at and others use created_at for the eventable it's tracking, how did you intend which is which?
There was a problem hiding this comment.
Because audits have a state (they have to be finalized) and they can't be edited, so updated_at is the correct thing. For the others, they can be edited but they are always "valid", so created_at is correct.
|
@dorner: Your PR |
This updates event calculations so that it processes them by event time, but still groups them by updated at, so we are ensured to get the "latest version" of the event that happened, e.g. if it was edited after the fact.
This also includes a migration to fix the event times of the known editable objects.
Marking this as draft because I want to add a test for the times. Coming tomorrow hopefully.