From 6568b4aaa9087068334833d4496e7ef2fcf0549f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 25 Mar 2013 12:24:46 -0400 Subject: [PATCH] Better error messages when hitting max mentions/images/links --- app/models/post.rb | 8 ++- app/models/site_setting.rb | 3 + config/locales/server.en.yml | 24 +++++-- spec/models/post_spec.rb | 125 +++++++++++++++++++++++------------ 4 files changed, 109 insertions(+), 51 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 4ee640960a5..a340590b8f2 100644 --- a/app/models/post.rb +++ b/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 diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 3d9125f734c..6d40d81571a 100644 --- a/app/models/site_setting.rb +++ b/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) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index be07c379540..c5d90fa3025 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 7aba5b9fa6a..efa6edf4171 100644 --- a/spec/models/post_spec.rb +++ b/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: " ")) } - let(:post_with_avatars) { Fabricate.build(:post, post_args.merge(raw: 'smiley wink')) } - let(:post_with_two_classy_images) { Fabricate.build(:post, post_args.merge(raw: " ")) } + 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: " ", user: visitor)) } + let(:post_with_avatars) { Fabricate.build(:post, post_args.merge(raw: 'smiley wink', user: visitor)) } + let(:post_with_two_classy_images) { Fabricate.build(:post, post_args.merge(raw: " ", 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 + context 'visitor' do + it "allows a visitor to post below the limit" do + post_one_image.should be_valid + end + + 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 multiple images for basic accounts" do - post_one_image.user.trust_level = TrustLevel.levels[:basic] - post_one_image.should be_valid - end - - it "doesn't allow a new user 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) - + 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: "discourse twitter")) } + 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: "discourse twitter", 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] - post_one_link.should be_valid + + before do + SiteSetting.stubs(:visitor_max_links).returns(1) end - it "doesn't allow multiple images for new accounts" do - post_two_links.user.trust_level = TrustLevel.levels[:new] - post_two_links.should_not be_valid + 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 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 - before do - SiteSetting.stubs(:max_mentions_per_post).returns(1) + 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 - it "wont accept the post as valid because there are too many mentions" do - post.should_not be_valid + 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 "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