From 3986367f3f5a77406a6adc45366ad4b55095e9c8 Mon Sep 17 00:00:00 2001 From: Leo McArdle Date: Wed, 23 Aug 2017 14:55:34 +0100 Subject: [PATCH] update pr based on review --- .../fix_primary_emails_for_staged_users.rb | 7 +--- ...ix_primary_emails_for_staged_users_spec.rb | 39 ++++++++----------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/jobs/onceoff/fix_primary_emails_for_staged_users.rb b/app/jobs/onceoff/fix_primary_emails_for_staged_users.rb index 303dc3ffeff..15fe43a14bb 100644 --- a/app/jobs/onceoff/fix_primary_emails_for_staged_users.rb +++ b/app/jobs/onceoff/fix_primary_emails_for_staged_users.rb @@ -10,14 +10,11 @@ module Jobs .count .each_key do |email| - query = users.where("email_tokens.email = ?", email) - .order(id: :asc) + query = users.where("email_tokens.email = ?", email).order(id: :asc) original_user = query.first - query.offset(1) - .each do |user| - + query.offset(1).each do |user| user.posts.each do |post| post.set_owner(original_user, acting_user) end diff --git a/spec/jobs/fix_primary_emails_for_staged_users_spec.rb b/spec/jobs/fix_primary_emails_for_staged_users_spec.rb index fc938e42028..ccc6ea2c1a5 100644 --- a/spec/jobs/fix_primary_emails_for_staged_users_spec.rb +++ b/spec/jobs/fix_primary_emails_for_staged_users_spec.rb @@ -1,36 +1,31 @@ require 'rails_helper' RSpec.describe Jobs::FixPrimaryEmailsForStagedUsers do - let(:common_email) { 'test@reply' } - let(:staged_user) { Fabricate(:user, staged: true, active: false) } - let(:staged_user2) { Fabricate(:user, staged: true, active: false) } - let(:staged_user3) { Fabricate(:user, staged: true, active: false) } - let(:active_user) { Fabricate(:coding_horror) } - - before do - [staged_user, staged_user2, staged_user3].each do |user| - user.email_tokens = [Fabricate.create(:email_token, email: common_email, user: user)] - end - - UserEmail.delete_all - end - it 'should clean up duplicated staged users' do - expect { described_class.new.execute_onceoff({}) } - .to change { User.count }.by(-2) + common_email = 'test@reply' - expect(User.all).to contain_exactly(Discourse.system_user, staged_user, active_user) - expect(staged_user.reload.email).to eq(common_email) - end + staged_user = Fabricate(:user, staged: true, active: false) + staged_user2 = Fabricate(:user, staged: true, active: false) + staged_user3 = Fabricate(:user, staged: true, active: false) - it 'should move posts owned by duplicate users to the original' do post1 = Fabricate(:post, user: staged_user2) post2 = Fabricate(:post, user: staged_user2) post3 = Fabricate(:post, user: staged_user3) - expect { described_class.new.execute_onceoff({}) } - .to change { staged_user.posts.count }.by(+3) + [staged_user, staged_user2, staged_user3].each do |user| + user.email_tokens = [Fabricate.create(:email_token, email: common_email, user: user)] + end + active_user = Fabricate(:coding_horror) + + UserEmail.delete_all + + expect { described_class.new.execute_onceoff({}) } + .to change { User.count }.by(-2) + .and change { staged_user.posts.count }.by(3) + + expect(User.all).to contain_exactly(Discourse.system_user, staged_user, active_user) expect(staged_user.posts.all).to contain_exactly(post1, post2, post3) + expect(staged_user.reload.email).to eq(common_email) end end