From d4be48e5c44e39d320e2e187a43ed23d4408c174 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 29 Sep 2022 16:33:04 +1000 Subject: [PATCH] 5. Trigger SubscriptionPlacementJob for each order cycle A new OrderCycleOpeningJob will trigger the job as before, but provides a way to consistently trigger other events when order cycles open. Ruby ranges with Arel allow us to be nice and concise :) --- app/jobs/order_cycle_opening_job.rb | 18 +++++++++++++ app/jobs/subscription_placement_job.rb | 10 +++---- config/sidekiq.yml | 4 +-- spec/jobs/order_cycle_opening_job_spec.rb | 23 ++++++++++++++++ spec/jobs/subscription_placement_job_spec.rb | 28 ++++++++++---------- 5 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 app/jobs/order_cycle_opening_job.rb create mode 100644 spec/jobs/order_cycle_opening_job_spec.rb diff --git a/app/jobs/order_cycle_opening_job.rb b/app/jobs/order_cycle_opening_job.rb new file mode 100644 index 000000000000..431728f080c7 --- /dev/null +++ b/app/jobs/order_cycle_opening_job.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# Trigger jobs for any order cycles that recently opened +class OrderCycleOpeningJob < ApplicationJob + def perform + recently_opened_order_cycles.each do |oc_id| + SubscriptionPlacementJob.perform_later(oc_id) + end + end + + private + + def recently_opened_order_cycles + OrderCycle + .where(orders_open_at: 1.hour.ago..Time.zone.now) + .pluck(:id) + end +end diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb index 8c9caf5221b7..3eefb0262143 100644 --- a/app/jobs/subscription_placement_job.rb +++ b/app/jobs/subscription_placement_job.rb @@ -3,8 +3,8 @@ require 'order_management/subscriptions/summarizer' class SubscriptionPlacementJob < ActiveJob::Base - def perform - proxy_orders.each do |proxy_order| + def perform(order_cycle_id) + proxy_orders(order_cycle_id).each do |proxy_order| place_order_for(proxy_order) end @@ -17,10 +17,10 @@ def summarizer @summarizer ||= OrderManagement::Subscriptions::Summarizer.new end - def proxy_orders - # Loads proxy orders for open order cycles that have not been placed yet + def proxy_orders(order_cycle_id) + # Loads proxy orders for order cycle, that have not been placed yet ProxyOrder.not_canceled.where(placed_at: nil) - .joins(:order_cycle).merge(OrderCycle.active) + .where(order_cycle_id: order_cycle_id) .joins(:subscription).merge(Subscription.not_canceled.not_paused) .order(:id) end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 3c4d93b6afc4..15b9d28ad308 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -10,9 +10,9 @@ :schedule: HeartbeatJob: every: ["5m", first_in: "0s"] - SubscriptionPlacementJob: - every: "5m" SubscriptionConfirmJob: every: "5m" + OrderCycleOpeningJob: + every: "5m" OrderCycleClosingJob: every: "5m" diff --git a/spec/jobs/order_cycle_opening_job_spec.rb b/spec/jobs/order_cycle_opening_job_spec.rb new file mode 100644 index 000000000000..84499c4da059 --- /dev/null +++ b/spec/jobs/order_cycle_opening_job_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe OrderCycleOpeningJob do + let(:oc_opened_before) { + create(:order_cycle, orders_open_at: Time.zone.now - 1.hour) + } + let(:oc_opened_now) { + create(:order_cycle, orders_open_at: Time.zone.now) + } + let(:oc_opening_soon) { + create(:order_cycle, orders_open_at: Time.zone.now + 1.minute) + } + + it "triggers jobs for recently opened order cycles only" do + expect(SubscriptionPlacementJob).to_not receive(:perform_later).with(oc_opened_before.id) + expect(SubscriptionPlacementJob).to receive(:perform_later).with(oc_opened_now.id) + expect(SubscriptionPlacementJob).to_not receive(:perform_later).with(oc_opening_soon.id) + + OrderCycleOpeningJob.perform_now + end +end diff --git a/spec/jobs/subscription_placement_job_spec.rb b/spec/jobs/subscription_placement_job_spec.rb index c4ee87bb9459..e6a2d3c3e5bf 100644 --- a/spec/jobs/subscription_placement_job_spec.rb +++ b/spec/jobs/subscription_placement_job_spec.rb @@ -25,29 +25,29 @@ } # OK it "ignores proxy orders where the OC has closed" do - expect(job.send(:proxy_orders)).to include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to include proxy_order proxy_order.update!(order_cycle_id: order_cycle2.id) - expect(job.send(:proxy_orders)).to_not include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to_not include proxy_order end it "ignores proxy orders for paused or cancelled subscriptions" do - expect(job.send(:proxy_orders)).to include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to include proxy_order subscription.update!(paused_at: 1.minute.ago) - expect(job.send(:proxy_orders)).to_not include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to_not include proxy_order subscription.update!(paused_at: nil) - expect(job.send(:proxy_orders)).to include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to include proxy_order subscription.update!(canceled_at: 1.minute.ago) - expect(job.send(:proxy_orders)).to_not include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to_not include proxy_order end it "ignores proxy orders that have been marked as cancelled or placed" do - expect(job.send(:proxy_orders)).to include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to include proxy_order proxy_order.update!(canceled_at: 5.minutes.ago) - expect(job.send(:proxy_orders)).to_not include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to_not include proxy_order proxy_order.update!(canceled_at: nil) - expect(job.send(:proxy_orders)).to include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to include proxy_order proxy_order.update!(placed_at: 5.minutes.ago) - expect(job.send(:proxy_orders)).to_not include proxy_order + expect(job.send(:proxy_orders, order_cycle1.id)).to_not include proxy_order end end @@ -67,7 +67,7 @@ allow(PlaceProxyOrder).to receive(:new) { service } allow(service).to receive(:call) - job.perform + job.perform(proxy_order.order_cycle_id) expect(service).to have_received(:call) end @@ -78,7 +78,7 @@ summarizer = TestSummarizer.new allow(OrderManagement::Subscriptions::Summarizer).to receive(:new).and_return(summarizer) - job.perform + job.perform(proxy_order.order_cycle_id) expect(summarizer.recorded_issues[order.id]) .to eq("Errors: Cannot transition state via :next from :address (Reason(s): Items cannot be shipped)") @@ -202,8 +202,8 @@ expect { # Start two jobs in parallel: threads = [ - Thread.new { SubscriptionPlacementJob.new.perform }, - Thread.new { SubscriptionPlacementJob.new.perform }, + Thread.new { SubscriptionPlacementJob.new.perform(proxy_order.order_cycle_id) }, + Thread.new { SubscriptionPlacementJob.new.perform(proxy_order.order_cycle_id) }, ] # Wait for both to jobs to pause.