Commit 396b0561 authored by Adam Hegyi's avatar Adam Hegyi

Tweak the user account data remediation migration

- Send the unconfirm notification email first and 1 minute later, send
the confirmation instructions.
- Make sure `unconfirm_email` field is set to NULL.
parent c20df30d
...@@ -8,7 +8,7 @@ class UnconfirmWrongfullyVerifiedEmails < ActiveRecord::Migration[6.0] ...@@ -8,7 +8,7 @@ class UnconfirmWrongfullyVerifiedEmails < ActiveRecord::Migration[6.0]
disable_ddl_transaction! disable_ddl_transaction!
INTERVAL = 5.minutes.to_i INTERVAL = 5.minutes.to_i
BATCH_SIZE = 1000 BATCH_SIZE = 500
MIGRATION = 'WrongfullyConfirmedEmailUnconfirmer' MIGRATION = 'WrongfullyConfirmedEmailUnconfirmer'
EMAIL_INDEX_NAME = 'tmp_index_for_email_unconfirmation_migration' EMAIL_INDEX_NAME = 'tmp_index_for_email_unconfirmation_migration'
......
...@@ -71,7 +71,7 @@ module Gitlab ...@@ -71,7 +71,7 @@ module Gitlab
def update_user_records(user_ids) def update_user_records(user_ids)
UserModel UserModel
.where(id: user_ids) .where(id: user_ids)
.update_all("confirmed_at = NULL, confirmation_sent_at = NOW(), confirmation_token=md5(users.id::varchar || users.created_at || users.encrypted_password || '#{Integer(Time.now.to_i)}')") .update_all("confirmed_at = NULL, confirmation_sent_at = NOW(), unconfirmed_email = NULL, confirmation_token=md5(users.id::varchar || users.created_at || users.encrypted_password || '#{Integer(Time.now.to_i)}')")
end end
def email_query_for_update(start_id, stop_id) def email_query_for_update(start_id, stop_id)
...@@ -81,15 +81,15 @@ module Gitlab ...@@ -81,15 +81,15 @@ module Gitlab
end end
def send_emails(email_records) def send_emails(email_records)
email_records.each do |email|
DeviseMailer.confirmation_instructions(email, email.confirmation_token).deliver_later
end
user_records = email_records.map(&:user).uniq user_records = email_records.map(&:user).uniq
user_records.each do |user| user_records.each do |user|
DeviseMailer.confirmation_instructions(user, user.confirmation_token).deliver_later
Gitlab::BackgroundMigration::Mailers::UnconfirmMailer.unconfirm_notification_email(user).deliver_later Gitlab::BackgroundMigration::Mailers::UnconfirmMailer.unconfirm_notification_email(user).deliver_later
DeviseMailer.confirmation_instructions(user, user.confirmation_token).deliver_later(wait: 1.minute)
end
email_records.each do |email|
DeviseMailer.confirmation_instructions(email, email.confirmation_token).deliver_later(wait: 1.minute)
end end
end end
end end
......
...@@ -10,7 +10,7 @@ RSpec.describe Gitlab::BackgroundMigration::WrongfullyConfirmedEmailUnconfirmer, ...@@ -10,7 +10,7 @@ RSpec.describe Gitlab::BackgroundMigration::WrongfullyConfirmedEmailUnconfirmer,
let(:one_year_ago) { 1.year.ago } let(:one_year_ago) { 1.year.ago }
let!(:user_needs_migration_1) { users.create!(name: 'user1', email: 'test1@test.com', state: 'active', projects_limit: 1, confirmed_at: confirmed_at_2_days_ago, confirmation_sent_at: one_year_ago) } let!(:user_needs_migration_1) { users.create!(name: 'user1', email: 'test1@test.com', state: 'active', projects_limit: 1, confirmed_at: confirmed_at_2_days_ago, confirmation_sent_at: one_year_ago) }
let!(:user_needs_migration_2) { users.create!(name: 'user2', email: 'test2@test.com', state: 'active', projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) } let!(:user_needs_migration_2) { users.create!(name: 'user2', email: 'test2@test.com', unconfirmed_email: 'unconfirmed@test.com', state: 'active', projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) }
let!(:user_does_not_need_migration) { users.create!(name: 'user3', email: 'test3@test.com', state: 'active', projects_limit: 1) } let!(:user_does_not_need_migration) { users.create!(name: 'user3', email: 'test3@test.com', state: 'active', projects_limit: 1) }
let!(:inactive_user) { users.create!(name: 'user4', email: 'test4@test.com', state: 'blocked', projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) } let!(:inactive_user) { users.create!(name: 'user4', email: 'test4@test.com', state: 'blocked', projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) }
let!(:alert_bot_user) { users.create!(name: 'user5', email: 'test5@test.com', state: 'active', user_type: 2, projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) } let!(:alert_bot_user) { users.create!(name: 'user5', email: 'test5@test.com', state: 'active', user_type: 2, projects_limit: 1, confirmed_at: confirmed_at_3_days_ago, confirmation_sent_at: one_year_ago) }
...@@ -48,6 +48,13 @@ RSpec.describe Gitlab::BackgroundMigration::WrongfullyConfirmedEmailUnconfirmer, ...@@ -48,6 +48,13 @@ RSpec.describe Gitlab::BackgroundMigration::WrongfullyConfirmedEmailUnconfirmer,
expect(bad_email_4_bot_user.reload.confirmation_sent_at).to be_within(1.second).of(one_year_ago) expect(bad_email_4_bot_user.reload.confirmation_sent_at).to be_within(1.second).of(one_year_ago)
end end
it 'clears the `unconfirmed_email` field' do
subject
user_needs_migration_2.reload
expect(user_needs_migration_2.unconfirmed_email).to be_nil
end
it 'does not change irrelevant user records' do it 'does not change irrelevant user records' do
subject subject
......
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