Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

make :scoped and :history work together

do not complain when :scoped and :history are used together

keep track of the modules in use in Configuration.

This adds a uses? method which will return whether or not a given module is
used, e.g.

friendly_id_config.uses?(:scoped)

remove note about history module's incompatibility with the scope module

add back the index for the non-scoped case

handle scopes in the find_one and exists? overrides

when using :scoped friendly_ids aren't unique anymore, so we can't just use the
first match we find. Instead we now consider all ids in "with_old_friendly_id"
and use according conditions in "find_one" and "exists?" which will return the
proper object when used on the relation as adviced in the :scoped module
documentation, e.g.

@city.restaurants.find("joes-diner")
  • Loading branch information...
commit 190ff219a4b9d672aeefeb70ba50368b30627aba 1 parent 8cd81ef
@aduffeck aduffeck authored committed
View
2  Guide.rdoc
@@ -296,8 +296,6 @@ model.
=== Considerations
-This module is incompatible with the +:scoped+ module.
-
Because recording slug history requires creating additional database records,
this module has an impact on the performance of the associated model's +create+
method.
View
10 lib/friendly_id/configuration.rb
@@ -30,6 +30,9 @@ class Configuration
# The default configuration options.
attr_reader :defaults
+ # The modules in use
+ attr_reader :modules
+
# The model class that this configuration belongs to.
# @return ActiveRecord::Base
attr_accessor :model_class
@@ -37,6 +40,7 @@ class Configuration
def initialize(model_class, values = nil)
@model_class = model_class
@defaults = {}
+ @modules = []
set values
end
@@ -59,9 +63,15 @@ def use(*modules)
modules.to_a.flatten.compact.map do |object|
mod = object.kind_of?(Module) ? object : FriendlyId.const_get(object.to_s.classify)
model_class.send(:include, mod)
+ @modules << mod.name.split("::").last.downcase.to_sym if mod.name.present?
end
end
+ # Returns whether the given module is in use
+ def uses?(modul)
+ @modules.include?(modul)
+ end
+
# The column that FriendlyId will use to find the record when querying by
# friendly id.
#
View
14 lib/friendly_id/history.rb
@@ -59,7 +59,6 @@ module History
# Configures the model instance to use the History add-on.
def self.included(model_class)
model_class.instance_eval do
- raise "FriendlyId::History is incompatible with FriendlyId::Scoped" if self < Scoped
@friendly_id_config.use :slugged
has_many :slugs, :as => :sluggable, :dependent => :destroy,
:class_name => Slug.to_s, :order => "#{Slug.quoted_table_name}.id DESC"
@@ -80,6 +79,7 @@ def create_slug
relation.delete_all unless result.empty?
slugs.create! do |record|
record.slug = friendly_id
+ record.scope = serialized_scope if friendly_id_config.uses?(:scoped)
end
end
@@ -90,7 +90,7 @@ module FinderMethods
def find_one(id)
return super(id) if id.unfriendly_id?
where(@klass.friendly_id_config.query_field => id).first or
- with_old_friendly_id(id) {|x| find_one_without_friendly_id(x)} or
+ with_old_friendly_id(id) {|x| where(:id => x).first} or
find_one_without_friendly_id(id)
end
@@ -98,7 +98,7 @@ def find_one(id)
def exists?(id = false)
return super if id.unfriendly_id?
exists_without_friendly_id?(@klass.friendly_id_config.query_field => id) or
- with_old_friendly_id(id) {|x| exists_without_friendly_id?(x)} or
+ with_old_friendly_id(id) {|x| exists_without_friendly_id?(:id => x)} or
exists_without_friendly_id?(id)
end
@@ -108,8 +108,8 @@ def exists?(id = false)
def with_old_friendly_id(slug, &block)
sql = "SELECT sluggable_id FROM #{Slug.quoted_table_name} WHERE sluggable_type = %s AND slug = %s"
sql = sql % [@klass.base_class.to_s, slug].map {|x| connection.quote(x)}
- sluggable_id = connection.select_values(sql).first
- yield sluggable_id if sluggable_id
+ sluggable_ids = connection.select_values(sql)
+ yield sluggable_ids if sluggable_ids
end
end
@@ -127,7 +127,9 @@ def conflicts
scope = Slug.where("slug = ? OR slug LIKE ?", normalized, wildcard)
scope = scope.where(:sluggable_type => sluggable_class.to_s)
scope = scope.where("sluggable_id <> ?", value) unless sluggable.new_record?
-
+ if sluggable.friendly_id_config.uses?(:scoped)
+ scope = scope.where("scope = ?", sluggable.serialized_scope)
+ end
length_command = "LENGTH"
length_command = "LEN" if sluggable.connection.adapter_name =~ /sqlserver/i
scope.order("#{length_command}(slug) DESC, slug DESC")
View
4 lib/friendly_id/migration.rb
@@ -5,10 +5,12 @@ def self.up
t.string :slug, :null => false
t.integer :sluggable_id, :null => false
t.string :sluggable_type, :limit => 40
+ t.string :scope
t.datetime :created_at
end
add_index :friendly_id_slugs, :sluggable_id
- add_index :friendly_id_slugs, [:slug, :sluggable_type], :unique => true
+ add_index :friendly_id_slugs, [:slug, :sluggable_type]
+ add_index :friendly_id_slugs, [:slug, :sluggable_type, :scope], :unique => true
add_index :friendly_id_slugs, :sluggable_type
end
View
21 lib/friendly_id/scoped.rb
@@ -108,13 +108,16 @@ module Scoped
# feature.
def self.included(model_class)
model_class.instance_eval do
- raise "FriendlyId::Scoped is incompatibe with FriendlyId::History" if self < History
include Slugged unless self < Slugged
friendly_id_config.class.send :include, Configuration
friendly_id_config.slug_generator_class.send :include, SlugGenerator
end
end
+ def serialized_scope
+ friendly_id_config.scope_columns.sort.map { |column| "#{column}:#{send(column)}" }.join(",")
+ end
+
# This module adds the +:scope+ configuration option to
# {FriendlyId::Configuration FriendlyId::Configuration}.
module Configuration
@@ -159,12 +162,18 @@ module SlugGenerator
private
def conflict
- columns = friendly_id_config.scope_columns
- matched = columns.inject(conflicts) do |memo, column|
- memo.where(column => sluggable.send(column))
+ if friendly_id_config.uses?(:history)
+ # When using the :history module +conflicts+ already returns only real conflicts, so there's no need to check
+ # for the scope columns again
+ conflicts.first
+ else
+ columns = friendly_id_config.scope_columns
+ matched = columns.inject(conflicts) do |memo, column|
+ memo.where(column => sluggable.send(column))
+ end
+
+ matched.first
end
-
- matched.first
end
end
end
View
75 test/history_test.rb
@@ -78,17 +78,6 @@ def model_class
end
end
-
- test "should raise error if used with scoped" do
- model_class = Class.new(ActiveRecord::Base) do
- self.abstract_class = true
- extend FriendlyId
- end
- assert_raises RuntimeError do
- model_class.friendly_id :name, :use => [:history, :scoped]
- end
- end
-
test "should handle renames" do
with_instance_of(model_class) do |record|
record.name = 'x'
@@ -146,4 +135,68 @@ class Editorialist < Journalist
def model_class
Editorialist
end
+end
+
+
+
+class City < ActiveRecord::Base
+ has_many :restaurants
+end
+
+class Restaurant < ActiveRecord::Base
+ extend FriendlyId
+ belongs_to :city
+ friendly_id :name, :use => [:history, :scoped], :scope => :city
+end
+
+class ScopedHistoryTest < MiniTest::Unit::TestCase
+ include FriendlyId::Test
+ include FriendlyId::Test::Shared::Core
+
+ def model_class
+ Restaurant
+ end
+
+ test "should find old scoped slugs" do
+ city = City.create!
+ with_instance_of(Restaurant) do |record|
+ record.city = city
+
+ record.name = "x"
+ record.save!
+
+ record.name = "y"
+ record.save!
+
+ assert_equal city.restaurants.find("x"), city.restaurants.find("y")
+ end
+ end
+
+ test "should consider old scoped slugs when creating slugs" do
+ city = City.create!
+ with_instance_of(Restaurant) do |record|
+ record.city = city
+
+ record.name = "x"
+ record.save!
+
+ record.name = "y"
+ record.save!
+
+ second_record = model_class.create! :city => city, :name => 'x'
+ assert_equal "x--2", second_record.friendly_id
+
+ third_record = model_class.create! :city => city, :name => 'y'
+ assert_equal "y--2", third_record.friendly_id
+ end
+ end
+
+ test "should allow equal slugs in different scopes" do
+ city = City.create!
+ second_city = City.create!
+ record = model_class.create! :city => city, :name => 'x'
+ second_record = model_class.create! :city => second_city, :name => 'x'
+
+ assert_equal record.slug, second_record.slug
+ end
end
View
15 test/schema.rb
@@ -34,6 +34,10 @@ def up
add_index table_name, :slug, :unique => true
end
+ scoped_tables.each do |table_name|
+ add_column table_name, :slug, :string
+ end
+
# This will be used to test scopes
add_column :novels, :novelist_id, :integer
add_column :novels, :publisher_id, :integer
@@ -57,6 +61,9 @@ def up
# This will be used to test relationships
add_column :books, :author_id, :integer
+ # Used to test :scoped and :history together
+ add_column :restaurants, :city_id, :integer
+
@done = true
end
@@ -66,12 +73,16 @@ def slugged_tables
%w[journalists articles novelists novels manuals translated_articles]
end
+ def scoped_tables
+ ["restaurants"]
+ end
+
def simple_tables
- %w[authors books publishers]
+ %w[authors books publishers cities]
end
def tables
- simple_tables + slugged_tables
+ simple_tables + slugged_tables + scoped_tables
end
end
end
View
11 test/scoped_test.rb
@@ -54,17 +54,6 @@ def model_class
end
end
- test "should raise error if used with history" do
- model_class = Class.new(ActiveRecord::Base) do
- self.abstract_class = true
- extend FriendlyId
- end
-
- assert_raises RuntimeError do
- model_class.friendly_id :name, :use => [:scoped, :history]
- end
- end
-
test "should apply scope with multiple columns" do
transaction do
novelist = Novelist.create! :name => "a"
Please sign in to comment.
Something went wrong with that request. Please try again.