raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-28 21:07:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2366 from coopdevs/refactor-ship-address-from-distributor

Browse Source 
Refactor ship address from distributor
This commit is contained in:
Pau Pérez Fabregat
2018-07-26 12:04:02 +02:00
committed by GitHub
parent a3cace46e0 eaf108718c
commit 5a4bc0c584
7 changed files with 210 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
app/models/order_updater.rb Normal file
View File

@@ -0,0 +1,57 @@
require 'delegate'
class OrderUpdater < SimpleDelegator
# TODO: This logic adapted from Spree 2.4, remove when we get there
# Handles state updating in a much more logical way than < 2.4
# Specifically, doesn't depend on payments.last to determine payment state
# Also swapped: == 0 for .zero?, .size == 0 for empty? and .size > 0 for !empty?
# See:
# https://github.com/spree/spree/commit/38b8456183d11fc1e00e395e7c9154c76ef65b85
# https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
def update_payment_state
last_state = order.payment_state
if payments.present? && payments.valid.empty?
order.payment_state = 'failed'
elsif order.state == 'canceled' && order.payment_total.zero?
order.payment_state = 'void'
else
# This part added so that we don't need to override order.outstanding_balance
balance = order.outstanding_balance
balance = -1 * order.payment_total if canceled_and_paid_for?
order.payment_state = 'balance_due' if balance > 0
order.payment_state = 'credit_owed' if balance < 0
order.payment_state = 'paid' if balance.zero?
# Original logic
# order.payment_state = 'balance_due' if order.outstanding_balance > 0
# order.payment_state = 'credit_owed' if order.outstanding_balance < 0
# order.payment_state = 'paid' if !order.outstanding_balance?
end
order.state_changed('payment') if last_state != order.payment_state
order.payment_state
end
def before_save_hook
shipping_address_from_distributor
end
private
# Taken from order.outstanding_balance in Spree 2.4
# See: https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
def canceled_and_paid_for?
order.canceled? && order.payments.present? && !order.payments.completed.empty?
end
# Sets the distributor's address as shipping address of the order for those
# shipments using a shipping method that doesn't require address, such us
# a pickup.
def shipping_address_from_distributor
shipments.each do |shipment|
shipping_method = shipment.shipping_method
next if shipping_method.require_ship_address
order.ship_address = order.distributor.address
end
end
end

13
app/models/spree/order_decorator.rb
View File

@@ -20,7 +20,6 @@ Spree::Order.class_eval do
validate :disallow_guest_order
attr_accessible :order_cycle_id, :distributor_id, :customer_id
before_validation :shipping_address_from_distributor
before_validation :associate_customer, unless: :customer_id?
before_validation :ensure_customer, unless: :customer_is_valid?
@@ -337,18 +336,6 @@ Spree::Order.class_eval do
private
def shipping_address_from_distributor
if distributor
# This method is confusing to conform to the vagaries of the multi-step checkout
# We copy over the shipping address when we have no shipping method selected
# We can refactor this when we drop the multi-step checkout option
#
if shipping_method.andand.require_ship_address == false
self.ship_address = address_from_distributor
end
end
end
def address_from_distributor
address = distributor.address.clone
if bill_address

41
app/models/spree/order_updater_decorator.rb
View File

@@ -1,41 +0,0 @@
module Spree
OrderUpdater.class_eval do
# TODO: This logic adapted from Spree 2.4, remove when we get there
# Handles state updating in a much more logical way than < 2.4
# Specifically, doesn't depend on payments.last to determine payment state
# Also swapped: == 0 for .zero?, .size == 0 for empty? and .size > 0 for !empty?
# See:
# https://github.com/spree/spree/commit/38b8456183d11fc1e00e395e7c9154c76ef65b85
# https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
def update_payment_state
last_state = order.payment_state
if payments.present? && payments.valid.empty?
order.payment_state = 'failed'
elsif order.state == 'canceled' && order.payment_total.zero?
order.payment_state = 'void'
else
# This part added so that we don't need to override order.outstanding_balance
balance = order.outstanding_balance
balance = -1 * order.payment_total if canceled_and_paid_for?
order.payment_state = 'balance_due' if balance > 0
order.payment_state = 'credit_owed' if balance < 0
order.payment_state = 'paid' if balance.zero?
# Original logic
# order.payment_state = 'balance_due' if order.outstanding_balance > 0
# order.payment_state = 'credit_owed' if order.outstanding_balance < 0
# order.payment_state = 'paid' if !order.outstanding_balance?
end
order.state_changed('payment') if last_state != order.payment_state
order.payment_state
end
private
# Taken from order.outstanding_balance in Spree 2.4
# See: https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
def canceled_and_paid_for?
order.canceled? && order.payments.present? && !order.payments.completed.empty?
end
end
end

1
config/initializers/spree.rb
View File

@@ -22,6 +22,7 @@ Spree.config do |config|
#config.override_actionmailer_config = false
config.package_factory = Stock::Package
config.order_updater_decorator = OrderUpdater
end
# TODO Work out why this is necessary

152
spec/models/order_updater_spec.rb Normal file
View File

@@ -0,0 +1,152 @@
require 'spec_helper'
describe OrderUpdater do
let(:order) { build(:order) }
let(:order_updater) { described_class.new(Spree::OrderUpdater.new(order)) }
context "#updating_payment_state" do
it "is failed if no valid payments" do
allow(order).to receive_message_chain(:payments, :valid, :empty?) { true }
order_updater.update_payment_state
expect(order.payment_state).to eq('failed')
end
context "payment total is greater than order total" do
before do
order.payment_total = 2
order.total = 1
end
it "is credit_owed" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'credit_owed'
end
end
context "order total is greater than payment total" do
before do
order.payment_total = 1
order.total = 2
end
it "is credit_owed" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'balance_due'
end
end
context "order total equals payment total" do
before do
order.payment_total = 30
order.total = 30
end
it "is paid" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'paid'
end
end
context "order is canceled" do
before { order.state = 'canceled' }
context "and is still unpaid" do
before do
order.payment_total = 0
order.total = 30
end
it "is void" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'void'
end
end
context "and is paid" do
before do
order.payment_total = 30
order.total = 30
order.stub_chain(:payments, :valid, :empty?).and_return(false)
order.stub_chain(:payments, :completed, :empty?).and_return(false)
end
it "is credit_owed" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'credit_owed'
end
end
context "and payment is refunded" do
before do
order.payment_total = 0
order.total = 30
order.stub_chain(:payments, :valid, :empty?).and_return(false)
order.stub_chain(:payments, :completed, :empty?).and_return(false)
end
it "is void" do
expect {
order_updater.update_payment_state
}.to change { order.payment_state }.to 'void'
end
end
end
end
context '#before_save_hook' do
let(:distributor) { build(:distributor_enterprise) }
let(:order) { build(:order, distributor: distributor) }
let(:shipment) { build(:shipment) }
let(:shipping_rate) do
Spree::ShippingRate.new(
shipping_method: shipping_method,
selected: true
)
end
before do
shipment.shipping_rates << shipping_rate
order.shipments << shipment
end
context 'when any of the shipping methods doesn\'t require a delivery address' do
let(:shipping_method) { build(:base_shipping_method, require_ship_address: false) }
let(:delivery_shipment) { build(:shipment) }
let(:delivery_shipping_rate) do
Spree::ShippingRate.new(
shipping_method: build(:base_shipping_method, require_ship_address: true),
selected: true
)
end
before do
delivery_shipment.shipping_rates << delivery_shipping_rate
order.shipments << delivery_shipment
end
it "populates the shipping address" do
order_updater.before_save_hook
expect(order.ship_address.firstname).to eq(distributor.address.firstname)
end
end
context 'when any of the shipping methods requires a delivery address' do
let(:shipping_method) { build(:base_shipping_method, require_ship_address: true) }
let(:address) { build(:address, firstname: 'will') }
before { order.ship_address = address }
it "does not populate the shipping address" do
order_updater.before_save_hook
expect(order.ship_address.firstname).to eq("will")
end
end
end
end

35
spec/models/spree/order_spec.rb
View File

@@ -509,41 +509,6 @@ describe Spree::Order do
end
end
describe "shipping address prepopulation" do
let(:distributor) { create(:distributor_enterprise) }
let(:order) { build(:order, distributor: distributor) }
before do
order.ship_address = distributor.address.clone
order.save # just to trigger our autopopulate the first time ;)
end
it "autopopulates the shipping address on save" do
order.should_receive(:shipping_address_from_distributor).and_return true
order.save
end
it "populates the shipping address if the shipping method doesn't require a delivery address" do
order.shipping_method = create(:shipping_method, require_ship_address: false)
order.ship_address.update_attribute :firstname, "will"
order.save
order.ship_address.firstname.should == distributor.address.firstname
end
it "does not populate the shipping address if the shipping method requires a delivery address" do
order.shipping_method = create(:shipping_method, require_ship_address: true)
order.ship_address.update_attribute :firstname, "will"
order.save
order.ship_address.firstname.should == "will"
end
it "doesn't attempt to create a shipment if the order is not yet valid" do
order.shipping_method = create(:shipping_method, require_ship_address: false)
#Shipment.should_not_r
order.create_shipment!
end
end
describe "checking if an order is an account invoice" do
let(:accounts_distributor) { create(:distributor_enterprise) }
let(:order_account_invoice) { create(:order, distributor: accounts_distributor) }

91
spec/models/spree/order_updater_spec.rb
View File

@@ -1,91 +0,0 @@
require 'spec_helper'
describe Spree::OrderUpdater do
# Copied pretty much verbatim from Spree 2.4. Remove this file once we get there,
# assuming the unchanged 2.4 logic still works for us.
# Only changes are stubs of :empty? instead of :size
context "updating payment state" do
let(:order) { Spree::Order.new }
let(:updater) { order.updater }
it "is failed if no valid payments" do
order.stub_chain(:payments, :valid, :empty?).and_return(true)
updater.update_payment_state
order.payment_state.should == 'failed'
end
context "payment total is greater than order total" do
it "is credit_owed" do
order.payment_total = 2
order.total = 1
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'credit_owed'
end
end
context "order total is greater than payment total" do
it "is credit_owed" do
order.payment_total = 1
order.total = 2
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'balance_due'
end
end
context "order total equals payment total" do
it "is paid" do
order.payment_total = 30
order.total = 30
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'paid'
end
end
context "order is canceled" do
before do
order.state = 'canceled'
end
context "and is still unpaid" do
it "is void" do
order.payment_total = 0
order.total = 30
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'void'
end
end
context "and is paid" do
it "is credit_owed" do
order.payment_total = 30
order.total = 30
order.stub_chain(:payments, :valid, :empty?).and_return(false)
order.stub_chain(:payments, :completed, :empty?).and_return(false)
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'credit_owed'
end
end
context "and payment is refunded" do
it "is void" do
order.payment_total = 0
order.total = 30
order.stub_chain(:payments, :valid, :empty?).and_return(false)
order.stub_chain(:payments, :completed, :empty?).and_return(false)
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'void'
end
end
end
end
end