Skip to content

Commit

Permalink
FEATURE: Add new setting to force user edit last post. (#6571)
Browse files Browse the repository at this point in the history
  • Loading branch information
nbianca authored and ZogStriP committed Nov 14, 2018
1 parent d003ae4 commit b6576d9
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 0 deletions.
5 changes: 5 additions & 0 deletions config/locales/server.en.yml
Expand Up @@ -218,6 +218,10 @@ en:
one: "We're sorry, but new users are temporarily limited to 1 reply in the same topic."
other: "We're sorry, but new users are temporarily limited to %{count} replies in the same topic."

max_consecutive_replies:
one: "No more than 1 reply is allowed. Please edit your previous reply instead, or wait for someone to reply to you."
other: "No more than %{count} consecutive replies are allowed. Please edit your previous reply instead, or wait for someone to reply to you."

embed:
start_discussion: "Start Discussion"
continue: "Continue Discussion"
Expand Down Expand Up @@ -1536,6 +1540,7 @@ en:
title_min_entropy: "The minimum entropy (unique characters, non-english count for more) required for a topic title."
body_min_entropy: "The minimum entropy (unique characters, non-english count for more) required for a post body."
allow_uppercase_posts: "Allow all caps in a topic title or a post body."
max_consecutive_replies: "Force users to edit their last post instead of replying"

title_fancy_entities: "Convert common ASCII characters to fancy HTML entities in topic titles, ala SmartyPants <a href='https://daringfireball.net/projects/smartypants/' target='_blank'>https://daringfireball.net/projects/smartypants/</a>"

Expand Down
2 changes: 2 additions & 0 deletions config/site_settings.yml
Expand Up @@ -594,6 +594,8 @@ posting:
default: false
locale_default:
ja: true
max_consecutive_replies:
default: 0
title_prettify:
default: true
locale_default:
Expand Down
27 changes: 27 additions & 0 deletions lib/validators/post_validator.rb
Expand Up @@ -17,6 +17,7 @@ def validate(record)
max_attachments_validator(record)
can_post_links_validator(record)
unique_post_validator(record)
force_edit_last_validator(record)
end

def presence(post)
Expand Down Expand Up @@ -141,6 +142,32 @@ def unique_post_validator(post)
end
end

def force_edit_last_validator(post)
return if SiteSetting.max_consecutive_replies == 0 || post.id || post.acting_user&.staff? || private_message?(post)

topic = post.topic

last_posts_count = DB.query_single(<<~SQL, topic_id: post.topic_id, user_id: post.acting_user.id, max_replies: SiteSetting.max_consecutive_replies).first
SELECT COUNT(*)
FROM (
SELECT user_id
FROM posts
WHERE deleted_at IS NULL
AND NOT hidden
AND topic_id = :topic_id
ORDER BY post_number DESC
LIMIT :max_replies
) c
WHERE c.user_id = :user_id
SQL
return if last_posts_count < SiteSetting.max_consecutive_replies

guardian = Guardian.new(post.acting_user)
if guardian.can_edit?(topic.posts.last)
post.errors.add(:base, I18n.t(:max_consecutive_replies, count: SiteSetting.max_consecutive_replies))
end
end

private

def acting_user_is_trusted?(post, level = 1)
Expand Down
38 changes: 38 additions & 0 deletions spec/components/validators/post_validator_spec.rb
Expand Up @@ -219,6 +219,43 @@
end
end

context "force_edit_last_validator" do

let(:user) { Fabricate(:user) }
let(:other_user) { Fabricate(:user) }
let(:topic) { Fabricate(:topic) }

before do
SiteSetting.max_consecutive_replies = 2
end

it "should not allow posting more than 2 consecutive replies" do
1.upto(3).each do |i|
post = Post.new(user: user, topic: topic, raw: "post number #{i}")
validator.force_edit_last_validator(post)
expect(post.errors.count).to eq(i > SiteSetting.max_consecutive_replies ? 1 : 0)
post.save
end
end

it "should always allow editing" do
post = Fabricate(:post, user: user, topic: topic)
post = Fabricate(:post, user: user, topic: topic)

revisor = PostRevisor.new(post)
revisor.revise!(post.user, raw: 'hello world123456789')
end

it "should allow posting more than 2 replies" do
3.times do
post = Fabricate(:post, user: user, topic: topic)
Fabricate(:post, user: other_user, topic: topic)
validator.force_edit_last_validator(post)
expect(post.errors.count).to eq(0)
end
end
end

shared_examples "almost no validations" do
it "skips most validations" do
validator.expects(:stripped_length).never
Expand All @@ -229,6 +266,7 @@
validator.expects(:max_attachments_validator).never
validator.expects(:newuser_links_validator).never
validator.expects(:unique_post_validator).never
validator.expects(:force_edit_last_validator).never
validator.validate(post)
end
end
Expand Down

2 comments on commit b6576d9

@SamSaffron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this, to me this validation is not really meant to be on save, it only needs to happen on create. Meaning this probably belongs in PostCreator, not in the validator.

By having it here we are introducing a validation on save that should never run on save (eg: when you edit a post) we really only care about this when you create a brand new post.

Can you follow up on this?

@ZogStriP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By having it here we are introducing a validation on save that should never run on save (eg: when you edit a post) we really only care about this when you create a brand new post.

Good point 👍 (cc @nbianca)

Please sign in to comment.