FEATURE: Allow watched words to be created as a group (#26632)

At the moment, there is no way to create a group of related watched words together.  If a user needed a set of words to be created together, they'll have to create them individually one at a time.

This change attempts to allow related watched words to be created as a group. The idea here is to have a list of words be tied together via a common `WatchedWordGroup` record.  Given a list of words, a `WatchedWordGroup` record is created and assigned to each `WatchedWord` record. The existing WatchedWord creation behaviour remains largely unchanged.

Co-authored-by: Selase Krakani <skrakani@gmail.com>
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit is contained in:
Vinoth Kannan 2024-04-29 15:50:55 +05:30 committed by GitHub
parent 0c8f531909
commit 143f06f2c6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 327 additions and 88 deletions

View File

@ -1,14 +1,13 @@
<div class="watched-word-input"> <div class="watched-word-input">
<label for="watched-word">{{i18n "admin.watched_words.form.label"}}</label> <label for="watched-word">{{i18n "admin.watched_words.form.label"}}</label>
<TextField <WatchedWords
@id="watched-word" @id="watched-words"
@value={{this.word}} @value={{this.words}}
@disabled={{this.formSubmitted}} @onChange={{action (mut this.words)}}
@autocorrect="off" @options={{hash
@autocapitalize="off" filterPlaceholder=this.placeholderKey
@placeholderKey={{this.placeholderKey}} disabled=this.formSubmitted
@title={{i18n this.placeholderKey}} }}
class="watched-word-input-field"
/> />
</div> </div>

View File

@ -1,11 +1,11 @@
import Component from "@ember/component"; import Component from "@ember/component";
import { action } from "@ember/object"; import { action } from "@ember/object";
import { equal, not } from "@ember/object/computed"; import { empty, equal } from "@ember/object/computed";
import { schedule } from "@ember/runloop";
import { service } from "@ember/service"; import { service } from "@ember/service";
import { isEmpty } from "@ember/utils"; import { isEmpty } from "@ember/utils";
import { classNames, tagName } from "@ember-decorators/component"; import { classNames, tagName } from "@ember-decorators/component";
import { observes } from "@ember-decorators/object"; import { observes } from "@ember-decorators/object";
import { popupAjaxError } from "discourse/lib/ajax-error";
import discourseComputed from "discourse-common/utils/decorators"; import discourseComputed from "discourse-common/utils/decorators";
import I18n from "discourse-i18n"; import I18n from "discourse-i18n";
import WatchedWord from "admin/models/watched-word"; import WatchedWord from "admin/models/watched-word";
@ -18,10 +18,11 @@ export default class WatchedWordForm extends Component {
formSubmitted = false; formSubmitted = false;
actionKey = null; actionKey = null;
showMessage = false; showMessage = false;
selectedTags = null;
isCaseSensitive = false; isCaseSensitive = false;
selectedTags = [];
words = [];
@not("word") submitDisabled; @empty("words") submitDisabled;
@equal("actionKey", "replace") canReplace; @equal("actionKey", "replace") canReplace;
@ -29,11 +30,6 @@ export default class WatchedWordForm extends Component {
@equal("actionKey", "link") canLink; @equal("actionKey", "link") canLink;
didInsertElement() {
super.didInsertElement(...arguments);
this.set("selectedTags", []);
}
@discourseComputed("siteSettings.watched_words_regular_expressions") @discourseComputed("siteSettings.watched_words_regular_expressions")
placeholderKey(watchedWordsRegularExpressions) { placeholderKey(watchedWordsRegularExpressions) {
if (watchedWordsRegularExpressions) { if (watchedWordsRegularExpressions) {
@ -43,29 +39,38 @@ export default class WatchedWordForm extends Component {
} }
} }
@observes("word") @observes("words.[]")
removeMessage() { removeMessage() {
if (this.showMessage && !isEmpty(this.word)) { if (this.showMessage && !isEmpty(this.words)) {
this.set("showMessage", false); this.set("showMessage", false);
} }
} }
@discourseComputed("word") @observes("actionKey")
isUniqueWord(word) { actionChanged() {
const words = this.filteredContent || []; this.setProperties({
const filtered = words.filter( showMessage: false,
(content) => content.action === this.actionKey
);
return filtered.every((content) => {
if (content.case_sensitive === true) {
return content.word !== word;
}
return content.word.toLowerCase() !== word.toLowerCase();
}); });
} }
focusInput() { @discourseComputed("words.[]")
schedule("afterRender", () => this.element.querySelector("input").focus()); isUniqueWord(words) {
const existingWords = this.filteredContent || [];
const filtered = existingWords.filter(
(content) => content.action === this.actionKey
);
const duplicate = filtered.find((content) => {
if (content.case_sensitive === true) {
return words.includes(content.word);
} else {
return words
.map((w) => w.toLowerCase())
.includes(content.word.toLowerCase());
}
});
return !duplicate;
} }
@action @action
@ -90,7 +95,7 @@ export default class WatchedWordForm extends Component {
this.set("formSubmitted", true); this.set("formSubmitted", true);
const watchedWord = WatchedWord.create({ const watchedWord = WatchedWord.create({
word: this.word, words: this.words,
replacement: replacement:
this.canReplace || this.canTag || this.canLink this.canReplace || this.canTag || this.canLink
? this.replacement ? this.replacement
@ -103,30 +108,23 @@ export default class WatchedWordForm extends Component {
.save() .save()
.then((result) => { .then((result) => {
this.setProperties({ this.setProperties({
word: "", words: [],
replacement: "", replacement: "",
formSubmitted: false,
selectedTags: [], selectedTags: [],
showMessage: true, showMessage: true,
message: I18n.t("admin.watched_words.form.success"), message: I18n.t("admin.watched_words.form.success"),
isCaseSensitive: false, isCaseSensitive: false,
}); });
this.action(WatchedWord.create(result)); if (result.words) {
this.focusInput(); result.words.forEach((word) => {
this.action(WatchedWord.create(word));
});
} else {
this.action(result);
}
}) })
.catch((e) => { .catch(popupAjaxError)
this.set("formSubmitted", false); .finally(this.set("formSubmitted", false));
const message = e.jqXHR.responseJSON?.errors
? I18n.t("generic_error_with_reason", {
error: e.jqXHR.responseJSON.errors.join(". "),
})
: I18n.t("generic_error");
this.dialog.alert({
message,
didConfirm: () => this.focusInput(),
didCancel: () => this.focusInput(),
});
});
} }
} }
} }

View File

@ -34,7 +34,7 @@ export default class WatchedWord extends EmberObject {
{ {
type: this.id ? "PUT" : "POST", type: this.id ? "PUT" : "POST",
data: { data: {
word: this.word, words: this.words,
replacement: this.replacement, replacement: this.replacement,
action_key: this.action, action_key: this.action,
case_sensitive: this.isCaseSensitive, case_sensitive: this.isCaseSensitive,

View File

@ -1,4 +1,4 @@
import { click, fillIn, visit } from "@ember/test-helpers"; import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers";
import { test } from "qunit"; import { test } from "qunit";
import { import {
acceptance, acceptance,
@ -57,20 +57,34 @@ acceptance("Admin - Watched Words", function (needs) {
test("add words", async function (assert) { test("add words", async function (assert) {
await visit("/admin/customize/watched_words/action/block"); await visit("/admin/customize/watched_words/action/block");
const submitButton = query(".watched-word-form button");
await click(".show-words-checkbox"); await click(".show-words-checkbox");
await fillIn(".watched-word-form input", "poutine"); await click(".select-kit-header.multi-select-header");
await click(".watched-word-form button");
let found = []; await fillIn(".select-kit-filter input", "poutine");
[...queryAll(".watched-words-list .watched-word")].forEach((elem) => { await triggerKeyEvent(".select-kit-filter input", "keydown", "Enter");
if (elem.innerText.trim() === "poutine") {
found.push(true); await fillIn(".select-kit-filter input", "cheese");
await triggerKeyEvent(".select-kit-filter input", "keydown", "Enter");
assert.equal(
query(".select-kit-header-wrapper .formatted-selection").innerText,
"poutine, cheese",
"has the correct words in the input field"
);
await click(submitButton);
const words = [...queryAll(".watched-words-list .watched-word")].map(
(elem) => {
return elem.innerText.trim();
} }
}); );
assert.strictEqual(found.length, 1); assert.ok(words.includes("poutine"), "has word 'poutine'");
assert.strictEqual(count(".watched-words-list .case-sensitive"), 0); assert.ok(words.includes("cheese"), "has word 'cheese'");
assert.equal(count(".watched-words-list .case-sensitive"), 0);
}); });
test("add case-sensitive words", async function (assert) { test("add case-sensitive words", async function (assert) {
@ -82,7 +96,11 @@ acceptance("Admin - Watched Words", function (needs) {
"Add button is disabled by default" "Add button is disabled by default"
); );
await click(".show-words-checkbox"); await click(".show-words-checkbox");
await fillIn(".watched-word-form input", "Discourse");
await click(".select-kit-header.multi-select-header");
await fillIn(".select-kit-filter input", "Discourse");
await triggerKeyEvent(".select-kit-filter input", "keydown", "Enter");
await click(".case-sensitivity-checkbox"); await click(".case-sensitivity-checkbox");
assert.strictEqual( assert.strictEqual(
submitButton.disabled, submitButton.disabled,
@ -95,7 +113,9 @@ acceptance("Admin - Watched Words", function (needs) {
.dom(".watched-words-list .watched-word") .dom(".watched-words-list .watched-word")
.hasText(`Discourse ${I18n.t("admin.watched_words.case_sensitive")}`); .hasText(`Discourse ${I18n.t("admin.watched_words.case_sensitive")}`);
await fillIn(".watched-word-form input", "discourse"); await click(".select-kit-header.multi-select-header");
await fillIn(".select-kit-filter input", "discourse");
await triggerKeyEvent(".select-kit-filter input", "keydown", "Enter");
await click(".case-sensitivity-checkbox"); await click(".case-sensitivity-checkbox");
await click(submitButton); await click(submitButton);

View File

@ -1,3 +1,4 @@
import EmberObject from "@ember/object";
import Pretender from "pretender"; import Pretender from "pretender";
import User from "discourse/models/user"; import User from "discourse/models/user";
import getURL from "discourse-common/lib/get-url"; import getURL from "discourse-common/lib/get-url";
@ -972,9 +973,24 @@ export function applyDefaultHandlers(pretender) {
pretender.delete("/admin/customize/watched_words/:id.json", success); pretender.delete("/admin/customize/watched_words/:id.json", success);
pretender.post("/admin/customize/watched_words.json", (request) => { pretender.post("/admin/customize/watched_words.json", (request) => {
const result = parsePostData(request.requestBody); const requestData = parsePostData(request.requestBody);
result.id = new Date().getTime();
result.case_sensitive = result.case_sensitive === "true"; const result = cloneJSON(
fixturesByUrl["/admin/customize/watched_words.json"]
);
result.words = [];
const words = requestData.words || requestData["words[]"];
words.forEach((word, index) => {
result.words[index] = EmberObject.create({
id: new Date().getTime(),
word,
action: requestData.action_key,
replacement: requestData.replacement,
case_sensitive: requestData.case_sensitive === "true",
});
});
return response(200, result); return response(200, result);
}); });

View File

@ -0,0 +1,20 @@
import { computed } from "@ember/object";
import { makeArray } from "discourse-common/lib/helpers";
import MultiSelectComponent from "select-kit/components/multi-select";
export default MultiSelectComponent.extend({
pluginApiIdentifiers: ["watched-words"],
classNames: ["watched-word-input-field"],
selectKitOptions: {
autoInsertNoneItem: false,
fullWidthOnMobile: true,
allowAny: true,
none: "admin.watched_words.form.words_or_phrases",
},
@computed("value.[]")
get content() {
return makeArray(this.value).map((x) => this.defaultItem(x, x));
},
});

View File

@ -347,6 +347,7 @@ table.screened-ip-addresses {
} }
.select-kit.multi-select.watched-word-input-field { .select-kit.multi-select.watched-word-input-field {
width: 300px; width: 300px;
margin-bottom: 9px;
} }
+ .btn-primary { + .btn-primary {

View File

@ -13,19 +13,36 @@ class Admin::WatchedWordsController < Admin::StaffController
end end
def create def create
watched_word = WatchedWord.create_or_update_word(watched_words_params) opts = watched_words_params
if watched_word.valid? action = opts[:action] || self.class.actions[opts[:action_key].to_sym]
StaffActionLogger.new(current_user).log_watched_words_creation(watched_word) words = opts.delete(:words)
render json: watched_word, root: false
watched_word_group = WatchedWordGroup.new(action: action)
watched_word_group.create_or_update_members(words, opts)
if watched_word_group.valid?
StaffActionLogger.new(current_user).log_watched_words_creation(watched_word_group)
render_json_dump WatchedWordListSerializer.new(
watched_word_group.watched_words,
scope: guardian,
root: false,
)
else else
render_json_error(watched_word) render_json_error(watched_word_group)
end end
end end
def destroy def destroy
watched_word = WatchedWord.find_by(id: params[:id]) watched_word = WatchedWord.find_by(id: params[:id])
raise Discourse::InvalidParameters.new(:id) unless watched_word raise Discourse::InvalidParameters.new(:id) unless watched_word
watched_word.destroy!
watched_word_group = watched_word.watched_word_group
if watched_word_group&.watched_words&.count == 1
watched_word_group.destroy!
else
watched_word.destroy!
end
StaffActionLogger.new(current_user).log_watched_words_deletion(watched_word) StaffActionLogger.new(current_user).log_watched_words_deletion(watched_word)
render json: success_json render json: success_json
end end
@ -100,6 +117,7 @@ class Admin::WatchedWordsController < Admin::StaffController
private private
def watched_words_params def watched_words_params
params.permit(:id, :word, :replacement, :action_key, :case_sensitive) @watched_words_params ||=
params.permit(:id, :replacement, :action, :action_key, :case_sensitive, words: [])
end end
end end

View File

@ -141,6 +141,9 @@ class UserHistory < ActiveRecord::Base
update_public_sidebar_section: 102, update_public_sidebar_section: 102,
destroy_public_sidebar_section: 103, destroy_public_sidebar_section: 103,
reset_bounce_score: 104, reset_bounce_score: 104,
create_watched_word_group: 105,
update_watched_word_group: 106,
delete_watched_word_group: 107,
) )
end end
@ -246,6 +249,9 @@ class UserHistory < ActiveRecord::Base
deleted_tag deleted_tag
chat_channel_status_change chat_channel_status_change
chat_auto_remove_membership chat_auto_remove_membership
create_watched_word_group
update_watched_word_group
delete_watched_word_group
] ]
end end

View File

@ -48,6 +48,16 @@ class WatchedWord < ActiveRecord::Base
) )
end end
belongs_to :watched_word_group
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.create_or_update_word(params) def self.create_or_update_word(params)
word = normalize_word(params[:word]) word = normalize_word(params[:word])
word = self.for(word: word).first_or_initialize(word: word) word = self.for(word: word).first_or_initialize(word: word)
@ -55,6 +65,7 @@ class WatchedWord < ActiveRecord::Base
word.action_key = params[:action_key] if params[:action_key] word.action_key = params[:action_key] if params[:action_key]
word.action = params[:action] if params[:action] word.action = params[:action] if params[:action]
word.case_sensitive = params[:case_sensitive] if !params[:case_sensitive].nil? word.case_sensitive = params[:case_sensitive] if !params[:case_sensitive].nil?
word.watched_word_group_id = params[:watched_word_group_id]
word.save word.save
word word
end end
@ -102,15 +113,17 @@ end
# #
# Table name: watched_words # Table name: watched_words
# #
# id :integer not null, primary key # id :integer not null, primary key
# word :string not null # word :string not null
# action :integer not null # action :integer not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# replacement :string # replacement :string
# case_sensitive :boolean default(FALSE), not null # case_sensitive :boolean default(FALSE), not null
# watched_word_group_id :bigint
# #
# Indexes # Indexes
# #
# index_watched_words_on_action_and_word (action,word) UNIQUE # index_watched_words_on_action_and_word (action,word) UNIQUE
# index_watched_words_on_watched_word_group_id (watched_word_group_id)
# #

View File

@ -0,0 +1,51 @@
# frozen_string_literal: true
class WatchedWordGroup < ActiveRecord::Base
validates :action, presence: true
has_many :watched_words, dependent: :destroy
def self.actions
WatchedWord.actions
end
def create_or_update_members(words, params)
WatchedWordGroup.transaction do
self.action_key = params[:action_key] if params[:action_key]
self.action = params[:action] if params[:action]
self.save! if self.changed?
words.each do |word|
watched_word =
WatchedWord.create_or_update_word(
params.merge(word: word, watched_word_group_id: self.id),
)
unless watched_word.valid?
self.errors.merge!(watched_word.errors)
raise ActiveRecord::Rollback
end
end
end
end
def action_key=(arg)
self.action = WatchedWordGroup.actions[arg.to_sym]
end
def action_log_details
action_key = WatchedWord.actions.key(self.action)
"#{action_key}#{watched_words.pluck(:word).join(", ")}"
end
end
# == Schema Information
#
# Table name: watched_word_groups
#
# id :bigint not null, primary key
# action :integer not null
# created_at :datetime not null
# updated_at :datetime not null
#

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class WatchedWordSerializer < ApplicationSerializer class WatchedWordSerializer < ApplicationSerializer
attributes :id, :word, :regexp, :replacement, :action, :case_sensitive attributes :id, :word, :regexp, :replacement, :action, :case_sensitive, :watched_word_group_id
def regexp def regexp
WordWatcher.word_to_regexp(word, engine: :js) WordWatcher.word_to_regexp(word, engine: :js)

View File

@ -6122,9 +6122,9 @@ en:
silence: "Silence new accounts if their very first post contains any of these words. The post will be automatically hidden until staff approves it." silence: "Silence new accounts if their very first post contains any of these words. The post will be automatically hidden until staff approves it."
link: "Replace words in posts with links." link: "Replace words in posts with links."
form: form:
label: "Has word or phrase" label: "Has words or phrases"
placeholder: "Enter word or phrase (* is a wildcard)" placeholder: "words or phrases (* is a wildcard)"
placeholder_regexp: "regular expression" placeholder_regexp: "regular expressions"
replace_label: "Replacement" replace_label: "Replacement"
replace_placeholder: "example" replace_placeholder: "example"
tag_label: "Tag" tag_label: "Tag"
@ -6137,6 +6137,7 @@ en:
upload_successful: "Upload successful. Words have been added." upload_successful: "Upload successful. Words have been added."
case_sensitivity_label: "Is case-sensitive" case_sensitivity_label: "Is case-sensitive"
case_sensitivity_description: "Only words with matching character casing" case_sensitivity_description: "Only words with matching character casing"
words_or_phrases: "words or phrases"
test: test:
button_label: "Test" button_label: "Test"
modal_title: "%{action}: Test Watched Words" modal_title: "%{action}: Test Watched Words"

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class CreateWatchedWordGroups < ActiveRecord::Migration[7.0]
def change
create_table :watched_word_groups do |t|
t.integer :action, null: false
t.timestamps
end
end
end

View File

@ -0,0 +1,8 @@
# frozen_string_literal: true
class AddWatchedWordGroupIdToWatchedWords < ActiveRecord::Migration[7.0]
def change
add_column :watched_words, :watched_word_group_id, :bigint
add_index :watched_words, :watched_word_group_id
end
end

View File

@ -0,0 +1,3 @@
# frozen_string_literal: true
Fabricator(:watched_word_group) { action WatchedWord.actions[:block] }

View File

@ -0,0 +1,56 @@
# frozen_string_literal: true
RSpec.describe WatchedWordGroup do
fab!(:watched_word_group)
fab!(:watched_word_1) { Fabricate(:watched_word, watched_word_group_id: watched_word_group.id) }
fab!(:watched_word_2) { Fabricate(:watched_word, watched_word_group_id: watched_word_group.id) }
describe "#create_or_update_members" do
it "updates watched word action" do
words = [watched_word_1.word, watched_word_2.word, "damn", "4sale"]
old_action = watched_word_group.action
watched_words_before_update = watched_word_group.watched_words
expect(old_action).to eq(WatchedWord.actions[:block])
expect(watched_words_before_update.map(&:action).uniq).to contain_exactly(old_action)
watched_word_group.create_or_update_members(words, action_key: :censor)
expect(watched_word_group.reload.errors).to be_empty
watched_words = watched_word_group.watched_words
expect(watched_words.size).to eq(4)
expect(watched_words.map(&:word)).to contain_exactly(*words)
expect(watched_words.map(&:action).uniq).to contain_exactly(WatchedWord.actions[:censor])
expect(watched_word_group.action).to eq(WatchedWord.actions[:censor])
end
it "leaves membership intact if update fails" do
words = [watched_word_1.word, watched_word_2.word, "a" * 120]
old_action = watched_word_group.action
watched_words_before_update = watched_word_group.watched_words
expect(watched_words_before_update.size).to eq(2)
expect(watched_words_before_update.map(&:word)).to contain_exactly(
watched_word_1.word,
watched_word_2.word,
)
expect(watched_words_before_update.map(&:action).uniq).to contain_exactly(old_action)
watched_word_group.create_or_update_members(
words,
action_key: WatchedWord.actions[watched_word_group.action],
)
expect(watched_word_group.reload.errors).not_to be_empty
watched_words = watched_word_group.watched_words
expect(watched_word_group.action).to eq(old_action)
expect(watched_words.size).to eq(2)
expect(watched_words.map(&:word)).to contain_exactly(watched_word_1.word, watched_word_2.word)
expect(watched_words.map(&:action).uniq).to contain_exactly(old_action)
end
end
end

View File

@ -86,6 +86,17 @@ RSpec.describe Admin::WatchedWordsController do
expect(WatchedWord.find_by(id: watched_word.id)).to eq(nil) expect(WatchedWord.find_by(id: watched_word.id)).to eq(nil)
expect(UserHistory.where(action: UserHistory.actions[:watched_word_destroy]).count).to eq(1) expect(UserHistory.where(action: UserHistory.actions[:watched_word_destroy]).count).to eq(1)
end end
it "should delete watched word group if it's the last word" do
watched_word_group = Fabricate(:watched_word_group)
watched_word.update!(watched_word_group: watched_word_group)
delete "/admin/customize/watched_words/#{watched_word.id}.json"
expect(response.status).to eq(200)
expect(WatchedWordGroup.exists?(id: watched_word_group.id)).to be_falsey
expect(WatchedWord.exists?(id: watched_word.id)).to be_falsey
end
end end
end end
@ -104,17 +115,24 @@ RSpec.describe Admin::WatchedWordsController do
before { sign_in(admin) } before { sign_in(admin) }
it "creates a word with default case sensitivity" do it "creates a word with default case sensitivity" do
post "/admin/customize/watched_words.json", params: { action_key: "flag", word: "Deals" } expect {
post "/admin/customize/watched_words.json",
params: {
action_key: "flag",
words: %w[Deals Offer],
}
}.to change { WatchedWord.count }.by(2)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(WatchedWord.take.word).to eq("Deals") expect(WatchedWord.take.word).to eq("Deals")
expect(WatchedWord.last.word).to eq("Offer")
end end
it "creates a word with the given case sensitivity" do it "creates a word with the given case sensitivity" do
post "/admin/customize/watched_words.json", post "/admin/customize/watched_words.json",
params: { params: {
action_key: "flag", action_key: "flag",
word: "PNG", words: ["PNG"],
case_sensitive: true, case_sensitive: true,
} }