diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb index ebc5f1aa0b1..13e4f8bd84d 100644 --- a/app/jobs/regular/update_username.rb +++ b/app/jobs/regular/update_username.rb @@ -3,21 +3,22 @@ module Jobs def execute(args) @user_id = args[:user_id] - - username = args[:old_username] - @raw_mention_regex = /(?:(?> 'original_username' + WHEN :old_username + THEN :new_username + ELSE NULL END, + 'display_username', CASE data :: JSONB ->> 'display_username' + WHEN :old_username + THEN :new_username + ELSE NULL END, + 'username', CASE data :: JSONB ->> 'username' + WHEN :old_username + THEN :new_username + ELSE NULL END + ) + )) :: JSON + WHERE EXISTS( + SELECT 1 + FROM posts AS p + WHERE p.topic_id = n.topic_id + AND p.post_number = n.post_number + AND p.user_id = :user_id) + OR (n.notification_type IN (:notification_types_with_correct_user_id) AND n.user_id = :user_id) + OR (n.notification_type = :invitee_accepted_notification_type + AND EXISTS( + SELECT 1 + FROM invites i + WHERE i.user_id = :user_id AND n.user_id = i.invited_by_id + ) + ) + SQL + end + protected def post_conditions(post_id_column) @@ -82,13 +131,17 @@ module Jobs end doc.css("aside.quote > div.title").each do |div| - # TODO Update avatar URL div.children.each do |child| child.content = child.content.gsub(@cooked_quote_username_regex, @new_username) if child.text? end + div.at_css("img.avatar")&.replace(avatar_img) end doc.to_html end + + def avatar_img + @avatar_img ||= PrettyText.avatar_img(User.find_by_id(@user_id).avatar_template, "tiny") + end end end diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index bc982523a01..b257ecd4d22 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -5,7 +5,7 @@ class UserMerger end def merge! - update_notifications + update_username move_posts update_user_ids merge_given_daily_likes @@ -23,52 +23,10 @@ class UserMerger protected - def update_notifications - params = { - source_user_id: @source_user.id, - source_username: @source_user.username, - target_username: @target_user.username, - notification_types_with_correct_user_id: [ - Notification.types[:granted_badge], - Notification.types[:group_message_summary] - ], - invitee_accepted_notification_type: Notification.types[:invitee_accepted] - } - - Notification.exec_sql(<<~SQL, params) - UPDATE notifications AS n - SET data = (data :: JSONB || - jsonb_strip_nulls( - jsonb_build_object( - 'original_username', CASE data :: JSONB ->> 'original_username' - WHEN :source_username - THEN :target_username - ELSE NULL END, - 'display_username', CASE data :: JSONB ->> 'display_username' - WHEN :source_username - THEN :target_username - ELSE NULL END, - 'username', CASE data :: JSONB ->> 'username' - WHEN :source_username - THEN :target_username - ELSE NULL END - ) - )) :: JSON - WHERE EXISTS( - SELECT 1 - FROM posts AS p - WHERE p.topic_id = n.topic_id - AND p.post_number = n.post_number - AND p.user_id = :source_user_id) - OR (n.notification_type IN (:notification_types_with_correct_user_id) AND n.user_id = :source_user_id) - OR (n.notification_type = :invitee_accepted_notification_type - AND EXISTS( - SELECT 1 - FROM invites i - WHERE i.user_id = :source_user_id AND n.user_id = i.invited_by_id - ) - ) - SQL + def update_username + Jobs::UpdateUsername.new.execute(user_id: @source_user.id, + old_username: @source_user.username, + new_username: @target_user.username) end def move_posts diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index ac5d3545b43..236372b125a 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -289,69 +289,6 @@ describe UserMerger do expect(Notification.where(user_id: target_user.id).count).to eq(2) expect(Notification.where(user_id: source_user.id).count).to eq(0) end - - def create_notification(type, notified_user, post, data = {}) - Fabricate( - :notification, - notification_type: Notification.types[type], - user: notified_user, - data: data.to_json, - topic: post&.topic, - post_number: post&.post_number - ) - end - - def notification_data(notification) - JSON.parse(notification.reload.data, symbolize_names: true) - end - - def original_and_display_username(user) - { original_username: user.username, display_username: user.username, foo: "bar" } - end - - def original_username_and_some_text_as_display_username(user) - { original_username: user.username, display_username: "some text", foo: "bar" } - end - - def only_display_username(user) - { display_username: user.username } - end - - def username_and_something_else(user) - { username: user.username, foo: "bar" } - end - - it "updates notification data" do - notified_user = Fabricate(:user) - p1 = Fabricate(:post, post_number: 1, user: source_user) - p2 = Fabricate(:post, post_number: 1, user: walter) - Fabricate(:invite, invited_by: notified_user, user: source_user) - Fabricate(:invite, invited_by: notified_user, user: walter) - - n01 = create_notification(:mentioned, notified_user, p1, original_and_display_username(source_user)) - n02 = create_notification(:mentioned, notified_user, p2, original_and_display_username(walter)) - n03 = create_notification(:mentioned, notified_user, p1, original_username_and_some_text_as_display_username(source_user)) - n04 = create_notification(:mentioned, notified_user, p1, only_display_username(source_user)) - n05 = create_notification(:invitee_accepted, notified_user, nil, only_display_username(source_user)) - n06 = create_notification(:invitee_accepted, notified_user, nil, only_display_username(walter)) - n07 = create_notification(:granted_badge, source_user, nil, username_and_something_else(source_user)) - n08 = create_notification(:granted_badge, walter, nil, username_and_something_else(walter)) - n09 = create_notification(:group_message_summary, source_user, nil, username_and_something_else(source_user)) - n10 = create_notification(:group_message_summary, walter, nil, username_and_something_else(walter)) - - merge_users! - - expect(notification_data(n01)).to eq(original_and_display_username(target_user)) - expect(notification_data(n02)).to eq(original_and_display_username(walter)) - expect(notification_data(n03)).to eq(original_username_and_some_text_as_display_username(target_user)) - expect(notification_data(n04)).to eq(only_display_username(target_user)) - expect(notification_data(n05)).to eq(only_display_username(target_user)) - expect(notification_data(n06)).to eq(only_display_username(walter)) - expect(notification_data(n07)).to eq(username_and_something_else(target_user)) - expect(notification_data(n08)).to eq(username_and_something_else(walter)) - expect(notification_data(n09)).to eq(username_and_something_else(target_user)) - expect(notification_data(n10)).to eq(username_and_something_else(walter)) - end end context "post actions" do @@ -1068,4 +1005,13 @@ describe UserMerger do expect(User.find_by_username(source_user.username)).to be_nil end + + it "updates the username" do + Jobs::UpdateUsername.any_instance + .expects(:execute) + .with(user_id: source_user.id, old_username: 'alice1', new_username: 'alice') + .once + + merge_users! + end end diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index dad6e4d1588..dd74ca95ad4 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -98,13 +98,15 @@ describe UsernameChanger do before { UserActionCreator.enable } after { UserActionCreator.disable } - def create_post_and_change_username(args = {}) + def create_post_and_change_username(args = {}, &block) post = create_post(args.merge(topic_id: topic.id)) args.delete(:revisions)&.each do |revision| post.revise(post.user, revision, force_new_version: true) end + block.call(post) if block + UsernameChanger.change(user, 'bar') post.reload end @@ -231,14 +233,26 @@ describe UsernameChanger do expect(post.revisions[2].modifications["cooked"][0]).to eq(%Q(

Hello @bar!

)) expect(post.revisions[2].modifications["cooked"][1]).to eq(%Q(

Hello @bar!!

)) end + + it 'replaces mentions in posts marked for deletion' do + post = create_post_and_change_username(raw: "Hello @foo") do |p| + PostDestroyer.new(p.user, p).destroy + end + + expect(post.raw).to_not include("@foo") + expect(post.cooked).to_not include("foo") + expect(post.revisions.count).to eq(1) + + expect(post.revisions[0].modifications["raw"][0]).to eq("Hello @bar") + expect(post.revisions[0].modifications["cooked"][0]).to eq(%Q(

Hello @bar

)) + end end context 'quotes' do let(:quoted_post) { create_post(user: user, topic: topic, post_number: 1, raw: "quoted post") } + let(:avatar_url) { user.avatar_template.gsub("{size}", "40") } it 'replaces the username in quote tags' do - avatar_url = user.avatar_template_url.gsub("{size}", "40") - post = create_post_and_change_username(raw: <<~RAW) Lorem ipsum @@ -280,7 +294,7 @@ describe UsernameChanger do