Refactor association methods + fix issue with #find_by_id

lib/associations/associations.rb

@@ -1,8 +1,19 @@
 module ActiveHash
   module Associations
+    def fetch_associations(target_model:, method: :find_by, filter:, conditions: nil)
+      base = apply_scope(model: target_model, conditions: conditions)
+      base.send(method, filter)
+    end
 
-    module ActiveRecordExtensions
+    def apply_scope(model:, conditions: nil)
+      if conditions && model.respond_to?(:scoped)
+        model.scoped(conditions: conditions)
+      else
+        model
+      end
+    end
 
+    module ActiveRecordExtensions
       def belongs_to(*args)
         our_args = args.dup
         options = our_args.extract_options!
@@ -34,7 +45,7 @@ def belongs_to_active_hash(association_id, options = {})
         options[:shortcuts] = [options[:shortcuts]] unless options[:shortcuts].kind_of?(Array)
 
         define_method(association_id) do
-          options[:class_name].constantize.send("find_by_#{options[:primary_key]}", send(options[:foreign_key]))
+          options[:class_name].constantize.send(:find_by, options[:primary_key] => send(options[:foreign_key]))
         end
 
         define_method("#{association_id}=") do |new_value|
@@ -92,65 +103,68 @@ def self.included(base)
     end
 
     module Methods
-      def has_many(association_id, options = {})
+      def association_metadata(type, association_name, options = {})
+        association_names = association_name.to_s
+        association_class = (options[:class_name] || association_names.classify).constantize
+        {
+          target_model: association_class,
+          primary_key: normalize_primary_key(type, association_class),
+          foreign_key: normalize_foreign_key(type, association_name, options)
+        }.merge(options)
+      end
 
-        define_method(association_id) do
-          options = {
-            :class_name => association_id.to_s.classify,
-            :foreign_key => self.class.to_s.foreign_key,
-            :primary_key => self.class.primary_key
-          }.merge(options)
-
-          klass = options[:class_name].constantize
-          primary_key_value = send(options[:primary_key])
-          foreign_key = options[:foreign_key].to_sym
-
-          if Object.const_defined?(:ActiveRecord) && ActiveRecord.const_defined?(:Relation) && klass < ActiveRecord::Relation
-            klass.where(foreign_key => primary_key_value)
-          elsif klass.respond_to?(:scoped)
-            klass.scoped(:conditions => {foreign_key => primary_key_value})
-          else
-            klass.where(foreign_key => primary_key_value)
-          end
-        end
+      def normalize_primary_key(type, association_class)
+        klass = type == :belongs_to ? association_class : name.constantize
+        klass.primary_key.to_sym
       end
 
-      def has_one(association_id, options = {})
-        define_method(association_id) do
-          options = {
-            :class_name => association_id.to_s.classify,
-            :foreign_key => self.class.to_s.foreign_key
-          }.merge(options)
+      def normalize_foreign_key(type, association_name, options)
+        return options[:foreign_key].to_sym if options[:foreign_key]
+        class_name = type == :belongs_to ? (options[:class_name] || association_name) : name
+        class_name.to_s.foreign_key.to_sym
+      end
 
-          scope = options[:class_name].constantize
+      def has_many(association_name, options = {})
+        meta = association_metadata(:has_many, association_name, options)
 
-          if scope.respond_to?(:scoped) && options[:conditions]
-            scope = scope.scoped(:conditions => options[:conditions])
-          end
-          scope.send("find_by_#{options[:foreign_key]}", id)
+        define_method(association_name) do
+          args = meta
+            .slice(:target_model, :conditions)
+            .merge(
+              method: :where,
+              filter: { meta[:foreign_key] => public_send(meta[:primary_key]) }
+            )
+          fetch_associations(args)
         end
       end
 
-      def belongs_to(association_id, options = {})
+      def has_one(association_name, options = {})
+        meta = association_metadata(:has_one, association_name , options)
 
-        options = {
-          :class_name => association_id.to_s.classify,
-          :foreign_key => association_id.to_s.foreign_key,
-          :primary_key => "id"
-        }.merge(options)
+        define_method(association_name) do
+          args = meta
+            .slice(:target_model, :conditions)
+            .merge(filter: { meta[:foreign_key] => public_send(meta[:primary_key]) })
+          fetch_associations(args)
+        end
+      end
 
-        field options[:foreign_key].to_sym
+      def belongs_to(association_name, options = {})
+        meta = association_metadata(:belongs_to, association_name, options)
 
-        define_method(association_id) do
-          options[:class_name].constantize.send("find_by_#{options[:primary_key]}", send(options[:foreign_key]))
-        end
+        field meta[:foreign_key].to_sym
 
-        define_method("#{association_id}=") do |new_value|
-          attributes[options[:foreign_key].to_sym] = new_value ? new_value.send(options[:primary_key]) : nil
+        define_method(association_name) do
+          args = meta
+            .slice(:target_model)
+            .merge(filter: { meta[:primary_key] => public_send(meta[:foreign_key]) })
+          fetch_associations(args)
         end
 
+        define_method("#{association_name}=") do |new_value|
+          attributes[meta[:foreign_key].to_sym] = new_value ? new_value.send(meta[:primary_key]) : nil
+        end
       end
     end
-
   end
 end

spec/associations/active_record_extensions_spec.rb

@@ -71,12 +71,12 @@ class Book < ActiveRecord::Base
             Author.has_many :books
           end
 
-          it "find the correct records" do
+          it "finds the correct records" do
             author = Author.create :id => 1
             author.books.should == [@book_1, @book_2]
           end
 
-          it "return a scope so that we can apply further scopes" do
+          it "returns a scope so that we can apply further scopes" do
             author = Author.create :id => 1
             author.books.published.should == [@book_1]
           end
@@ -96,7 +96,7 @@ class Book < ActiveRecord::Base
             author.books.should == [@book_2, @book_3]
           end
 
-          it "return a scope so that we can apply further scopes" do
+          it "returns a scope so that we can apply further scopes" do
             author = Author.create :id => 1, :book_identifier => 2
             author.books.published.should == [@book_3]
           end
@@ -115,26 +115,23 @@ class Book < ActiveRecord::Base
             author.books.should == [@book_1, @book_2]
           end
 
-          it "return a scope so that we can apply further scopes" do
+          it "returns a scope so that we can apply further scopes" do
             author = Author.create :id => 1
             author.books.published.should == [@book_1]
           end
         end
 
         it "only uses 1 query" do
           Author.has_many :books
-          author = Author.create :id => 1
-          Book.should_receive(:find_by_sql)
+          author = Author.create!(id: 1)
+          Book.should_receive(:where)
           author.books.to_a
         end
       end
-
     end
 
     describe ActiveHash::Associations::ActiveRecordExtensions do
-
       describe "#belongs_to" do
-
         if ActiveRecord::VERSION::MAJOR > 3
           it "doesn't interfere with AR's procs in belongs_to methods" do
             School.belongs_to :country, lambda { where() }
@@ -218,7 +215,7 @@ class Book < ActiveRecord::Base
             school.city_name.should == 'gothan'
           end
 
-          it "have custom shortcut" do
+          it "has custom shortcut" do
             School.belongs_to_active_hash :city, :shortcuts => :friendly_name
             City.data = [{:id => 1, :friendly_name => 'Gothan City'}]
             city = City.find_by_friendly_name 'Gothan City'
@@ -258,9 +255,8 @@ class Book < ActiveRecord::Base
     end
 
     describe "#belongs_to" do
-
       context "with an ActiveRecord parent" do
-        it "find the correct records" do
+        it "finds the correct records" do
           City.belongs_to :country
           country = Country.create
           city = City.create :country_id => country.id
@@ -282,7 +278,7 @@ class Book < ActiveRecord::Base
           Author.has_one :book
         end
 
-        it "find the correct records" do
+        it "finds the correct records" do
           book = Book.create! :author_id => 1, :published => true
           author = Author.create :id => 1
           author.book.should == book

spec/associations/associations_spec.rb

@@ -32,7 +32,7 @@ class SchoolStatus < ActiveHash::Base
           @excluded_author = Author.create :city_id => 2
         end
 
-        it "find the correct records" do
+        it "finds the correct records" do
           City.has_many :authors
           city = City.create :id => 1
           city.authors.should == [@included_author_1, @included_author_2]