Skip to content

Commit

Permalink
Ensure static attributes are executed before dynamic ones.
Browse files Browse the repository at this point in the history
Static attributes must be executed first because dynamic attributes might
rely on them. This is really important when using the :parent relationship.

Previous code didn't work fine in situations like this one:
  Factory.define(:generic_user, :class => User) do |u|
    u.email { |user| "#{user.name}@example.com }
  end

  Factory.define(:flavio, :parent => :generic_user) do |u|
    u.name "flavio"
  end

When building a :user object the previous code would have set the email
attribute and then the name attribute. This results in a user object with
an invalid email address: the 'name' attribute is yet not set while the
'email' attribute is evaluated.

Closes #159
  • Loading branch information
Flavio Castelli authored and joshuaclayton committed Aug 5, 2011
1 parent 841e012 commit 28f541e
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 7 deletions.
11 changes: 11 additions & 0 deletions lib/factory_girl/attribute.rb
Expand Up @@ -8,6 +8,7 @@ class AttributeDefinitionError < RuntimeError
end

class Attribute #:nodoc:
include Comparable

attr_reader :name

Expand All @@ -28,6 +29,16 @@ def add_to(proxy)
def association?
false
end

def priority
1
end

def <=>(another)
return nil unless another.is_a? Attribute
self.priority <=> another.priority
end

end

end
5 changes: 4 additions & 1 deletion lib/factory_girl/attribute/static.rb
Expand Up @@ -11,7 +11,10 @@ def initialize(name, value)
def add_to(proxy)
proxy.set(name, @value)
end
end

def priority
0
end
end
end
end
4 changes: 3 additions & 1 deletion lib/factory_girl/factory.rb
Expand Up @@ -49,7 +49,9 @@ def inherit_from(parent) #:nodoc:
new_attributes << attribute.clone
end
end
@attributes.unshift *new_attributes

@attributes += new_attributes
@attributes.sort!
end

def define_attribute(attribute)
Expand Down
12 changes: 7 additions & 5 deletions spec/acceptance/parent_spec.rb
Expand Up @@ -8,11 +8,11 @@
FactoryGirl.define do
factory :user do
name "John"
email "john@example.com"
email { "#{name.downcase}@example.com" }

factory :admin do
name "admin"
admin true
email "admin@example.com"
end

factory :guest do
Expand All @@ -28,16 +28,18 @@
its(:email) { should == "john@example.com" }
end

describe "the child class" do
describe "the child class redefining parent's static method used by a dynamic method" do
subject { FactoryGirl.create(:admin) }
it { should be_kind_of(User) }
it { should be_admin }
its(:name) { should == "John" }
its(:name) { should == "admin" }
its(:email) { should == "admin@example.com" }
end

describe "the child class with parent attribute" do
describe "the child class redefining parent's dynamic method" do
subject { FactoryGirl.create(:guest) }
it { should_not be_admin }
its(:name) { should == "John" }
its(:email) { should eql "John-guest@example.com" }
end
end
Expand Down
9 changes: 9 additions & 0 deletions spec/factory_girl/attribute_spec.rb
Expand Up @@ -31,4 +31,13 @@
it "should convert names to symbols" do
FactoryGirl::Attribute.new('name').name.should == :name
end

it "should return nil when is compared with a non-attribute object" do
(@attr <=> "foo").should == nil
end

it "should use priority to perform comparisons" do
attr2 = FactoryGirl::Attribute.new('name')
(@attr <=> attr2).should == 0
end
end

0 comments on commit 28f541e

Please sign in to comment.