From 9a6e8a111357aab4d62552886bbc8d1d0b9932cd Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Wed, 8 Jun 2022 20:10:34 +0100 Subject: [PATCH] Add an OrderCycleShippingMethod model to handle attaching shipping methods to order cycles --- app/models/order_cycle_shipping_method.rb | 58 ++++++++ config/locales/en.yml | 10 ++ ...052_create_order_cycle_shipping_methods.rb | 98 +++++++++++++ db/schema.rb | 11 ++ spec/factories/order_cycle_factory.rb | 12 ++ .../order_cycle_shipping_method_spec.rb | 130 ++++++++++++++++++ 6 files changed, 319 insertions(+) create mode 100644 app/models/order_cycle_shipping_method.rb create mode 100644 db/migrate/20220429092052_create_order_cycle_shipping_methods.rb create mode 100644 spec/models/order_cycle_shipping_method_spec.rb diff --git a/app/models/order_cycle_shipping_method.rb b/app/models/order_cycle_shipping_method.rb new file mode 100644 index 0000000000..dbdd0ccd0d --- /dev/null +++ b/app/models/order_cycle_shipping_method.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class OrderCycleShippingMethod < ActiveRecord::Base + belongs_to :order_cycle + belongs_to :shipping_method, class_name: "Spree::ShippingMethod" + + validate :shipping_method_belongs_to_order_cycle_distributor + validate :shipping_method_available_at_checkout + validate :order_cycle_not_simple + validate :order_cycle_shipping_methods_customisable + validates_uniqueness_of :shipping_method, scope: :order_cycle_id + + before_destroy :check_shipping_method_not_selected_on_any_orders + + private + + def shipping_method_not_selected_on_any_orders? + !Spree::Order.joins(shipments: :shipping_rates).where( + "order_cycle_id = ? AND spree_shipping_rates.shipping_method_id = ?", + order_cycle_id, shipping_method_id + ).exists? + end + + def check_shipping_method_not_selected_on_any_orders + return if order_cycle.nil? || + shipping_method.nil? || + shipping_method_not_selected_on_any_orders? + + errors.add(:base, :shipping_method_already_used_in_order_cycle) + throw :abort + end + + def order_cycle_not_simple + return if order_cycle.nil? || !order_cycle.simple? + + errors.add(:order_cycle, :must_not_be_simple) + end + + def order_cycle_shipping_methods_customisable + return if order_cycle.nil? || order_cycle.shipping_methods_customisable? + + errors.add(:order_cycle, :must_support_customisable_shipping_methods) + end + + def shipping_method_available_at_checkout + return if shipping_method.nil? || shipping_method.frontend? + + errors.add(:shipping_method, :must_be_available_at_checkout) + end + + def shipping_method_belongs_to_order_cycle_distributor + return if order_cycle.nil? || + shipping_method.nil? || + shipping_method.distributors.where(id: order_cycle.distributor_ids).exists? + + errors.add(:shipping_method, :must_belong_to_order_cycle_distributor) + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index be87cc9fab..1feb920654 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -77,6 +77,16 @@ en: attributes: orders_close_at: after_orders_open_at: must be after open date + order_cycle_shipping_method: + attributes: + base: + shipping_method_already_used_in_order_cycle: "This shipping method has already been selected on orders in this order cycle and cannot be removed" + order_cycle: + must_not_be_simple: "is simple, all shipping methods are available by default and cannot be customised" + must_support_customisable_shipping_methods: "shipping methods cannot be customised, all shipping methods are available by default" + shipping_method: + must_be_available_at_checkout: "must be available at checkout" + must_belong_to_order_cycle_distributor: "must be from a distributor on the order cycle" variant_override: count_on_hand: using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings" diff --git a/db/migrate/20220429092052_create_order_cycle_shipping_methods.rb b/db/migrate/20220429092052_create_order_cycle_shipping_methods.rb new file mode 100644 index 0000000000..ae78acc94a --- /dev/null +++ b/db/migrate/20220429092052_create_order_cycle_shipping_methods.rb @@ -0,0 +1,98 @@ +class CreateOrderCycleShippingMethods < ActiveRecord::Migration[6.1] + # Before this migration every available shipping method was available to customers on order + # cycles by default. However this migration only populates :order_cycles_shipping_methods records + # for active or upcoming order cycles because retroactively calculating which shipping methods + # should be attached to past, closed order cycles is probaby tricky so skipping that because it + # may not be even necessary. Instead this adds a :shipping_methods_customisable flag to order + # cycles so we have a record of order cycles created before this feature was deployed. + # + # Note: Redefining the Spree::ShippingMethod class in this migration as suggested by the + # :good_migrations gem was not passing Good Migrations checks. This redefines the classes inside + # a Migration class to to bypass this problem. + + class Migration + class DistributorShippingMethod < ActiveRecord::Base + self.table_name = "distributors_shipping_methods" + belongs_to :shipping_method, class_name: "Migration::ShippingMethod", touch: true + belongs_to :distributor, class_name: "Enterprise", touch: true + end + + class Enterprise < ActiveRecord::Base + end + + class Exchange < ActiveRecord::Base + self.table_name = "exchanges" + belongs_to :receiver, class_name: 'Migration::Enterprise' + end + + class OrderCycle < ActiveRecord::Base + self.table_name = "order_cycles" + has_many :order_cycle_shipping_methods, class_name: "Migration::OrderCycleShippingMethod" + has_many :shipping_methods, class_name: "Migration::ShippingMethod", through: :order_cycle_shipping_methods + + has_many :cached_outgoing_exchanges, -> { where incoming: false }, class_name: "Migration::Exchange" + has_many :distributors, -> { distinct }, source: :receiver, through: :cached_outgoing_exchanges + + belongs_to :coordinator, class_name: 'Migration::Enterprise' + + scope :active, lambda { + where('order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?', + Time.zone.now, + Time.zone.now) + } + scope :upcoming, lambda { where('order_cycles.orders_open_at > ?', Time.zone.now) } + end + + class OrderCycleShippingMethod < ActiveRecord::Base + self.table_name = "order_cycle_shipping_methods" + belongs_to :shipping_method, class_name: "Migration::ShippingMethod" + end + + class ShippingMethod < ActiveRecord::Base + self.table_name = "spree_shipping_methods" + end + + def self.attach_all_shipping_methods_to_non_simple_active_or_upcoming_order_cycles + non_simple_active_or_upcoming_order_cycles.find_each do |order_cycle| + order_cycle.shipping_method_ids = DistributorShippingMethod. + where("display_on != 'back_end'"). + where(distributor_id: order_cycle.distributor_ids). + joins(:shipping_method). + pluck(:shipping_method_id) + end + end + + def self.set_shipping_methods_customisable_to_false_on_past_order_cycles + OrderCycle.update_all(shipping_methods_customisable: false) + active_or_upcoming_order_cycles.update_all(shipping_methods_customisable: true) + end + + private + + def self.active_or_upcoming_order_cycles + OrderCycle.active.or(OrderCycle.upcoming) + end + + def self.non_simple_active_or_upcoming_order_cycles + active_or_upcoming_order_cycles.joins(:coordinator).where("sells != 'own'") + end + end + + def up + create_table :order_cycle_shipping_methods do |t| + t.references :order_cycle + t.references :shipping_method, foreign_key: { to_table: :spree_shipping_methods } + t.timestamps + end + + add_column :order_cycles, :shipping_methods_customisable, :boolean, default: true + + Migration.set_shipping_methods_customisable_to_false_on_past_order_cycles + Migration.attach_all_shipping_methods_to_non_simple_active_or_upcoming_order_cycles + end + + def down + remove_column :order_cycles, :shipping_methods_customisable + drop_table :order_cycle_shipping_methods + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c9b1b00fe..bca8dabe52 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -318,6 +318,15 @@ ActiveRecord::Schema.define(version: 2022_09_07_055044) do t.index ["schedule_id"], name: "index_order_cycle_schedules_on_schedule_id" end + create_table "order_cycle_shipping_methods", force: :cascade do |t| + t.bigint "order_cycle_id" + t.bigint "shipping_method_id" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["order_cycle_id"], name: "index_order_cycle_shipping_methods_on_order_cycle_id" + t.index ["shipping_method_id"], name: "index_order_cycle_shipping_methods_on_shipping_method_id" + end + create_table "order_cycles", id: :serial, force: :cascade do |t| t.string "name", limit: 255 t.datetime "orders_open_at" @@ -328,6 +337,7 @@ ActiveRecord::Schema.define(version: 2022_09_07_055044) do t.datetime "processed_at" t.boolean "automatic_notifications", default: false t.boolean "mails_sent", default: false + t.boolean "shipping_methods_customisable", default: true end create_table "producer_properties", id: :serial, force: :cascade do |t| @@ -1253,6 +1263,7 @@ ActiveRecord::Schema.define(version: 2022_09_07_055044) do add_foreign_key "exchanges", "order_cycles", name: "exchanges_order_cycle_id_fk" add_foreign_key "order_cycle_schedules", "order_cycles", name: "oc_schedules_order_cycle_id_fk" add_foreign_key "order_cycle_schedules", "schedules", name: "oc_schedules_schedule_id_fk" + add_foreign_key "order_cycle_shipping_methods", "spree_shipping_methods", column: "shipping_method_id" add_foreign_key "order_cycles", "enterprises", column: "coordinator_id", name: "order_cycles_coordinator_id_fk" add_foreign_key "producer_properties", "enterprises", column: "producer_id", name: "producer_properties_producer_id_fk" add_foreign_key "producer_properties", "spree_properties", column: "property_id", name: "producer_properties_property_id_fk" diff --git a/spec/factories/order_cycle_factory.rb b/spec/factories/order_cycle_factory.rb index 8dbfcc6acd..469fb94019 100644 --- a/spec/factories/order_cycle_factory.rb +++ b/spec/factories/order_cycle_factory.rb @@ -59,6 +59,10 @@ FactoryBot.define do end end + # Note: Order cycles are sometimes referred to as 'simple if they are for a shop selling their + # own produce i.e. :sells = 'own'. However the 'simple_order_cycle' name does not mean this + # and may need to be renamed to avoid potential confusion because it actually can create + # 'non-simple' order cycles too for distributors selling produce from other enterprises. factory :simple_order_cycle, class: OrderCycle do sequence(:name) { |n| "Order Cycle #{n}" } @@ -116,4 +120,12 @@ FactoryBot.define do orders_open_at { 2.weeks.ago } orders_close_at { 1.week.ago } end + + factory :distributor_order_cycle, parent: :simple_order_cycle do + coordinator { create(:distributor_enterprise) } + end + + factory :sells_own_order_cycle, parent: :simple_order_cycle do + coordinator { create(:enterprise, sells: "own") } + end end diff --git a/spec/models/order_cycle_shipping_method_spec.rb b/spec/models/order_cycle_shipping_method_spec.rb new file mode 100644 index 0000000000..fcc5fd5290 --- /dev/null +++ b/spec/models/order_cycle_shipping_method_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe OrderCycleShippingMethod do + it "is valid when the shipping method is available at checkout" do + shipping_method = create(:shipping_method, display_on: nil) + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).to be_valid + + shipping_method.display_on = "both" + + expect(order_cycle_shipping_method).to be_valid + end + + it "is not valid when the shipping method is only available in the backoffice" do + shipping_method = create(:shipping_method, display_on: "back_end") + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).to_not be_valid + expect(order_cycle_shipping_method.errors.to_a).to include( + "Shipping method must be available at checkout" + ) + end + + it "is not valid if the order cycle is simple i.e. :sells is 'own'" do + order_cycle = create(:sells_own_order_cycle) + shipping_method = create(:shipping_method, distributors: [order_cycle.coordinator]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).to_not be_valid + expect(order_cycle_shipping_method.errors.to_a).to include( + "Order cycle is simple, all shipping methods are available by default and cannot be customised" + ) + end + + it "is not valid if order cycle doesn't support customised shipping methods + e.g. the order cycle was created before the custom shipping methods feature was available" do + order_cycle = create(:distributor_order_cycle, shipping_methods_customisable: false) + shipping_method = create(:shipping_method, distributors: [order_cycle.coordinator]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).to_not be_valid + expect(order_cycle_shipping_method.errors.to_a).to include( + "Order cycle shipping methods cannot be customised, all shipping methods are available by default" + ) + end + + it "is valid if the shipping method belongs to one of the order cycle distributors" do + shipping_method = create(:shipping_method) + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).to be_valid + end + + it "is not valid if the shipping method does not belong to one of the order cycle distributors" do + shipping_method = create(:shipping_method) + enterprise = create(:enterprise) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + + order_cycle_shipping_method = OrderCycleShippingMethod.new( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + + expect(order_cycle_shipping_method).not_to be_valid + expect(order_cycle_shipping_method.errors.to_a).to eq [ + "Shipping method must be from a distributor on the order cycle" + ] + end + + it "can be destroyed if the shipping method hasn't been used on any orders in the order cycle" do + shipping_method = create(:shipping_method) + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + + order_cycle_shipping_method = OrderCycleShippingMethod.create!( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + order_cycle_shipping_method.destroy + + expect(order_cycle_shipping_method).to be_destroyed + end + + it "cannot be destroyed if the shipping method has been used on some orders in the order cycle" do + shipping_method = create(:shipping_method) + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + order_cycle = create(:simple_order_cycle, distributors: [enterprise]) + order = create(:order_ready_for_payment, distributor: enterprise, order_cycle: order_cycle) + + order_cycle_shipping_method = OrderCycleShippingMethod.create!( + order_cycle: order_cycle, + shipping_method: shipping_method + ) + order_cycle_shipping_method.destroy + + expect(order_cycle_shipping_method).not_to be_destroyed + expect(order_cycle_shipping_method.errors.to_a).to eq [ + "This shipping method has already been selected on orders in this order cycle and cannot be removed" + ] + end +end