From 050bc8e619e2a8b8c15dbe69bc0fcc5325d6e322 Mon Sep 17 00:00:00 2001 From: Ka Hin Ng Date: Thu, 16 Apr 2020 09:41:08 -0400 Subject: [PATCH] Fix/Slug Validation in unset_slug_if_invalid Signed-off-by: Rob Szumlakowski --- lib/friendly_id/slugged.rb | 2 +- test/slugged_test.rb | 96 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/lib/friendly_id/slugged.rb b/lib/friendly_id/slugged.rb index 6e0e13542..d7f0699cf 100644 --- a/lib/friendly_id/slugged.rb +++ b/lib/friendly_id/slugged.rb @@ -375,7 +375,7 @@ def slug_generator private :slug_generator def unset_slug_if_invalid - if errors.present? && attribute_changed?(friendly_id_config.query_field.to_s) + if errors[friendly_id_config.query_field].present? && attribute_changed?(friendly_id_config.query_field.to_s) diff = changes[friendly_id_config.query_field] send "#{friendly_id_config.slug_column}=", diff.first end diff --git a/test/slugged_test.rb b/test/slugged_test.rb index 388864119..07b6b0274 100644 --- a/test/slugged_test.rb +++ b/test/slugged_test.rb @@ -92,7 +92,7 @@ def should_generate_new_friendly_id? end end - test "should not set slug on create if unrelated validations fail" do + test "should set slug on create if unrelated validations fail" do klass = Class.new model_class do validates_presence_of :active friendly_id :name, :use => :slugged @@ -106,11 +106,30 @@ def self.name instance = klass.new :name => 'foo' refute instance.save refute instance.valid? + assert_equal 'foo', instance.slug + end + end + + test "should not set slug on create if slug validation fails" do + klass = Class.new model_class do + validates_presence_of :active + validates_length_of :slug, :minimum => 2 + friendly_id :name, :use => :slugged + + def self.name + "Journalist" + end + end + + transaction do + instance = klass.new :name => 'x' + refute instance.save + refute instance.valid? assert_nil instance.slug end end - test "should not set slug on create if unrelated validations fail with custom slug_column" do + test "should set slug on create if unrelated validations fail with custom slug_column" do klass = Class.new(ActiveRecord::Base) do self.table_name = 'authors' extend FriendlyId @@ -126,11 +145,31 @@ def self.name instance = klass.new :name => 'foo' refute instance.save refute instance.valid? + assert_equal 'foo', instance.subdomain + end + end + + test "should not set slug on create if custom slug column validations fail" do + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'authors' + extend FriendlyId + validates_length_of :subdomain, :minimum => 2 + friendly_id :name, :use => :slugged, :slug_column => :subdomain + + def self.name + "Author" + end + end + + transaction do + instance = klass.new :name => 'x' + refute instance.save + refute instance.valid? assert_nil instance.subdomain end end - test "should not update slug on save if unrelated validations fail" do + test "should keep new slug on save if unrelated validations fail" do klass = Class.new model_class do validates_presence_of :active friendly_id :name, :use => :slugged @@ -149,6 +188,28 @@ def self.name instance.active = nil refute instance.save refute instance.valid? + assert_equal 'foobar', instance.slug + end + end + + test "should not update slug on save if slug validations fail" do + klass = Class.new model_class do + validates_length_of :slug, :minimum => 2 + friendly_id :name, :use => :slugged + + def self.name + "Journalist" + end + end + + transaction do + instance = klass.new :name => 'foo', :active => true + assert instance.save + assert instance.valid? + instance.name = 'x' + instance.slug = nil + instance.active = nil + refute instance.save assert_equal 'foo', instance.slug end end @@ -425,6 +486,7 @@ class ToParamTest < TestCaseClass class Journalist < ActiveRecord::Base extend FriendlyId validates_presence_of :active + validates_length_of :slug, :minimum => 2 friendly_id :name, :use => :slugged attr_accessor :to_param_in_callback @@ -438,9 +500,21 @@ class Journalist < ActiveRecord::Base assert_nil Journalist.new.to_param end - test "to_param should return nil if record failed validation" do + test "to_param should return original slug if record failed validation" do journalist = Journalist.new :name => 'Clark Kent', :active => nil refute journalist.save + assert_equal 'clark-kent', journalist.to_param + end + + test "to_param should clear slug attributes if slug attribute fails validation" do + journalist = Journalist.new :name => 'x', :active => true + refute journalist.save + assert_nil journalist.to_param + end + + test "to_param should clear slug attribute if slug attribute fails validation and unrelated validation fails" do + journalist = Journalist.new :name => 'x', :active => nil + refute journalist.save assert_nil journalist.to_param end @@ -452,7 +526,7 @@ class Journalist < ActiveRecord::Base end end - test "to_param should use original slug if existing record changes but fails to save" do + test "to_param should use new slug if existing record changes but fails to save" do transaction do journalist = Journalist.new :name => 'Clark Kent', :active => true assert journalist.save @@ -460,6 +534,18 @@ class Journalist < ActiveRecord::Base journalist.slug = nil journalist.active = nil refute journalist.save + assert_equal 'superman', journalist.to_param + end + end + + test "to_param should use original slug if new slug attribute is not valid" do + transaction do + journalist = Journalist.new :name => 'Clark Kent', :active => true + assert journalist.save + journalist.name = 'x' + journalist.slug = nil + journalist.active = nil + refute journalist.save assert_equal 'clark-kent', journalist.to_param end end