From 87de5d9e8f3149d32c3f7f81ee7c97510e516a81 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 10 Apr 2019 17:42:49 -0400 Subject: [PATCH] Record the reason that a post was put into the queue --- lib/new_post_manager.rb | 102 ++++++++++++++--------- spec/components/new_post_manager_spec.rb | 24 ++++-- spec/integration/watched_words_spec.rb | 1 + spec/requests/posts_controller_spec.rb | 5 +- 4 files changed, 83 insertions(+), 49 deletions(-) diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index b5de841f9ba..d0eca260147 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -73,16 +73,31 @@ class NewPostManager def self.post_needs_approval?(manager) user = manager.user - return false if exempt_user?(user) + return :skip if exempt_user?(user) - (user.trust_level <= TrustLevel.levels[:basic] && user.post_count < SiteSetting.approve_post_count) || - (user.trust_level < SiteSetting.approve_unless_trust_level.to_i) || - (manager.args[:title].present? && user.trust_level < SiteSetting.approve_new_topics_unless_trust_level.to_i) || - is_fast_typer?(manager) || - matches_auto_silence_regex?(manager) || - WordWatcher.new("#{manager.args[:title]} #{manager.args[:raw]}").requires_approval? || - (SiteSetting.approve_unless_staged && user.staged) || - post_needs_approval_in_its_category?(manager) + return :post_count if ( + user.trust_level <= TrustLevel.levels[:basic] && + user.post_count < SiteSetting.approve_post_count + ) + + return :trust_level if user.trust_level < SiteSetting.approve_unless_trust_level.to_i + + return :new_topics_unless_trust_level if ( + manager.args[:title].present? && + user.trust_level < SiteSetting.approve_new_topics_unless_trust_level.to_i + ) + + return :fast_typer if is_fast_typer?(manager) + + return :auto_silence_regex if matches_auto_silence_regex?(manager) + + return :watched_word if WordWatcher.new("#{manager.args[:title]} #{manager.args[:raw]}").requires_approval? + + return :staged if SiteSetting.approve_unless_staged? && user.staged? + + return :category if post_needs_approval_in_its_category?(manager) + + :skip end def self.post_needs_approval_in_its_category?(manager) @@ -98,44 +113,46 @@ class NewPostManager end def self.default_handler(manager) - if post_needs_approval?(manager) - validator = Validators::PostValidator.new - post = Post.new(raw: manager.args[:raw]) - post.user = manager.user - validator.validate(post) - if post.errors[:raw].present? + reason = post_needs_approval?(manager) + return if reason == :skip + + validator = Validators::PostValidator.new + post = Post.new(raw: manager.args[:raw]) + post.user = manager.user + validator.validate(post) + + if post.errors[:raw].present? + result = NewPostResult.new(:created_post, false) + result.errors[:base] << post.errors[:raw] + return result + elsif manager.args[:topic_id] + topic = Topic.unscoped.where(id: manager.args[:topic_id]).first + + unless manager.user.guardian.can_create_post_on_topic?(topic) result = NewPostResult.new(:created_post, false) - result.errors[:base] << post.errors[:raw] + result.errors[:base] << I18n.t(:topic_not_found) return result - elsif manager.args[:topic_id] - topic = Topic.unscoped.where(id: manager.args[:topic_id]).first - - unless manager.user.guardian.can_create_post_on_topic?(topic) - result = NewPostResult.new(:created_post, false) - result.errors[:base] << I18n.t(:topic_not_found) - return result - end - elsif manager.args[:category] - category = Category.find_by(id: manager.args[:category]) - - unless manager.user.guardian.can_create_topic_on_category?(category) - result = NewPostResult.new(:created_post, false) - result.errors[:base] << I18n.t("js.errors.reasons.forbidden") - return result - end end + elsif manager.args[:category] + category = Category.find_by(id: manager.args[:category]) - result = manager.enqueue - - if is_fast_typer?(manager) - UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.new_user_typed_too_fast")) - elsif matches_auto_silence_regex?(manager) - UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.content_matches_auto_silence_regex")) + unless manager.user.guardian.can_create_topic_on_category?(category) + result = NewPostResult.new(:created_post, false) + result.errors[:base] << I18n.t("js.errors.reasons.forbidden") + return result end - - result end + + result = manager.enqueue(reason) + + if is_fast_typer?(manager) + UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.new_user_typed_too_fast")) + elsif matches_auto_silence_regex?(manager) + UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.content_matches_auto_silence_regex")) + end + + result end def self.queue_enabled? @@ -181,9 +198,12 @@ class NewPostManager def enqueue(reason = nil) result = NewPostResult.new(:enqueued) + payload = { raw: @args[:raw], tags: @args[:tags] } + payload[:reason] = reason.to_s if reason + reviewable = ReviewableQueuedPost.new( created_by: @user, - payload: { raw: @args[:raw], tags: @args[:tags] }, + payload: payload, topic_id: @args[:topic_id], reviewable_by_moderator: true ) diff --git a/spec/components/new_post_manager_spec.rb b/spec/components/new_post_manager_spec.rb index b00def4e333..837c81d0a9e 100644 --- a/spec/components/new_post_manager_spec.rb +++ b/spec/components/new_post_manager_spec.rb @@ -78,6 +78,7 @@ describe NewPostManager do result = NewPostManager.default_handler(manager) expect(NewPostManager.queue_enabled?).to eq(true) expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:post_count) end end @@ -90,6 +91,7 @@ describe NewPostManager do result = NewPostManager.default_handler(manager) expect(NewPostManager.queue_enabled?).to eq(true) expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:post_count) end end @@ -109,7 +111,7 @@ describe NewPostManager do SiteSetting.approve_post_count = 100 user = Fabricate(:user) category_group = Fabricate(:category_group, permission_type: 2) - group_user = Fabricate(:group_user, group: category_group.group, user_id: user.id) + Fabricate(:group_user, group: category_group.group, user_id: user.id) manager = NewPostManager.new( user, @@ -130,6 +132,7 @@ describe NewPostManager do result = NewPostManager.default_handler(manager) expect(NewPostManager.queue_enabled?).to eq(true) expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:trust_level) end end @@ -163,6 +166,7 @@ describe NewPostManager do result = NewPostManager.default_handler(manager) expect(NewPostManager.queue_enabled?).to eq(true) expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:staged) end end @@ -188,6 +192,7 @@ describe NewPostManager do result = NewPostManager.default_handler(manager) expect(NewPostManager.queue_enabled?).to eq(true) expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:new_topics_unless_trust_level) end end @@ -325,19 +330,19 @@ describe NewPostManager do it "handles post_needs_approval? correctly" do u = user default = NewPostManager.new(u, {}) - expect(NewPostManager.post_needs_approval?(default)).to eq(false) + expect(NewPostManager.post_needs_approval?(default)).to eq(:skip) with_check = NewPostManager.new(u, first_post_checks: true) - expect(NewPostManager.post_needs_approval?(with_check)).to eq(true) + expect(NewPostManager.post_needs_approval?(with_check)).to eq(:fast_typer) u.user_stat.post_count = 1 with_check_and_post = NewPostManager.new(u, first_post_checks: true) - expect(NewPostManager.post_needs_approval?(with_check_and_post)).to eq(false) + expect(NewPostManager.post_needs_approval?(with_check_and_post)).to eq(:skip) u.user_stat.post_count = 0 u.trust_level = 1 with_check_tl1 = NewPostManager.new(u, first_post_checks: true) - expect(NewPostManager.post_needs_approval?(with_check_tl1)).to eq(false) + expect(NewPostManager.post_needs_approval?(with_check_tl1)).to eq(:skip) end end @@ -359,7 +364,9 @@ describe NewPostManager do category: category.id ) - expect(manager.perform.action).to eq(:enqueued) + result = manager.perform + expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:category) end end @@ -373,7 +380,10 @@ describe NewPostManager do it 'enqueues new posts' do manager = NewPostManager.new(user, raw: 'this is a new post', topic_id: topic.id) - expect(manager.perform.action).to eq(:enqueued) + + result = manager.perform + expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:category) end it "doesn't blow up with invalid topic_id" do diff --git a/spec/integration/watched_words_spec.rb b/spec/integration/watched_words_spec.rb index f144f5533d5..45bbdb21e2e 100644 --- a/spec/integration/watched_words_spec.rb +++ b/spec/integration/watched_words_spec.rb @@ -75,6 +75,7 @@ describe WatchedWord do manager = NewPostManager.new(tl2_user, raw: "My dog's name is #{require_approval_word.word}.", topic_id: topic.id) result = manager.perform expect(result.action).to eq(:enqueued) + expect(result.reason).to eq(:watched_word) end it "looks at title too" do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index f993926b966..17b2b71a317 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -800,7 +800,8 @@ describe PostsController do user.reload expect(user).to be_silenced - rp = ReviewableQueuedPost.first + rp = ReviewableQueuedPost.find_by(created_by: user) + expect(rp.payload['reason']).to eq('fast_typer') mod = Fabricate(:moderator) rp.perform(mod, :approve) @@ -849,6 +850,8 @@ describe PostsController do parsed = ::JSON.parse(response.body) expect(parsed["action"]).to eq("enqueued") + reviewable = ReviewableQueuedPost.find_by(created_by: user) + expect(reviewable.payload['reason']).to eq('auto_silence_regex') user.reload expect(user).to be_silenced