Commit 682e7701 authored by Alexandru Croitor's avatar Alexandru Croitor Committed by Patrick Derichs

Introduce bulk insert of weight changes for an array of issues

Create FKs to issues and users as part of `references`
Use bulk insert in weight events tracking
parent f6f49861
# frozen_string_literal: true # frozen_string_literal: true
class CreateResourceWeightEvent < ActiveRecord::Migration[5.2] class CreateResourceWeightEvent < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false DOWNTIME = false
disable_ddl_transaction! def change
def up
create_table :resource_weight_events do |t| create_table :resource_weight_events do |t|
t.integer :user_id, null: false t.references :user, null: false, foreign_key: { on_delete: :cascade },
t.integer :issue_id, null: false index: { name: 'index_resource_weight_events_on_user_id' }
t.references :issue, null: false, foreign_key: { on_delete: :cascade },
index: false
t.integer :weight t.integer :weight
t.datetime_with_timezone :created_at, null: false t.datetime_with_timezone :created_at, null: false
t.index [:user_id], name: 'index_resource_weight_events_on_user_id'
t.index [:issue_id, :weight], name: 'index_resource_weight_events_on_issue_id_and_weight' t.index [:issue_id, :weight], name: 'index_resource_weight_events_on_issue_id_and_weight'
end end
add_concurrent_foreign_key :resource_weight_events, :users, column: :user_id, on_delete: :cascade
add_concurrent_foreign_key :resource_weight_events, :issues, column: :issue_id, on_delete: :cascade
end
def down
drop_table :resource_weight_events
end end
end end
...@@ -3606,8 +3606,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do ...@@ -3606,8 +3606,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do
end end
create_table "resource_weight_events", force: :cascade do |t| create_table "resource_weight_events", force: :cascade do |t|
t.integer "user_id", null: false t.bigint "user_id", null: false
t.integer "issue_id", null: false t.bigint "issue_id", null: false
t.integer "weight" t.integer "weight"
t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "created_at", null: false
t.index ["issue_id", "weight"], name: "index_resource_weight_events_on_issue_id_and_weight" t.index ["issue_id", "weight"], name: "index_resource_weight_events_on_issue_id_and_weight"
...@@ -4754,8 +4754,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do ...@@ -4754,8 +4754,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do
add_foreign_key "resource_label_events", "labels", on_delete: :nullify add_foreign_key "resource_label_events", "labels", on_delete: :nullify
add_foreign_key "resource_label_events", "merge_requests", on_delete: :cascade add_foreign_key "resource_label_events", "merge_requests", on_delete: :cascade
add_foreign_key "resource_label_events", "users", on_delete: :nullify add_foreign_key "resource_label_events", "users", on_delete: :nullify
add_foreign_key "resource_weight_events", "issues", name: "fk_5eb5cb92a1", on_delete: :cascade add_foreign_key "resource_weight_events", "issues", on_delete: :cascade
add_foreign_key "resource_weight_events", "users", name: "fk_bfc406b47c", on_delete: :cascade add_foreign_key "resource_weight_events", "users", on_delete: :cascade
add_foreign_key "reviews", "merge_requests", on_delete: :cascade add_foreign_key "reviews", "merge_requests", on_delete: :cascade
add_foreign_key "reviews", "projects", on_delete: :cascade add_foreign_key "reviews", "projects", on_delete: :cascade
add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify
......
...@@ -29,7 +29,7 @@ module EE ...@@ -29,7 +29,7 @@ module EE
end end
def handle_weight_change_note def handle_weight_change_note
if issuable.previous_changes.include?('weight') if !issuable.is_a?(Epic) && issuable.previous_changes.include?('weight')
note = create_weight_change_note note = create_weight_change_note
track_weight_change(note) track_weight_change(note)
...@@ -43,11 +43,11 @@ module EE ...@@ -43,11 +43,11 @@ module EE
def track_weight_change(note) def track_weight_change(note)
return unless weight_changes_tracking_enabled? return unless weight_changes_tracking_enabled?
EE::ResourceEvents::ChangeWeightService.new(issuable, current_user, note.created_at).execute EE::ResourceEvents::ChangeWeightService.new([issuable], current_user, note.created_at).execute
end end
def weight_changes_tracking_enabled? def weight_changes_tracking_enabled?
::Feature.enabled?(:track_resource_weight_change_events, issuable.project) ::Feature.enabled?(:track_issue_weight_change_events, issuable.project)
end end
end end
end end
......
...@@ -3,32 +3,36 @@ ...@@ -3,32 +3,36 @@
module EE module EE
module ResourceEvents module ResourceEvents
class ChangeWeightService class ChangeWeightService
attr_reader :resource, :user, :event_created_at attr_reader :resources, :user, :event_created_at
def initialize(resource, user, created_at) def initialize(resources, user, created_at)
@resource = resource @resources = resources
@user = user @user = user
@event_created_at = created_at @event_created_at = created_at
end end
def execute def execute
create_event_by_issue if first_weight_event? ::Gitlab::Database.bulk_insert(ResourceWeightEvent.table_name, resource_weight_changes) unless resource_weight_changes.empty?
ResourceWeightEvent
.new(user: user, issue: resource, weight: resource.weight, created_at: event_created_at)
.save
end end
private private
def first_weight_event? def resource_weight_changes
ResourceWeightEvent.by_issue(resource).none? @weight_changes ||= resources.map do |resource|
changes = []
base_data = { user_id: user.id, issue_id: resource.id }
changes << base_data.merge({ weight: previous_weight(resource), created_at: resource.updated_at }) if first_weight_event?(resource)
changes << base_data.merge({ weight: resource.weight, created_at: event_created_at })
end.flatten
end
def previous_weight(resource)
resource.previous_changes['weight']&.first
end end
def create_event_by_issue def first_weight_event?(resource)
ResourceWeightEvent previous_weight(resource) && ResourceWeightEvent.by_issue(resource).none?
.new(user: user, issue: resource, weight: resource.weight, created_at: resource.updated_at)
.save
end end
end end
end end
......
...@@ -47,7 +47,7 @@ describe Issuable::CommonSystemNotesService do ...@@ -47,7 +47,7 @@ describe Issuable::CommonSystemNotesService do
context 'when resource weight event tracking is enabled' do context 'when resource weight event tracking is enabled' do
before do before do
stub_feature_flags(track_resource_weight_change_events: true) stub_feature_flags(track_issue_weight_change_events: true)
end end
it 'creates a resource weight event' do it 'creates a resource weight event' do
...@@ -63,7 +63,7 @@ describe Issuable::CommonSystemNotesService do ...@@ -63,7 +63,7 @@ describe Issuable::CommonSystemNotesService do
context 'when resource weight event tracking is disabled' do context 'when resource weight event tracking is disabled' do
before do before do
stub_feature_flags(track_resource_weight_change_events: false) stub_feature_flags(track_issue_weight_change_events: false)
end end
it 'does not created a resource weight event' do it 'does not created a resource weight event' do
......
...@@ -8,7 +8,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do ...@@ -8,7 +8,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do
let(:issue) { create(:issue, weight: 3) } let(:issue) { create(:issue, weight: 3) }
let(:created_at_time) { Time.new(2019, 1, 1, 12, 30, 48) } let(:created_at_time) { Time.new(2019, 1, 1, 12, 30, 48) }
subject { described_class.new(issue, user, created_at_time).execute } subject { described_class.new([issue], user, created_at_time).execute }
before do before do
ResourceWeightEvent.new(issue: issue, user: user).save! ResourceWeightEvent.new(issue: issue, user: user).save!
...@@ -35,6 +35,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do ...@@ -35,6 +35,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do
context 'when there is no existing weight event record' do context 'when there is no existing weight event record' do
before do before do
ResourceWeightEvent.delete_all ResourceWeightEvent.delete_all
issue.update(weight: 5)
end end
it 'creates the expected event records' do it 'creates the expected event records' do
...@@ -44,7 +45,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do ...@@ -44,7 +45,7 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do
expect_event_record(record, weight: 3, created_at: issue.reload.updated_at) expect_event_record(record, weight: 3, created_at: issue.reload.updated_at)
record = ResourceWeightEvent.last record = ResourceWeightEvent.last
expect_event_record(record, weight: 3, created_at: created_at_time) expect_event_record(record, weight: 5, created_at: created_at_time)
end end
end end
...@@ -54,4 +55,25 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do ...@@ -54,4 +55,25 @@ RSpec.describe EE::ResourceEvents::ChangeWeightService do
expect(record.weight).to eq(weight) expect(record.weight).to eq(weight)
expect(record.created_at).to eq(created_at) expect(record.created_at).to eq(created_at)
end end
describe 'bulk issue weight updates' do
let(:issues) { create_list(:issue, 3, weight: 1) }
before do
issues.each { |issue| issue.update!(weight: 3) }
end
it 'bulk insert weight changes' do
expect do
described_class.new(issues, user, created_at_time).execute
end.to change { ResourceWeightEvent.count }.by(6)
end
it 'calls first_weight_event? once per resource' do
service = described_class.new(issues, user, created_at_time)
allow(service).to receive(:first_weight_event?).exactly(3).times
service.execute
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment