From 5c37a5d0f2d34954294a6632074124bcd553c5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Fri, 5 Aug 2022 12:18:17 +0200 Subject: [PATCH] FIX: Allow to add the same watched word with a different case (#17799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we can’t add a case-sensitive watched word if another one exists with a different case. For example, the existing watched word `Meta` has been created and is case-sensitive. Now an admin tries to add `metA` while marking it as case-sensitive too, this won’t work and the word won’t be added. This patch changes this behavior by allowing to add same words that have different cases, so the example above will now work as expected. We still check for uniqueness but case-sensitivy is now taken into account. It means that if the watched word `meta` already exists and is not case-sensitive then it will not be possible to add `Meta` (case-sensitive or not) as `meta` already matches every possible variations of this word. --- .../addon/components/watched-word-form.js | 9 ++++-- .../acceptance/admin-watched-words-test.js | 8 +++++ app/models/watched_word.rb | 5 ++- spec/models/watched_word_spec.rb | 31 +++++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/watched-word-form.js b/app/assets/javascripts/admin/addon/components/watched-word-form.js index 0902e0b3e4d..e00032f22c1 100644 --- a/app/assets/javascripts/admin/addon/components/watched-word-form.js +++ b/app/assets/javascripts/admin/addon/components/watched-word-form.js @@ -47,9 +47,12 @@ export default Component.extend({ const filtered = words.filter( (content) => content.action === this.actionKey ); - return filtered.every( - (content) => content.word.toLowerCase() !== word.toLowerCase() - ); + return filtered.every((content) => { + if (content.case_sensitive === true) { + return content.word !== word; + } + return content.word.toLowerCase() !== word.toLowerCase(); + }); }, actions: { diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-watched-words-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-watched-words-test.js index 52aff93232a..e2d6b667024 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/admin-watched-words-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/admin-watched-words-test.js @@ -86,6 +86,14 @@ acceptance("Admin - Watched Words", function (needs) { assert .dom(".watched-words-list .watched-word") .hasText(`Discourse ${I18n.t("admin.watched_words.case_sensitive")}`); + + fillIn(".watched-word-form input", "discourse"); + click(".case-sensitivity-checkbox"); + await click(".watched-word-form button"); + + assert + .dom(".watched-words-list .watched-word") + .hasText(`discourse ${I18n.t("admin.watched_words.case_sensitive")}`); }); test("remove words", async function (assert) { diff --git a/app/models/watched_word.rb b/app/models/watched_word.rb index 66cee4588ad..e396544e87d 100644 --- a/app/models/watched_word.rb +++ b/app/models/watched_word.rb @@ -40,6 +40,9 @@ class WatchedWord < ActiveRecord::Base after_destroy :clear_cache scope :by_action, -> { order("action ASC, word ASC") } + scope :for, ->(word:) do + where("(word ILIKE :word AND case_sensitive = 'f') OR (word LIKE :word AND case_sensitive = 't')", word: word) + end def self.normalize_word(w) w.strip.squeeze('*') @@ -61,7 +64,7 @@ class WatchedWord < ActiveRecord::Base def self.create_or_update_word(params) new_word = normalize_word(params[:word]) - w = WatchedWord.where("word ILIKE ?", new_word).first || WatchedWord.new(word: new_word) + w = self.for(word: new_word).first_or_initialize(word: new_word) w.replacement = params[:replacement] if params[:replacement] w.action_key = params[:action_key] if params[:action_key] w.action = params[:action] if params[:action] diff --git a/spec/models/watched_word_spec.rb b/spec/models/watched_word_spec.rb index cf3ab22618d..ef593d29f2a 100644 --- a/spec/models/watched_word_spec.rb +++ b/spec/models/watched_word_spec.rb @@ -125,5 +125,36 @@ RSpec.describe WatchedWord do expect(updated.case_sensitive?).to eq(false) end + context "when a case-sensitive word already exists" do + subject(:create_or_update) do + described_class.create_or_update_word(word: word, action_key: :block, case_sensitive: true) + end + + fab!(:existing_word) { Fabricate(:watched_word, case_sensitive: true, word: "Meta") } + + context "when providing the exact same word" do + let(:word) { existing_word.word } + + it "doesn't create a new watched word" do + expect { create_or_update }.not_to change { described_class.count } + end + + it "returns the existing watched word" do + expect(create_or_update).to eq(existing_word) + end + end + + context "when providing the same word with a different case" do + let(:word) { "metA" } + + it "creates a new watched word" do + expect(create_or_update).not_to eq(existing_word) + end + + it "returns the new watched word" do + expect(create_or_update).to have_attributes word: "metA", case_sensitive: true, action: 1 + end + end + end end end