Skip to content
Snippets Groups Projects
Commit b012174d authored by Alexandru Croitor's avatar Alexandru Croitor Committed by Nick Thomas
Browse files

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.
parent e4b28b42
No related branches found
No related tags found
No related merge requests found
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class MigrateDiscussionIdOnPromotedEpics < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
# We have ~5000 unique discussion_ids -> this migration will take about 102 minutes
# (5000/100 * 2 minutes + 2 minutes initial delay) on gitlab.com.
DOWNTIME = false
BATCH_SIZE = 100
DELAY_INTERVAL = 2.minutes
MIGRATION = 'FixPromotedEpicsDiscussionIds'
disable_ddl_transaction!
class SystemNoteMetadata < ActiveRecord::Base
self.table_name = 'system_note_metadata'
self.inheritance_column = :_type_disabled
end
class Note < ActiveRecord::Base
include EachBatch
has_one :system_note_metadata, class_name: 'MigrateDiscussionIdOnPromotedEpics::SystemNoteMetadata'
self.table_name = 'notes'
self.inheritance_column = :_type_disabled
def self.fetch_discussion_ids_query
promoted_epics_query = Note
.joins(:system_note_metadata)
.where(system: true)
.where(noteable_type: 'Epic')
.where(system_note_metadata: { action: 'moved' })
.select("DISTINCT noteable_id")
Note.where(noteable_type: 'Epic')
.where(noteable_id: promoted_epics_query)
.distinct.pluck(:discussion_id)
end
end
def up
add_concurrent_index(:system_note_metadata, :note_id, where: "action='moved'", name: 'temp_index_system_note_metadata_on_moved_note_id')
add_concurrent_index(:notes, [:id, :noteable_id], where: "noteable_type='Epic' AND system", name: 'temp_index_notes_on_id_and_noteable_id' )
all_discussion_ids = Note.fetch_discussion_ids_query
all_discussion_ids.in_groups_of(BATCH_SIZE, false).each_with_index do |ids, index|
delay = DELAY_INTERVAL * (index + 1)
BackgroundMigrationWorker.perform_in(delay, MIGRATION, [ids])
end
remove_concurrent_index(:system_note_metadata, :note_id, where: "action='moved'", name: 'temp_index_system_note_metadata_on_moved_note_id')
remove_concurrent_index(:notes, [:id, :noteable_id], where: "noteable_type='Epic' AND system", name: 'temp_index_notes_on_id_and_noteable_id')
end
def down
# no-op
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# This migration updates discussion ids for epics that were promoted from issue so that the discussion id on epics
# is different from discussion id on issue, which was causing problems when replying to epic discussions as it would
# identify the discussion as related to an issue and complaint about missing project_id
class FixPromotedEpicsDiscussionIds
# notes model to iterate through the notes to be updated
class Note < ActiveRecord::Base
self.table_name = 'notes'
self.inheritance_column = :_type_disabled
end
def perform(discussion_ids)
Note.where(noteable_type: 'Epic')
.where(discussion_id: discussion_ids)
.update_all("discussion_id=MD5(discussion_id)||substring(discussion_id from 1 for 8)")
end
end
end
end
# 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
# 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
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment