diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 9494595048..bd710779cc 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -101,7 +101,6 @@ 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 @@ -686,6 +685,7 @@ Metrics/ModuleLength: - 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 + - engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb - lib/open_food_network/column_preference_defaults.rb - spec/controllers/admin/enterprises_controller_spec.rb - spec/controllers/admin/order_cycles_controller_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1c003c0504..e498ce594a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -70,7 +70,6 @@ Lint/DuplicateHashKey: # Offense count: 4 Lint/DuplicateMethods: Exclude: - - 'engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb' - 'lib/discourse/single_sign_on.rb' # Offense count: 10 @@ -882,9 +881,6 @@ 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' - 'engines/order_management/app/services/reports/authorizer.rb' - 'engines/order_management/app/services/reports/parameters/base.rb' @@ -904,9 +900,6 @@ 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' - 'engines/web/app/controllers/web/angular_templates_controller.rb' - 'engines/web/app/controllers/web/api/cookies_consent_controller.rb' @@ -1542,7 +1535,6 @@ Style/Send: Exclude: - 'app/controllers/spree/checkout_controller.rb' - 'app/models/spree/shipping_method_decorator.rb' - - 'engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb' - 'spec/controllers/admin/subscriptions_controller_spec.rb' - 'spec/controllers/checkout_controller_spec.rb' - 'spec/controllers/spree/admin/base_controller_spec.rb' diff --git a/app/controllers/admin/subscriptions_controller.rb b/app/controllers/admin/subscriptions_controller.rb index da99409469..0b3952d5a6 100644 --- a/app/controllers/admin/subscriptions_controller.rb +++ b/app/controllers/admin/subscriptions_controller.rb @@ -1,5 +1,4 @@ require 'open_food_network/permissions' -require 'open_food_network/proxy_order_syncer' module Admin class SubscriptionsController < ResourceController diff --git a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb index aa9a61c469..2faa93b6ee 100644 --- a/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb +++ b/engines/order_management/app/services/order_management/subscriptions/subscription_summary.rb @@ -3,7 +3,7 @@ module OrderManagement module Subscriptions class SubscriptionSummary - attr_reader :shop_id, :order_count, :success_count, :issues + attr_reader :shop_id, :issues def initialize(shop_id) @shop_id = shop_id 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 index ab01eddc4c..501f6fa67e 100644 --- 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 @@ -6,7 +6,8 @@ module OrderManagement 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 + 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 @@ -16,14 +17,46 @@ module OrderManagement 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(:closed_oc) { # Closed + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now - 1.minute, + orders_close_at: now) + } + let(:open_oc_closes_before_begins_at_oc) { # Open, but closes before begins at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now - 1.minute, + orders_close_at: now + 59.seconds) + } + let(:open_oc) { # Open & closes between begins at and ends at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now - 1.minute, + orders_close_at: now + 90.seconds) + } + let(:upcoming_closes_before_begins_at_oc) { # Upcoming, but closes before begins at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now + 30.seconds, + orders_close_at: now + 59.seconds) + } + let(:upcoming_closes_on_begins_at_oc) { # Upcoming & closes on begins at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now + 30.seconds, + orders_close_at: now + 1.minute) + } + let(:upcoming_closes_on_ends_at_oc) { # Upcoming & closes on ends at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now + 30.seconds, + orders_close_at: now + 2.minutes) + } + let(:upcoming_closes_after_ends_at_oc) { # Upcoming & closes after ends at + create(:simple_order_cycle, schedules: [schedule], + orders_open_at: now + 30.seconds, + orders_close_at: now + 121.seconds) + } + 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) } @@ -50,7 +83,7 @@ module OrderManagement end end - context "and the schedule includes an open oc that closes between begins_at and ends_at" do + context "and the schedule has 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) @@ -218,7 +251,9 @@ module OrderManagement end context "for an oc not included in the relevant schedule" do - let!(:proxy_order) { create(:proxy_order, subscription: subscription, order_cycle: open_oc) } + 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 @@ -355,7 +390,7 @@ module OrderManagement end end - context "and the schedule includes an open oc that closes between begins_at and ends_at" do + context "and the schedule has 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) diff --git a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb index 26a79af4d1..2da6d1f7fb 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/subscription_summarizer_spec.rb @@ -16,7 +16,7 @@ module OrderManagement context "when a summary for the order's distributor doesn't already exist" do it "initializes a new summary object, and returns it" do expect(summarizer.instance_variable_get(:@summaries).count).to be 0 - summary = summarizer.send(:summary_for, order) + summary = summarizer.__send__(:summary_for, order) expect(summary.shop_id).to be 123 expect(summarizer.instance_variable_get(:@summaries).count).to be 1 end @@ -31,7 +31,7 @@ module OrderManagement it "returns the existing summary object" do expect(summarizer.instance_variable_get(:@summaries).count).to be 1 - expect(summarizer.send(:summary_for, order)).to eq summary + expect(summarizer.__send__(:summary_for, order)).to eq summary expect(summarizer.instance_variable_get(:@summaries).count).to be 1 end end