diff --git a/app/assets/javascripts/admin/addon/models/admin-user.js b/app/assets/javascripts/admin/addon/models/admin-user.js index 3f40cb7474..a2686912ae 100644 --- a/app/assets/javascripts/admin/addon/models/admin-user.js +++ b/app/assets/javascripts/admin/addon/models/admin-user.js @@ -9,6 +9,7 @@ import bootbox from "bootbox"; import discourseComputed from "discourse-common/utils/decorators"; import getURL from "discourse-common/lib/get-url"; import { iconHTML } from "discourse-common/lib/icon-library"; +import messageBus from "message-bus-client"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { propertyNotEqual } from "discourse/lib/computed"; @@ -493,7 +494,7 @@ const AdminUser = User.extend({ const user = this; const location = document.location.pathname; - bootbox.dialog(I18n.t("admin.user.merging_user")); + const bootboxDiv = bootbox.dialog(I18n.t("admin.user.merging_user")); let formData = { context: location }; if (opts && opts.targetUsername) { @@ -504,20 +505,25 @@ const AdminUser = User.extend({ type: "POST", data: formData, }) - .then((data) => { - if (data.merged) { - if (/^\/admin\/users\/list\//.test(location)) { - DiscourseURL.redirectTo(location); - } else { - DiscourseURL.redirectTo( - `/admin/users/${data.user.id}/${data.user.username}` - ); - } + .then((response) => { + if (response.success) { + messageBus.subscribe("/merge_user", (data) => { + if (data.merged) { + if (/^\/admin\/users\/list\//.test(location)) { + DiscourseURL.redirectTo(location); + } else { + DiscourseURL.redirectTo( + `/admin/users/${data.user.id}/${data.user.username}` + ); + } + } else if (data.message) { + bootboxDiv.find(".modal-body").html(data.message); + } else if (data.failed) { + bootbox.alert(I18n.t("admin.user.merge_failed")); + } + }); } else { bootbox.alert(I18n.t("admin.user.merge_failed")); - if (data.user) { - user.setProperties(data.user); - } } }) .catch(() => { diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 5815b0c34c..d44c38dc4d 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -502,14 +502,9 @@ class Admin::UsersController < Admin::AdminController raise Discourse::NotFound if target_user.blank? guardian.ensure_can_merge_users!(@user, target_user) - serializer_opts = { root: false, scope: guardian } - if user = UserMerger.new(@user, target_user, current_user).merge! - user_json = AdminDetailedUserSerializer.new(user, serializer_opts).as_json - render json: success_json.merge(merged: true, user: user_json) - else - render json: failed_json.merge(user: AdminDetailedUserSerializer.new(@user, serializer_opts).as_json) - end + Jobs.enqueue(:merge_user, user_id: @user.id, target_user_id: target_user.id, current_user_id: current_user.id) + render json: success_json end def reset_bounce_score diff --git a/app/jobs/regular/merge_user.rb b/app/jobs/regular/merge_user.rb new file mode 100644 index 0000000000..e759b08d18 --- /dev/null +++ b/app/jobs/regular/merge_user.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Jobs + class MergeUser < ::Jobs::Base + + def execute(args) + target_user_id = args[:target_user_id] + current_user_id = args[:current_user_id] + + user = User.find_by(id: args[:user_id]) + target_user = User.find_by(id: args[:target_user_id]) + current_user = User.find_by(id: args[:current_user_id]) + guardian = Guardian.new(current_user) + serializer_opts = { root: false, scope: guardian } + + if user = UserMerger.new(user, target_user, current_user).merge! + user_json = AdminDetailedUserSerializer.new(user, serializer_opts).as_json + ::MessageBus.publish '/merge_user', { success: 'OK' }.merge(merged: true, user: user_json), user_ids: [current_user.id] + else + ::MessageBus.publish '/merge_user', { failed: 'FAILED' }.merge(user: AdminDetailedUserSerializer.new(@user, serializer_opts).as_json), user_ids: [current_user.id] + end + end + end +end diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index 989c67e796..d6a0a1eefe 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -5,6 +5,7 @@ class UserMerger @source_user = source_user @target_user = target_user @acting_user = acting_user + @user_id = source_user.id @source_primary_email = source_user.email end @@ -30,6 +31,9 @@ class UserMerger protected def update_username + return if @source_user.username == @target_user.username + + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.updating_username") }, user_ids: [@acting_user.id] if @acting_user UsernameChanger.update_username(user_id: @source_user.id, old_username: @source_user.username, new_username: @target_user.username, @@ -43,6 +47,10 @@ class UserMerger .order(:topic_id, :post_number) .pluck(:topic_id, :id) + return if posts.count == 0 + + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.changing_post_ownership") }, user_ids: [@acting_user.id] if @acting_user + last_topic_id = nil post_ids = [] @@ -70,6 +78,8 @@ class UserMerger end def merge_given_daily_likes + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.merging_given_daily_likes") }, user_ids: [@acting_user.id] if @acting_user + sql = <<~SQL INSERT INTO given_daily_likes AS g (user_id, likes_given, given_date, limit_reached) SELECT @@ -102,6 +112,8 @@ class UserMerger end def merge_post_timings + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.merging_post_timings") }, user_ids: [@acting_user.id] if @acting_user + update_user_id(:post_timings, conditions: ["x.topic_id = y.topic_id", "x.post_number = y.post_number"]) sql = <<~SQL @@ -116,6 +128,8 @@ class UserMerger end def merge_user_visits + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.merging_user_visits") }, user_ids: [@acting_user.id] if @acting_user + update_user_id(:user_visits, conditions: "x.visited_at = y.visited_at") sql = <<~SQL @@ -132,6 +146,8 @@ class UserMerger end def update_site_settings + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.updating_site_settings") }, user_ids: [@acting_user.id] if @acting_user + SiteSetting.all_settings(true).each do |setting| if setting[:type] == "username" && setting[:value] == @source_user.username SiteSetting.set_and_log(setting[:setting], @target_user.username) @@ -140,6 +156,8 @@ class UserMerger end def update_user_stats + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.updating_user_stats") }, user_ids: [@acting_user.id] if @acting_user + # topics_entered DB.exec(<<~SQL, target_user_id: @target_user.id) UPDATE user_stats @@ -194,6 +212,8 @@ class UserMerger end def merge_user_attributes + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.merging_user_attributes") }, user_ids: [@acting_user.id] if @acting_user + DB.exec(<<~SQL, source_user_id: @source_user.id, target_user_id: @target_user.id) UPDATE users AS t SET created_at = LEAST(t.created_at, s.created_at), @@ -235,6 +255,8 @@ class UserMerger end def update_user_ids + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.updating_user_ids") }, user_ids: [@acting_user.id] if @acting_user + Category.where(user_id: @source_user.id).update_all(user_id: @target_user.id) update_user_id(:category_users, conditions: ["x.category_id = y.category_id"]) @@ -356,6 +378,8 @@ class UserMerger end def delete_source_user + ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.deleting_source_user") }, user_ids: [@acting_user.id] if @acting_user + @source_user.reload @source_user.skip_email_validation = true diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 877d4f37fc..47c22b496d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2496,7 +2496,18 @@ en: admin: email: sent_test: "sent!" - + user: + merge_user: + updating_username: "Updating username..." + changing_post_ownership: "Changing post ownership..." + merging_given_daily_likes: "Merging given daily likes..." + merging_post_timings: "Merging post timings..." + merging_user_visits: "Merging user visits..." + updating_site_settings: "Updating site settings..." + updating_user_stats: "Updating user stats..." + merging_user_attributes: "Merging user attributes..." + updating_user_ids: "Updating user ids..." + deleting_source_user: "Deleting source user..." user: deactivated: "Was deactivated due to too many bounced emails to '%{email}'." deactivated_by_staff: "Deactivated by staff" diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 60404b6a96..06219ebebc 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -1079,13 +1079,12 @@ RSpec.describe Admin::UsersController do fab!(:first_post) { Fabricate(:post, topic: topic, user: user) } it 'should merge source user to target user' do + Jobs.run_immediately! post "/admin/users/#{user.id}/merge.json", params: { target_username: target_user.username } expect(response.status).to eq(200) - expect(response.parsed_body["merged"]).to be_truthy - expect(response.parsed_body["user"]["id"]).to eq(target_user.id) expect(topic.reload.user_id).to eq(target_user.id) expect(first_post.reload.user_id).to eq(target_user.id) end