From 48c2e48b24fae141ce0960d163585e4aa2d2632d Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Wed, 8 Jun 2022 20:29:21 +0100 Subject: [PATCH] Update the OrderCycleForm service so it supports attaching shipping methods --- app/services/order_cycle_form.rb | 20 +++- spec/services/order_cycle_form_spec.rb | 140 +++++++++++++++++++++---- 2 files changed, 141 insertions(+), 19 deletions(-) diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb index 6e96b9af68..ba6aa590e7 100644 --- a/app/services/order_cycle_form.rb +++ b/app/services/order_cycle_form.rb @@ -11,11 +11,13 @@ class OrderCycleForm @user = user @permissions = OpenFoodNetwork::Permissions.new(user) @schedule_ids = order_cycle_params.delete(:schedule_ids) + @shipping_method_ids = order_cycle_params.delete(:shipping_method_ids) end def save schedule_ids = build_schedule_ids order_cycle.assign_attributes(order_cycle_params) + order_cycle.validate_shipping_methods = false return false unless order_cycle.valid? order_cycle.transaction do @@ -23,10 +25,12 @@ class OrderCycleForm order_cycle.schedule_ids = schedule_ids order_cycle.save! apply_exchange_changes + attach_shipping_methods sync_subscriptions true end - rescue ActiveRecord::RecordInvalid + rescue ActiveRecord::RecordInvalid => e + add_exception_to_order_cycle_errors(e) false end @@ -34,12 +38,26 @@ class OrderCycleForm attr_accessor :order_cycle, :order_cycle_params, :user, :permissions + def add_exception_to_order_cycle_errors(exception) + error = exception.message.split(":").last.strip + order_cycle.errors.add(:base, error) if !order_cycle.errors.to_a.include?(error) + end + def apply_exchange_changes return if exchanges_unchanged? OpenFoodNetwork::OrderCycleFormApplicator.new(order_cycle, user).go! end + def attach_shipping_methods + return if @shipping_method_ids.nil? + + order_cycle.reload # so outgoing exchanges are up-to-date for shipping method validations + order_cycle.validate_shipping_methods = true + order_cycle.shipping_method_ids = @shipping_method_ids + order_cycle.save! + end + def exchanges_unchanged? [:incoming_exchanges, :outgoing_exchanges].all? do |direction| order_cycle_params[direction].nil? diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb index 4d5bce792c..c7b472257d 100644 --- a/spec/services/order_cycle_form_spec.rb +++ b/spec/services/order_cycle_form_spec.rb @@ -119,34 +119,138 @@ describe OrderCycleForm do end end - describe "updating exchanges" do - let(:user) { instance_double(Spree::User) } - let(:order_cycle) { create(:simple_order_cycle) } - let(:form_applicator_mock) { instance_double(OpenFoodNetwork::OrderCycleFormApplicator) } - let(:form) { OrderCycleForm.new(order_cycle, params, user) } + context "distributor order cycle" do + let(:order_cycle) { create(:distributor_order_cycle) } + let(:distributor) { order_cycle.coordinator } + let(:supplier) { create(:supplier_enterprise) } + let(:user) { distributor.owner } + let(:shipping_method) { create(:shipping_method, distributors: [distributor]) } + let(:variant) { create(:variant, product: create(:product, supplier: supplier)) } let(:params) { { name: 'Some new name' } } - - before do - allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock } - allow(form_applicator_mock).to receive(:go!) + let(:form) { OrderCycleForm.new(order_cycle, params, user) } + let(:outgoing_exchange_params) do + { + enterprise_id: distributor.id, + incoming: false, + active: true, + variants: { variant.id => true }, + pickup_time: "Saturday morning", + enterprise_fee_ids: [] + } end - context "when exchange params are provided" do - let(:exchange_params) { { incoming_exchanges: [], outgoing_exchanges: [] } } - before { params.merge!(exchange_params) } - - it "runs the OrderCycleFormApplicator, and saves other changes" do + context "basic update i.e. without exchanges or shipping methods" do + it do expect(form.save).to be true - expect(form_applicator_mock).to have_received(:go!) expect(order_cycle.name).to eq 'Some new name' end end - context "when no exchange params are provided" do - it "does not run the OrderCycleFormApplicator, but saves other changes" do + context "updating basics, incoming and outcoming exchanges, shipping methods simultaneously" do + before do + params.merge!( + incoming_exchanges: [{ + enterprise_id: supplier.id, + incoming: true, + active: true, + variants: { variant.id => true }, + receival_instructions: "Friday evening", + enterprise_fee_ids: [] + }], + outgoing_exchanges: [outgoing_exchange_params], + shipping_method_ids: [shipping_method.id] + ) + end + + it "saves everything i.e. the basics, incoming and outgoing exchanges and shipping methods" do expect(form.save).to be true - expect(form_applicator_mock).to_not have_received(:go!) expect(order_cycle.name).to eq 'Some new name' + expect(order_cycle.cached_incoming_exchanges.count).to eq 1 + expect(order_cycle.cached_outgoing_exchanges.count).to eq 1 + expect(order_cycle.shipping_methods).to eq [shipping_method] + end + end + + context "updating outgoing exchanges without specifying any shipping methods" do + before do + params.merge!( + outgoing_exchanges: [outgoing_exchange_params], + shipping_method_ids: nil + ) + end + + it "saves the outgoing exchanges, + it doesn't return a validation error because no shipping methods are present yet" do + expect(form.save).to be true + expect(order_cycle.cached_outgoing_exchanges.count).to eq 1 + end + end + + context "updating outgoing exchanges but specifying an invalid shipping method" do + let(:other_distributor_shipping_method) do + create(:shipping_method, distributors: [create(:distributor_enterprise)]) + end + + before do + params.merge!( + outgoing_exchanges: [outgoing_exchange_params], + shipping_method_ids: [other_distributor_shipping_method.id] + ) + end + + it "returns a validation error" do + expect(form.save).to be false + expect(order_cycle.errors.to_a).to eq [ + "Shipping method must be from a distributor on the order cycle" + ] + end + end + + context "when shipping methods already exist + and doing an update without the :shipping_methods_id parameter" do + it "doesn't return a validation error on shipping methods" do + order_cycle = create(:distributor_order_cycle, with_distributor_and_shipping_method: true) + + form = OrderCycleForm.new( + order_cycle, + params.except(:shipping_method_ids), + order_cycle.coordinator + ) + + expect(form.save).to be true + end + end + + context "updating shipping methods" do + context "and it's valid" do + it "saves the changes" do + distributor = create(:distributor_enterprise) + shipping_method = create(:shipping_method, distributors: [distributor]) + order_cycle = create(:distributor_order_cycle, distributors: [distributor]) + + form = OrderCycleForm.new(order_cycle, + { shipping_method_ids: [shipping_method.id] }, + order_cycle.coordinator) + + expect(form.save).to be true + expect(order_cycle.shipping_methods).to eq [shipping_method] + end + end + + context "and it's invalid" do + it "returns a validation error" do + distributor = create(:distributor_enterprise) + order_cycle = create(:distributor_order_cycle, distributors: [distributor]) + + form = OrderCycleForm.new(order_cycle, + { shipping_method_ids: [] }, + order_cycle.coordinator) + + expect(form.save).to be false + expect(order_cycle.errors.to_a).to eq [ + "You need to select at least one shipping method for each distributor" + ] + end end end end