From 71ca292c923ec97cd50bb88ceb424bd6f8c95238 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 8 Oct 2024 16:59:23 +1100
Subject: [PATCH] Synchronise stock with DFC catalog during checkout

This will delay the checkout request by a few seconds if there's stock
to sync. But we minimise the chance of missing reduced stock from orders
on another platform.

We still have a gap between the checkout and placing a backorder. In
that time we can't guarantee enough stock. But let's tackle that after
the pilot.
---
 .../concerns/checkout_callbacks.rb            | 14 +++++++++
 app/jobs/stock_sync_job.rb                    | 31 ++++++++++++++-----
 spec/controllers/checkout_controller_spec.rb  | 14 +++++++++
 spec/jobs/stock_sync_job_spec.rb              | 24 ++++++++++++++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb
index f703fdc790..3b0fba0b1c 100644
--- a/app/controllers/concerns/checkout_callbacks.rb
+++ b/app/controllers/concerns/checkout_callbacks.rb
@@ -9,6 +9,12 @@ module CheckoutCallbacks
     # Otherwise we fail on duplicate indexes or end up with negative stock.
     prepend_around_action CurrentOrderLocker, only: [:edit, :update]
 
+    # We want to download the latest stock data before anything else happens.
+    # We don't want it to be in the same database transaction as the order
+    # locking because this action locks a different set of variants and it
+    # could cause race conditions.
+    prepend_around_action :sync_stock, only: :update
+
     prepend_before_action :check_hub_ready_for_checkout
     prepend_before_action :check_order_cycle_expiry
     prepend_before_action :require_order_cycle
@@ -25,6 +31,14 @@ module CheckoutCallbacks
 
   private
 
+  def sync_stock
+    if current_order&.state == "confirmation"
+      StockSyncJob.sync_linked_catalogs_now(current_order)
+    end
+
+    yield
+  end
+
   def load_order
     @order = current_order
     @order.manual_shipping_selection = true
diff --git a/app/jobs/stock_sync_job.rb b/app/jobs/stock_sync_job.rb
index 04a849c3c7..14d27f54e3 100644
--- a/app/jobs/stock_sync_job.rb
+++ b/app/jobs/stock_sync_job.rb
@@ -9,15 +9,8 @@ class StockSyncJob < ApplicationJob
   # enqueue a new job. That should save some time loading the order with
   # all the stock data to make this decision.
   def self.sync_linked_catalogs(order)
-    stock_controlled_variants = order.variants.reject(&:on_demand)
-    links = SemanticLink.where(variant_id: stock_controlled_variants.map(&:id))
-    semantic_ids = links.pluck(:semantic_id)
-    catalog_ids = semantic_ids.map do |product_id|
-      FdcUrlBuilder.new(product_id).catalog_url
-    end
-
     user = order.distributor.owner
-    catalog_ids.uniq.each do |catalog_id|
+    catalog_ids(order).each do |catalog_id|
       perform_later(user, catalog_id)
     end
   rescue StandardError => e
@@ -28,6 +21,28 @@ class StockSyncJob < ApplicationJob
     end
   end
 
+  def self.sync_linked_catalogs_now(order)
+    user = order.distributor.owner
+    catalog_ids(order).each do |catalog_id|
+      perform_now(user, catalog_id)
+    end
+  rescue StandardError => e
+    # Errors here shouldn't affect the shopping. So let's report them
+    # separately:
+    Bugsnag.notify(e) do |payload|
+      payload.add_metadata(:order, order)
+    end
+  end
+
+  def self.catalog_ids(order)
+    stock_controlled_variants = order.variants.reject(&:on_demand)
+    links = SemanticLink.where(variant_id: stock_controlled_variants.map(&:id))
+    semantic_ids = links.pluck(:semantic_id)
+    semantic_ids.map do |product_id|
+      FdcUrlBuilder.new(product_id).catalog_url
+    end.uniq
+  end
+
   def perform(user, catalog_id)
     products = load_products(user, catalog_id)
     products_by_id = products.index_by(&:semanticId)
diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb
index ea761d876b..ed1da29653 100644
--- a/spec/controllers/checkout_controller_spec.rb
+++ b/spec/controllers/checkout_controller_spec.rb
@@ -451,6 +451,20 @@ RSpec.describe CheckoutController, type: :controller do
           expect(response).to redirect_to order_path(order, order_token: order.token)
           expect(order.reload.state).to eq "complete"
         end
+
+        it "syncs stock before locking the order" do
+          actions = []
+          expect(StockSyncJob).to receive(:sync_linked_catalogs_now) do
+            actions << "sync stock"
+          end
+          expect(CurrentOrderLocker).to receive(:around) do
+            actions << "lock order"
+          end
+
+          put(:update, params:)
+
+          expect(actions).to eq ["sync stock", "lock order"]
+        end
       end
 
       context "when accepting T&Cs is required" do
diff --git a/spec/jobs/stock_sync_job_spec.rb b/spec/jobs/stock_sync_job_spec.rb
index f80bc42285..8fd7029e18 100644
--- a/spec/jobs/stock_sync_job_spec.rb
+++ b/spec/jobs/stock_sync_job_spec.rb
@@ -37,6 +37,30 @@ RSpec.describe StockSyncJob do
     end
   end
 
+  describe ".sync_linked_catalogs_now" do
+    subject { StockSyncJob.sync_linked_catalogs_now(order) }
+    it "ignores products without semantic link" do
+      expect(StockSyncJob).not_to receive(:perform_now)
+      expect { subject }.not_to enqueue_job(StockSyncJob)
+    end
+
+    it "performs stock check now" do
+      beans.semantic_links << SemanticLink.new(
+        semantic_id: beans_retail_link
+      )
+
+      expect(StockSyncJob).to receive(:perform_now).with(user, catalog_link)
+      expect { subject }.not_to raise_error
+    end
+
+    it "reports errors" do
+      expect(order).to receive(:variants).and_raise("test error")
+      expect(Bugsnag).to receive(:notify).and_call_original
+
+      expect { subject }.not_to raise_error
+    end
+  end
+
   describe "#perform" do
     subject { StockSyncJob.perform_now(user, catalog_link) }