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

@@ -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)
+        begin
-        post.cooked = update_cooked(post.cooked)
+          post.raw = update_raw(post.raw)
+          post.cooked = update_cooked(post.cooked)
 
-        # update without running validations and hooks
+          # update without running validations and hooks
-        post.update_columns(raw: post.raw, cooked: post.cooked)
+          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")
+        begin
-          revision.modifications["raw"]&.map! { |raw| update_raw(raw) }
+          if revision.modifications.key?("raw") || revision.modifications.key?("cooked")
-          revision.modifications["cooked"]&.map! { |cooked| update_cooked(cooked) }
+            revision.modifications["raw"]&.map! { |raw| update_raw(raw) }
-          revision.save!
+            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,
+        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]
       }
 
       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(
+        WHERE data ILIKE '%' || :old_username || '%'
-                  SELECT 1
+      SQL
-                  FROM posts AS p
+    end
-                  WHERE p.topic_id = n.topic_id
+
-                        AND p.post_number = n.post_number
+    def update_post_custom_fields
-                        AND p.user_id = :user_id)
+      PostCustomField.exec_sql(<<~SQL, old_username: @old_username, new_username: @new_username)
-              OR (n.notification_type IN (:notification_types_with_correct_user_id) AND n.user_id = :user_id)
+        UPDATE post_custom_fields
-              OR (n.notification_type = :invitee_accepted_notification_type
+        SET value = :new_username
-                  AND EXISTS(
+        WHERE name = 'action_code_who' AND value = :old_username
-                      SELECT 1
-                      FROM invites i
-                      WHERE i.user_id = :user_id AND n.user_id = i.invited_by_id
-                  )
-              )
       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|

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 }
+      before do
-      after { UserActionCreator.disable }
+        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(<p>Hello <a class="mention" href="/u/bar">@bar</a></p>))
         end
+
+        it 'works when users are mentioned with HTML' do
+          post = create_post_and_change_username(raw: '<a class="mention">@foo</a> and <a class="mention">@someuser</a>')
+
+          expect(post.raw).to eq('<a class="mention">@bar</a> and <a class="mention">@someuser</a>')
+          expect(post.cooked).to match_html('<p><a class="mention">@bar</a> and <a class="mention">@someuser</a></p>')
+        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