Better error messages when hitting max mentions/images/links

app/models/post.rb

@@ -114,17 +114,19 @@ class Post < ActiveRecord::Base
   end
 
   def max_mention_validator
-    errors.add(:raw, I18n.t(:too_many_mentions)) if raw_mentions.size > SiteSetting.max_mentions_per_post
+    max_mentions = SiteSetting.visitor_max_mentions_per_post
+    max_mentions = SiteSetting.max_mentions_per_post if user.present? && user.has_trust_level?(:basic)
+    errors.add(:base, I18n.t(:too_many_mentions, count: max_mentions)) if raw_mentions.size > max_mentions
   end
 
   def max_images_validator
     return if user.present? && user.has_trust_level?(:basic)
-    errors.add(:raw, I18n.t(:too_many_images)) if image_count > 0
+    errors.add(:base, I18n.t(:too_many_images, count: SiteSetting.visitor_max_images)) if image_count > SiteSetting.visitor_max_images
   end
 
   def max_links_validator
     return if user.present? && user.has_trust_level?(:basic)
-    errors.add(:raw, I18n.t(:too_many_links)) if link_count > 1
+    errors.add(:base, I18n.t(:too_many_links, count: SiteSetting.visitor_max_links)) if link_count > SiteSetting.visitor_max_links
   end
 
 

app/models/site_setting.rb

@@ -74,6 +74,7 @@ class SiteSetting < ActiveRecord::Base
   setting(:post_undo_action_window_mins, 10)
   setting(:system_username, '')
   setting(:max_mentions_per_post, 5)
+  setting(:visitor_max_mentions_per_post, 2)
 
   setting(:uncategorized_name, 'uncategorized')
 
@@ -155,6 +156,8 @@ class SiteSetting < ActiveRecord::Base
   setting(:max_word_length, 30)
 
   setting(:new_user_period_days, 2)
+  setting(:visitor_max_links, 2)
+  setting(:visitor_max_images, 0)
 
   setting(:title_fancy_entities, true)
 

config/locales/server.en.yml

@@ -11,9 +11,21 @@ en:
 
   via: "%{username} via %{site_name}"
   is_reserved: "is reserved"
-  too_many_mentions: "has too many users mentioned"
-  too_many_images: "has too many images"
-  too_many_links: "has too many links"
+
+  too_many_mentions:
+    zero: "Sorry, you can't mention any users at your trust level."
+    one: "Sorry, you can only mention one user at your trust level."
+    other: "Sorry, you can only mention %{count} users at your trust level."
+
+  too_many_images:
+    zero: "Sorry, you can't put any images in a post at your trust level."
+    one: "Sorry, we only allow one image in a post at your trust level."
+    other: "Sorry, we only allow %{count} images in a post at your trust level."
+  too_many_links:
+    zero: "Sorry, you can't put any links in a post at your trust level."
+    one: "Sorry, we only allow one link in a post at your trust level."
+    other: "Sorry, we only allow %{count} links in a post at your trust level."
+
   just_posted_that: "is too similar to what you recently posted"
   has_already_been_used: "has already been used"
   invalid_characters: "contains invalid characters"
@@ -391,7 +403,6 @@ en:
     previous_visit_timeout_hours: "How long a visit lasts before we consider it the 'previous' visit, in hours"
 
     uncategorized_name: "The default category for topics that have no category in the /categories page"
-    max_mentions_per_post: "Maximum number of @name notifications you can use in a single post"
 
     rate_limit_create_topic: "How many seconds, after creating a topic, before you can create another topic"
     rate_limit_create_post: "How many seconds, after creating a post, before you can create another post"
@@ -415,6 +426,10 @@ en:
     basic_requires_topics_entered: "How many a topics a new user must enter before promotion to basic (1) trust level"
     basic_requires_read_posts: "How many posts a new user must read before promotion to basic (1) trust level"
     basic_requires_time_spent_mins: "How many minutes a new user must read posts before promotion to basic (1) trust level"
+    visitor_max_links: "How many links a visitor can add to a post"
+    visitor_max_images: "How many images a visitor can add to a post"
+    visitor_max_mentions_per_post: "Maximum number of @name notifications a visitor can use in a post"
+    max_mentions_per_post: "Maximum number of @name notifications you can use in a post"
 
     auto_link_images_wider_than: "Images wider than this, in pixels, will get auto link and lightbox treatment"
 
@@ -424,6 +439,7 @@ en:
     title_min_entropy: "The minimum allowed entropy (unique characters) required for a topic title"
     body_min_entropy: "The minimum allowed entropy (unique characters) required for a post body"
     new_user_period_days: "How long a user is highlighted as being new, in days"
+
     title_fancy_entities: "Convert fancy HTML entities in topic titles"
 
     min_title_similar_length: "The minimum length of a title before it will be checked for similar topics"

spec/models/post_spec.rb

@@ -83,11 +83,12 @@ describe Post do
   end
 
   describe "maximum images" do
-    let(:post_no_images) { Fabricate.build(:post, post_args) }
-    let(:post_one_image) { Fabricate.build(:post, post_args.merge(raw: "![sherlock](http://bbc.co.uk/sherlock.jpg)")) }
-    let(:post_two_images) { Fabricate.build(:post, post_args.merge(raw: "<img src='http://discourse.org/logo.png'> <img src='http://bbc.co.uk/sherlock.jpg'>")) }
-    let(:post_with_avatars) { Fabricate.build(:post, post_args.merge(raw: '<img alt="smiley" title=":smiley:" src="/assets/emoji/smiley.png" class="avatar"> <img alt="wink" title=":wink:" src="/assets/emoji/wink.png" class="avatar">')) }
-    let(:post_with_two_classy_images) { Fabricate.build(:post, post_args.merge(raw: "<img src='http://discourse.org/logo.png' class='classy'> <img src='http://bbc.co.uk/sherlock.jpg' class='classy'>")) }
+    let(:visitor) { Fabricate(:user, trust_level: TrustLevel.levels[:visitor]) }
+    let(:post_no_images) { Fabricate.build(:post, post_args.merge(user: visitor)) }
+    let(:post_one_image) { Fabricate.build(:post, post_args.merge(raw: "![sherlock](http://bbc.co.uk/sherlock.jpg)", user: visitor)) }
+    let(:post_two_images) { Fabricate.build(:post, post_args.merge(raw: "<img src='http://discourse.org/logo.png'> <img src='http://bbc.co.uk/sherlock.jpg'>", user: visitor)) }
+    let(:post_with_avatars) { Fabricate.build(:post, post_args.merge(raw: '<img alt="smiley" title=":smiley:" src="/assets/emoji/smiley.png" class="avatar"> <img alt="wink" title=":wink:" src="/assets/emoji/wink.png" class="avatar">', user: visitor)) }
+    let(:post_with_two_classy_images) { Fabricate.build(:post, post_args.merge(raw: "<img src='http://discourse.org/logo.png' class='classy'> <img src='http://bbc.co.uk/sherlock.jpg' class='classy'>", user: visitor)) }
 
     it "returns 0 images for an empty post" do
       Fabricate.build(:post).image_count.should == 0
@@ -111,29 +112,33 @@ describe Post do
     end
 
     context "validation" do
-      it "allows a new user to make a post with one image" do
-        post_no_images.user.trust_level = TrustLevel.levels[:new]
-        post_no_images.should be_valid
+
+      before do
+        SiteSetting.stubs(:visitor_max_images).returns(1)
       end
 
-      it "doesn't allow multiple images for new accounts" do
-        post_one_image.user.trust_level = TrustLevel.levels[:new]
-        post_one_image.should_not be_valid
-      end
-
-      it "allows multiple images for basic accounts" do
-        post_one_image.user.trust_level = TrustLevel.levels[:basic]
+      context 'visitor' do
+        it "allows a visitor to post below the limit" do
           post_one_image.should be_valid
         end
 
-      it "doesn't allow a new user to edit their post to insert an image" do
+        it "doesn't allow more than the maximum" do
+          post_two_images.should_not be_valid
+        end
+
+        it "doesn't allow a visitor to edit their post to insert an image" do
           post_no_images.user.trust_level = TrustLevel.levels[:new]
           post_no_images.save
           -> {
             post_no_images.revise(post_no_images.user, post_two_images.raw)
             post_no_images.reload
           }.should_not change(post_no_images, :raw)
+        end
+      end
 
+      it "allows more images from a non-visitor account" do
+        post_two_images.user.trust_level = TrustLevel.levels[:basic]
+        post_two_images.should be_valid
       end
 
     end
@@ -141,8 +146,9 @@ describe Post do
   end
 
   describe "maximum links" do
-    let(:post_one_link) { Fabricate.build(:post, post_args.merge(raw: "[sherlock](http://www.bbc.co.uk/programmes/b018ttws)")) }
-    let(:post_two_links) { Fabricate.build(:post, post_args.merge(raw: "<a href='http://discourse.org'>discourse</a> <a href='http://twitter.com'>twitter</a>")) }
+    let(:visitor) { Fabricate(:user, trust_level: TrustLevel.levels[:visitor]) }
+    let(:post_one_link) { Fabricate.build(:post, post_args.merge(raw: "[sherlock](http://www.bbc.co.uk/programmes/b018ttws)", user: visitor)) }
+    let(:post_two_links) { Fabricate.build(:post, post_args.merge(raw: "<a href='http://discourse.org'>discourse</a> <a href='http://twitter.com'>twitter</a>", user: visitor)) }
 
     it "returns 0 images for an empty post" do
       Fabricate.build(:post).link_count.should == 0
@@ -157,32 +163,32 @@ describe Post do
     end
 
     context "validation" do
-      it "allows a new user to make a post with one image" do
-        post_one_link.user.trust_level = TrustLevel.levels[:new]
+
+      before do
+        SiteSetting.stubs(:visitor_max_links).returns(1)
+      end
+
+      context 'visitor' do
+        it "returns true when within the amount of links allowed" do
           post_one_link.should be_valid
         end
 
-      it "doesn't allow multiple images for new accounts" do
-        post_two_links.user.trust_level = TrustLevel.levels[:new]
+        it "doesn't allow more links than allowed" do
           post_two_links.should_not be_valid
         end
+      end
 
       it "allows multiple images for basic accounts" do
         post_two_links.user.trust_level = TrustLevel.levels[:basic]
         post_two_links.should be_valid
       end
+
     end
 
   end
 
 
-  describe "maximum @mentions" do
-
-    let(:post) { Fabricate.build(:post, post_args.merge(raw: "@Jake @Finn")) }
-
-    it "will accept a post with 2 @mentions as valid" do
-      post.should be_valid
-    end
+  describe "@mentions" do
 
     context 'raw_mentions' do
 
@@ -223,14 +229,45 @@ describe Post do
 
     end
 
-    context "With a @mention limit of 1" do
+    context "max mentions" do
+
+      let(:visitor) { Fabricate(:user, trust_level: TrustLevel.levels[:visitor]) }
+      let(:post_with_one_mention) { Fabricate.build(:post, post_args.merge(raw: "@Jake is the person I'm mentioning", user: visitor)) }
+      let(:post_with_two_mentions) { Fabricate.build(:post, post_args.merge(raw: "@Jake @Finn are the people I'm mentioning", user: visitor)) }
+
+      context 'visitor' do
         before do
+          SiteSetting.stubs(:visitor_max_mentions_per_post).returns(1)
+          SiteSetting.stubs(:max_mentions_per_post).returns(5)
+        end
+
+        it "allows a visitor to have visitor_max_mentions_per_post mentions" do
+          post_with_one_mention.should be_valid
+        end
+
+        it "doesn't allow a visitor to have more than visitor_max_mentions_per_post mentions" do
+          post_with_two_mentions.should_not be_valid
+        end
+      end
+
+      context "non-visitor" do
+        before do
+          SiteSetting.stubs(:visitor_max_mentions_per_post).returns(0)
           SiteSetting.stubs(:max_mentions_per_post).returns(1)
         end
 
-      it "wont accept the post as valid because there are too many mentions" do
-        post.should_not be_valid
+        it "allows vmax_mentions_per_post mentions" do
+          post_with_one_mention.user.trust_level = TrustLevel.levels[:basic]
+          post_with_one_mention.should be_valid
         end
+
+        it "doesn't allow to have more than max_mentions_per_post mentions" do
+          post_with_two_mentions.user.trust_level = TrustLevel.levels[:basic]
+          post_with_two_mentions.should_not be_valid
+        end
+      end
+
+
     end
 
   end