diff --git a/db/migrate/20191210211253_create_resource_weight_event.rb b/db/migrate/20191210211253_create_resource_weight_event.rb
index ef3d55148fc572dc9a2e4fb1074be81c2a233e4e..d36b8aafa3397989e036941665d5f6f160aa7e35 100644
--- a/db/migrate/20191210211253_create_resource_weight_event.rb
+++ b/db/migrate/20191210211253_create_resource_weight_event.rb
@@ -1,28 +1,18 @@
# frozen_string_literal: true
class CreateResourceWeightEvent < ActiveRecord::Migration[5.2]
- include Gitlab::Database::MigrationHelpers
-
DOWNTIME = false
- disable_ddl_transaction!
-
- def up
+ def change
create_table :resource_weight_events do |t|
- t.integer :user_id, null: false
- t.integer :issue_id, null: false
+ t.references :user, null: false, foreign_key: { on_delete: :cascade },
+ 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.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'
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
diff --git a/db/schema.rb b/db/schema.rb
index e55bc626ce235adc56e145993f2b9635cc4cffe1..8a4736b33b8030127bcfb4705315853f91f9978c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -3606,8 +3606,8 @@
end
create_table "resource_weight_events", force: :cascade do |t|
- t.integer "user_id", null: false
- t.integer "issue_id", null: false
+ t.bigint "user_id", null: false
+ t.bigint "issue_id", null: false
t.integer "weight"
t.datetime_with_timezone "created_at", null: false
t.index ["issue_id", "weight"], name: "index_resource_weight_events_on_issue_id_and_weight"
@@ -4754,8 +4754,8 @@
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", "users", on_delete: :nullify
- add_foreign_key "resource_weight_events", "issues", name: "fk_5eb5cb92a1", on_delete: :cascade
- add_foreign_key "resource_weight_events", "users", name: "fk_bfc406b47c", on_delete: :cascade
+ add_foreign_key "resource_weight_events", "issues", 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", "projects", on_delete: :cascade
add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify
diff --git a/ee/app/services/ee/issuable/common_system_notes_service.rb b/ee/app/services/ee/issuable/common_system_notes_service.rb
index 65894cd0a7249e260046821a5d4c095c5f527362..1060fd1e8f845a57fca99a9e3163c70830e73b8e 100644
--- a/ee/app/services/ee/issuable/common_system_notes_service.rb
+++ b/ee/app/services/ee/issuable/common_system_notes_service.rb
@@ -29,7 +29,7 @@ def handle_date_change_note
end
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
track_weight_change(note)
@@ -43,11 +43,11 @@ def create_weight_change_note
def track_weight_change(note)
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
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
diff --git a/ee/app/services/ee/resource_events/change_weight_service.rb b/ee/app/services/ee/resource_events/change_weight_service.rb
index da0faf9ffbc86b57ea08b801f2f66f04073f078a..8a787194f5145edcb416459d5cf177f22098d663 100644
--- a/ee/app/services/ee/resource_events/change_weight_service.rb
+++ b/ee/app/services/ee/resource_events/change_weight_service.rb
@@ -3,32 +3,36 @@
module EE
module ResourceEvents
class ChangeWeightService
- attr_reader :resource, :user, :event_created_at
+ attr_reader :resources, :user, :event_created_at
- def initialize(resource, user, created_at)
- @resource = resource
+ def initialize(resources, user, created_at)
+ @resources = resources
@user = user
@event_created_at = created_at
end
def execute
- create_event_by_issue if first_weight_event?
-
- ResourceWeightEvent
- .new(user: user, issue: resource, weight: resource.weight, created_at: event_created_at)
- .save
+ ::Gitlab::Database.bulk_insert(ResourceWeightEvent.table_name, resource_weight_changes) unless resource_weight_changes.empty?
end
private
- def first_weight_event?
- ResourceWeightEvent.by_issue(resource).none?
+ def resource_weight_changes
+ @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
- def create_event_by_issue
- ResourceWeightEvent
- .new(user: user, issue: resource, weight: resource.weight, created_at: resource.updated_at)
- .save
+ def first_weight_event?(resource)
+ previous_weight(resource) && ResourceWeightEvent.by_issue(resource).none?
end
end
end
diff --git a/ee/spec/services/ee/issuable/common_system_notes_service_spec.rb b/ee/spec/services/ee/issuable/common_system_notes_service_spec.rb
index 07a504135e39004aacbd83db31039b3d90a36818..d5b0af8efabb2e42fc05d2f618fea9aa712470ce 100644
--- a/ee/spec/services/ee/issuable/common_system_notes_service_spec.rb
+++ b/ee/spec/services/ee/issuable/common_system_notes_service_spec.rb
@@ -47,7 +47,7 @@
context 'when resource weight event tracking is enabled' do
before do
- stub_feature_flags(track_resource_weight_change_events: true)
+ stub_feature_flags(track_issue_weight_change_events: true)
end
it 'creates a resource weight event' do
@@ -63,7 +63,7 @@
context 'when resource weight event tracking is disabled' do
before do
- stub_feature_flags(track_resource_weight_change_events: false)
+ stub_feature_flags(track_issue_weight_change_events: false)
end
it 'does not created a resource weight event' do
diff --git a/ee/spec/services/ee/resource_events/change_weight_service_spec.rb b/ee/spec/services/ee/resource_events/change_weight_service_spec.rb
index baff401f5d61d0e82ffd6c39db6d6b2c170c1e15..03995e0a76e3672dd8bc18d85c60c8788f37cdb0 100644
--- a/ee/spec/services/ee/resource_events/change_weight_service_spec.rb
+++ b/ee/spec/services/ee/resource_events/change_weight_service_spec.rb
@@ -8,7 +8,7 @@
let(:issue) { create(:issue, weight: 3) }
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
ResourceWeightEvent.new(issue: issue, user: user).save!
@@ -35,6 +35,7 @@
context 'when there is no existing weight event record' do
before do
ResourceWeightEvent.delete_all
+ issue.update(weight: 5)
end
it 'creates the expected event records' do
@@ -44,7 +45,7 @@
expect_event_record(record, weight: 3, created_at: issue.reload.updated_at)
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
@@ -54,4 +55,25 @@ def expect_event_record(record, weight:, created_at:)
expect(record.weight).to eq(weight)
expect(record.created_at).to eq(created_at)
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