From 406018e7ebbeb538b47ef4c0ed614f855f82925f Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Thu, 27 Mar 2025 14:54:14 +1100
Subject: [PATCH] Simplify resetting variants

I was hoping to reduce the query count but it stayed the same. In fact,
I'm expecting the query count to be higher with this version. The
DfcCatalogImporter queries all variants and links at once while the OC
job is not pre-loading the variants at the moment. We can optimise the
job though.

If we kept the old version and there were multiple catalogs per variant
then we would call the importer with the reset multiple times. The job
is iterating through each link only once though. So depending on the
ratio of catalogs to variants, I'm not sure which version would be more
efficient.

No version is properly dealing with the edge-case of multiple catalogs
per variant anyway. Imagine there are two catalogs with different stock
levels. Which one do we choose? Or do we add up?
And if the variant disappears from one catalog we still want to sell it
through the other. But depending on the order of processing, we may
reset the variant if it's missing in the last catalog. But let's worry
about that when it actually happens. Maybe it will be better to restrict
variants to one catalog.
---
 app/jobs/open_order_cycle_job.rb     | 10 +++++-----
 app/services/dfc_catalog_importer.rb | 14 +++++++++-----
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/app/jobs/open_order_cycle_job.rb b/app/jobs/open_order_cycle_job.rb
index b11660ad94..c2f7dc9469 100644
--- a/app/jobs/open_order_cycle_job.rb
+++ b/app/jobs/open_order_cycle_job.rb
@@ -57,14 +57,14 @@ class OpenOrderCycleJob < ApplicationJob
       catalog = DfcCatalog.load(dfc_user, catalog_url)
       catalog.apply_wholesale_values!
 
-      variants = Spree::Variant.where(id: catalog_links.pluck(:subject_id))
-      importer = DfcCatalogImporter.new(variants, catalog)
-      importer.reset_absent_variants
-
       catalog_links.each do |link|
         catalog_item = catalog.item(link.semantic_id)
 
-        SuppliedProductImporter.update_product(catalog_item, link.subject) if catalog_item
+        if catalog_item
+          SuppliedProductImporter.update_product(catalog_item, link.subject)
+        else
+          DfcCatalogImporter.reset_variant(link.subject)
+        end
       end
     end
   end
diff --git a/app/services/dfc_catalog_importer.rb b/app/services/dfc_catalog_importer.rb
index ab5837caee..16d81ac6d3 100644
--- a/app/services/dfc_catalog_importer.rb
+++ b/app/services/dfc_catalog_importer.rb
@@ -1,6 +1,14 @@
 # frozen_string_literal: true
 
 class DfcCatalogImporter
+  def self.reset_variant(variant)
+    if variant.on_demand
+      variant.on_demand = false
+    else
+      variant.on_hand = 0
+    end
+  end
+
   attr_reader :catalog, :existing_variants
 
   def initialize(existing_variants, catalog)
@@ -22,11 +30,7 @@ class DfcCatalogImporter
   # we don't want to lose the connection to previous orders.
   def reset_absent_variants
     absent_variants.map do |variant|
-      if variant.on_demand
-        variant.on_demand = false
-      else
-        variant.on_hand = 0
-      end
+      self.class.reset_variant(variant)
     end
   end