From d962340651935283838858ccd885e41128bce3da Mon Sep 17 00:00:00 2001
From: CodeMeister <CodeMeister@codemeister.dev>
Date: Mon, 28 Oct 2024 12:26:25 +0000
Subject: [PATCH] Fixes '<attribute>' and '<attribute>_id' conflict.

  - AttributeAssigner refactored to ignore a
    matching attribute alias, if it also
    matches the name of another attribute.

  - Tests added for attribute assignment.
---
 lib/factory_bot/attribute_assigner.rb         | 31 +++++--
 spec/factory_bot/attribute_assignment_spec.rb | 86 +++++++++++++++++++
 2 files changed, 110 insertions(+), 7 deletions(-)
 create mode 100644 spec/factory_bot/attribute_assignment_spec.rb

diff --git a/lib/factory_bot/attribute_assigner.rb b/lib/factory_bot/attribute_assigner.rb
index 51ab7236..08a5e1c1 100644
--- a/lib/factory_bot/attribute_assigner.rb
+++ b/lib/factory_bot/attribute_assigner.rb
@@ -72,7 +72,7 @@ def attribute_names_to_assign
         non_ignored_attribute_names +
         override_names -
         ignored_attribute_names -
-        alias_names_to_ignore
+        aliased_attribute_names_to_ignore
     end
 
     def non_ignored_attribute_names
@@ -91,22 +91,39 @@ def override_names
       @evaluator.__override_names__
     end
 
+    def attribute_names
+      @attribute_list.names
+    end
+
     def hash_instance_methods_to_respond_to
       @attribute_list.names + override_names + @build_class.instance_methods
     end
 
-    def alias_names_to_ignore
+    ##
+    # Creat a list of attribute names that will be
+    # overridden by an alias, so any defaults can
+    # ignored.
+    #
+    def aliased_attribute_names_to_ignore
       @attribute_list.non_ignored.flat_map { |attribute|
         override_names.map do |override|
-          attribute.name if ignorable_alias?(attribute, override)
+          attribute.name if aliased_attribute?(attribute, override)
         end
       }.compact
     end
 
-    def ignorable_alias?(attribute, override)
-      attribute.alias_for?(override) &&
-        attribute.name != override &&
-        !ignored_attribute_names.include?(override)
+    ##
+    # Is the override an alias for the attribute and not the
+    # actual name of another attribute?
+    #
+    # Note: Checking against the names of all attributes, resolves any
+    #       issues with having both <attribute> and <attribute>_id
+    #       in the same factory.
+    #
+    def aliased_attribute?(attribute, override)
+      return false if attribute_names.include?(override)
+
+      attribute.alias_for?(override)
     end
   end
 end
diff --git a/spec/factory_bot/attribute_assignment_spec.rb b/spec/factory_bot/attribute_assignment_spec.rb
new file mode 100644
index 00000000..b900ffdf
--- /dev/null
+++ b/spec/factory_bot/attribute_assignment_spec.rb
@@ -0,0 +1,86 @@
+describe "Attribute Assignment" do
+  it "sets the <attribute> default value if not overriden" do
+    name = :user
+    define_class("User") { attr_accessor :name, :response }
+    factory = FactoryBot::Factory.new(name)
+    FactoryBot::Internal.register_factory(factory)
+
+    attr_1 = FactoryBot::Declaration::Dynamic.new(:name, false, -> { "orig name" })
+    attr_2 = FactoryBot::Declaration::Dynamic.new(:response, false, -> { "orig response" })
+    factory.declare_attribute(attr_1)
+    factory.declare_attribute(attr_2)
+
+    user = factory.run(FactoryBot::Strategy::Build, {})
+
+    expect(user.name).to eq "orig name"
+    expect(user.response).to eq "orig response"
+  end
+
+  it "set the <attribute> when directly named" do
+    name = :user
+    define_class("User") { attr_accessor :name, :response }
+    factory = FactoryBot::Factory.new(name)
+    FactoryBot::Internal.register_factory(factory)
+
+    attr_1 = FactoryBot::Declaration::Dynamic.new(:name, false, -> { "orig name" })
+    attr_2 = FactoryBot::Declaration::Dynamic.new(:response, false, -> { "orig response" })
+    factory.declare_attribute(attr_1)
+    factory.declare_attribute(attr_2)
+
+    user = factory.run(
+      FactoryBot::Strategy::Build,
+      name: "new name",
+      response: "new response"
+    )
+
+    expect(user.name).to eq "new name"
+    expect(user.response).to eq "new response"
+  end
+
+  it "Does not set a default if the attribute's alias is used" do
+    name = :user
+    define_class("User") { attr_accessor :name, :response_id }
+    factory = FactoryBot::Factory.new(name)
+    FactoryBot::Internal.register_factory(factory)
+
+    attr_1 = FactoryBot::Declaration::Dynamic.new(:name, false, -> { "orig name" })
+    attr_2 = FactoryBot::Declaration::Dynamic.new(:response_id, false, -> { 42 })
+    factory.declare_attribute(attr_1)
+    factory.declare_attribute(attr_2)
+
+    user = factory.run(
+      FactoryBot::Strategy::AttributesFor,
+      name: "new name",
+      response: 13.75
+    )
+
+    expect(user[:name]).to eq "new name"
+    expect(user[:response]).to eq 13.75
+    expect(user[:response_id]).to be_nil
+  end
+
+  it "sets both <attribute> and <attribute>_id correctly" do
+    name = :user
+    define_class("User") { attr_accessor :name, :response, :response_id }
+    factory = FactoryBot::Factory.new(name)
+    FactoryBot::Internal.register_factory(factory)
+
+    attr_1 = FactoryBot::Declaration::Dynamic.new(:name, false, -> { "orig name" })
+    attr_2 = FactoryBot::Declaration::Dynamic.new(:response, false, -> { "orig response" })
+    attr_3 = FactoryBot::Declaration::Dynamic.new(:response_id, false, -> { 42 })
+    factory.declare_attribute(attr_1)
+    factory.declare_attribute(attr_2)
+    factory.declare_attribute(attr_3)
+
+    user = factory.run(
+      FactoryBot::Strategy::Build,
+      name: "new name",
+      response: "new response",
+      response_id: 13.75
+    )
+
+    expect(user.name).to eq "new name"
+    expect(user.response).to eq "new response"
+    expect(user.response_id).to eq 13.75
+  end
+end