From ae0ceb61a169e531fe9b7f8d13e7c4c1f7273acb Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 11 Feb 2020 18:22:36 +0000 Subject: [PATCH] Move ProxyOrderSyncer to OrderManagement engine --- .rubocop_manual_todo.yml | 4 +- .rubocop_todo.yml | 6 +- app/controllers/admin/schedules_controller.rb | 4 +- app/services/order_cycle_form.rb | 4 +- app/services/subscription_form.rb | 4 +- .../subscriptions/proxy_order_syncer.rb | 99 +++++ .../subscriptions/proxy_order_syncer_spec.rb | 69 +++ .../subscriptions/proxy_order_syncer_spec.rb | 402 ++++++++++++++++++ lib/open_food_network/proxy_order_syncer.rb | 95 ----- .../admin/schedules_controller_spec.rb | 4 +- .../proxy_order_syncer_spec.rb | 400 ----------------- spec/performance/proxy_order_syncer_spec.rb | 67 --- spec/services/order_cycle_form_spec.rb | 6 +- 13 files changed, 587 insertions(+), 577 deletions(-) create mode 100644 engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb create mode 100644 engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb create mode 100644 engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb delete mode 100644 lib/open_food_network/proxy_order_syncer.rb delete mode 100644 spec/lib/open_food_network/proxy_order_syncer_spec.rb delete mode 100644 spec/performance/proxy_order_syncer_spec.rb diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 9b0fd3b537..9494595048 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -101,6 +101,7 @@ Layout/LineLength: - app/services/subscriptions_count.rb - app/services/variants_stock_levels.rb - engines/web/app/helpers/web/cookies_policy_helper.rb + - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb - lib/discourse/single_sign_on.rb - lib/open_food_network/available_payment_method_filter.rb - lib/open_food_network/bulk_coop_report.rb @@ -239,7 +240,6 @@ Layout/LineLength: - spec/lib/open_food_network/packing_report_spec.rb - spec/lib/open_food_network/permissions_spec.rb - spec/lib/open_food_network/products_and_inventory_report_spec.rb - - spec/lib/open_food_network/proxy_order_syncer_spec.rb - spec/lib/open_food_network/scope_variant_to_hub_spec.rb - spec/lib/open_food_network/tag_rule_applicator_spec.rb - spec/lib/open_food_network/users_and_enterprises_report_spec.rb @@ -684,6 +684,7 @@ Metrics/ModuleLength: - app/helpers/injection_helper.rb - app/helpers/spree/admin/navigation_helper.rb - app/helpers/spree/admin/base_helper.rb + - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb - lib/open_food_network/column_preference_defaults.rb - spec/controllers/admin/enterprises_controller_spec.rb @@ -700,7 +701,6 @@ Metrics/ModuleLength: - spec/lib/open_food_network/order_grouper_spec.rb - spec/lib/open_food_network/permissions_spec.rb - spec/lib/open_food_network/products_and_inventory_report_spec.rb - - spec/lib/open_food_network/proxy_order_syncer_spec.rb - spec/lib/open_food_network/scope_variant_to_hub_spec.rb - spec/lib/open_food_network/tag_rule_applicator_spec.rb - spec/lib/open_food_network/users_and_enterprises_report_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 323426fda9..1c003c0504 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -882,6 +882,7 @@ Style/FrozenStringLiteralComment: - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/report_service.rb' - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb' - 'engines/order_management/app/services/order_management/reports/enterprise_fee_summary/summarizer.rb' + - 'engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb' - 'engines/order_management/app/services/order_management/subscriptions/subscription_summarizer.rb' - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb' - 'engines/order_management/app/services/reports.rb' @@ -895,6 +896,7 @@ Style/FrozenStringLiteralComment: - 'engines/order_management/lib/order_management/engine.rb' - 'engines/order_management/lib/order_management/version.rb' - 'engines/order_management/order_management.gemspec' + - 'engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb' - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/authorizer_spec.rb' - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/parameters_spec.rb' - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/permissions_spec.rb' @@ -902,6 +904,7 @@ Style/FrozenStringLiteralComment: - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/renderers/html_renderer_spec.rb' - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_data/enterprise_fee_type_total_spec.rb' - 'engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb' + - 'engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summizer_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summary_spec.rb' - 'engines/order_management/spec/spec_helper.rb' @@ -954,7 +957,6 @@ Style/FrozenStringLiteralComment: - 'lib/open_food_network/products_and_inventory_report.rb' - 'lib/open_food_network/products_and_inventory_report_base.rb' - 'lib/open_food_network/property_merge.rb' - - 'lib/open_food_network/proxy_order_syncer.rb' - 'lib/open_food_network/rack_request_blocker.rb' - 'lib/open_food_network/referer_parser.rb' - 'lib/open_food_network/reports/bulk_coop_allocation_report.rb' @@ -1212,7 +1214,6 @@ Style/FrozenStringLiteralComment: - 'spec/lib/open_food_network/permissions_spec.rb' - 'spec/lib/open_food_network/products_and_inventory_report_spec.rb' - 'spec/lib/open_food_network/property_merge_spec.rb' - - 'spec/lib/open_food_network/proxy_order_syncer_spec.rb' - 'spec/lib/open_food_network/referer_parser_spec.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' - 'spec/lib/open_food_network/reports/row_spec.rb' @@ -1306,7 +1307,6 @@ Style/FrozenStringLiteralComment: - 'spec/models/variant_override_spec.rb' - 'spec/performance/injection_helper_spec.rb' - 'spec/performance/orders_controller_spec.rb' - - 'spec/performance/proxy_order_syncer_spec.rb' - 'spec/performance/shop_controller_spec.rb' - 'spec/requests/checkout/failed_checkout_spec.rb' - 'spec/requests/checkout/paypal_spec.rb' diff --git a/app/controllers/admin/schedules_controller.rb b/app/controllers/admin/schedules_controller.rb index 580a6a9c72..c568bf0ad7 100644 --- a/app/controllers/admin/schedules_controller.rb +++ b/app/controllers/admin/schedules_controller.rb @@ -1,5 +1,5 @@ require 'open_food_network/permissions' -require 'open_food_network/proxy_order_syncer' +require 'order_management/subscriptions/proxy_order_syncer' module Admin class SchedulesController < ResourceController @@ -81,7 +81,7 @@ module Admin return unless removed_ids.any? || new_ids.any? subscriptions = Subscription.where(schedule_id: @schedule) - syncer = OpenFoodNetwork::ProxyOrderSyncer.new(subscriptions) + syncer = OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions) syncer.sync! end end diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb index 2ab5d109de..7026d5f5b6 100644 --- a/app/services/order_cycle_form.rb +++ b/app/services/order_cycle_form.rb @@ -1,6 +1,6 @@ require 'open_food_network/permissions' -require 'open_food_network/proxy_order_syncer' require 'open_food_network/order_cycle_form_applicator' +require 'order_management/subscriptions/proxy_order_syncer' class OrderCycleForm def initialize(order_cycle, params, user) @@ -58,7 +58,7 @@ class OrderCycleForm return unless schedule_ids? return unless schedule_sync_required? - OpenFoodNetwork::ProxyOrderSyncer.new(subscriptions_to_sync).sync! + OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions_to_sync).sync! end def schedule_sync_required? diff --git a/app/services/subscription_form.rb b/app/services/subscription_form.rb index da82063b5c..fa33b216b2 100644 --- a/app/services/subscription_form.rb +++ b/app/services/subscription_form.rb @@ -1,4 +1,4 @@ -require 'open_food_network/proxy_order_syncer' +require 'order_management/subscriptions/proxy_order_syncer' class SubscriptionForm attr_accessor :subscription, :params, :order_update_issues, :validator, :order_syncer, :estimator @@ -29,6 +29,6 @@ class SubscriptionForm private def proxy_order_syncer - OpenFoodNetwork::ProxyOrderSyncer.new(subscription) + OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscription) end end diff --git a/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb new file mode 100644 index 0000000000..b53085f1c1 --- /dev/null +++ b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: false + +module OrderManagement + module Subscriptions + class ProxyOrderSyncer + attr_reader :subscription + + delegate :order_cycles, :proxy_orders, :begins_at, :ends_at, to: :subscription + + def initialize(subscriptions) + case subscriptions + when Subscription + @subscription = subscriptions + when ActiveRecord::Relation + @subscriptions = subscriptions.not_ended.not_canceled + else + raise "ProxyOrderSyncer must be initialized with " \ + "an instance of Subscription or ActiveRecord::Relation" + end + end + + def sync! + return sync_subscriptions! if @subscriptions + + return initialise_proxy_orders! unless @subscription.id + + sync_subscription! + end + + private + + def sync_subscriptions! + @subscriptions.each do |subscription| + @subscription = subscription + sync_subscription! + end + end + + def initialise_proxy_orders! + uninitialised_order_cycle_ids.each do |order_cycle_id| + Rails.logger.info "Initializing Proxy Order " \ + "of subscription #{@subscription.id} in order cycle #{order_cycle_id}" + proxy_orders << ProxyOrder.new(subscription: subscription, order_cycle_id: order_cycle_id) + end + end + + def sync_subscription! + Rails.logger.info "Syncing Proxy Orders of subscription #{@subscription.id}" + create_proxy_orders! + remove_orphaned_proxy_orders! + end + + def create_proxy_orders! + return unless not_closed_in_range_order_cycles.any? + + query = "INSERT INTO proxy_orders (subscription_id, order_cycle_id, updated_at, created_at)" + query << " VALUES #{insert_values}" + query << " ON CONFLICT DO NOTHING" + + ActiveRecord::Base.connection.exec_query(query) + end + + def uninitialised_order_cycle_ids + not_closed_in_range_order_cycles.pluck(:id) - proxy_orders.map(&:order_cycle_id) + end + + def remove_orphaned_proxy_orders! + orphaned_proxy_orders.scoped.delete_all + end + + # Remove Proxy Orders that have not been placed yet + # and are in Order Cycles that are out of range + def orphaned_proxy_orders + orphaned = proxy_orders.where(placed_at: nil) + order_cycle_ids = in_range_order_cycles.pluck(:id) + return orphaned unless order_cycle_ids.any? + + orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids) + end + + def insert_values + now = Time.now.utc.iso8601 + not_closed_in_range_order_cycles + .map{ |oc| "(#{subscription.id},#{oc.id},'#{now}','#{now}')" } + .join(",") + end + + def not_closed_in_range_order_cycles + in_range_order_cycles.merge(OrderCycle.not_closed) + end + + def in_range_order_cycles + order_cycles.where("orders_close_at >= ? AND orders_close_at <= ?", + begins_at, + ends_at || 100.years.from_now) + end + end + end +end diff --git a/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb new file mode 100644 index 0000000000..b62267b714 --- /dev/null +++ b/engines/order_management/spec/performance/order_management/subscriptions/proxy_order_syncer_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module OrderManagement + module Subscriptions + describe ProxyOrderSyncer, performance: true do + let(:start) { Time.zone.now.beginning_of_day } + let!(:schedule) { create(:schedule, order_cycles: order_cycles) } + + let!(:order_cycles) do + Array.new(10) do |i| + create(:simple_order_cycle, orders_open_at: start + i.days, + orders_close_at: start + (i + 1).days ) + end + end + + let!(:subscriptions) do + Array.new(150) do |_i| + create(:subscription, schedule: schedule, begins_at: start, ends_at: start + 10.days) + end + Subscription.where(schedule_id: schedule) + end + + context "measuring performance for initialisation" do + it "reports the average run time for adding 10 OCs to 150 subscriptions" do + expect(ProxyOrder.count).to be 0 + times = [] + 10.times do + syncer = ProxyOrderSyncer.new(subscriptions.reload) + + t1 = Time.zone.now + syncer.sync! + t2 = Time.zone.now + diff = t2 - t1 + times << diff + puts diff.round(2) + + expect(ProxyOrder.count).to be 1500 + ProxyOrder.destroy_all + end + puts "AVG: #{(times.sum / times.count).round(2)}" + end + end + + context "measuring performance for removal" do + it "reports the average run time for removing 8 OCs from 150 subscriptions" do + times = [] + 10.times do + syncer = ProxyOrderSyncer.new(subscriptions.reload) + syncer.sync! + expect(ProxyOrder.count).to be 1500 + subscriptions.update_all(begins_at: start + 8.days + 1.minute) + syncer = ProxyOrderSyncer.new(subscriptions.reload) + + t1 = Time.zone.now + syncer.sync! + t2 = Time.zone.now + diff = t2 - t1 + times << diff + puts diff.round(2) + + expect(ProxyOrder.count).to be 300 + subscriptions.update_all(begins_at: start) + end + puts "AVG: #{(times.sum / times.count).round(2)}" + end + end + end + end +end diff --git a/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb new file mode 100644 index 0000000000..ab01eddc4c --- /dev/null +++ b/engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb @@ -0,0 +1,402 @@ +# frozen_string_literal: true + +module OrderManagement + module Subscriptions + describe ProxyOrderSyncer do + describe "initialization" do + let!(:subscription) { create(:subscription) } + + it "raises an error when initialized with an object that is not a Subscription or an ActiveRecord::Relation" do + expect{ ProxyOrderSyncer.new(subscription) }.to_not raise_error + expect{ ProxyOrderSyncer.new(Subscription.where(id: subscription.id)) }.to_not raise_error + expect{ ProxyOrderSyncer.new("something") }.to raise_error RuntimeError + end + end + + describe "#sync!" do + let(:now) { Time.zone.now } + let(:schedule) { create(:schedule) } + let(:closed_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now) } # Closed + let(:open_oc_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } # Open, but closes before begins at + let(:open_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } # Open & closes between begins at and ends at + let(:upcoming_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } # Upcoming, but closes before begins at + let(:upcoming_closes_on_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } # Upcoming & closes on begins at + let(:upcoming_closes_on_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } # Upcoming & closes on ends at + let(:upcoming_closes_after_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } # Upcoming & closes after ends at + let(:subscription) { build(:subscription, schedule: schedule, begins_at: now + 1.minute, ends_at: now + 2.minutes) } + let(:proxy_orders) { subscription.reload.proxy_orders } + let(:order_cycles) { proxy_orders.map(&:order_cycle) } + let(:syncer) { ProxyOrderSyncer.new(subscription) } + + context "when the subscription is not persisted" do + before do + oc # Ensure oc is created before we attempt to sync + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) + end + + context "and the schedule includes a closed oc (ie. closed before opens_at)" do + let(:oc) { closed_oc } + it "does not create a new proxy order for that oc" do + expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes an open oc that closes before begins_at" do + let(:oc) { open_oc_closes_before_begins_at_oc } + it "does not create a new proxy order for that oc" do + expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes an open oc that closes between begins_at and ends_at" do + let(:oc) { open_oc } + it "creates a new proxy order for that oc" do + expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes before begins_at" do + let(:oc) { upcoming_closes_before_begins_at_oc } + it "does not create a new proxy order for that oc" do + expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes upcoming oc that closes on begins_at" do + let(:oc) { upcoming_closes_on_begins_at_oc } + it "creates a new proxy order for that oc" do + expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes after ends_at" do + let(:oc) { upcoming_closes_on_ends_at_oc } + it "creates a new proxy order for that oc" do + expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes after ends_at" do + let(:oc) { upcoming_closes_after_ends_at_oc } + it "does not create a new proxy order for that oc" do + expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + end + + context "when the subscription is persisted" do + before { expect(subscription.save!).to be true } + + context "when a proxy order exists" do + let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: oc) } + + context "for an oc included in the relevant schedule" do + context "and the proxy order has already been placed" do + before { proxy_order.update_attributes(placed_at: 5.minutes.ago) } + + context "the oc is closed (ie. closed before opens_at)" do + let(:oc) { closed_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the schedule includes an open oc that closes before begins_at" do + let(:oc) { open_oc_closes_before_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is open and closes between begins_at and ends_at" do + let(:oc) { open_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes before begins_at" do + let(:oc) { upcoming_closes_before_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes on begins_at" do + let(:oc) { upcoming_closes_on_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes on ends_at" do + let(:oc) { upcoming_closes_on_ends_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes after ends_at" do + let(:oc) { upcoming_closes_after_ends_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + end + + context "and the proxy order has not already been placed" do + context "the oc is closed (ie. closed before opens_at)" do + let(:oc) { closed_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the schedule includes an open oc that closes before begins_at" do + let(:oc) { open_oc_closes_before_begins_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is open and closes between begins_at and ends_at" do + let(:oc) { open_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes before begins_at" do + let(:oc) { upcoming_closes_before_begins_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is upcoming and closes on begins_at" do + let(:oc) { upcoming_closes_on_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes on ends_at" do + let(:oc) { upcoming_closes_on_ends_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes after ends_at" do + let(:oc) { upcoming_closes_after_ends_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + end + end + + context "for an oc not included in the relevant schedule" do + let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) } + before do + open_oc.schedule_ids = [] + expect(open_oc.save!).to be true + end + + context "and the proxy order has already been placed" do + before { proxy_order.update_attributes(placed_at: 5.minutes.ago) } + + context "the oc is closed (ie. closed before opens_at)" do + let(:oc) { closed_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the schedule includes an open oc that closes before begins_at" do + let(:oc) { open_oc_closes_before_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is open and closes between begins_at and ends_at" do + let(:oc) { open_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes before begins_at" do + let(:oc) { upcoming_closes_before_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes on begins_at" do + let(:oc) { upcoming_closes_on_begins_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes on ends_at" do + let(:oc) { upcoming_closes_on_ends_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + + context "and the oc is upcoming and closes after ends_at" do + let(:oc) { upcoming_closes_after_ends_at_oc } + it "keeps the proxy order" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) + expect(proxy_orders).to include proxy_order + end + end + end + + context "and the proxy order has not already been placed" do + # This shouldn't really happen, but it is possible + context "the oc is closed (ie. closed before opens_at)" do + let(:oc) { closed_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + # This shouldn't really happen, but it is possible + context "and the oc is open and closes between begins_at and ends_at" do + let(:oc) { open_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is upcoming and closes before begins_at" do + let(:oc) { upcoming_closes_before_begins_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is upcoming and closes on begins_at" do + let(:oc) { upcoming_closes_on_begins_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is upcoming and closes on ends_at" do + let(:oc) { upcoming_closes_on_ends_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + + context "and the oc is upcoming and closes after ends_at" do + let(:oc) { upcoming_closes_after_ends_at_oc } + it "removes the proxy order" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) + expect(proxy_orders).to_not include proxy_order + end + end + end + end + end + + context "when a proxy order does not exist" do + context "and the schedule includes a closed oc (ie. closed before opens_at)" do + let!(:oc) { closed_oc } + it "does not create a new proxy order for that oc" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes an open oc that closes before begins_at" do + let(:oc) { open_oc_closes_before_begins_at_oc } + it "does not create a new proxy order for that oc" do + expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes an open oc that closes between begins_at and ends_at" do + let!(:oc) { open_oc } + it "creates a new proxy order for that oc" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes before begins_at" do + let!(:oc) { upcoming_closes_before_begins_at_oc } + it "does not create a new proxy order for that oc" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + + context "and the schedule includes upcoming oc that closes on begins_at" do + let!(:oc) { upcoming_closes_on_begins_at_oc } + it "creates a new proxy order for that oc" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes on ends_at" do + let!(:oc) { upcoming_closes_on_ends_at_oc } + it "creates a new proxy order for that oc" do + expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) + expect(order_cycles).to include oc + end + end + + context "and the schedule includes upcoming oc that closes after ends_at" do + let!(:oc) { upcoming_closes_after_ends_at_oc } + it "does not create a new proxy order for that oc" do + expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) + expect(order_cycles).to_not include oc + end + end + end + end + end + end + end +end diff --git a/lib/open_food_network/proxy_order_syncer.rb b/lib/open_food_network/proxy_order_syncer.rb deleted file mode 100644 index 62dc575115..0000000000 --- a/lib/open_food_network/proxy_order_syncer.rb +++ /dev/null @@ -1,95 +0,0 @@ -module OpenFoodNetwork - class ProxyOrderSyncer - attr_reader :subscription - - delegate :order_cycles, :proxy_orders, :begins_at, :ends_at, to: :subscription - - def initialize(subscriptions) - case subscriptions - when Subscription - @subscription = subscriptions - when ActiveRecord::Relation - @subscriptions = subscriptions.not_ended.not_canceled - else - raise "ProxyOrderSyncer must be initialized with " \ - "an instance of Subscription or ActiveRecord::Relation" - end - end - - def sync! - return sync_subscriptions! if @subscriptions - - return initialise_proxy_orders! unless @subscription.id - - sync_subscription! - end - - private - - def sync_subscriptions! - @subscriptions.each do |subscription| - @subscription = subscription - sync_subscription! - end - end - - def initialise_proxy_orders! - uninitialised_order_cycle_ids.each do |order_cycle_id| - Rails.logger.info "Initializing Proxy Order " \ - "of subscription #{@subscription.id} in order cycle #{order_cycle_id}" - proxy_orders << ProxyOrder.new(subscription: subscription, order_cycle_id: order_cycle_id) - end - end - - def sync_subscription! - Rails.logger.info "Syncing Proxy Orders of subscription #{@subscription.id}" - create_proxy_orders! - remove_orphaned_proxy_orders! - end - - def create_proxy_orders! - return unless not_closed_in_range_order_cycles.any? - - query = "INSERT INTO proxy_orders (subscription_id, order_cycle_id, updated_at, created_at)" - query << " VALUES #{insert_values}" - query << " ON CONFLICT DO NOTHING" - - ActiveRecord::Base.connection.exec_query(query) - end - - def uninitialised_order_cycle_ids - not_closed_in_range_order_cycles.pluck(:id) - proxy_orders.map(&:order_cycle_id) - end - - def remove_orphaned_proxy_orders! - orphaned_proxy_orders.scoped.delete_all - end - - # Remove Proxy Orders that have not been placed yet - # and are in Order Cycles that are out of range - def orphaned_proxy_orders - orphaned = proxy_orders.where(placed_at: nil) - order_cycle_ids = in_range_order_cycles.pluck(:id) - return orphaned unless order_cycle_ids.any? - - orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids) - end - - def insert_values - now = Time.now.utc.iso8601 - not_closed_in_range_order_cycles - .map{ |oc| "(#{subscription.id},#{oc.id},'#{now}','#{now}')" } - .join(",") - end - - def not_closed_in_range_order_cycles - in_range_order_cycles.merge(OrderCycle.not_closed) - end - - def in_range_order_cycles - order_cycles.where("orders_close_at >= ? AND orders_close_at <= ?", - begins_at, - ends_at || 100.years.from_now) - end - end -end diff --git a/spec/controllers/admin/schedules_controller_spec.rb b/spec/controllers/admin/schedules_controller_spec.rb index 19f2e4b032..5f213b4292 100644 --- a/spec/controllers/admin/schedules_controller_spec.rb +++ b/spec/controllers/admin/schedules_controller_spec.rb @@ -90,7 +90,7 @@ describe Admin::SchedulesController, type: :controller do it "syncs proxy orders when order_cycle_ids change" do syncer_mock = double(:syncer) - allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock } + allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock } expect(syncer_mock).to receive(:sync!).exactly(2).times spree_put :update, format: :json, id: coordinated_schedule.id, schedule: { order_cycle_ids: [coordinated_order_cycle.id, coordinated_order_cycle2.id] } @@ -150,7 +150,7 @@ describe Admin::SchedulesController, type: :controller do it "sync proxy orders" do syncer_mock = double(:syncer) - allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock } + allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock } expect(syncer_mock).to receive(:sync!).once create_schedule params diff --git a/spec/lib/open_food_network/proxy_order_syncer_spec.rb b/spec/lib/open_food_network/proxy_order_syncer_spec.rb deleted file mode 100644 index 9b059d178f..0000000000 --- a/spec/lib/open_food_network/proxy_order_syncer_spec.rb +++ /dev/null @@ -1,400 +0,0 @@ -require 'open_food_network/proxy_order_syncer' - -module OpenFoodNetwork - describe ProxyOrderSyncer do - describe "initialization" do - let!(:subscription) { create(:subscription) } - - it "raises an error when initialized with an object that is not a Subscription or an ActiveRecord::Relation" do - expect{ ProxyOrderSyncer.new(subscription) }.to_not raise_error - expect{ ProxyOrderSyncer.new(Subscription.where(id: subscription.id)) }.to_not raise_error - expect{ ProxyOrderSyncer.new("something") }.to raise_error RuntimeError - end - end - - describe "#sync!" do - let(:now) { Time.zone.now } - let(:schedule) { create(:schedule) } - let(:closed_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now) } # Closed - let(:open_oc_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 59.seconds) } # Open, but closes before begins at - let(:open_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now - 1.minute, orders_close_at: now + 90.seconds) } # Open & closes between begins at and ends at - let(:upcoming_closes_before_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 59.seconds) } # Upcoming, but closes before begins at - let(:upcoming_closes_on_begins_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 1.minute) } # Upcoming & closes on begins at - let(:upcoming_closes_on_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 2.minutes) } # Upcoming & closes on ends at - let(:upcoming_closes_after_ends_at_oc) { create(:simple_order_cycle, schedules: [schedule], orders_open_at: now + 30.seconds, orders_close_at: now + 121.seconds) } # Upcoming & closes after ends at - let(:subscription) { build(:subscription, schedule: schedule, begins_at: now + 1.minute, ends_at: now + 2.minutes) } - let(:proxy_orders) { subscription.reload.proxy_orders } - let(:order_cycles) { proxy_orders.map(&:order_cycle) } - let(:syncer) { ProxyOrderSyncer.new(subscription) } - - context "when the subscription is not persisted" do - before do - oc # Ensure oc is created before we attempt to sync - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) - end - - context "and the schedule includes a closed oc (ie. closed before opens_at)" do - let(:oc) { closed_oc } - it "does not create a new proxy order for that oc" do - expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes an open oc that closes before begins_at" do - let(:oc) { open_oc_closes_before_begins_at_oc } - it "does not create a new proxy order for that oc" do - expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes an open oc that closes between begins_at and ends_at" do - let(:oc) { open_oc } - it "creates a new proxy order for that oc" do - expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes before begins_at" do - let(:oc) { upcoming_closes_before_begins_at_oc } - it "does not create a new proxy order for that oc" do - expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes upcoming oc that closes on begins_at" do - let(:oc) { upcoming_closes_on_begins_at_oc } - it "creates a new proxy order for that oc" do - expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes after ends_at" do - let(:oc) { upcoming_closes_on_ends_at_oc } - it "creates a new proxy order for that oc" do - expect{ subscription.save! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes after ends_at" do - let(:oc) { upcoming_closes_after_ends_at_oc } - it "does not create a new proxy order for that oc" do - expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - end - - context "when the subscription is persisted" do - before { expect(subscription.save!).to be true } - - context "when a proxy order exists" do - let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: oc) } - - context "for an oc included in the relevant schedule" do - context "and the proxy order has already been placed" do - before { proxy_order.update_attributes(placed_at: 5.minutes.ago) } - - context "the oc is closed (ie. closed before opens_at)" do - let(:oc) { closed_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the schedule includes an open oc that closes before begins_at" do - let(:oc) { open_oc_closes_before_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is open and closes between begins_at and ends_at" do - let(:oc) { open_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes before begins_at" do - let(:oc) { upcoming_closes_before_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes on begins_at" do - let(:oc) { upcoming_closes_on_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes on ends_at" do - let(:oc) { upcoming_closes_on_ends_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes after ends_at" do - let(:oc) { upcoming_closes_after_ends_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - end - - context "and the proxy order has not already been placed" do - context "the oc is closed (ie. closed before opens_at)" do - let(:oc) { closed_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the schedule includes an open oc that closes before begins_at" do - let(:oc) { open_oc_closes_before_begins_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is open and closes between begins_at and ends_at" do - let(:oc) { open_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes before begins_at" do - let(:oc) { upcoming_closes_before_begins_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is upcoming and closes on begins_at" do - let(:oc) { upcoming_closes_on_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes on ends_at" do - let(:oc) { upcoming_closes_on_ends_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes after ends_at" do - let(:oc) { upcoming_closes_after_ends_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - end - end - - context "for an oc not included in the relevant schedule" do - let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) } - before do - open_oc.schedule_ids = [] - expect(open_oc.save!).to be true - end - - context "and the proxy order has already been placed" do - before { proxy_order.update_attributes(placed_at: 5.minutes.ago) } - - context "the oc is closed (ie. closed before opens_at)" do - let(:oc) { closed_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the schedule includes an open oc that closes before begins_at" do - let(:oc) { open_oc_closes_before_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is open and closes between begins_at and ends_at" do - let(:oc) { open_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes before begins_at" do - let(:oc) { upcoming_closes_before_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes on begins_at" do - let(:oc) { upcoming_closes_on_begins_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes on ends_at" do - let(:oc) { upcoming_closes_on_ends_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - - context "and the oc is upcoming and closes after ends_at" do - let(:oc) { upcoming_closes_after_ends_at_oc } - it "keeps the proxy order" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(1) - expect(proxy_orders).to include proxy_order - end - end - end - - context "and the proxy order has not already been placed" do - # This shouldn't really happen, but it is possible - context "the oc is closed (ie. closed before opens_at)" do - let(:oc) { closed_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - # This shouldn't really happen, but it is possible - context "and the oc is open and closes between begins_at and ends_at" do - let(:oc) { open_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is upcoming and closes before begins_at" do - let(:oc) { upcoming_closes_before_begins_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is upcoming and closes on begins_at" do - let(:oc) { upcoming_closes_on_begins_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is upcoming and closes on ends_at" do - let(:oc) { upcoming_closes_on_ends_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - - context "and the oc is upcoming and closes after ends_at" do - let(:oc) { upcoming_closes_after_ends_at_oc } - it "removes the proxy order" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(1).to(0) - expect(proxy_orders).to_not include proxy_order - end - end - end - end - end - - context "when a proxy order does not exist" do - context "and the schedule includes a closed oc (ie. closed before opens_at)" do - let!(:oc) { closed_oc } - it "does not create a new proxy order for that oc" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes an open oc that closes before begins_at" do - let(:oc) { open_oc_closes_before_begins_at_oc } - it "does not create a new proxy order for that oc" do - expect{ subscription.save! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes an open oc that closes between begins_at and ends_at" do - let!(:oc) { open_oc } - it "creates a new proxy order for that oc" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes before begins_at" do - let!(:oc) { upcoming_closes_before_begins_at_oc } - it "does not create a new proxy order for that oc" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - - context "and the schedule includes upcoming oc that closes on begins_at" do - let!(:oc) { upcoming_closes_on_begins_at_oc } - it "creates a new proxy order for that oc" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes on ends_at" do - let!(:oc) { upcoming_closes_on_ends_at_oc } - it "creates a new proxy order for that oc" do - expect{ syncer.sync! }.to change(ProxyOrder, :count).from(0).to(1) - expect(order_cycles).to include oc - end - end - - context "and the schedule includes upcoming oc that closes after ends_at" do - let!(:oc) { upcoming_closes_after_ends_at_oc } - it "does not create a new proxy order for that oc" do - expect{ syncer.sync! }.to_not change(ProxyOrder, :count).from(0) - expect(order_cycles).to_not include oc - end - end - end - end - end - end -end diff --git a/spec/performance/proxy_order_syncer_spec.rb b/spec/performance/proxy_order_syncer_spec.rb deleted file mode 100644 index 95678c68a2..0000000000 --- a/spec/performance/proxy_order_syncer_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'open_food_network/proxy_order_syncer' - -module OpenFoodNetwork - describe ProxyOrderSyncer, performance: true do - let(:start) { Time.zone.now.beginning_of_day } - let!(:schedule) { create(:schedule, order_cycles: order_cycles) } - - let!(:order_cycles) do - Array.new(10) do |i| - create(:simple_order_cycle, orders_open_at: start + i.days, - orders_close_at: start + (i + 1).days ) - end - end - - let!(:subscriptions) do - Array.new(150) do |_i| - create(:subscription, schedule: schedule, begins_at: start, ends_at: start + 10.days) - end - Subscription.where(schedule_id: schedule) - end - - context "measuring performance for initialisation" do - it "reports the average run time for adding 10 OCs to 150 subscriptions" do - expect(ProxyOrder.count).to be 0 - times = [] - 10.times do - syncer = ProxyOrderSyncer.new(subscriptions.reload) - - t1 = Time.zone.now - syncer.sync! - t2 = Time.zone.now - diff = t2 - t1 - times << diff - puts diff.round(2) - - expect(ProxyOrder.count).to be 1500 - ProxyOrder.destroy_all - end - puts "AVG: #{(times.sum / times.count).round(2)}" - end - end - - context "measuring performance for removal" do - it "reports the average run time for removing 8 OCs from 150 subscriptions" do - times = [] - 10.times do - syncer = ProxyOrderSyncer.new(subscriptions.reload) - syncer.sync! - expect(ProxyOrder.count).to be 1500 - subscriptions.update_all(begins_at: start + 8.days + 1.minute) - syncer = ProxyOrderSyncer.new(subscriptions.reload) - - t1 = Time.zone.now - syncer.sync! - t2 = Time.zone.now - diff = t2 - t1 - times << diff - puts diff.round(2) - - expect(ProxyOrder.count).to be 300 - subscriptions.update_all(begins_at: start) - end - puts "AVG: #{(times.sum / times.count).round(2)}" - end - end - end -end diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb index 476adb3e39..e4d5d144d1 100644 --- a/spec/services/order_cycle_form_spec.rb +++ b/spec/services/order_cycle_form_spec.rb @@ -1,3 +1,5 @@ +require 'order_management/subscriptions/proxy_order_syncer' + describe OrderCycleForm do describe "save" do describe "creating a new order cycle from params" do @@ -66,10 +68,10 @@ describe OrderCycleForm do context "where I manage the order_cycle's coordinator" do let(:form) { OrderCycleForm.new(coordinated_order_cycle, params, user) } - let(:syncer_mock) { instance_double(OpenFoodNetwork::ProxyOrderSyncer, sync!: true) } + let(:syncer_mock) { instance_double(OrderManagement::Subscriptions::ProxyOrderSyncer, sync!: true) } before do - allow(OpenFoodNetwork::ProxyOrderSyncer).to receive(:new) { syncer_mock } + allow(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new) { syncer_mock } end context "and I add an schedule that I own, and remove another that I own" do