raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Only require OrderCycleShippingMethod records if people want to override the default shipping methods

Browse Source 
It makes things much simpler if we return all shipping methods by default without needing OrderCycleShippingMethod records to be added to the database.

Co-authored-by: Maikel <maikel@email.org.au>
This commit is contained in:
Cillian O'Ruanaidh
2022-06-17 10:48:36 +01:00
committed by Filipe
parent 4e0bf75ecf
commit fc4f951a1a
16 changed files with 243 additions and 369 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
app/models/order_cycle.rb
View File

@@ -25,20 +25,19 @@ class OrderCycle < ApplicationRecord
has_many :order_cycle_schedules
has_many :schedules, through: :order_cycle_schedules
has_many :order_cycle_shipping_methods
has_many :shipping_methods, class_name: "Spree::ShippingMethod",
through: :order_cycle_shipping_methods
has_many :preferred_shipping_methods, class_name: "Spree::ShippingMethod",
through: :order_cycle_shipping_methods,
source: :shipping_method
has_paper_trail meta: { custom_data: proc { |order_cycle| order_cycle.schedule_ids.to_s } }
attr_accessor :incoming_exchanges, :outgoing_exchanges
attribute :validate_shipping_methods, :boolean, default: true
before_update :reset_processed_at, if: :will_save_change_to_orders_close_at?
after_save :sync_subscriptions, if: :opening?
validates :name, :coordinator_id, presence: true
validate :at_least_one_shipping_method_selected_for_each_distributor
validate :no_invalid_shipping_methods
validate :no_invalid_order_cycle_shipping_methods
validate :orders_close_at_after_orders_open_at?
preference :product_selection_from_coordinator_inventory_only, :boolean, default: false
@@ -165,8 +164,6 @@ class OrderCycle < ApplicationRecord
end
def attachable_shipping_methods
return Spree::ShippingMethod.none if simple? || !shipping_methods_customisable?
Spree::ShippingMethod.frontend.
joins(:distributor_shipping_methods).
where("distributor_id IN (?)", distributor_ids).
@@ -182,12 +179,9 @@ class OrderCycle < ApplicationRecord
oc.preferred_product_selection_from_coordinator_inventory_only = preferred_product_selection_from_coordinator_inventory_only
# rubocop:enable Layout/LineLength
oc.schedule_ids = schedule_ids
oc.shipping_methods_customisable = true
oc.save!
oc.validate_shipping_methods = false
exchanges.each { |e| e.clone!(oc) }
oc.validate_shipping_methods = true
oc.shipping_method_ids = shipping_method_ids
oc.preferred_shipping_method_ids = preferred_shipping_method_ids
sync_subscriptions
oc.reload
end
@@ -301,6 +295,14 @@ class OrderCycle < ApplicationRecord
items.each { |li| scoper.scope(li.variant) }
end
def shipping_methods
if simple? || preferred_shipping_methods.none?
attachable_shipping_methods
else
preferred_shipping_methods
end
end
def simple?
coordinator.sells == 'own'
end
@@ -308,20 +310,21 @@ class OrderCycle < ApplicationRecord
private
def all_distributors_have_at_least_one_shipping_method?
distributors.all? { |distributor| (distributor.shipping_method_ids & shipping_method_ids).any? }
distributors.all? do |distributor|
(distributor.shipping_method_ids & preferred_shipping_method_ids).any?
end
end
def at_least_one_shipping_method_selected_for_each_distributor
return if !validate_shipping_methods? ||
return if preferred_shipping_methods.none? ||
coordinator.nil? ||
simple? ||
!shipping_methods_customisable? ||
all_distributors_have_at_least_one_shipping_method?
errors.add(:base, :at_least_one_shipping_method_per_distributor)
end
def no_invalid_shipping_methods
def no_invalid_order_cycle_shipping_methods
return if order_cycle_shipping_methods.all?(&:valid?)
errors.add(

7
app/models/order_cycle_shipping_method.rb
View File

@@ -7,7 +7,6 @@ class OrderCycleShippingMethod < ActiveRecord::Base
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
@@ -36,12 +35,6 @@ class OrderCycleShippingMethod < ActiveRecord::Base
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?

20
app/models/spree/shipping_method.rb
View File

@@ -115,18 +115,14 @@ module Spree
private
def no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method?
def no_active_or_upcoming_order_cycle_distributors_with_only_one_shipping_method?
return true if new_record?
OrderCycle.active.or(OrderCycle.upcoming).joins(:coordinator, :shipping_methods).where("
sells != 'own' AND
spree_shipping_methods.id = ? AND
NOT EXISTS(
SELECT 1
FROM order_cycle_shipping_methods
WHERE order_cycle_id = order_cycles.id AND
shipping_method_id != ?
)", id, id).none?
distributors.
with_order_cycles_as_distributor_outer.
merge(OrderCycle.active.or(OrderCycle.upcoming)).none? do |distributor|
distributor.shipping_method_ids.one?
end
end
def at_least_one_shipping_category
@@ -146,7 +142,7 @@ module Spree
end
def check_destroy_wont_leave_order_cycles_without_shipping_methods
return if no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method?
return if no_active_or_upcoming_order_cycle_distributors_with_only_one_shipping_method?
errors.add(:base, :destroy_leaves_order_cycles_without_shipping_methods)
throw :abort
@@ -154,7 +150,7 @@ module Spree
def switching_to_backoffice_only_wont_leave_order_cycles_without_shipping_methods
return if frontend? ||
no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method?
no_active_or_upcoming_order_cycle_distributors_with_only_one_shipping_method?
errors.add(:base, :switching_to_backoffice_only_leaves_order_cycles_without_shipping_methods)
end

22
app/services/order_cycle_form.rb
View File

@@ -11,13 +11,12 @@ 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)
@preferred_shipping_method_ids = order_cycle_params.delete(:preferred_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
@@ -25,7 +24,7 @@ class OrderCycleForm
order_cycle.schedule_ids = schedule_ids
order_cycle.save!
apply_exchange_changes
attach_shipping_methods
attach_preferred_shipping_methods
sync_subscriptions
true
end
@@ -49,12 +48,11 @@ class OrderCycleForm
OpenFoodNetwork::OrderCycleFormApplicator.new(order_cycle, user).go!
end
def attach_shipping_methods
return if @shipping_method_ids.nil?
def attach_preferred_shipping_methods
return if @preferred_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.preferred_shipping_method_ids = preferred_shipping_method_ids
order_cycle.save!
end
@@ -64,6 +62,16 @@ class OrderCycleForm
end
end
def preferred_shipping_method_ids
@preferred_shipping_method_ids = @preferred_shipping_method_ids.reject(&:blank?).map(&:to_i)
if order_cycle.attachable_shipping_methods.map(&:id).sort == @preferred_shipping_method_ids.sort
@preferred_shipping_method_ids = []
end
@preferred_shipping_method_ids
end
def schedule_ids?
@schedule_ids.present?
end

2
app/services/permitted_attributes/order_cycle.rb
View File

@@ -17,7 +17,7 @@ module PermittedAttributes
:name, :orders_open_at, :orders_close_at, :coordinator_id,
:preferred_product_selection_from_coordinator_inventory_only,
:automatic_notifications,
{ schedule_ids: [], shipping_method_ids: [], coordinator_fee_ids: [] }
{ schedule_ids: [], preferred_shipping_method_ids: [], coordinator_fee_ids: [] }
]
end

2
app/views/admin/order_cycles/checkout_options.html.haml
View File

@@ -13,6 +13,8 @@
%fieldset.no-border-bottom
%legend{ align: 'center'}= t('.checkout_options')
= hidden_field_tag "order_cycle[preferred_shipping_method_ids][]", ""
%table.checkout-options
%thead
%tr

1
config/locales/en.yml
View File

@@ -90,7 +90,6 @@ en:
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"

89
db/migrate/20220429092052_create_order_cycle_shipping_methods.rb
View File

@@ -1,98 +1,17 @@
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
add_index :order_cycle_shipping_methods,
[:order_cycle_id, :shipping_method_id],
name: "order_cycle_shipping_methods_join_index",
unique: true
end
def down
remove_column :order_cycles, :shipping_methods_customisable
drop_table :order_cycle_shipping_methods
end
end

2
db/schema.rb
View File

@@ -323,6 +323,7 @@ ActiveRecord::Schema.define(version: 2022_09_07_055044) do
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", "shipping_method_id"], name: "order_cycle_shipping_methods_join_index", unique: true
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
@@ -337,7 +338,6 @@ 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|

3
lib/tasks/sample_data/order_cycle_factory.rb
View File

@@ -57,9 +57,6 @@ module SampleData
log "- #{name}"
cycle = create_order_cycle_with_fee(name, coordinator)
create_exchanges(cycle, supplier_names, distributor_names, data)
return if cycle.reload.attachable_shipping_methods.none?
cycle.shipping_method_ids = cycle.attachable_shipping_methods.pluck(:id)
end
def create_order_cycle_with_fee(name, coordinator)

8
spec/factories/subscription_factory.rb
View File

@@ -3,13 +3,7 @@
FactoryBot.define do
factory :subscription, class: Subscription do
shop { create :enterprise }
schedule do
order_cycle = create(:distributor_order_cycle,
coordinator: shop,
distributors: [shop],
shipping_methods: [shipping_method])
create(:schedule, order_cycles: [order_cycle])
end
schedule { create(:schedule, order_cycles: [create(:simple_order_cycle, coordinator: shop)]) }
customer { create(:customer, enterprise: shop) }
bill_address { create(:address, :randomized) }
ship_address { create(:address, :randomized) }

16
spec/models/order_cycle_shipping_method_spec.rb
View File

@@ -51,22 +51,6 @@ describe OrderCycleShippingMethod do
)
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])

98
spec/models/order_cycle_spec.rb
View File

@@ -28,9 +28,13 @@ describe OrderCycle do
distributor_ii = create(:distributor_enterprise)
distributor_i_shipping_method = create(:shipping_method, distributors: [distributor_i])
distributor_ii_shipping_method = create(:shipping_method, distributors: [distributor_ii])
order_cycle = create(:distributor_order_cycle, distributors: [distributor_i, distributor_ii])
order_cycle = create(:distributor_order_cycle,
distributors: [distributor_i, distributor_ii])
order_cycle.shipping_method_ids = [distributor_i_shipping_method.id, distributor_ii_shipping_method.id]
order_cycle.preferred_shipping_method_ids = [
distributor_i_shipping_method.id,
distributor_ii_shipping_method.id
]
expect(order_cycle).to be_valid
end
@@ -43,19 +47,24 @@ describe OrderCycle do
distributor_ii_shipping_method = create(:shipping_method, distributors: [distributor_i])
order_cycle = create(:distributor_order_cycle, distributors: [distributor_i, distributor_ii])
order_cycle.shipping_method_ids = [distributor_i_shipping_method.id]
order_cycle.preferred_shipping_method_ids = [distributor_i_shipping_method.id]
expect(order_cycle).to be_invalid
expect(order_cycle.errors.to_a).to eq ["You need to select at least one shipping method for each distributor"]
expect(order_cycle.errors.to_a).to eq [
"You need to select at least one shipping method for each distributor"
]
end
end
end
describe "#no_invalid_shipping_methods" do
context "when a shipping method is not valid" do
it "adds a validation error, and it is more meaningful than the default 'Order cycle shipping methods is invalid'" do
order_cycle = create(:distributor_order_cycle, with_distributor_and_shipping_method: true)
shipping_method = order_cycle.shipping_methods.first
describe "#no_invalid_order_cycle_shipping_methods" do
context "when a order cycle shipping method is not valid" do
it "adds a validation error,
and it is more meaningful than the default 'Order cycle shipping methods is invalid'" do
shipping_method = create(:shipping_method)
distributor = create(:distributor_enterprise, shipping_methods: [shipping_method])
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
order_cycle.preferred_shipping_methods << shipping_method
shipping_method.update_column(:display_on, "back_end")
@@ -69,23 +78,6 @@ describe OrderCycle do
end
end
describe "#validate_shipping_methods" do
it "doesn't skip shipping method validations by default" do
order_cycle = create(:distributor_order_cycle, distributors: [create(:distributor_enterprise)])
expect(order_cycle).to be_invalid
expect(order_cycle.errors.to_a).to eq ["You need to select at least one shipping method for each distributor"]
end
it "allows shipping method validations to be skipped because distributors need to be saved before shipping methods" do
order_cycle = create(:distributor_order_cycle, distributors: [create(:distributor_enterprise)])
order_cycle.validate_shipping_methods = false
expect(order_cycle).to be_valid
end
end
it "has a coordinator and associated fees" do
oc = create(:simple_order_cycle)
@@ -198,7 +190,7 @@ describe OrderCycle do
end
it "reports its distributors" do
oc = create(:simple_order_cycle, validate_shipping_methods: false)
oc = create(:simple_order_cycle)
e1 = create(:exchange, incoming: false,
order_cycle: oc, sender: oc.coordinator, receiver: create(:enterprise))
@@ -209,7 +201,7 @@ describe OrderCycle do
end
it "checks for existance of distributors" do
oc = create(:simple_order_cycle, validate_shipping_methods: false)
oc = create(:simple_order_cycle)
d1 = create(:distributor_enterprise)
d2 = create(:distributor_enterprise)
create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1, incoming: false)
@@ -470,22 +462,14 @@ describe OrderCycle do
expect(cloned_exchange_attributes).to match_array original_exchange_attributes
end
context "distributor order cycle created before the customisable shipping methods feature was available" do
it "allows the clone to have customisable shipping methods" do
order_cycle = create(:distributor_order_cycle, shipping_methods_customisable: false)
order_cycle.clone!
order_cycle_clone = OrderCycle.last
expect(order_cycle_clone.shipping_methods_customisable).to eq(true)
end
end
context "when it has shipping methods which can longer be applied validly e.g. shipping method is backoffice only" do
context "when it has preferred shipping methods which can longer be applied validly
e.g. shipping method is backoffice only" do
it "raises an error (TODO: display a message to user explaining why clone failed)" do
distributor = create(:distributor_enterprise)
shipping_method = create(:shipping_method, distributors: [distributor])
order_cycle = create(:distributor_order_cycle, distributors: [distributor], shipping_methods: [shipping_method])
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
order_cycle.preferred_shipping_methods << shipping_method
shipping_method.update_column(:display_on, "back_end")
expect {
@@ -721,6 +705,38 @@ describe OrderCycle do
end
end
describe "#shipping_methods" do
let(:distributor) { create(:distributor_enterprise) }
it "returns all attachable shipping methods if the order cycle is simple" do
oc = create(:sells_own_order_cycle, distributors: [distributor])
shipping_method = create(:shipping_method, distributors: [distributor])
expect(oc.shipping_methods).to eq [shipping_method]
end
context "distributor order cycle i.e. non-simple" do
let(:oc) { create(:distributor_order_cycle, distributors: [distributor]) }
it "returns all attachable shipping methods if no preferred shipping methods have been chosen" do
shipping_method = create(:shipping_method, distributors: [distributor])
expect(oc.preferred_shipping_methods).to be_empty
expect(oc.shipping_methods).to eq [shipping_method]
end
it "returns preferred shipping methods if they have been specified" do
shipping_method_i = create(:shipping_method, distributors: [distributor])
shipping_method_ii = create(:shipping_method, distributors: [distributor])
oc.preferred_shipping_methods << shipping_method_ii
expect(oc.shipping_methods).to eq [shipping_method_ii]
end
end
end
describe "#simple?" do
it "returns true if the coordinator sells their own products i.e. shops" do
order_cycle = build(:simple_order_cycle, coordinator: build(:enterprise, sells: "own"))

63
spec/models/spree/shipping_method_spec.rb
View File

@@ -182,22 +182,38 @@ module Spree
end
context "when it is being changed to backoffice only" do
let!(:order_cycle) { create(:distributor_order_cycle, with_distributor_and_shipping_method: true) }
let(:distributor) { order_cycle.distributors.first }
let(:shipping_method) { order_cycle.shipping_methods.first }
let!(:order_cycle) { create(:distributor_order_cycle, distributors: [distributor]) }
let(:distributor) { create(:distributor_enterprise, shipping_methods: [shipping_method]) }
let(:shipping_method) { create(:shipping_method) }
context "when the shipping method is the only shipping method on a distributor order cycle" do
context "when one of its distributors has no other shipping methods available
on an active or upcoming order cycle" do
it "should not be valid" do
shipping_method.display_on = "back_end"
expect(shipping_method).not_to be_valid
expect(shipping_method.errors.to_a).to eq ["Unable to switch to backoffice only, some open or upcoming order cycles would be left without any shipping methods"]
expect(shipping_method.errors.to_a).to eq [
"Unable to switch to backoffice only, some open or upcoming order cycles would be " \
"left without any shipping methods"
]
end
end
context "when the order cycles the shipping method is attached to has other valid shipping methods" do
it "is is valid" do
order_cycle.shipping_methods << create(:shipping_method, distributors: [distributor])
context "when one of its distributors has no other shipping methods available
on an order cycle which isn't active or upcoming" do
it "is valid" do
order_cycle.update!(orders_open_at: 2.weeks.ago, orders_close_at: 1.week.ago)
shipping_method.display_on = "back_end"
expect(shipping_method).to be_valid
end
end
context "when one of its distributors has other shipping methods available
on an active or upcoming order cycle" do
it "is valid" do
create(:shipping_method, distributors: [distributor])
shipping_method.display_on = "back_end"
@@ -242,9 +258,10 @@ module Spree
context "#destroy" do
let(:shipping_method) { create(:shipping_method) }
let(:distributor) { create(:distributor_enterprise, shipping_methods: [shipping_method]) }
let!(:order_cycle) { create(:distributor_order_cycle, distributors: [distributor], shipping_methods: [shipping_method]) }
let!(:order_cycle) { create(:distributor_order_cycle, distributors: [distributor]) }
context "when the shipping method is the only shipping method on a distributor order cycle" do
context "when its distributors are part of some order cycles
and have no other shipping methods available" do
it "can be deleted if the order cycle is closed" do
order_cycle.update!(orders_close_at: 1.minute.ago)
@@ -259,7 +276,10 @@ module Spree
shipping_method.destroy
expect(shipping_method).not_to be_deleted
expect(shipping_method.errors.to_a).to eq ["Unable to delete, some open or upcoming order cycles would be left without any shipping methods"]
expect(shipping_method.errors.to_a).to eq [
"Unable to delete, some open or upcoming order cycles would be left without any " \
"shipping methods"
]
end
it "cannot be deleted if the order cycle is upcoming" do
@@ -268,13 +288,28 @@ module Spree
shipping_method.destroy
expect(shipping_method).not_to be_deleted
expect(shipping_method.errors.to_a).to eq ["Unable to delete, some open or upcoming order cycles would be left without any shipping methods"]
expect(shipping_method.errors.to_a).to eq [
"Unable to delete, some open or upcoming order cycles would be left without any " \
"shipping methods"
]
end
end
context "when the order cycles the shipping method is attached to has other valid shipping methods" do
context "when its distributors are part of some active or upcoming order cycles
and have other shipping methods available" do
it "can be deleted" do
order_cycle.shipping_methods << create(:shipping_method, distributors: [distributor])
create(:shipping_method, distributors: [distributor])
shipping_method.destroy
expect(shipping_method).to be_deleted
end
end
context "when its distributors have no other shipping methods available
but aren't part of active/upcoming order cycles" do
it "can be deleted" do
order_cycle.update!(orders_open_at: 2.weeks.ago, orders_close_at: 1.week.ago)
shipping_method.destroy

219
spec/services/order_available_shipping_methods_spec.rb
View File

@@ -26,54 +26,41 @@ describe OrderAvailableShippingMethods do
end
context "when no tag rules are in effect" do
context "order cycle selling own produce only i.e. shipping methods cannot be customised" do
it "returns all shipping methods belonging to the enterprise" do
order_cycle = create(:sells_own_order_cycle)
enterprise = order_cycle.coordinator
shipping_method = create(:shipping_method, distributors: [enterprise])
other_enterprise = create(:enterprise)
other_enterprise_shipping_method = create(:shipping_method,
distributors: [other_enterprise])
order = build(:order, distributor: enterprise, order_cycle: order_cycle)
context "sells own order cycle i.e. simple" do
it "only returns the shipping methods which are available on the order cycle
and belong to the order distributor" do
distributor_i = create(:distributor_enterprise)
distributor_ii = create(:distributor_enterprise)
distributor_iii = create(:distributor_enterprise)
shipping_method_i = create(:shipping_method, distributors: [distributor_i])
shipping_method_ii = create(:shipping_method, distributors: [distributor_ii])
shipping_method_iii = create(:shipping_method, distributors: [distributor_iii])
order_cycle = create(:sells_own_order_cycle, distributors: [distributor_i, distributor_ii])
order = build(:order, distributor: distributor_i, order_cycle: order_cycle)
available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a
expect(order_cycle.shipping_methods).to be_empty
expect(available_shipping_methods).to eq [shipping_method]
expect(available_shipping_methods).to eq [shipping_method_i]
end
end
context "distributor order cycle" do
it "only returns shipping methods which belong to the order distributor
and have been added to the order cycle" do
distributor = create(:distributor_enterprise)
shipping_method_i = create(:shipping_method, distributors: [distributor])
shipping_method_ii = create(:shipping_method, distributors: [distributor])
order_cycle = create(:simple_order_cycle,
distributors: [distributor], shipping_methods: [shipping_method_i])
order = build(:order, distributor: distributor, order_cycle: order_cycle)
available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a
expect(available_shipping_methods).to eq order_cycle.shipping_methods
expect(available_shipping_methods).to eq [shipping_method_i]
end
it "doesn't return shipping methods which have been added to the order cycle
when they don't belong to the order distributor" do
context "distributor order cycle i.e. not simple" do
it "only returns the shipping methods which are available on the order cycle
and belong to the order distributor" do
distributor_i = create(:distributor_enterprise)
distributor_ii = create(:distributor_enterprise)
shipping_method_i = create(:shipping_method, distributors: [distributor_i])
shipping_method_ii = create(:shipping_method, distributors: [distributor_ii])
order_cycle = create(:simple_order_cycle,
distributors: [distributor_i, distributor_ii],
shipping_methods: [shipping_method_i, shipping_method_ii])
order = build(:order, distributor: distributor_ii, order_cycle: order_cycle)
shipping_method_ii = create(:shipping_method, distributors: [distributor_i])
shipping_method_iii = create(:shipping_method, distributors: [distributor_ii])
shipping_method_iv = create(:shipping_method, distributors: [distributor_ii])
order_cycle = create(:distributor_order_cycle,
distributors: [distributor_i, distributor_ii])
order_cycle.preferred_shipping_methods << [shipping_method_i, shipping_method_iii]
order = build(:order, distributor: distributor_i, order_cycle: order_cycle)
available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a
expect(available_shipping_methods).not_to eq order_cycle.shipping_methods
expect(available_shipping_methods).to eq [shipping_method_ii]
expect(available_shipping_methods).to eq [shipping_method_i]
end
end
end
@@ -116,66 +103,39 @@ describe OrderAvailableShippingMethods do
'hidden')
}
context "order cycle selling own produce only" do
let(:order_cycle) { create(:sells_own_order_cycle) }
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
let(:order_cycle) { create(:sells_own_order_cycle) }
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
context "when the customer is nil" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a }
context "when the customer is nil" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a }
it "applies default action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
context "when a customer is present" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
context "and the customer's tags match" do
before do
customer.update_attribute(:tag_list, 'local')
end
it "applies the action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
context "and the customer's tags don't match" do
before do
customer.update_attribute(:tag_list, 'something')
end
it "applies the default action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
it "applies default action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
context "distributor order cycle" do
context "when the shipping method without the tag rule is attached to the order cycle
and the shipping method with the tag rule is not" do
let(:order_cycle) do
create(:distributor_order_cycle,
distributors: [distributor], shipping_methods: [untagged_sm])
context "when a customer is present" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
context "and the customer's tags match" do
before do
customer.update_attribute(:tag_list, 'local')
end
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
context "when the customer's tags match" do
before do
customer.update_attribute(:tag_list, 'local')
end
it "applies the action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
it "doesn't display the shipping method with the prefered visibility 'visible' tag
even though the customer's tags match
because it hasn't been attached to the order cycle" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
context "and the customer's tags don't match" do
before do
customer.update_attribute(:tag_list, 'something')
end
it "applies the default action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
end
@@ -190,77 +150,38 @@ describe OrderAvailableShippingMethods do
'visible')
}
context "order cycle selling own produce only" do
let(:order_cycle) { create(:sells_own_order_cycle) }
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
let(:order_cycle) { create(:sells_own_order_cycle) }
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
context "when the customer is nil" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a }
context "when the customer is nil" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a }
it "applies default action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
context "when a customer is present" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
context "and the customer's tags match" do
before do
customer.update_attribute(:tag_list, 'local')
end
it "applies the action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
context "and the customer's tags don't match" do
before do
customer.update_attribute(:tag_list, 'something')
end
it "applies the default action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
it "applies default action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
context "distributor order cycle" do
context "when the shipping method without the tag rule is attached to the order cycle
and the shipping method with the tag rule is not" do
let(:order_cycle) do
create(:distributor_order_cycle,
distributors: [distributor], shipping_methods: [untagged_sm])
end
let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) }
context "when a customer is present" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
context "when the customer is nil" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a }
it "doesn't display the shipping method tagged to be visible by default
because it is not attached to the order cycle" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
context "and the customer's tags match" do
before do
customer.update_attribute(:tag_list, 'local')
end
context "when a customer is present" do
let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a }
it "applies the action (hide)" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
context "when the customer's tags don't match" do
before do
customer.update_attribute(:tag_list, 'something')
end
context "and the customer's tags don't match" do
before do
customer.update_attribute(:tag_list, 'something')
end
it "doesn't display the shipping method tagged to be visible by default
because it is not attached to the order cycle" do
expect(available_shipping_methods).to include untagged_sm
expect(available_shipping_methods).to_not include tagged_sm
end
end
it "applies the default action (show)" do
expect(available_shipping_methods).to include tagged_sm, untagged_sm
end
end
end

27
spec/services/order_cycle_form_spec.rb
View File

@@ -146,7 +146,8 @@ describe OrderCycleForm do
end
end
context "updating basics, incoming and outcoming exchanges, shipping methods simultaneously" do
context "updating basics, incoming exchanges, outcoming exchanges
and preferred shipping methods simultaneously" do
before do
params.merge!(
incoming_exchanges: [{
@@ -158,7 +159,7 @@ describe OrderCycleForm do
enterprise_fee_ids: []
}],
outgoing_exchanges: [outgoing_exchange_params],
shipping_method_ids: [shipping_method.id]
preferred_shipping_method_ids: [shipping_method.id]
)
end
@@ -175,7 +176,7 @@ describe OrderCycleForm do
before do
params.merge!(
outgoing_exchanges: [outgoing_exchange_params],
shipping_method_ids: nil
preferred_shipping_method_ids: nil
)
end
@@ -194,7 +195,7 @@ describe OrderCycleForm do
before do
params.merge!(
outgoing_exchanges: [outgoing_exchange_params],
shipping_method_ids: [other_distributor_shipping_method.id]
preferred_shipping_method_ids: [other_distributor_shipping_method.id]
)
end
@@ -209,11 +210,13 @@ describe OrderCycleForm do
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)
distributor = create(:distributor_enterprise)
shipping_method = create(:shipping_method, distributors: [distributor])
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
form = OrderCycleForm.new(
order_cycle,
params.except(:shipping_method_ids),
params.except(:preferred_shipping_method_ids),
order_cycle.coordinator
)
@@ -229,7 +232,7 @@ describe OrderCycleForm do
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
form = OrderCycleForm.new(order_cycle,
{ shipping_method_ids: [shipping_method.id] },
{ preferred_shipping_method_ids: [shipping_method.id] },
order_cycle.coordinator)
expect(form.save).to be true
@@ -239,11 +242,15 @@ describe OrderCycleForm do
context "and it's invalid" do
it "returns a validation error" do
distributor = create(:distributor_enterprise)
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
distributor_i = create(:distributor_enterprise)
distributor_ii = create(:distributor_enterprise)
shipping_method_i = create(:shipping_method, distributors: [distributor_i])
shipping_method_ii = create(:shipping_method, distributors: [distributor_ii])
order_cycle = create(:distributor_order_cycle,
distributors: [distributor_i, distributor_ii])
form = OrderCycleForm.new(order_cycle,
{ shipping_method_ids: [] },
{ preferred_shipping_method_ids: [shipping_method_i.id] },
order_cycle.coordinator)
expect(form.save).to be false