From 5e01a93fc09685e1ca1d838aebba9ae9df6d3da0 Mon Sep 17 00:00:00 2001
From: Benjamin Fleischer <github@benjaminfleischer.com>
Date: Sun, 30 Apr 2017 15:09:18 -0500
Subject: [PATCH] Update comments regarding lazy_association and TODOs

---
 lib/active_model/serializer/concerns/caching.rb     |  1 +
 lib/active_model/serializer/lazy_association.rb     | 13 -------------
 lib/active_model_serializers/adapter/json_api.rb    |  1 +
 .../adapter/json_api/relationship.rb                |  3 +++
 4 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/lib/active_model/serializer/concerns/caching.rb b/lib/active_model/serializer/concerns/caching.rb
index f633447a..1de86246 100644
--- a/lib/active_model/serializer/concerns/caching.rb
+++ b/lib/active_model/serializer/concerns/caching.rb
@@ -193,6 +193,7 @@ module ActiveModel
             cache_keys << object_cache_key(serializer, adapter_instance)
 
             serializer.associations(include_directive).each do |association|
+              # TODO(BF): Process relationship without evaluating lazy_association
               association_serializer = association.lazy_association.serializer
               if association_serializer.respond_to?(:each)
                 association_serializer.each do |sub_serializer|
diff --git a/lib/active_model/serializer/lazy_association.rb b/lib/active_model/serializer/lazy_association.rb
index 1ba40f1b..8c4dad61 100644
--- a/lib/active_model/serializer/lazy_association.rb
+++ b/lib/active_model/serializer/lazy_association.rb
@@ -40,17 +40,6 @@ module ActiveModel
         cached_result[:virtual_value] || reflection_options[:virtual_value]
       end
 
-      # NOTE(BF): Kurko writes:
-      # 1. This class is doing a lot more than it should. It has business logic (key/meta/links) and
-      #   it also looks like a factory (serializer/serialize_object/instantiate_serializer/serializer_class).
-      #   It's hard to maintain classes that you can understand what it's really meant to be doing,
-      #   so it ends up having all sorts of methods.
-      #   Perhaps we could replace all these methods with a class called... Serializer.
-      #   See how association is doing the job a serializer again?
-      # 2. I've seen code like this in many other places.
-      #   Perhaps we should just have it all in one place: Serializer.
-      #   We already have a class called Serializer, I know,
-      #   and that is doing things that are not responsibility of a serializer.
       def serializer_class
         return @serializer_class if defined?(@serializer_class)
         serializer_for_options = { namespace: namespace }
@@ -82,8 +71,6 @@ module ActiveModel
         end
       end
 
-      # NOTE(BF): This serializer throw/catch should only happen when the serializer is a collection
-      # serializer.  This is a good reason for the reflection to have a to_many? type method.
       def instantiate_serializer(object)
         serializer_options = association_options.fetch(:parent_serializer_options).except(:serializer)
         serializer_options[:serializer_context_class] = association_options.fetch(:parent_serializer).class
diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb
index 0e44ddd2..c252deaf 100644
--- a/lib/active_model_serializers/adapter/json_api.rb
+++ b/lib/active_model_serializers/adapter/json_api.rb
@@ -257,6 +257,7 @@ module ActiveModelSerializers
 
       def process_relationships(serializer, include_slice)
         serializer.associations(include_slice).each do |association|
+          # TODO(BF): Process relationship without evaluating lazy_association
           process_relationship(association.lazy_association.serializer, include_slice[association.key])
         end
       end
diff --git a/lib/active_model_serializers/adapter/json_api/relationship.rb b/lib/active_model_serializers/adapter/json_api/relationship.rb
index 44c74878..fc2f116f 100644
--- a/lib/active_model_serializers/adapter/json_api/relationship.rb
+++ b/lib/active_model_serializers/adapter/json_api/relationship.rb
@@ -33,6 +33,7 @@ module ActiveModelSerializers
 
         private
 
+        # TODO(BF): Avoid db hit on belong_to_ releationship by using foreign_key on self
         def data_for(association)
           if association.collection?
             data_for_many(association)
@@ -42,6 +43,7 @@ module ActiveModelSerializers
         end
 
         def data_for_one(association)
+          # TODO(BF): Process relationship without evaluating lazy_association
           serializer = association.lazy_association.serializer
           if (virtual_value = association.virtual_value)
             virtual_value
@@ -53,6 +55,7 @@ module ActiveModelSerializers
         end
 
         def data_for_many(association)
+          # TODO(BF): Process relationship without evaluating lazy_association
           collection_serializer = association.lazy_association.serializer
           if collection_serializer.respond_to?(:each)
             collection_serializer.map do |serializer|