FIX: Don't cook user fields to apply watched words (#17590)

The previous method for reused the PrettyText logic which applied the
watched word logic, but had the unwanted effect of cooking the text too.
This meant that regular text values were converted to HTML.

Follow up to commit 5a4c35f627.

app/models/user.rb

@@ -1283,7 +1283,7 @@ class User < ActiveRecord::Base
 
   def apply_watched_words
     validatable_user_fields.each do |id, value|
-      set_user_field(id, PrettyText.cook(value).gsub(/^<p>(.*)<\/p>$/, "\\1"))
+      set_user_field(id, WordWatcher.apply_to_text(value))
     end
   end
 

app/models/user_profile.rb

@@ -164,7 +164,7 @@ class UserProfile < ActiveRecord::Base
   end
 
   def apply_watched_words
-    self.location = PrettyText.cook(location).gsub(/^<p>(.*)<\/p>$/, "\\1")
+    self.location = WordWatcher.apply_to_text(location)
   end
 
   def website_domain_validator

app/services/word_watcher.rb

@@ -99,7 +99,7 @@ class WordWatcher
   end
 
   def self.censor(html)
-    regexp = WordWatcher.word_matcher_regexp(:censor)
+    regexp = word_matcher_regexp(:censor)
     return html if regexp.blank?
 
     doc = Nokogiri::HTML5::fragment(html)
@@ -110,12 +110,24 @@ class WordWatcher
   end
 
   def self.censor_text(text)
-    regexp = WordWatcher.word_matcher_regexp(:censor)
+    regexp = word_matcher_regexp(:censor)
     return text if regexp.blank?
 
     censor_text_with_regexp(text, regexp)
   end
 
+  def self.apply_to_text(text)
+    if regexp = word_matcher_regexp(:censor)
+      text = censor_text_with_regexp(text, regexp)
+    end
+
+    %i[replace link]
+      .flat_map { |type| word_matcher_regexps(type).to_a }
+      .reduce(text) do |t, (word_regexp, replacement)|
+        t.gsub(Regexp.new(word_regexp)) { |match| "#{match[0]}#{replacement}" }
+      end
+  end
+
   def self.clear_cache!
     WatchedWord.actions.each do |a, i|
       Discourse.cache.delete word_matcher_regexp_key(a)

spec/models/user_profile_spec.rb

@@ -45,6 +45,11 @@ RSpec.describe UserProfile do
       context "when it does not contain watched words" do
         it { is_expected.to be_valid }
       end
+
+      it "is not cooked" do
+        profile.location = "https://discourse.org"
+        expect { profile.save! }.not_to change { profile.location }
+      end
     end
 
     describe "#bio_raw" do

spec/models/user_spec.rb

@@ -224,6 +224,16 @@ RSpec.describe User do
         it { is_expected.to be_valid }
       end
 
+      context "when user fields contain URL" do
+        let(:value) { "https://discourse.org" }
+        let(:user_field_value) { user.reload.user_fields[user_field.id.to_s] }
+
+        it "is not cooked" do
+          user.save!
+          expect(user_field_value).to eq "https://discourse.org"
+        end
+      end
+
       context "with a multiselect user field" do
         fab!(:user_field) do
           Fabricate(:user_field, field_type: 'multiselect', show_on_profile: true) do

spec/services/word_watcher_spec.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 describe WordWatcher do
-
   let(:raw) { "Do you like liquorice?\n\nI really like them. One could even say that I am *addicted* to liquorice. And if\nyou can mix it up with some anise, then I'm in heaven ;)" }
 
   after do
@@ -15,67 +14,69 @@ describe WordWatcher do
     context 'format of the result regexp' do
       it "is correct when watched_words_regular_expressions = true" do
         SiteSetting.watched_words_regular_expressions = true
-        regexp = WordWatcher.word_matcher_regexp(:block)
+        regexp = described_class.word_matcher_regexp(:block)
         expect(regexp.inspect).to eq("/(#{word1})|(#{word2})/i")
       end
 
       it "is correct when watched_words_regular_expressions = false" do
         SiteSetting.watched_words_regular_expressions = false
-        regexp = WordWatcher.word_matcher_regexp(:block)
+        regexp = described_class.word_matcher_regexp(:block)
         expect(regexp.inspect).to eq("/(?:\\W|^)(#{word1}|#{word2})(?=\\W|$)/i")
       end
     end
   end
 
-  describe "word_matches_for_action?" do
+  describe "#word_matches_for_action?" do
     it "is falsey when there are no watched words" do
-      expect(WordWatcher.new(raw).word_matches_for_action?(:require_approval)).to be_falsey
+      expect(described_class.new(raw).word_matches_for_action?(:require_approval)).to be_falsey
     end
 
     context "with watched words" do
       fab!(:anise) { Fabricate(:watched_word, word: "anise", action: WatchedWord.actions[:require_approval]) }
 
       it "is falsey without a match" do
-        expect(WordWatcher.new("No liquorice for me, thanks...").word_matches_for_action?(:require_approval)).to be_falsey
+        expect(described_class.new("No liquorice for me, thanks...").word_matches_for_action?(:require_approval)).to be_falsey
       end
 
       it "is returns matched words if there's a match" do
-        m = WordWatcher.new(raw).word_matches_for_action?(:require_approval)
-        expect(m).to be_truthy
-        expect(m[1]).to eq(anise.word)
+        matches = described_class.new(raw).word_matches_for_action?(:require_approval)
+        expect(matches).to be_truthy
+        expect(matches[1]).to eq(anise.word)
       end
 
       it "finds at start of string" do
-        m = WordWatcher.new("#{anise.word} is garbage").word_matches_for_action?(:require_approval)
-        expect(m[1]).to eq(anise.word)
+        matches = described_class.new("#{anise.word} is garbage").word_matches_for_action?(:require_approval)
+        expect(matches[1]).to eq(anise.word)
       end
 
       it "finds at end of string" do
-        m = WordWatcher.new("who likes #{anise.word}").word_matches_for_action?(:require_approval)
-        expect(m[1]).to eq(anise.word)
+        matches = described_class.new("who likes #{anise.word}").word_matches_for_action?(:require_approval)
+        expect(matches[1]).to eq(anise.word)
       end
 
       it "finds non-letters in place of letters" do
         Fabricate(:watched_word, word: "co(onut", action: WatchedWord.actions[:require_approval])
-        m = WordWatcher.new("This co(onut is delicious.").word_matches_for_action?(:require_approval)
-        expect(m[1]).to eq("co(onut")
+
+        matches = described_class.new("This co(onut is delicious.").word_matches_for_action?(:require_approval)
+        expect(matches[1]).to eq("co(onut")
       end
 
       it "handles * for wildcards" do
         Fabricate(:watched_word, word: "a**le*", action: WatchedWord.actions[:require_approval])
-        m = WordWatcher.new("I acknowledge you.").word_matches_for_action?(:require_approval)
-        expect(m[1]).to eq("acknowledge")
+
+        matches = described_class.new("I acknowledge you.").word_matches_for_action?(:require_approval)
+        expect(matches[1]).to eq("acknowledge")
       end
 
       context "word boundary" do
         it "handles word boundary" do
           Fabricate(:watched_word, word: "love", action: WatchedWord.actions[:require_approval])
-          expect(WordWatcher.new("I Love, bananas.").word_matches_for_action?(:require_approval)[1]).to eq("Love")
-          expect(WordWatcher.new("I LOVE; apples.").word_matches_for_action?(:require_approval)[1]).to eq("LOVE")
-          expect(WordWatcher.new("love: is a thing.").word_matches_for_action?(:require_approval)[1]).to eq("love")
-          expect(WordWatcher.new("I love. oranges").word_matches_for_action?(:require_approval)[1]).to eq("love")
-          expect(WordWatcher.new("I :love. pineapples").word_matches_for_action?(:require_approval)[1]).to eq("love")
-          expect(WordWatcher.new("peace ,love and understanding.").word_matches_for_action?(:require_approval)[1]).to eq("love")
+          expect(described_class.new("I Love, bananas.").word_matches_for_action?(:require_approval)[1]).to eq("Love")
+          expect(described_class.new("I LOVE; apples.").word_matches_for_action?(:require_approval)[1]).to eq("LOVE")
+          expect(described_class.new("love: is a thing.").word_matches_for_action?(:require_approval)[1]).to eq("love")
+          expect(described_class.new("I love. oranges").word_matches_for_action?(:require_approval)[1]).to eq("love")
+          expect(described_class.new("I :love. pineapples").word_matches_for_action?(:require_approval)[1]).to eq("love")
+          expect(described_class.new("peace ,love and understanding.").word_matches_for_action?(:require_approval)[1]).to eq("love")
         end
       end
 
@@ -85,9 +86,11 @@ describe WordWatcher do
             %w{bananas hate hates}.each do |word|
               Fabricate(:watched_word, word: word, action: WatchedWord.actions[:block])
             end
-            matches = WordWatcher.new("I hate bananas").word_matches_for_action?(:block, all_matches: true)
+
+            matches = described_class.new("I hate bananas").word_matches_for_action?(:block, all_matches: true)
             expect(matches).to contain_exactly('hate', 'bananas')
-            matches = WordWatcher.new("She hates bananas too").word_matches_for_action?(:block, all_matches: true)
+
+            matches = described_class.new("She hates bananas too").word_matches_for_action?(:block, all_matches: true)
             expect(matches).to contain_exactly('hates', 'bananas')
           end
         end
@@ -101,10 +104,10 @@ describe WordWatcher do
             Fabricate(:watched_word, word: "(pine)?apples", action: WatchedWord.actions[:block])
             Fabricate(:watched_word, word: "((move|store)(d)?)|((watch|listen)(ed|ing)?)", action: WatchedWord.actions[:block])
 
-            matches = WordWatcher.new("pine pineapples apples").word_matches_for_action?(:block, all_matches: true)
+            matches = described_class.new("pine pineapples apples").word_matches_for_action?(:block, all_matches: true)
             expect(matches).to contain_exactly('pineapples', 'apples')
 
-            matches = WordWatcher.new("go watched watch ed ing move d moveed moved moving").word_matches_for_action?(:block, all_matches: true)
+            matches = described_class.new("go watched watch ed ing move d moveed moved moving").word_matches_for_action?(:block, all_matches: true)
             expect(matches).to contain_exactly(*%w{watched watch move moved})
           end
         end
@@ -113,20 +116,23 @@ describe WordWatcher do
       context "emojis" do
         it "handles emoji" do
           Fabricate(:watched_word, word: ":joy:", action: WatchedWord.actions[:require_approval])
-          m = WordWatcher.new("Lots of emojis here :joy:").word_matches_for_action?(:require_approval)
-          expect(m[1]).to eq(":joy:")
+
+          matches = described_class.new("Lots of emojis here :joy:").word_matches_for_action?(:require_approval)
+          expect(matches[1]).to eq(":joy:")
         end
 
         it "handles unicode emoji" do
           Fabricate(:watched_word, word: "🎃", action: WatchedWord.actions[:require_approval])
-          m = WordWatcher.new("Halloween party! 🎃").word_matches_for_action?(:require_approval)
-          expect(m[1]).to eq("🎃")
+
+          matches = described_class.new("Halloween party! 🎃").word_matches_for_action?(:require_approval)
+          expect(matches[1]).to eq("🎃")
         end
 
         it "handles emoji skin tone" do
           Fabricate(:watched_word, word: ":woman:t5:", action: WatchedWord.actions[:require_approval])
-          m = WordWatcher.new("To Infinity and beyond! 🚀 :woman:t5:").word_matches_for_action?(:require_approval)
-          expect(m[1]).to eq(":woman:t5:")
+
+          matches = described_class.new("To Infinity and beyond! 🚀 :woman:t5:").word_matches_for_action?(:require_approval)
+          expect(matches[1]).to eq(":woman:t5:")
         end
       end
 
@@ -141,8 +147,9 @@ describe WordWatcher do
             word: /\btest\b/,
             action: WatchedWord.actions[:block]
           )
-          m = WordWatcher.new("this is not a test.").word_matches_for_action?(:block)
-          expect(m[0]).to eq("test")
+
+          matches = described_class.new("this is not a test.").word_matches_for_action?(:block)
+          expect(matches[0]).to eq("test")
         end
 
         it "supports regular expressions as a site setting" do
@@ -151,12 +158,15 @@ describe WordWatcher do
             word: /tro[uo]+t/,
             action: WatchedWord.actions[:require_approval]
           )
-          m = WordWatcher.new("Evil Trout is cool").word_matches_for_action?(:require_approval)
-          expect(m[0]).to eq("Trout")
-          m = WordWatcher.new("Evil Troot is cool").word_matches_for_action?(:require_approval)
-          expect(m[0]).to eq("Troot")
-          m = WordWatcher.new("trooooooooot").word_matches_for_action?(:require_approval)
-          expect(m[0]).to eq("trooooooooot")
+
+          matches = described_class.new("Evil Trout is cool").word_matches_for_action?(:require_approval)
+          expect(matches[0]).to eq("Trout")
+
+          matches = described_class.new("Evil Troot is cool").word_matches_for_action?(:require_approval)
+          expect(matches[0]).to eq("Troot")
+
+          matches = described_class.new("trooooooooot").word_matches_for_action?(:require_approval)
+          expect(matches[0]).to eq("trooooooooot")
         end
 
         it "support uppercase" do
@@ -166,16 +176,29 @@ describe WordWatcher do
             action: WatchedWord.actions[:require_approval]
           )
 
-          m = WordWatcher.new('Amazing place').word_matches_for_action?(:require_approval)
-          expect(m).to be_nil
-          m = WordWatcher.new('Amazing applesauce').word_matches_for_action?(:require_approval)
-          expect(m[0]).to eq('applesauce')
-          m = WordWatcher.new('Amazing AppleSauce').word_matches_for_action?(:require_approval)
-          expect(m[0]).to eq('AppleSauce')
+          matches = described_class.new('Amazing place').word_matches_for_action?(:require_approval)
+          expect(matches).to be_nil
+
+          matches = described_class.new('Amazing applesauce').word_matches_for_action?(:require_approval)
+          expect(matches[0]).to eq('applesauce')
+
+          matches = described_class.new('Amazing AppleSauce').word_matches_for_action?(:require_approval)
+          expect(matches[0]).to eq('AppleSauce')
         end
       end
 
     end
   end
 
+  describe ".apply_to_text" do
+    fab!(:censored_word) { Fabricate(:watched_word, word: "censored", action: WatchedWord.actions[:censor]) }
+    fab!(:replaced_word) { Fabricate(:watched_word, word: "to replace", replacement: "replaced", action: WatchedWord.actions[:replace]) }
+    fab!(:link_word) { Fabricate(:watched_word, word: "https://notdiscourse.org", replacement: "https://discourse.org", action: WatchedWord.actions[:link]) }
+
+    it "replaces all types of words" do
+      text = "hello censored world to replace https://notdiscourse.org"
+      expected = "hello #{described_class::REPLACEMENT_LETTER * 8} world replaced https://discourse.org"
+      expect(described_class.apply_to_text(text)).to eq(expected)
+    end
+  end
 end