From 27a9037c890a4a3689c71cd0125ff6477c17bd07 Mon Sep 17 00:00:00 2001
From: Jordan Brough <git.j@brgh.net>
Date: Sat, 19 Dec 2015 08:14:00 -0700
Subject: [PATCH] Add comments and make the return value explicit

---
 core/app/models/spree/stock/coordinator.rb | 40 +++++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/core/app/models/spree/stock/coordinator.rb b/core/app/models/spree/stock/coordinator.rb
index f1bdae5ded1..ed6f71ff99c 100644
--- a/core/app/models/spree/stock/coordinator.rb
+++ b/core/app/models/spree/stock/coordinator.rb
@@ -65,15 +65,45 @@ def build_packages(packages = Array.new)
 
       private
 
+      # This finds the variants we're looking for in each active stock location.
+      # It returns a hash like:
+      #   {
+      #     <stock location> => <set of variant ids>,
+      #     <stock location> => <set of variant ids>,
+      #     ...,
+      #   }
+      # This is done in an awkward way for performance reasons.  It uses two
+      # queries that are kept as performant as possible, and only loads the
+      # minimum required ActiveRecord objects.
       def stock_location_variant_ids
-        location_variant_ids = StockItem.where(variant_id: unallocated_variant_ids).joins(:stock_location).merge(StockLocation.active).pluck(:stock_location_id, :variant_id)
+        # associate the variant ids we're interested in with stock location ids
+        location_variant_ids = StockItem.
+          where(variant_id: unallocated_variant_ids).
+          joins(:stock_location).
+          merge(StockLocation.active).
+          pluck(:stock_location_id, :variant_id)
 
-        location_lookup = StockLocation.where(id: location_variant_ids.map(&:first).uniq).map { |l| [l.id, l] }.to_h
+        # load activerecord objects for the stock location ids and turn them
+        # into a lookup hash like:
+        #   {
+        #     <stock location id> => <stock location>,
+        #     ...,
+        #   }
+        location_lookup = StockLocation.
+          where(id: location_variant_ids.map(&:first).uniq).
+          map { |l| [l.id, l] }.
+          to_h
 
-        location_variant_ids.each_with_object({}) do |(location_id, variant_id), hash|
-          hash[location_lookup[location_id]] ||= Set.new
-          hash[location_lookup[location_id]] << variant_id
+        # build the final lookup hash of
+        #   {<stock location> => <set of variant ids>, ...}
+        # using the previous results
+        hash = location_variant_ids.each_with_object({}) do |(location_id, variant_id), hash|
+          location = location_lookup[location_id]
+          hash[location] ||= Set.new
+          hash[location] << variant_id
         end
+
+        hash
       end
 
       def unallocated_inventory_units