From f24b20d28c7790722d7bac3251a5b5f6b0b6ceda Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 6 Sep 2018 22:45:22 +0100 Subject: [PATCH 1/4] Added adapted implementation of order.shipping_method to order_decorator --- app/models/spree/order_decorator.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 458742ebf4a..779162b9b2d 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -333,6 +333,13 @@ def pending_payments payments.select {|p| p.state == "checkout"} # Original definition end + # Although Spree 2 supports multi shipments per order, in OFN we keep the rule one shipment per order + # Thus, this method returns the shipping method of the first and only shipment in the order + def shipping_method + return if shipments.empty? + shipments.first.shipping_method + end + private def address_from_distributor From 62951f7d483aef34e0be21bf05c153915ae69679 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 10 Sep 2018 15:16:19 +0100 Subject: [PATCH 2/4] Removing duplicated factories inserted by previous PRs: 2668 and 2670 --- spec/factories.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 4824f5be6b4..9c0a88e3181 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -280,16 +280,6 @@ after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } end - factory :shipping_method_with_flat_rate, parent: :shipping_method do - calculator { Spree::Calculator::FlatRate.new(preferred_amount: 50.0) } - end - - factory :shipment_with_flat_rate, parent: :shipment do - after(:create) do |shipment| - shipment.add_shipping_method(create(:shipping_method_with_flat_rate), true) - end - end - factory :order_with_totals_and_distribution, :parent => :order do #possibly called :order_with_line_items in newer Spree distributor { create(:distributor_enterprise) } order_cycle { create(:simple_order_cycle) } From 3577f3790df980eea1ff91f4b22093a71da3e7e2 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 10 Sep 2018 16:50:34 +0100 Subject: [PATCH 3/4] Moved order.shipping_method to the OrderShippingMethod concern --- app/models/concerns/order_shipping_method.rb | 21 ++++++++++++++++++ app/models/spree/order_decorator.rb | 10 +++------ .../concerns/order_shipping_method_spec.rb | 22 +++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 app/models/concerns/order_shipping_method.rb create mode 100644 spec/models/concerns/order_shipping_method_spec.rb diff --git a/app/models/concerns/order_shipping_method.rb b/app/models/concerns/order_shipping_method.rb new file mode 100644 index 00000000000..75e729f24e9 --- /dev/null +++ b/app/models/concerns/order_shipping_method.rb @@ -0,0 +1,21 @@ +require 'active_support/concern' + +# This module is an adapter for OFN to work with Spree 2 code. +# +# Although Spree 2 supports multiple shipments per order, in OFN we have only one shipment per order. +# A shipment is associated to a shipping_method through a selected shipping_rate. +# See https://github.com/openfoodfoundation/openfoodnetwork/wiki/Spree-Upgrade:-Migration-to-multiple-shipments +# for details. +# +# Methods in this module may become deprecated. +module OrderShippingMethod + extend ActiveSupport::Concern + + # Returns the shipping method of the first and only shipment in the order + # + # @return [ShippingMethod] + def shipping_method + return if shipments.empty? + shipments.first.shipping_method + end +end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 779162b9b2d..8a88078b05d 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -2,12 +2,15 @@ require 'open_food_network/distribution_change_validator' require 'open_food_network/feature_toggle' require 'open_food_network/tag_rule_applicator' +require 'concerns/order_shipping_method' ActiveSupport::Notifications.subscribe('spree.order.contents_changed') do |name, start, finish, id, payload| payload[:order].reload.update_distribution_charge! end Spree::Order.class_eval do + include OrderShippingMethod + belongs_to :order_cycle belongs_to :distributor, class_name: 'Enterprise' belongs_to :customer @@ -333,13 +336,6 @@ def pending_payments payments.select {|p| p.state == "checkout"} # Original definition end - # Although Spree 2 supports multi shipments per order, in OFN we keep the rule one shipment per order - # Thus, this method returns the shipping method of the first and only shipment in the order - def shipping_method - return if shipments.empty? - shipments.first.shipping_method - end - private def address_from_distributor diff --git a/spec/models/concerns/order_shipping_method_spec.rb b/spec/models/concerns/order_shipping_method_spec.rb new file mode 100644 index 00000000000..86c178e1158 --- /dev/null +++ b/spec/models/concerns/order_shipping_method_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe OrderShippingMethod do + let(:order) { create(:order) } + + describe '#shipping_method' do + context 'when order has no shipments' do + it 'returns nil' do + expect(order.shipping_method).to be_nil + end + end + + context 'when order has single shipment' do + it 'returns the shipments shipping_method' do + shipment = create(:shipment_with_flat_rate) + order.shipments = [shipment] + + expect(order.shipping_method).to eq shipment.shipping_method + end + end + end +end From c00e6a5dc7aadcb9665ba162f81f73f971f723c3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 10 Sep 2018 17:02:50 +0100 Subject: [PATCH 4/4] Added DB uniqueness constraint on order_id to shipments By forbidding more than a row per order in the spree_shipments table we ensure all orders have no more than one shipment associated --- ...0155506_add_uniqueness_of_order_id_to_spree_shipments.rb | 6 ++++++ db/schema.rb | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20180910155506_add_uniqueness_of_order_id_to_spree_shipments.rb diff --git a/db/migrate/20180910155506_add_uniqueness_of_order_id_to_spree_shipments.rb b/db/migrate/20180910155506_add_uniqueness_of_order_id_to_spree_shipments.rb new file mode 100644 index 00000000000..21ad966ad49 --- /dev/null +++ b/db/migrate/20180910155506_add_uniqueness_of_order_id_to_spree_shipments.rb @@ -0,0 +1,6 @@ +class AddUniquenessOfOrderIdToSpreeShipments < ActiveRecord::Migration + def change + remove_index :spree_shipments, :order_id + add_index :spree_shipments, :order_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d7f1677246..007691a87ca 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20180812214434) do +ActiveRecord::Schema.define(:version => 20180910155506) do create_table "account_invoices", :force => true do |t| t.integer "user_id", :null => false @@ -876,7 +876,7 @@ end add_index "spree_shipments", ["number"], :name => "index_shipments_on_number" - add_index "spree_shipments", ["order_id"], :name => "index_spree_shipments_on_order_id" + add_index "spree_shipments", ["order_id"], :name => "index_spree_shipments_on_order_id", :unique => true create_table "spree_shipping_categories", :force => true do |t| t.string "name"