mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-02 21:57:17 +00:00
A shop won't be shown as open if it doesn't have useable shipping method so these shipping method validations are really necessary
This commit is contained in:
Cillian O'Ruanaidh
committed by
Filipe
Filipe
parent
a3a52a07b7
commit
593da4996f
@@ -179,7 +179,8 @@ class OrderCycle < ApplicationRecord
|
||||
oc.schedule_ids = schedule_ids
|
||||
oc.save!
|
||||
exchanges.each { |e| e.clone!(oc) }
|
||||
oc.selected_shipping_method_ids = selected_shipping_method_ids
|
||||
oc.selected_shipping_method_ids = attachable_shipping_methods.map(&:id) &
|
||||
selected_shipping_method_ids
|
||||
sync_subscriptions
|
||||
oc.reload
|
||||
end
|
||||
|
||||
@@ -5,42 +5,10 @@ class OrderCycleShippingMethod < ApplicationRecord
|
||||
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
|
||||
validates :shipping_method, uniqueness: { 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
|
||||
).empty?
|
||||
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 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? ||
|
||||
|
||||
@@ -30,11 +30,8 @@ module Spree
|
||||
validates :name, presence: true
|
||||
validate :distributor_validation
|
||||
validate :at_least_one_shipping_category
|
||||
validate :switching_to_backoffice_only_wont_leave_order_cycles_without_shipping_methods
|
||||
validates :display_on, inclusion: { in: DISPLAY_ON_OPTIONS.values }, allow_nil: true
|
||||
|
||||
before_destroy :check_destroy_wont_leave_order_cycles_without_shipping_methods
|
||||
|
||||
after_save :touch_distributors
|
||||
|
||||
scope :managed_by, lambda { |user|
|
||||
@@ -140,19 +137,5 @@ module Spree
|
||||
def distributor_validation
|
||||
validates_with DistributorsValidator
|
||||
end
|
||||
|
||||
def check_destroy_wont_leave_order_cycles_without_shipping_methods
|
||||
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
|
||||
end
|
||||
|
||||
def switching_to_backoffice_only_wont_leave_order_cycles_without_shipping_methods
|
||||
return if frontend? ||
|
||||
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
|
||||
end
|
||||
end
|
||||
|
||||
@@ -73,25 +73,13 @@ en:
|
||||
attributes:
|
||||
base:
|
||||
card_expired: "has expired"
|
||||
spree/shipping_method:
|
||||
attributes:
|
||||
base:
|
||||
switching_to_backoffice_only_leaves_order_cycles_without_shipping_methods: Unable to switch to backoffice only, some open or upcoming order cycles would be left without any shipping methods
|
||||
destroy_leaves_order_cycles_without_shipping_methods: Unable to delete, some open or upcoming order cycles would be left without any shipping methods
|
||||
order_cycle:
|
||||
attributes:
|
||||
base:
|
||||
at_least_one_shipping_method_per_distributor: "You need to select at least one shipping method for each distributor"
|
||||
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"
|
||||
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:
|
||||
|
||||
@@ -3,55 +3,6 @@
|
||||
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 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])
|
||||
@@ -80,37 +31,4 @@ describe OrderCycleShippingMethod do
|
||||
"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
|
||||
|
||||
@@ -406,17 +406,20 @@ describe OrderCycle 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
|
||||
it "only attaches the valid ones to the clone" do
|
||||
distributor = create(:distributor_enterprise)
|
||||
shipping_method = create(:shipping_method, distributors: [distributor])
|
||||
shipping_method_i = create(:shipping_method, distributors: [distributor])
|
||||
shipping_method_ii = create(
|
||||
:shipping_method,
|
||||
distributors: [distributor],
|
||||
display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end]
|
||||
)
|
||||
order_cycle = create(:distributor_order_cycle, distributors: [distributor])
|
||||
order_cycle.selected_shipping_methods << shipping_method
|
||||
order_cycle.selected_shipping_methods = [shipping_method_i, shipping_method_ii]
|
||||
|
||||
shipping_method.update_column(:display_on, "back_end")
|
||||
cloned_order_cycle = order_cycle.clone!
|
||||
|
||||
expect {
|
||||
order_cycle.clone!
|
||||
}.to raise_error ActiveRecord::RecordInvalid
|
||||
expect(cloned_order_cycle.shipping_methods).to eq [shipping_method_i]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -182,47 +182,6 @@ module Spree
|
||||
it { expect(shipping_method).to be_valid }
|
||||
end
|
||||
end
|
||||
|
||||
context "when it is being changed to backoffice only" do
|
||||
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 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"
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
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"
|
||||
|
||||
expect(shipping_method).to be_valid
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Regression test for Spree #4320
|
||||
@@ -256,68 +215,5 @@ module Spree
|
||||
expect(shipping_method.shipments).to include(shipment)
|
||||
end
|
||||
end
|
||||
|
||||
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]) }
|
||||
|
||||
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)
|
||||
|
||||
shipping_method.destroy
|
||||
|
||||
expect(shipping_method).to be_deleted
|
||||
end
|
||||
|
||||
it "cannot be deleted if the order cycle is active" do
|
||||
order_cycle.update!(orders_open_at: 1.day.ago, orders_close_at: 1.week.from_now)
|
||||
|
||||
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"
|
||||
]
|
||||
end
|
||||
|
||||
it "cannot be deleted if the order cycle is upcoming" do
|
||||
order_cycle.update!(orders_open_at: 1.day.from_now, orders_close_at: 1.week.from_now)
|
||||
|
||||
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"
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
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
|
||||
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
|
||||
|
||||
expect(shipping_method).to be_deleted
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -247,15 +247,15 @@ describe OrderCycleForm do
|
||||
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])
|
||||
distributors: [distributor_i])
|
||||
|
||||
form = OrderCycleForm.new(order_cycle,
|
||||
{ selected_shipping_method_ids: [shipping_method_i.id] },
|
||||
{ selected_shipping_method_ids: [shipping_method_ii.id] },
|
||||
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"
|
||||
"Shipping method must be from a distributor on the order cycle"
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user