diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb new file mode 100644 index 0000000000..ebc5f1aa0b --- /dev/null +++ b/app/jobs/regular/update_username.rb @@ -0,0 +1,94 @@ +module Jobs + class UpdateUsername < Jobs::Base + + def execute(args) + @user_id = args[:user_id] + + username = args[:old_username] + @raw_mention_regex = /(?:(? 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 + end + + doc.to_html + end + end +end diff --git a/app/services/user_anonymizer.rb b/app/services/user_anonymizer.rb index f4c11a7bfc..526a629750 100644 --- a/app/services/user_anonymizer.rb +++ b/app/services/user_anonymizer.rb @@ -20,9 +20,7 @@ class UserAnonymizer @prev_email = @user.email @prev_username = @user.username - if !UsernameChanger.change(@user, make_anon_username) - raise "Failed to change username" - end + raise "Failed to change username" unless UsernameChanger.change(@user, make_anon_username) @user.reload @user.password = SecureRandom.hex @@ -32,9 +30,7 @@ class UserAnonymizer @user.title = nil @user.uploaded_avatar_id = nil - if @opts.has_key?(:anonymize_ip) - anonymize_ips(@opts[:anonymize_ip]) - end + anonymize_ips(@opts[:anonymize_ip]) if @opts.has_key?(:anonymize_ip) @user.save diff --git a/app/services/username_changer.rb b/app/services/username_changer.rb index a81f089415..bb2cb1fcca 100644 --- a/app/services/username_changer.rb +++ b/app/services/username_changer.rb @@ -1,7 +1,10 @@ +require_dependency 'jobs/regular/update_username' + class UsernameChanger def initialize(user, new_username, actor = nil) @user = user + @old_username = user.username @new_username = new_username @actor = actor end @@ -10,14 +13,30 @@ class UsernameChanger self.new(user, new_username, actor).change end - def change - if @actor && @user.username != @new_username - StaffActionLogger.new(@actor).log_username_change(@user, @user.username, @new_username) + def change(asynchronous: true) + if @actor && @old_username != @new_username + StaffActionLogger.new(@actor).log_username_change(@user, @old_username, @new_username) end - # future work: update mentions and quotes - @user.username = @new_username - @user.save + if @user.save + + args = { + user_id: @user.id, + old_username: @old_username, + new_username: @new_username + } + + if asynchronous + Jobs.enqueue(:update_username, args) + else + Jobs::UpdateUsername.new.execute(args) + end + + return true + end + + false end + end diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index 3776ca7064..40dce95817 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -48,6 +48,20 @@ task "users:merge", [:source_username, :target_username] => [:environment] do |_ puts "", "Users merged!", "" end +task "users:rename", [:old_username, :new_username] => [:environment] do |_, args| + old_username = args[:old_username] + new_username = args[:new_username] + + if !old_username || !new_username + puts "ERROR: Expecting rake posts:rename[old_username,new_username]" + exit 1 + end + + changer = UsernameChanger.new(find_user(old_username), new_username) + changer.change(asynchronous: false) + puts "", "User renamed!", "" +end + def find_user(username) user = User.find_by_username(username) diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index 79e18091c9..dad6e4d158 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -90,6 +90,225 @@ describe UsernameChanger do expect(result).to eq(false) end end + + context 'posts and revisions' do + let(:user) { Fabricate(:user, username: 'foo') } + let(:topic) { Fabricate(:topic, user: user) } + + before { UserActionCreator.enable } + after { UserActionCreator.disable } + + def create_post_and_change_username(args = {}) + 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 + + UsernameChanger.change(user, 'bar') + post.reload + end + + context 'mentions' do + it 'rewrites cooked correctly' do + post = create_post_and_change_username(raw: "Hello @foo") + expect(post.cooked).to eq(%Q(
Hello @bar
)) + + post.rebake! + expect(post.cooked).to eq(%Q(Hello @bar
)) + end + + it 'ignores case when replacing mentions' do + post = create_post_and_change_username(raw: "There's no difference between @foo and @Foo") + + expect(post.raw).to eq("There's no difference between @bar and @bar") + expect(post.cooked).to eq(%Q(There’s no difference between @bar and @bar
)) + end + + it 'replaces mentions when there are leading symbols' do + post = create_post_and_change_username(raw: ".@foo -@foo %@foo _@foo ,@foo ;@foo @@foo") + + expect(post.raw).to eq(".@bar -@bar %@bar _@bar ,@bar ;@bar @@bar") + expect(post.cooked).to match_html(<<~HTML) +.@bar + -@bar + %@bar + _@bar + ,@bar + ;@bar + @@bar
+ HTML + end + + it 'replaces mentions within double and single quotes' do + post = create_post_and_change_username(raw: %Q("@foo" '@foo')) + + expect(post.raw).to eq(%Q("@bar" '@bar')) + expect(post.cooked).to eq(%Q()) + end + + it 'replaces mentions when there are trailing symbols' do + post = create_post_and_change_username(raw: "@foo. @foo, @foo: @foo; @foo-") + + expect(post.raw).to eq("@bar. @bar, @bar: @bar; @bar-") + expect(post.cooked).to match_html(<<~HTML) +@bar. + @bar, + @bar: + @bar; + @bar-
+ HTML + end + + it 'does not replace mention when followed by an underscore' do + post = create_post_and_change_username(raw: "@foo_") + + expect(post.raw).to eq("@foo_") + expect(post.cooked).to eq(%Q(@foo_
)) + end + + it 'does not replace mentions when there are leading alphanumeric chars' do + post = create_post_and_change_username(raw: "a@foo 2@foo") + + expect(post.raw).to eq("a@foo 2@foo") + expect(post.cooked).to eq(%Q(a@foo 2@foo
)) + end + + it 'does not replace username within email address' do + post = create_post_and_change_username(raw: "mail@foo.com") + + expect(post.raw).to eq("mail@foo.com") + expect(post.cooked).to eq(%Q()) + end + + it 'does not replace username in a mention of a similar username' do + Fabricate(:user, username: 'foobar') + Fabricate(:user, username: 'foo-bar') + Fabricate(:user, username: 'foo_bar') + Fabricate(:user, username: 'foo1') + + post = create_post_and_change_username(raw: "@foo @foobar @foo-bar @foo_bar @foo1") + + expect(post.raw).to eq("@bar @foobar @foo-bar @foo_bar @foo1") + expect(post.cooked).to match_html(<<~HTML) +@bar + @foobar + @foo-bar + @foo_bar + @foo1
+ HTML + end + + it 'updates the path to the user even when it links to /user instead of /u' do + post = create_post_and_change_username(raw: "Hello @foo") + post.update_column(:cooked, post.cooked.gsub("/u/foo", "/users/foo")) + + expect(post.raw).to eq("Hello @bar") + expect(post.cooked).to eq(%Q(Hello @bar
)) + end + + it 'replaces mentions within revisions' do + revisions = [{ raw: "Hello Foo" }, { raw: "Hello @foo!" }, { raw: "Hello @foo!!" }] + post = create_post_and_change_username(raw: "Hello @foo", revisions: revisions) + + expect(post.raw).to eq("Hello @bar!!") + expect(post.cooked).to eq(%Q(Hello @bar!!
)) + + expect(post.revisions.count).to eq(3) + + expect(post.revisions[0].modifications["raw"][0]).to eq("Hello @bar") + expect(post.revisions[0].modifications["raw"][1]).to eq("Hello Foo") + expect(post.revisions[0].modifications["cooked"][0]).to eq(%Q(Hello @bar
)) + expect(post.revisions[0].modifications["cooked"][1]).to eq(%Q(Hello Foo
)) + + expect(post.revisions[1].modifications["raw"][0]).to eq("Hello Foo") + expect(post.revisions[1].modifications["raw"][1]).to eq("Hello @bar!") + expect(post.revisions[1].modifications["cooked"][0]).to eq(%Q(Hello Foo
)) + expect(post.revisions[1].modifications["cooked"][1]).to eq(%Q(Hello @bar!
)) + + expect(post.revisions[2].modifications["raw"][0]).to eq("Hello @bar!") + expect(post.revisions[2].modifications["raw"][1]).to eq("Hello @bar!!") + 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 + end + + context 'quotes' do + let(:quoted_post) { create_post(user: user, topic: topic, post_number: 1, raw: "quoted post") } + + 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 + + [quote="foo, post:1, topic:#{quoted_post.topic.id}"] + quoted post + [/quote] + + [quote='foo'] + quoted post + [/quote] + + [quote=foo, post:1, topic:#{quoted_post.topic.id}] + quoted post + [/quote] + + dolor sit amet + RAW + + expect(post.raw).to eq(<<~RAW.strip) + Lorem ipsum + + [quote="bar, post:1, topic:#{quoted_post.topic.id}"] + quoted post + [/quote] + + [quote='bar'] + quoted post + [/quote] + + [quote=bar, post:1, topic:#{quoted_post.topic.id}] + quoted post + [/quote] + + dolor sit amet + RAW + + expect(post.cooked).to match_html(<<~HTML) +Lorem ipsum
+ + + +dolor sit amet
+ HTML + end + + # TODO spec for quotes in revisions + end + end + end end