Skip to content

Resource event model

Jan Provaznik requested to merge jprovazn-resource-events into master

What does this MR do?

It introduces a new model for tracking label events on issuable resources, similar to system note when changing labels but with more structured data. This MR introduces a DB model for these events and a service which creates an event when issuable labels are changed. Note that these events are not created, used or displayed anywhere yet - this will be done in a separate issue (https://gitlab.com/gitlab-org/gitlab-ce/issues/48483).

The reason for not creating these events yet is that we don't want to have both label system notes and label events at the same time because it would make switching in https://gitlab.com/gitlab-org/gitlab-ce/issues/48483 more complicated.

The plan is to keep other types of events too (e.g. milestone changes) - eventually events may replace system notes, these events will require other columns (for milestones we will keep reference to milestones table, or for changing issue weight we will keep an integer column with new weight...). Adding new column for each type of event wouldn't be ideal in this case so instead for each new type of event, there will be a new table (e.g. for milestones it will be resource_milestone_events). Consequence of this design is that when listing all events for a resource we will have to do N queries, where N = number of types of events.

Expected behavior:

  • event is deleted when its resource (issue, MR, epic) is deleted
  • event is kept when the user who made the change is deleted (but user reference is nullified)
  • event is kept when the referenced label is deleted (but label reference is nullified)

Are there points in the code the reviewer needs to double check?

  • DB performance - events table will grow only (except when issuable itself is deleted), there will be millions of events and we should double-check performance potential issues in use-cases listed in the associated issue (#47993 (closed))
  • To avoid polymorphic associations, each issuable will have extra column in the table (in CE it's issue_id and merge_request_id, in EE there will be additional epic_id)
  • In future, given that each type of event has separate table, we will do N queries (N = number of types of events) for listing all events for a resource. We can not use UNION in this case because each event table will have different types of columns.

Why was this MR needed?

Having more structured events (in compare to system notes) allows us to:

There is already Event model which tracks events on "upper" level and it has slightly different purpose. Although there is at first look overlap of functionality, I think it makes more sense to track resource-related events separately because the existing Event model doesn't allow to keep additional details about the event (e.g. we could track an event "labels changed on the resource", but we couldn't track info about what labels were changed).

TODO

  • add tests for creating/deleting events and for nullifying references

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #47993 (closed)

Edited by Jan Provaznik

Merge request reports