From 487c20959c874666db726d69c00abe139ba1aeab Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 20 Jun 2016 16:38:15 -0400 Subject: [PATCH] FEATURE: max topics/replies per day for new users now starts counting from the first post, not signup date --- app/models/post.rb | 2 +- app/models/topic.rb | 2 +- app/models/user.rb | 4 +-- spec/models/topic_spec.rb | 63 +++++++++++++++++++++++++-------------- spec/models/user_spec.rb | 20 ++++++++++--- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index eaa3c7c2554..1946d4fa71f 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -109,7 +109,7 @@ class Post < ActiveRecord::Base end def limit_posts_per_day - if user && user.first_day_user? && post_number && post_number > 1 + if user && user.new_user_posting_on_first_day? && post_number && post_number > 1 RateLimiter.new(user, "first-day-replies-per-day", SiteSetting.max_replies_in_first_day, 1.day.to_i) end end diff --git a/app/models/topic.rb b/app/models/topic.rb index 5cd53f9673e..ade61d0ccba 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -268,7 +268,7 @@ class Topic < ActiveRecord::Base # Additional rate limits on topics: per day and private messages per day def limit_topics_per_day - if user && user.first_day_user? + if user && user.new_user_posting_on_first_day? limit_first_day_topics_per_day else apply_per_day_rate_limit_for("topics", :max_topics_per_day) diff --git a/app/models/user.rb b/app/models/user.rb index c44a6fd3a5d..2a9900cb989 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -410,10 +410,10 @@ class User < ActiveRecord::Base self.password_hash == hash_password(password, salt) end - def first_day_user? + def new_user_posting_on_first_day? !staff? && trust_level < TrustLevel[2] && - created_at >= 24.hours.ago + (self.first_post_created_at.nil? || self.first_post_created_at >= 24.hours.ago) end def new_user? diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 093475d4982..36b6376b47e 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1499,34 +1499,53 @@ describe Topic do end end - it "limits new users to max_topics_in_first_day and max_posts_in_first_day" do - SiteSetting.stubs(:max_topics_in_first_day).returns(1) - SiteSetting.stubs(:max_replies_in_first_day).returns(1) - SiteSetting.stubs(:client_settings_json).returns(SiteSetting.client_settings_json_uncached) - RateLimiter.stubs(:rate_limit_create_topic).returns(100) - RateLimiter.stubs(:disabled?).returns(false) - RateLimiter.clear_all! + context "new user limits" do + before do + SiteSetting.max_topics_in_first_day = 1 + SiteSetting.max_replies_in_first_day = 1 + SiteSetting.stubs(:client_settings_json).returns(SiteSetting.client_settings_json_uncached) + RateLimiter.stubs(:rate_limit_create_topic).returns(100) + RateLimiter.stubs(:disabled?).returns(false) + RateLimiter.clear_all! + end - start = Time.now.tomorrow.beginning_of_day + it "limits new users to max_topics_in_first_day and max_posts_in_first_day" do + start = Time.now.tomorrow.beginning_of_day - freeze_time(start) + freeze_time(start) - user = Fabricate(:user) - topic_id = create_post(user: user).topic_id + user = Fabricate(:user) + topic_id = create_post(user: user).topic_id - freeze_time(start + 10.minutes) - expect { - create_post(user: user) - }.to raise_error(RateLimiter::LimitExceeded) + freeze_time(start + 10.minutes) + expect { create_post(user: user) }.to raise_error(RateLimiter::LimitExceeded) - freeze_time(start + 20.minutes) - create_post(user: user, topic_id: topic_id) - - freeze_time(start + 30.minutes) - - expect { + freeze_time(start + 20.minutes) create_post(user: user, topic_id: topic_id) - }.to raise_error(RateLimiter::LimitExceeded) + + freeze_time(start + 30.minutes) + expect { create_post(user: user, topic_id: topic_id) }.to raise_error(RateLimiter::LimitExceeded) + end + + it "starts counting when they make their first post/topic" do + start = Time.now.tomorrow.beginning_of_day + + freeze_time(start) + + user = Fabricate(:user) + + freeze_time(start + 25.hours) + topic_id = create_post(user: user).topic_id + + freeze_time(start + 26.hours) + expect { create_post(user: user) }.to raise_error(RateLimiter::LimitExceeded) + + freeze_time(start + 27.hours) + create_post(user: user, topic_id: topic_id) + + freeze_time(start + 28.hours) + expect { create_post(user: user, topic_id: topic_id) }.to raise_error(RateLimiter::LimitExceeded) + end end describe ".count_exceeds_minimun?" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e08759b6d97..2c5b0aea224 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -806,17 +806,29 @@ describe User do end - describe "#first_day_user?" do + describe "#new_user_posting_on_first_day?" do def test_user?(opts={}) - Fabricate.build(:user, {created_at: Time.now}.merge(opts)).first_day_user? + Fabricate.build(:user, {created_at: Time.zone.now}.merge(opts)).new_user_posting_on_first_day? end - it "works" do + it "handles when user has never posted" do expect(test_user?).to eq(true) expect(test_user?(moderator: true)).to eq(false) expect(test_user?(trust_level: TrustLevel[2])).to eq(false) - expect(test_user?(created_at: 2.days.ago)).to eq(false) + expect(test_user?(created_at: 2.days.ago)).to eq(true) + end + + it "is true for a user who posted less than 24 hours ago but was created over 1 day ago" do + u = Fabricate(:user, created_at: 28.hours.ago) + u.user_stat.first_post_created_at = 1.hour.ago + expect(u.new_user_posting_on_first_day?).to eq(true) + end + + it "is false if first post was more than 24 hours ago" do + u = Fabricate(:user, created_at: 28.hours.ago) + u.user_stat.first_post_created_at = 25.hours.ago + expect(u.new_user_posting_on_first_day?).to eq(false) end end