From 6738101ebdb90c9f1445a1af7fc0f34b32742c63 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 26 Feb 2025 10:50:45 +1100 Subject: [PATCH] Raise error if record not found, and don't retry too many times After 10 minutes, I'd consider that it failed to open the order cycle. Who would want their products to sync, or get a notification at a random time during the order cycle? Best viewed with whitespace ignored. --- app/jobs/open_order_cycle_job.rb | 7 ++++--- spec/jobs/open_order_cycle_job_spec.rb | 8 +++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/jobs/open_order_cycle_job.rb b/app/jobs/open_order_cycle_job.rb index 60dd985157..80c4e1d7b8 100644 --- a/app/jobs/open_order_cycle_job.rb +++ b/app/jobs/open_order_cycle_job.rb @@ -4,13 +4,14 @@ # # Currently, an order cycle is considered open in the shopfront when orders_open_at >= now. # But now there are some pre-conditions for opening an order cycle, so we would like to change that. -# Instead, the presence of opened_at (and absence of closed_at) should indicate it is open. +# Instead, the presence of opened_at (and absence of processed_at) should indicate it is open. class OpenOrderCycleJob < ApplicationJob + sidekiq_options retry_for: 10.minutes + def perform(order_cycle_id) ActiveRecord::Base.transaction do # Fetch order cycle if it's still unopened, and lock DB row until finished - order_cycle = OrderCycle.lock.find_by(id: order_cycle_id, opened_at: nil) - return if order_cycle.nil? + order_cycle = OrderCycle.lock.find_by!(id: order_cycle_id, opened_at: nil) sync_remote_variants(order_cycle) diff --git a/spec/jobs/open_order_cycle_job_spec.rb b/spec/jobs/open_order_cycle_job_spec.rb index 023a029fe4..0a25161360 100644 --- a/spec/jobs/open_order_cycle_job_spec.rb +++ b/spec/jobs/open_order_cycle_job_spec.rb @@ -89,7 +89,13 @@ RSpec.describe OpenOrderCycleJob do # Resume and complete both jobs: breakpoint.unlock - threads.each(&:join) + + # Join each thread to the main thread to ensure they end. + # Any exceptions that were raised, are raised to the main thread. + # We're expecting RecordNotFound because the record was locked by the first concurrent thread. + expect{ + threads.each(&:join) + }.to raise_error ActiveRecord::RecordNotFound end end end