diff --git a/app/controllers/pools_controller.rb b/app/controllers/pools_controller.rb index 3b43ed819..d02b6f1aa 100644 --- a/app/controllers/pools_controller.rb +++ b/app/controllers/pools_controller.rb @@ -36,7 +36,7 @@ class PoolsController < ApplicationController def create @pool = Pool.create(params[:pool]) - flash[:notice] = "Pool created" + flash[:notice] = @pool.valid? ? "Pool created" : @pool.errors.full_messages.join("; ") respond_with(@pool) end diff --git a/app/models/pool.rb b/app/models/pool.rb index 42a84c5b2..47d201552 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -3,11 +3,10 @@ require 'ostruct' class Pool < ApplicationRecord class RevertError < Exception ; end - validates_uniqueness_of :name, :case_sensitive => false - validates_format_of :name, :with => /\A[^,]+\Z/, :message => "cannot have commas" + validates_uniqueness_of :name, :case_sensitive => false, :if => :name_changed? + validate :validate_name, :if => :name_changed? validates_inclusion_of :category, :in => %w(series collection) validate :updater_can_change_category - validate :name_does_not_conflict_with_metatags validate :updater_can_remove_posts belongs_to :creator, :class_name => "User" belongs_to :updater, :class_name => "User" @@ -15,7 +14,6 @@ class Pool < ApplicationRecord before_validation :normalize_name before_validation :initialize_is_active, :on => :create before_validation :initialize_creator, :on => :create - before_validation :strip_name after_save :update_category_pseudo_tags_for_posts_async after_save :create_version after_create :synchronize! @@ -119,7 +117,7 @@ class Pool < ApplicationRecord end def self.normalize_name(name) - name.gsub(/\s+/, "_") + name.gsub(/[_[:space:]]+/, "_").gsub(/\A_|_\z/, "") end def self.normalize_name_for_search(name) @@ -138,7 +136,7 @@ class Pool < ApplicationRecord if name =~ /^\d+$/ where("id = ?", name.to_i).first elsif name - where("lower(name) = ?", normalize_name(name).mb_chars.downcase).first + where("lower(name) = ?", normalize_name_for_search(name)).first else nil end @@ -356,10 +354,6 @@ class Pool < ApplicationRecord super + [:creator_name] end - def strip_name - self.name = name.to_s.strip - end - def update_category_pseudo_tags_for_posts_async if category_changed? delay(:queue => "default").update_category_pseudo_tags_for_posts @@ -387,12 +381,14 @@ class Pool < ApplicationRecord end end - def name_does_not_conflict_with_metatags - if %w(any none series collection).include?(name.downcase.tr(" ", "_")) - errors[:base] << "Pools cannot have the following names: any, none, series, collection" - false - else - true + def validate_name + case name + when /\A(any|none|series|collection)\z/i + errors[:name] << "cannot be any of the following names: any, none, series, collection" + when /,/ + errors[:name] << "cannot contain commas" + when "" + errors[:name] << "cannot be blank" end end diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index 35651a5ba..09abb1623 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -249,7 +249,10 @@ class PoolTest < ActiveSupport::TestCase end should "normalize its name" do - @pool.update_attributes(:name => "A B") + @pool.update(:name => " A B ") + assert_equal("A_B", @pool.name) + + @pool.update(:name => "__A__B__") assert_equal("A_B", @pool.name) end @@ -257,6 +260,16 @@ class PoolTest < ActiveSupport::TestCase @pool.update_attributes(:post_ids => " 1 2 ") assert_equal("1 2", @pool.post_ids) end + + context "when validating names" do + should_not allow_value("foo,bar").for(:name) + should_not allow_value("___").for(:name) + should_not allow_value(" ").for(:name) + + %w[any none series collection].each do |type| + should_not allow_value(type).for(:name) + end + end end context "An existing pool" do