From 74c4af279a7516ae9dcbc92154cdc83917dc4e2a Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 15 May 2018 21:05:51 +0200 Subject: [PATCH] Improvements to user renaming * don't update search index if post belongs to deleted topic * log errors instead of crashing when updating post or revision fails * update mentions even when the href attribute is missing * run the background job with low priority * replace username in all notifications * update `action_code_who` used by small action posts --- app/jobs/regular/update_username.rb | 67 ++++++++++++++------------ spec/services/username_changer_spec.rb | 27 ++++++++++- 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb index d074bdd9299..e8a45d9366c 100644 --- a/app/jobs/regular/update_username.rb +++ b/app/jobs/regular/update_username.rb @@ -1,6 +1,8 @@ module Jobs class UpdateUsername < Jobs::Base + sidekiq_options queue: 'low' + def execute(args) @user_id = args[:user_id] @old_username = args[:old_username] @@ -16,26 +18,35 @@ module Jobs update_posts update_revisions update_notifications + update_post_custom_fields end def update_posts Post.with_deleted.where(post_conditions("posts.id"), post_condition_args).find_each do |post| - post.raw = update_raw(post.raw) - post.cooked = update_cooked(post.cooked) + begin + post.raw = update_raw(post.raw) + post.cooked = update_cooked(post.cooked) - # update without running validations and hooks - post.update_columns(raw: post.raw, cooked: post.cooked) + # update without running validations and hooks + post.update_columns(raw: post.raw, cooked: post.cooked) - SearchIndexer.index(post, force: true) + SearchIndexer.index(post, force: true) if post.topic + rescue => e + Discourse.warn_exception(e, message: "Failed to update post with id #{post.id}") + end end end def update_revisions PostRevision.where(post_conditions("post_revisions.post_id"), post_condition_args).find_each do |revision| - if revision.modifications.key?("raw") || revision.modifications.key?("cooked") - revision.modifications["raw"]&.map! { |raw| update_raw(raw) } - revision.modifications["cooked"]&.map! { |cooked| update_cooked(cooked) } - revision.save! + begin + if revision.modifications.key?("raw") || revision.modifications.key?("cooked") + revision.modifications["raw"]&.map! { |raw| update_raw(raw) } + revision.modifications["cooked"]&.map! { |cooked| update_cooked(cooked) } + revision.save! + end + rescue => e + Discourse.warn_exception(e, message: "Failed to update post revision with id #{revision.id}") end end end @@ -44,16 +55,11 @@ module Jobs params = { user_id: @user_id, old_username: @old_username, - new_username: @new_username, - notification_types_with_correct_user_id: [ - Notification.types[:granted_badge], - Notification.types[:group_message_summary] - ], - invitee_accepted_notification_type: Notification.types[:invitee_accepted] + new_username: @new_username } Notification.exec_sql(<<~SQL, params) - UPDATE notifications AS n + UPDATE notifications SET data = (data :: JSONB || jsonb_strip_nulls( jsonb_build_object( @@ -66,25 +72,24 @@ module Jobs THEN :new_username ELSE NULL END, 'username', CASE data :: JSONB ->> 'username' + WHEN :old_username + THEN :new_username + ELSE NULL END, + 'username2', CASE data :: JSONB ->> 'username2' 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 - ) - ) + WHERE data ILIKE '%' || :old_username || '%' + SQL + end + + def update_post_custom_fields + PostCustomField.exec_sql(<<~SQL, old_username: @old_username, new_username: @new_username) + UPDATE post_custom_fields + SET value = :new_username + WHERE name = 'action_code_who' AND value = :old_username SQL end @@ -125,7 +130,7 @@ module Jobs doc.css("a.mention").each do |a| a.content = a.content.gsub(@cooked_mention_username_regex, "@#{@new_username}") - a["href"] = a["href"].gsub(@cooked_mention_user_path_regex, "/u/#{@new_username}") + a["href"] = a["href"].gsub(@cooked_mention_user_path_regex, "/u/#{@new_username}") if a["href"] end doc.css("aside.quote > div.title").each do |div| diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index c02ea460aff..01d788bc683 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -95,8 +95,14 @@ describe UsernameChanger do let(:user) { Fabricate(:user, username: 'foo') } let(:topic) { Fabricate(:topic, user: user) } - before { UserActionCreator.enable } - after { UserActionCreator.disable } + before do + UserActionCreator.enable + Discourse.expects(:warn_exception).never + end + + after do + UserActionCreator.disable + end def create_post_and_change_username(args = {}, &block) post = create_post(args.merge(topic_id: topic.id)) @@ -256,6 +262,13 @@ describe UsernameChanger do expect(post.revisions[0].modifications["raw"][0]).to eq("Hello @bar") expect(post.revisions[0].modifications["cooked"][0]).to eq(%Q(

Hello @bar

)) end + + it 'works when users are mentioned with HTML' do + post = create_post_and_change_username(raw: '@foo and @someuser') + + expect(post.raw).to eq('@bar and @someuser') + expect(post.cooked).to match_html('

@bar and @someuser

') + end end context 'quotes' do @@ -427,6 +440,16 @@ describe UsernameChanger do HTML end end + + it 'updates username in small action posts' do + invited_by = Fabricate(:user) + p1 = topic.add_small_action(invited_by, 'invited_user', 'foo') + p2 = topic.add_small_action(invited_by, 'invited_user', 'foobar') + UsernameChanger.change(user, 'bar') + + expect(p1.reload.custom_fields['action_code_who']).to eq('bar') + expect(p2.reload.custom_fields['action_code_who']).to eq('foobar') + end end context 'notifications' do