diff --git a/app/assets/javascripts/admin/components/watched-word-uploader.js.es6 b/app/assets/javascripts/admin/components/watched-word-uploader.js.es6 index 5f047a2bc6e..b1706337f28 100644 --- a/app/assets/javascripts/admin/components/watched-word-uploader.js.es6 +++ b/app/assets/javascripts/admin/components/watched-word-uploader.js.es6 @@ -2,13 +2,13 @@ import computed from "ember-addons/ember-computed-decorators"; import UploadMixin from "discourse/mixins/upload"; export default Ember.Component.extend(UploadMixin, { - type: "csv", + type: "txt", classNames: "watched-words-uploader", uploadUrl: "/admin/logs/watched_words/upload", addDisabled: Ember.computed.alias("uploading"), validateUploadedFilesOptions() { - return { csvOnly: true }; + return { skipValidation: true }; }, @computed("actionKey") diff --git a/app/assets/javascripts/admin/controllers/admin-watched-words-action.js.es6 b/app/assets/javascripts/admin/controllers/admin-watched-words-action.js.es6 index 2e38279b54d..2e1260533ac 100644 --- a/app/assets/javascripts/admin/controllers/admin-watched-words-action.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-watched-words-action.js.es6 @@ -1,5 +1,7 @@ import computed from "ember-addons/ember-computed-decorators"; import WatchedWord from "admin/models/watched-word"; +import { ajax } from "discourse/lib/ajax"; +import { fmt } from "discourse/lib/computed"; export default Ember.Controller.extend({ actionNameKey: null, @@ -8,6 +10,10 @@ export default Ember.Controller.extend({ "adminWatchedWords.filtered", "adminWatchedWords.showWords" ), + downloadLink: fmt( + "actionNameKey", + "/admin/logs/watched_words/action/%@/download" + ), findAction(actionName) { return (this.get("adminWatchedWords.model") || []).findBy( @@ -17,13 +23,13 @@ export default Ember.Controller.extend({ }, @computed("actionNameKey", "adminWatchedWords.model") - filteredContent(actionNameKey) { - if (!actionNameKey) { - return []; - } + currentAction(actionName) { + return this.findAction(actionName); + }, - const a = this.findAction(actionNameKey); - return a ? a.words : []; + @computed("currentAction.words.[]", "adminWatchedWords.model") + filteredContent(words) { + return words || []; }, @computed("actionNameKey") @@ -31,10 +37,9 @@ export default Ember.Controller.extend({ return I18n.t("admin.watched_words.action_descriptions." + actionNameKey); }, - @computed("actionNameKey", "adminWatchedWords.model") - wordCount(actionNameKey) { - const a = this.findAction(actionNameKey); - return a ? a.words.length : 0; + @computed("currentAction.count") + wordCount(count) { + return count || 0; }, actions: { @@ -62,10 +67,9 @@ export default Ember.Controller.extend({ }, recordRemoved(arg) { - const a = this.findAction(this.actionNameKey); - if (a) { - a.words.removeObject(arg); - a.decrementProperty("count"); + if (this.currentAction) { + this.currentAction.words.removeObject(arg); + this.currentAction.decrementProperty("count"); } }, @@ -73,6 +77,30 @@ export default Ember.Controller.extend({ WatchedWord.findAll().then(data => { this.set("adminWatchedWords.model", data); }); + }, + + clearAll() { + const actionKey = this.actionNameKey; + bootbox.confirm( + I18n.t(`admin.watched_words.clear_all_confirm_${actionKey}`), + I18n.t("no_value"), + I18n.t("yes_value"), + result => { + if (result) { + ajax(`/admin/logs/watched_words/action/${actionKey}.json`, { + method: "DELETE" + }).then(() => { + const action = this.findAction(actionKey); + if (action) { + action.setProperties({ + words: [], + count: 0 + }); + } + }); + } + } + ); } } }); diff --git a/app/assets/javascripts/admin/templates/components/watched-word-uploader.hbs b/app/assets/javascripts/admin/templates/components/watched-word-uploader.hbs index 4042bc5c828..d2422da00b7 100644 --- a/app/assets/javascripts/admin/templates/components/watched-word-uploader.hbs +++ b/app/assets/javascripts/admin/templates/components/watched-word-uploader.hbs @@ -1,7 +1,6 @@ -
{{i18n 'admin.watched_words.one_word_per_line'}} diff --git a/app/assets/javascripts/admin/templates/watched-words-action.hbs b/app/assets/javascripts/admin/templates/watched-words-action.hbs index ad04b20c5b9..9e811570c7b 100644 --- a/app/assets/javascripts/admin/templates/watched-words-action.hbs +++ b/app/assets/javascripts/admin/templates/watched-words-action.hbs @@ -3,14 +3,24 @@

{{actionDescription}}

-{{watched-word-form - actionKey=actionNameKey - action=(action "recordAdded") - filteredContent=filteredContent - regularExpressions=adminWatchedWords.regularExpressions}} + {{watched-word-form + actionKey=actionNameKey + action=(action "recordAdded") + filteredContent=filteredContent + regularExpressions=adminWatchedWords.regularExpressions}} -{{watched-word-uploader uploading=uploading actionKey=actionNameKey done=(action "uploadComplete")}} +
+
+ {{d-button + class="btn-default download-link" + href=downloadLink + icon="download" + label="admin.watched_words.download"}} +
+ {{watched-word-uploader uploading=uploading actionKey=actionNameKey done=(action "uploadComplete")}} +
+
+ +
+ {{d-button + class="btn-danger clear-all" + label="admin.watched_words.clear_all" + icon="trash-alt" + action=(action "clearAll")}} +
diff --git a/app/assets/javascripts/discourse/lib/url.js.es6 b/app/assets/javascripts/discourse/lib/url.js.es6 index 18af8aee7ec..66b82829ecf 100644 --- a/app/assets/javascripts/discourse/lib/url.js.es6 +++ b/app/assets/javascripts/discourse/lib/url.js.es6 @@ -23,7 +23,8 @@ const SERVER_SIDE_ONLY = [ /\.rss$/, /\.json$/, /^\/admin\/upgrade$/, - /^\/logs($|\/)/ + /^\/logs($|\/)/, + /^\/admin\/logs\/watched_words\/action\/[^\/]+\/download$/ ]; export function rewritePath(path) { diff --git a/app/assets/stylesheets/common/admin/staff_logs.scss b/app/assets/stylesheets/common/admin/staff_logs.scss index bbfd6ae8880..349480b8fac 100644 --- a/app/assets/stylesheets/common/admin/staff_logs.scss +++ b/app/assets/stylesheets/common/admin/staff_logs.scss @@ -362,17 +362,33 @@ table.screened-ip-addresses { display: inline-block; width: 250px; margin-bottom: 1em; - float: left; + vertical-align: top; +} + +.admin-watched-words { + .clear-all-row { + display: flex; + margin-top: 10px; + justify-content: flex-end; + } } .watched-word-controls { display: flex; flex-wrap: wrap; margin-bottom: 1em; + justify-content: space-between; + .download-upload-controls { + display: flex; + } + .download { + justify-content: flex-end; + } } .watched-words-list { margin-top: 20px; + display: inline-block; } .watched-word { @@ -395,13 +411,17 @@ table.screened-ip-addresses { } .watched-words-uploader { - margin-left: auto; + margin-left: 5px; + display: flex; + flex-direction: column; + align-items: flex-end; @media screen and (max-width: 500px) { flex: 1 1 100%; margin-top: 0.5em; } .instructions { font-size: $font-down-1; + margin-top: 5px; } } diff --git a/app/controllers/admin/watched_words_controller.rb b/app/controllers/admin/watched_words_controller.rb index a9b3a490c24..b6e013041be 100644 --- a/app/controllers/admin/watched_words_controller.rb +++ b/app/controllers/admin/watched_words_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class Admin::WatchedWordsController < Admin::AdminController + skip_before_action :check_xhr, only: [:download] def index render_json_dump WatchedWordListSerializer.new(WatchedWord.by_action, scope: guardian, root: false) @@ -35,12 +36,36 @@ class Admin::WatchedWordsController < Admin::AdminController rescue => e data = failed_json.merge(errors: [e.message]) end - MessageBus.publish("/uploads/csv", data.as_json, client_ids: [params[:client_id]]) + MessageBus.publish("/uploads/txt", data.as_json, client_ids: [params[:client_id]]) end render json: success_json end + def download + params.require(:id) + name = watched_words_params[:id].to_sym + action = WatchedWord.actions[name] + raise Discourse::NotFound if !action + + content = WatchedWord.where(action: action).pluck(:word).join("\n") + headers['Content-Length'] = content.bytesize.to_s + send_data content, + filename: "#{Discourse.current_hostname}-watched-words-#{name}.txt", + content_type: "text/plain" + end + + def clear_all + params.require(:id) + name = watched_words_params[:id].to_sym + action = WatchedWord.actions[name] + raise Discourse::NotFound if !action + + WatchedWord.where(action: action).delete_all + WordWatcher.clear_cache! + render json: success_json + end + private def watched_words_params diff --git a/app/controllers/export_csv_controller.rb b/app/controllers/export_csv_controller.rb index 1f0e59ef068..8c724947250 100644 --- a/app/controllers/export_csv_controller.rb +++ b/app/controllers/export_csv_controller.rb @@ -12,6 +12,7 @@ class ExportCsvController < ApplicationController end private + def export_params @_export_params ||= begin params.require(:entity) diff --git a/app/services/word_watcher.rb b/app/services/word_watcher.rb index 0cddc96fc24..1e12f860029 100644 --- a/app/services/word_watcher.rb +++ b/app/services/word_watcher.rb @@ -14,17 +14,27 @@ class WordWatcher WatchedWord.where(action: WatchedWord.actions[action.to_sym]).exists? end - def self.word_matcher_regexp(action) - s = Discourse.cache.fetch(word_matcher_regexp_key(action), expires_in: 1.day) do - words = words_for_action(action) - if words.empty? - nil - else - regexp = '(' + words.map { |w| word_to_regexp(w) }.join('|'.freeze) + ')' - SiteSetting.watched_words_regular_expressions? ? regexp : "(? "watched_words#index" + get "action/:id/download" => "watched_words#download" + delete "action/:id" => "watched_words#clear_all" end end post "watched_words/upload" => "watched_words#upload" diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 26712309cd4..a9a12786940 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -172,9 +172,16 @@ class NewPostManager end def perform - if !self.class.exempt_user?(@user) && matches = WordWatcher.new("#{@args[:title]} #{@args[:raw]}").should_block? + if !self.class.exempt_user?(@user) && matches = WordWatcher.new("#{@args[:title]} #{@args[:raw]}").should_block?.presence result = NewPostResult.new(:created_post, false) - result.errors.add(:base, I18n.t('contains_blocked_words', word: matches[0])) + if matches.size == 1 + key = 'contains_blocked_word' + translation_args = { word: matches[0] } + else + key = 'contains_blocked_words' + translation_args = { words: matches.join(', ') } + end + result.errors.add(:base, I18n.t(key, translation_args)) return result end diff --git a/lib/validators/post_validator.rb b/lib/validators/post_validator.rb index d5a33a79bb5..09c564b3b19 100644 --- a/lib/validators/post_validator.rb +++ b/lib/validators/post_validator.rb @@ -60,8 +60,15 @@ class Validators::PostValidator < ActiveModel::Validator end def watched_words(post) - if !post.acting_user&.staged && matches = WordWatcher.new(post.raw).should_block? - post.errors.add(:base, I18n.t('contains_blocked_words', word: matches[0])) + if !post.acting_user&.staged && matches = WordWatcher.new(post.raw).should_block?.presence + if matches.size == 1 + key = 'contains_blocked_word' + translation_args = { word: matches[0] } + else + key = 'contains_blocked_words' + translation_args = { words: matches.join(', ') } + end + post.errors.add(:base, I18n.t(key, translation_args)) end end diff --git a/spec/integration/watched_words_spec.rb b/spec/integration/watched_words_spec.rb index 68a7c2c05c0..b78990090e8 100644 --- a/spec/integration/watched_words_spec.rb +++ b/spec/integration/watched_words_spec.rb @@ -13,6 +13,7 @@ describe WatchedWord do let(:require_approval_word) { Fabricate(:watched_word, action: WatchedWord.actions[:require_approval]) } let(:flag_word) { Fabricate(:watched_word, action: WatchedWord.actions[:flag]) } let(:block_word) { Fabricate(:watched_word, action: WatchedWord.actions[:block]) } + let(:another_block_word) { Fabricate(:watched_word, action: WatchedWord.actions[:block]) } before_all do WordWatcher.clear_cache! @@ -27,7 +28,7 @@ describe WatchedWord do expect { result = manager.perform expect(result).to_not be_success - expect(result.errors[:base]&.first).to eq(I18n.t('contains_blocked_words', word: block_word.word)) + expect(result.errors[:base]&.first).to eq(I18n.t('contains_blocked_word', word: block_word.word)) }.to_not change { Post.count } end @@ -51,6 +52,15 @@ describe WatchedWord do should_block_post(manager) end + it "should block the post if it contains multiple blocked words" do + manager = NewPostManager.new(moderator, raw: "Want some #{block_word.word} #{another_block_word.word} for cheap?", topic_id: topic.id) + expect { + result = manager.perform + expect(result).to_not be_success + expect(result.errors[:base]&.first).to eq(I18n.t('contains_blocked_words', words: [block_word.word, another_block_word.word].join(', '))) + }.to_not change { Post.count } + end + it "should block in a private message too" do manager = NewPostManager.new( tl2_user, diff --git a/spec/requests/admin/watched_words_controller_spec.rb b/spec/requests/admin/watched_words_controller_spec.rb index 1b8127c373d..97cac195f94 100644 --- a/spec/requests/admin/watched_words_controller_spec.rb +++ b/spec/requests/admin/watched_words_controller_spec.rb @@ -49,4 +49,70 @@ RSpec.describe Admin::WatchedWordsController do end end end + + describe '#download' do + context 'not logged in as admin' do + it "doesn't allow performing #download" do + get "/admin/logs/watched_words/action/block/download" + expect(response.status).to eq(404) + end + end + + context 'logged in as admin' do + before do + sign_in(admin) + end + + it "words of different actions are downloaded separately" do + block_word_1 = Fabricate(:watched_word, action: WatchedWord.actions[:block]) + block_word_2 = Fabricate(:watched_word, action: WatchedWord.actions[:block]) + censor_word_1 = Fabricate(:watched_word, action: WatchedWord.actions[:censor]) + + get "/admin/logs/watched_words/action/block/download" + expect(response.status).to eq(200) + block_words = response.body.split("\n") + expect(block_words).to contain_exactly(block_word_1.word, block_word_2.word) + + get "/admin/logs/watched_words/action/censor/download" + expect(response.status).to eq(200) + censor_words = response.body.split("\n") + expect(censor_words).to eq([censor_word_1.word]) + end + end + end + + context '#clear_all' do + context 'non admins' do + it "doesn't allow them to perform #clear_all" do + word = Fabricate(:watched_word, action: WatchedWord.actions[:block]) + delete "/admin/logs/watched_words/action/block" + expect(response.status).to eq(404) + expect(WatchedWord.pluck(:word)).to include(word.word) + end + end + + context 'admins' do + before do + sign_in(admin) + end + + it "allows them to perform #clear_all" do + word = Fabricate(:watched_word, action: WatchedWord.actions[:block]) + delete "/admin/logs/watched_words/action/block.json" + expect(response.status).to eq(200) + expect(WatchedWord.pluck(:word)).not_to include(word.word) + end + + it "doesn't delete words of multiple actions in one call" do + block_word = Fabricate(:watched_word, action: WatchedWord.actions[:block]) + flag_word = Fabricate(:watched_word, action: WatchedWord.actions[:flag]) + + delete "/admin/logs/watched_words/action/flag.json" + expect(response.status).to eq(200) + all_words = WatchedWord.pluck(:word) + expect(all_words).to include(block_word.word) + expect(all_words).not_to include(flag_word.word) + end + end + end end diff --git a/spec/services/word_watcher_spec.rb b/spec/services/word_watcher_spec.rb index d7d83caa484..89d854ae1a5 100644 --- a/spec/services/word_watcher_spec.rb +++ b/spec/services/word_watcher_spec.rb @@ -10,6 +10,25 @@ describe WordWatcher do $redis.flushall end + describe '.word_matcher_regexp' do + let!(:word1) { Fabricate(:watched_word, action: WatchedWord.actions[:block]).word } + let!(:word2) { Fabricate(:watched_word, action: WatchedWord.actions[:block]).word } + + 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) + 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) + expect(regexp.inspect).to eq("/(?