From b012174d6fb5dcef10db1dc67754bb5c59aa2a07 Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Wed, 11 Sep 2019 12:17:51 +0000 Subject: Change discussion_ids on promoted epics notes Notes on epics promoted from an issue used to get same discussion_id as the notes from the issue the epic was promoted from, which would cause problems when trying to reply to the epic discussion. --- .../fix_promoted_epics_discussion_ids_spec.rb | 49 +++++++++++++ ...migrate_discussion_id_on_promoted_epics_spec.rb | 81 ++++++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 spec/lib/gitlab/background_migration/fix_promoted_epics_discussion_ids_spec.rb create mode 100644 spec/migrations/migrate_discussion_id_on_promoted_epics_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/background_migration/fix_promoted_epics_discussion_ids_spec.rb b/spec/lib/gitlab/background_migration/fix_promoted_epics_discussion_ids_spec.rb new file mode 100644 index 00000000000..73c855ac184 --- /dev/null +++ b/spec/lib/gitlab/background_migration/fix_promoted_epics_discussion_ids_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::FixPromotedEpicsDiscussionIds, :migration, schema: 20190715193142 do + let(:namespaces) { table(:namespaces) } + let(:users) { table(:users) } + let(:epics) { table(:epics) } + let(:notes) { table(:notes) } + + let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') } + let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } + let(:epic1) { epics.create!(id: 1, author_id: user.id, iid: 1, group_id: namespace.id, title: 'Epic with discussion', title_html: 'Epic with discussion') } + + def create_note(discussion_id) + notes.create!(note: 'note comment', + noteable_id: epic1.id, + noteable_type: 'Epic', + discussion_id: discussion_id) + end + + def expect_valid_discussion_id(id) + expect(id).to match(/\A\h{40}\z/) + end + + describe '#perform with batch of discussion ids' do + it 'updates discussion ids' do + note1 = create_note('00000000') + note2 = create_note('00000000') + note3 = create_note('10000000') + + subject.perform(%w(00000000 10000000)) + + expect_valid_discussion_id(note1.reload.discussion_id) + expect_valid_discussion_id(note2.reload.discussion_id) + expect_valid_discussion_id(note3.reload.discussion_id) + expect(note1.discussion_id).to eq(note2.discussion_id) + expect(note1.discussion_id).not_to eq(note3.discussion_id) + end + + it 'skips notes with discussion id not in range' do + note4 = create_note('20000000') + + subject.perform(%w(00000000 10000000)) + + expect(note4.reload.discussion_id).to eq('20000000') + end + end +end diff --git a/spec/migrations/migrate_discussion_id_on_promoted_epics_spec.rb b/spec/migrations/migrate_discussion_id_on_promoted_epics_spec.rb new file mode 100644 index 00000000000..5e25d1aed82 --- /dev/null +++ b/spec/migrations/migrate_discussion_id_on_promoted_epics_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190715193142_migrate_discussion_id_on_promoted_epics.rb') + +describe MigrateDiscussionIdOnPromotedEpics, :migration, :sidekiq do + let(:migration_class) { described_class::MIGRATION } + let(:migration_name) { migration_class.to_s.demodulize } + + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:users) { table(:users) } + let(:issues) { table(:issues) } + let(:epics) { table(:epics) } + let(:notes) { table(:notes) } + let(:system_note_metadata) { table(:system_note_metadata) } + + let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') } + let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } + + def create_promotion_note(model, id) + note = create_note(model, id, { system: true, + note: 'promoted from issue XXX' }) + system_note_metadata.create!(note_id: note.id, action: 'moved') + end + + def create_epic + epics.create!(author_id: user.id, iid: 1, + group_id: namespace.id, + title: 'Epic with discussion', + title_html: 'Epic with discussion') + end + + def create_note(model, id, extra_params = {}) + params = { + note: 'note', + noteable_id: model.id, + noteable_type: model.class.name, + discussion_id: id + }.merge(extra_params) + + notes.create!(params) + end + + context 'with promoted epic' do + let(:epic1) { create_epic } + let!(:note1) { create_promotion_note(epic1, 'id1') } + + it 'correctly schedules background migrations in batches' do + create_note(epic1, 'id2') + create_note(epic1, 'id3') + + stub_const("#{described_class.name}::BATCH_SIZE", 2) + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(migration_name).to be_scheduled_delayed_migration(2.minutes, %w(id1 id2)) + expect(migration_name).to be_scheduled_delayed_migration(4.minutes, %w(id3)) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end + + it 'schedules only promoted epics' do + issue = issues.create!(description: 'first', state: 'opened') + create_promotion_note(issue, 'id2') + create_note(create_epic, 'id3') + + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(migration_name).to be_scheduled_delayed_migration(2.minutes, %w(id1)) + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + end + end + end + end +end -- cgit v1.2.3