Skip to content

Commit

Permalink
added the ability not to delete historic slugs when the sluggable cla…
Browse files Browse the repository at this point in the history
…ss is destroyed

When using acts_as_paranoid on a sluggable models, we aren't actually
deleting the models when calling destroy. If we delete the slugs,
we’re losing the history. We're alsosetting up a condition where a
 unique contstraint may end up being violated
  • Loading branch information
Victor Kmita committed Jan 12, 2016
1 parent 8069fc9 commit 2ad50e1
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
6 changes: 6 additions & 0 deletions lib/friendly_id/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,15 @@ module Base
# allows an alternate configuration syntax, and conditional configuration
# logic.
#
# @option options [Symbol,Boolean] :dependent Available when using `:history`.
# Sets the value used for the slugged association's dependent option. Use
# `false` if you do not want to dependently destroy the associated slugged
# record. Defaults to `:destroy`.
#
# @yieldparam config The model class's {FriendlyId::Configuration friendly_id_config}.
def friendly_id(base = nil, options = {}, &block)
yield friendly_id_config if block_given?
friendly_id_config.dependent = options.delete :dependent
friendly_id_config.use options.delete :use
friendly_id_config.send :set, base ? options.merge(:base => base) : options
include Model
Expand Down
3 changes: 3 additions & 0 deletions lib/friendly_id/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class Configuration
# The module to use for finders
attr_accessor :finder_methods

# The value used for the slugged association's dependent option
attr_accessor :dependent

def initialize(model_class, values = nil)
@model_class = model_class
@defaults = {}
Expand Down
9 changes: 8 additions & 1 deletion lib/friendly_id/history.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,16 @@ def find_post
=end
module History

module Configuration
def dependent_value
dependent.nil? ? :destroy : dependent
end
end

def self.setup(model_class)
model_class.instance_eval do
friendly_id_config.use :slugged
friendly_id_config.class.send :include, History::Configuration
friendly_id_config.finder_methods = FriendlyId::History::FinderMethods
if friendly_id_config.uses? :finders
relation.class.send(:include, friendly_id_config.finder_methods)
Expand All @@ -72,7 +79,7 @@ def self.included(model_class)
model_class.class_eval do
has_many :slugs, -> {order("#{Slug.quoted_table_name}.id DESC")}, {
:as => :sluggable,
:dependent => :destroy,
:dependent => @friendly_id_config.dependent_value,
:class_name => Slug.to_s
}

Expand Down
34 changes: 29 additions & 5 deletions test/history_test.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
require "helper"

class Manual < ActiveRecord::Base
extend FriendlyId
friendly_id :name, :use => [:slugged, :history]
end

class HistoryTest < TestCaseClass

include FriendlyId::Test
include FriendlyId::Test::Shared::Core

class Manual < ActiveRecord::Base
extend FriendlyId
friendly_id :name, :use => [:slugged, :history]
end

def model_class
Manual
end
Expand Down Expand Up @@ -172,6 +172,30 @@ def model_class
end
end

class DependentDestroyTest < HistoryTest

include FriendlyId::Test

class Manual < ActiveRecord::Base
extend FriendlyId
friendly_id :name, :use => :history, :dependent => false
end

def model_class
Manual
end

test "should allow disabling of dependent destroy" do
transaction do
assert FriendlyId::Slug.find_by_slug('test').nil?
l = model_class.create! :name => 'test'
assert FriendlyId::Slug.find_by_slug('test').present?
l.destroy
assert FriendlyId::Slug.find_by_slug('test').present?
end
end
end

class HistoryTestWithSti < HistoryTest
class Journalist < ActiveRecord::Base
extend FriendlyId
Expand Down

0 comments on commit 2ad50e1

Please sign in to comment.