Merge pull request #6 from Matt-Yorkley/8009-handling-shipping-adress

8009-handling-shipping-adress: REDUX
This commit is contained in:
jibees
2021-09-07 09:03:22 +02:00
committed by GitHub
17 changed files with 190 additions and 200 deletions

View File

@@ -175,7 +175,7 @@ class CheckoutController < ::BaseController
return action_failed unless @order.process_payments!
end
next if OrderWorkflow.new(@order).next({ shipping_method_id: shipping_method_id })
next if OrderWorkflow.new(@order).next({ "shipping_method_id" => shipping_method_id })
return action_failed
end

View File

@@ -14,7 +14,7 @@ module CheckoutCallbacks
prepend_before_action :require_distributor_chosen
before_action :load_order, :associate_user, :load_saved_addresses
before_action :load_shipping_methods, :load_countries, if: -> { checkout_step == "details"}
before_action :load_shipping_methods, :load_countries, if: -> { params[:step] == "details"}
before_action :ensure_order_not_completed
before_action :ensure_checkout_allowed
@@ -27,6 +27,8 @@ module CheckoutCallbacks
def load_order
@order = current_order
@order.manual_shipping_selection = true
@order.checkout_processing = true
redirect_to(main_app.shop_path) && return if redirect_to_shop?
redirect_to_cart_path && return unless valid_order_line_items?

View File

@@ -15,146 +15,69 @@ class SplitCheckoutController < ::BaseController
helper OrderHelper
def edit
return handle_redirect_from_stripe if valid_payment_intent_provided?
redirect_to_step unless checkout_step
OrderWorkflow.new(@order).next if @order.cart?
# This is only required because of spree_paypal_express. If we implement
# a version of paypal that uses this controller, and more specifically
# the #action_failed method, then we can remove this call
# OrderCheckoutRestart.new(@order).call
@order.errors.clear
@order.bill_address.errors.clear
@order.ship_address.errors.clear
@ship_address_same_as_billing = "1" if ship_address_matches_bill_address?
rescue Spree::Core::GatewayError => e
rescue_from_spree_gateway_error(e)
redirect_to_step unless params[:step]
end
def update
load_shipping_method
handle_shipping_method_selection
confirm_or_update = confirm_order || update_order
if confirm_or_update && advance_order_state
if confirm_order || update_order
clear_invalid_payments
advance_order_state
redirect_to_step
else
if @shipping_method_id.blank?
@order.errors.add(:base, "no_shipping_method_selected")
end
flash.now[:error] = "#{I18n.t('split_checkout.errors.global')}"
flash.now[:error] = I18n.t('split_checkout.errors.global')
render :edit
end
end
private
def ship_address_matches_bill_address?
attrs_to_check = %w(firstname lastname address1 address2 state_id city zipcode phone country_id)
((@order.bill_address.attributes.to_a -
@order.ship_address.attributes.to_a).map(&:first) & attrs_to_check).blank?
end
def handle_shipping_method_selection
return unless @shipping_method
populate_ship_address_params
end
def load_shipping_method
if params[:shipping_method_id]
@shipping_method = Spree::ShippingMethod.where(id: params[:shipping_method_id]).first
@shipping_method_id = params[:shipping_method_id]
else
@shipping_method = @order.shipping_method
@shipping_method_id = @shipping_method&.id
end
end
def populate_ship_address_params
return unless params.dig(:order, :ship_address_attributes).present? &&
params.dig(:order, :bill_address_attributes).present?
if params.dig(:order, "Checkout.ship_address_same_as_billing") == "1"
params[:order][:ship_address_attributes] = params[:order][:bill_address_attributes]
return
end
address_attrs = [
:firstname,
:lastname,
:phone,
:address1,
:address2,
:city,
:state_id,
:zipcode,
:country_id,
:id
]
address_attrs.each do |attr|
next if params[:order][:ship_address_attributes][attr].present?
params[:order][:ship_address_attributes][attr] = params[:order][:bill_address_attributes][attr]
end
end
def clear_invalid_payments
@order.payments.with_state(:invalid).delete_all
end
def confirm_order
return unless @order.confirmation? && params[:confirm_order]
return unless validate_summary! && @order.errors.empty?
if params["accept_terms"] != "1"
@order.errors.add(:base, "terms_not_accepted")
return false
end
@order.confirm!
end
def update_order
return unless params[:order]
return if @order.state == "address" && params[:shipping_method_id].blank?
return if @order.errors.any?
@order.select_shipping_method(params[:shipping_method_id])
@order.update(order_params)
send("validate_#{params[:step]}!")
@order.errors.empty?
end
def advance_order_state
return true if @order.complete?
return if @order.complete?
workflow_options = raw_params.slice(:shipping_method_id)
if @order.payments.empty?
OrderWorkflow.new(@order).advance_to_payment(workflow_options)
else
OrderWorkflow.new(@order).advance_to_confirmation(workflow_options)
end
OrderWorkflow.new(@order).advance_checkout(raw_params.slice(:shipping_method_id))
end
def checkout_step
@checkout_step ||= params[:step]
def validate_details!
return true if params[:shipping_method_id].present?
@order.errors.add :shipping_method, I18n.t('split_checkout.errors.select_a_shipping_method')
end
def validate_payment!
return true if params.dig(:order, :payments_attributes).present?
@order.errors.add :payment_method, I18n.t('split_checkout.errors.select_a_payment_method')
end
def validate_summary!
return true if params[:accept_terms]
@order.errors.add(:terms_and_conditions, t("split_checkout.errors.terms_not_accepted"))
end
def order_params
return @order_params unless @order_params.nil?
@order_params = params.require(:order).permit(
:email, :shipping_method_id, :special_instructions,
bill_address_attributes: PermittedAttributes::Address.attributes,
ship_address_attributes: PermittedAttributes::Address.attributes,
payments_attributes: [:payment_method_id]
)
if @order_params[:payments_attributes]
# Set payment amount
@order_params[:payments_attributes].first[:amount] = @order.total
end
@order_params
@order_params ||= Checkout::Params.new(@order, params).call
end
def redirect_to_step
@@ -169,33 +92,4 @@ class SplitCheckoutController < ::BaseController
redirect_to order_path(@order)
end
end
def valid_payment_intent_provided?
return false unless params["payment_intent"]&.starts_with?("pi_")
last_payment = OrderPaymentFinder.new(@order).last_payment
@order.state == "payment" &&
last_payment&.state == "requires_authorization" &&
last_payment&.response_code == params["payment_intent"]
end
def handle_redirect_from_stripe
return checkout_failed unless @order.process_payments!
if OrderWorkflow.new(@order).next && order_complete?
checkout_succeeded
redirect_to(order_path(@order)) && return
else
checkout_failed
end
end
def order_complete?
@order.state == "complete" || @order.completed?
end
def rescue_from_spree_gateway_error(error)
flash[:error] = t(:spree_gateway_error_flash_for_checkout, error: error.message)
action_failed(error)
end
end

View File

@@ -4,14 +4,22 @@ module ApplicationHelper
include RawParams
include Pagy::Frontend
def error_message_on(object, method, _options = {})
def error_message_on(object, method, options = {})
object = convert_to_model(object)
obj = object.respond_to?(:errors) ? object : instance_variable_get("@#{object}")
if obj && obj.errors[method].present?
errors = obj.errors[method].map { |err| h(err) }.join('<br />').html_safe
content_tag(:span, errors, class: 'formError')
return "" unless obj && obj.errors[method].present?
errors = obj.errors[method].map { |err| h(err) }.join('<br />').html_safe
if options[:standalone]
content_tag(
:div,
content_tag(:span, errors, class: 'formError standalone'),
class: 'checkout-input'
)
else
''
content_tag(:span, errors, class: 'formError')
end
end

View File

@@ -1,6 +1,10 @@
# frozen_string_literal: true
module CheckoutHelper
def shipping_and_billing_match?(order)
order.ship_address == order.bill_address
end
def guest_checkout_allowed?
current_order.distributor.allow_guest_orders?
end

View File

@@ -13,6 +13,10 @@ require 'active_support/concern'
module OrderShipment
extend ActiveSupport::Concern
included do
attr_accessor :manual_shipping_selection
end
def shipment
shipments.first
end

View File

@@ -4,12 +4,10 @@ require 'spree/order/checkout'
require 'open_food_network/enterprise_fee_calculator'
require 'open_food_network/feature_toggle'
require 'open_food_network/tag_rule_applicator'
require 'concerns/order_shipment'
module Spree
class Order < ApplicationRecord
prepend OrderShipment
include OrderShipment
include Checkout
include Balance
@@ -21,11 +19,14 @@ module Spree
order.payment_required?
}
go_to_state :confirmation, if: ->(order) {
Flipper.enabled? :split_checkout, order.user
Flipper.enabled? :split_checkout
}
go_to_state :complete
end
attr_accessor :use_billing
attr_accessor :checkout_processing
token_resource
belongs_to :user, class_name: Spree.user_class.to_s
@@ -81,8 +82,6 @@ module Spree
before_validation :associate_customer, unless: :customer_id?
before_validation :ensure_customer, unless: :customer_is_valid?
attr_accessor :use_billing
before_create :link_by_email
after_create :create_tax_charge!
@@ -94,7 +93,6 @@ module Spree
validates :email, presence: true,
format: /\A([\w.%+\-']+)@([\w\-]+\.)+(\w{2,})\z/i,
if: :require_email
validates :payments, presence: true, if: ->(order) { order.confirmation? && payment_required? }
make_permalink field: :number
@@ -140,6 +138,13 @@ module Spree
scope :by_state, lambda { |state| where(state: state) }
scope :not_state, lambda { |state| where.not(state: state) }
def initialize(*_args)
@checkout_processing = nil
@manual_shipping_selection = nil
super
end
# For compatiblity with Calculator::PriceSack
def amount
line_items.inject(0.0) { |sum, li| sum + li.amount }
@@ -628,7 +633,7 @@ module Spree
# Determine if email is required (we don't want validation errors before we hit the checkout)
def require_email
return true unless new_record? || (state == 'cart')
return true unless (new_record? || cart?) && !checkout_processing
end
def ensure_line_items_present

View File

@@ -76,6 +76,7 @@ module Spree
before_transition to: :delivery, do: :create_proposed_shipments
before_transition to: :delivery, do: :ensure_available_shipping_rates
before_transition to: :payment, do: :create_tax_charge!
before_transition to: :confirmation, do: :validate_payment_method!
after_transition to: :complete, do: :finalize!
after_transition to: :resumed, do: :after_resume
@@ -135,6 +136,16 @@ module Spree
updated_at: Time.zone.now,
)
end
private
def validate_payment_method!
return unless checkout_processing
return if payments.any?
errors.add :payment_method, I18n.t('split_checkout.errors.select_a_payment_method')
throw :halt
end
end
end
end

View File

@@ -84,7 +84,9 @@ module Spree
end
def shipping_method
selected_shipping_rate.try(:shipping_method) || shipping_rates.first.try(:shipping_method)
method = selected_shipping_rate.try(:shipping_method)
method ||= shipping_rates.first.try(:shipping_method) unless order.manual_shipping_selection
method
end
def add_shipping_method(shipping_method, selected = false)
@@ -263,7 +265,7 @@ module Spree
fee_adjustment.amount = selected_shipping_rate.cost if fee_adjustment.open?
fee_adjustment.save!
fee_adjustment.reload
elsif selected_shipping_rate_id
elsif shipping_method
shipping_method.create_adjustment(adjustment_label,
self,
true,

View File

@@ -0,0 +1,63 @@
# frozen_string_literal: true
module Checkout
class Params
def initialize(order, params)
@params = params
@order = order
end
def call
return {} unless params[:order]
apply_strong_parameters
set_address_details
set_payment_amount
@order_params
end
private
attr_reader :params, :order
def apply_strong_parameters
@order_params = params.require(:order).permit(
:email, :shipping_method_id, :special_instructions,
bill_address_attributes: ::PermittedAttributes::Address.attributes,
ship_address_attributes: ::PermittedAttributes::Address.attributes,
payments_attributes: [:payment_method_id]
)
end
def set_address_details
return unless addresses_present?
if params[:ship_address_same_as_billing]
set_ship_address_from_bill_address
else
set_basic_details
end
end
def set_payment_amount
return unless @order_params[:payments_attributes]
@order_params[:payments_attributes].first[:amount] = order.total
end
def addresses_present?
@order_params[:ship_address_attributes] && @order_params[:bill_address_attributes]
end
def set_ship_address_from_bill_address
@order_params[:ship_address_attributes] = @order_params[:bill_address_attributes]
end
def set_basic_details
[:firstname, :lastname, :phone].each do |attr|
@order_params[:ship_address_attributes][attr] = @order_params[:bill_address_attributes][attr]
end
end
end
end

View File

@@ -23,26 +23,21 @@ class OrderWorkflow
result
end
def advance_to_payment(options = {})
if options[:shipping_method_id]
order.select_shipping_method(options[:shipping_method_id])
end
def advance_to_payment
advance_to_state("payment", advance_order_options)
end
def advance_to_confirmation(options = {})
if options[:shipping_method_id]
order.select_shipping_method(options[:shipping_method_id])
end
def advance_checkout(options = {})
advance_to = order.state.in?(["cart", "address", "delivery"]) ? "payment" : "confirmation"
advance_to_state("confirmation")
advance_to_state(advance_to, advance_order_options.merge(options))
end
private
def advance_order_options
shipping_method_id = order.shipping_method.id if order.shipping_method.present?
{ shipping_method_id: shipping_method_id }
{ "shipping_method_id" => shipping_method_id }
end
def advance_to_state(target_state, options = {})
@@ -71,8 +66,8 @@ class OrderWorkflow
end
def after_transition_hook(options)
if order.state == "delivery" && (options[:shipping_method_id])
order.select_shipping_method(options[:shipping_method_id])
if order.state == "delivery"
order.select_shipping_method(options["shipping_method_id"])
end
persist_all_payments if order.state == "payment"

View File

@@ -64,17 +64,13 @@
= f.label :checkout_default_bill_address, t(:checkout_default_bill_address)
%div.checkout-substep{ "data-controller": "toggle shippingmethod" }
- selected_shipping_method = @order.shipping_method&.id || @shipping_method_id
-# DELIVERY ADDRESS
- selected_shipping_method = @order.shipping_method&.id || params[:shipping_method_id]
%div.checkout-title
= t("split_checkout.step1.delivery_address.title")
- display_ship_address = false
- ship_method_description = nil
- if selected_shipping_method == nil && @order.errors.messages_for(:base).include?("no_shipping_method_selected")
%div.checkout-input
%span.formError.standalone
= t("split_checkout.step1.delivery_address.errors.no_shipping_method_selected")
- @shipping_methods.each do |shipping_method|
%div.checkout-input
= fields_for shipping_method do |shipping_method_form|
@@ -89,10 +85,12 @@
= shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s }
%em.light
= payment_or_shipping_price(shipping_method, @order)
- display_ship_address = display_ship_address || (shipping_method.id == selected_shipping_method.to_i && shipping_method.require_ship_address)
- display_ship_address = (shipping_method.id == selected_shipping_method.to_i && shipping_method.require_ship_address)
- if shipping_method.id == selected_shipping_method.to_i
- ship_method_description = shipping_method.description
= f.error_message_on :shipping_method, standalone: true
%div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", style: "display: #{ship_method_description == nil ? 'none' : 'block'}" }
#distributor_address.panel
%span{"data-shippingmethod-target": "shippingMethodDescriptionContent"} #{ship_method_description}
@@ -103,10 +101,10 @@
= @order.order_cycle.pickup_time_for(@order.distributor)
%div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" }
= f.check_box "Checkout.ship_address_same_as_billing", { id: "Checkout.ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: @ship_address_same_as_billing == "1" }
= f.label "Checkout.ship_address_same_as_billing", t(:checkout_address_same), { for: "Checkout.ship_address_same_as_billing" }
= f.check_box :ship_address_same_as_billing, { id: "ship_address_same_as_billing", name: "ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: shipping_and_billing_match?(@order) }, 1, nil
= f.label :ship_address_same_as_billing, t(:checkout_address_same), { for: "ship_address_same_as_billing" }
%div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{display_ship_address == false || @ship_address_same_as_billing == "1" ? 'none' : 'block'}" }
%div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{!display_ship_address || shipping_and_billing_match?(@order) ? 'none' : 'block'}" }
= f.fields :ship_address, model: @order.ship_address do |ship_address|
%div.checkout-input
= ship_address.label :address1, t("split_checkout.step1.address.address1.label")
@@ -138,9 +136,8 @@
- if spree_current_user
%div.checkout-input{ "data-toggle-target": "content", style: "display: none" }
= f.check_box "Checkout.default_ship_address", { id: "Checkout.default_ship_address" }
= f.label "Checkout.default_ship_address", t(:checkout_default_ship_address), { for: "Checkout.default_ship_address" }
= f.check_box :default_ship_address, { id: "default_ship_address", name: "default_ship_address" }
= f.label :default_ship_address, t(:checkout_default_ship_address), { for: "default_ship_address" }
.div.checkout-input
= f.label :special_instructions, t(:checkout_instructions)

View File

@@ -2,21 +2,22 @@
%div.checkout-substep{"data-controller": "paymentmethod"}
%div.checkout-title
= t("split_checkout.step2.payment_method.title")
- selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id
- available_payment_methods.each do |payment_method|
%div.checkout-input
= f.radio_button :payment_method_id, payment_method.id,
name: "order[payments_attributes][][payment_method_id]",
id: "payment_method_#{payment_method.id}",
name: "order[payments_attributes][][payment_method_id]",
checked: (payment_method.id == selected_payment_method),
"data-action": "paymentmethod#selectPaymentMethod",
"data-paymentmethod-description": "#{payment_method.description}"
= f.label payment_method.id, "#{payment_method.name} (#{payment_or_shipping_price(payment_method, @order)})", {for: "payment_method_" + payment_method.id.to_s }
= f.label :payment_method_id, "#{payment_method.name} (#{payment_or_shipping_price(payment_method, @order)})", for: "payment_method_#{payment_method.id}"
= f.error_message_on :payment_method, standalone: true
%div
.panel{"data-paymentmethod-target": "panel", style: "display: none"}
%div.checkout-substep
= t("split_checkout.step2.explaination")

View File

@@ -72,14 +72,11 @@
= render 'spree/orders/summary', order: @order
%div.checkout-substep.medium-6
- if @order.errors.messages_for(:base).include?("terms_not_accepted")
%div.checkout-input
%span.formError.standalone
= t("split_checkout.errors.terms_not_accepted")
%div.checkout-input
= f.check_box :accept_terms, {id: 'accept_terms', name: "accept_terms", "checked": "#{all_terms_and_conditions_already_accepted?}"}
= f.label :accept_terms, t('split_checkout.step3.terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions.url, target: '_blank'), tos_link: link_to_platform_terms), {for: "accept_terms"}
= f.check_box :accept_terms, { id: "accept_terms", name: "accept_terms", "checked": "#{all_terms_and_conditions_already_accepted?}" }, 1, nil
= f.label :accept_terms, t('split_checkout.step3.terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions.url, target: '_blank'), tos_link: link_to_platform_terms), { for: "accept_terms" }
= f.error_message_on :terms_and_conditions, standalone: true
%div.checkout-input
= t("split_checkout.step3.agree")

View File

@@ -1660,8 +1660,6 @@ en:
label: Country
delivery_address:
title: Delivery address
errors:
no_shipping_method_selected: No shipping method selected
submit: Next - Payment method
cancel: Back to Edit basket
step2:
@@ -1697,6 +1695,8 @@ en:
required: Field cannot be blank
invalid_number: "Please enter a valid phone number"
invalid_email: "Please enter a valid email address"
select_a_shipping_method: Select a shipping method
select_a_payment_method: Select a payment method
order_paid: PAID
order_not_paid: NOT PAID
order_total: Total order

View File

@@ -22,17 +22,8 @@ module OrderManagement
shipping_rates.sort_by! { |r| r.cost || 0 }
unless shipping_rates.empty?
if frontend_only
shipping_rates.each do |rate|
if rate.shipping_method.frontend?
rate.selected = true
break
end
end
else
shipping_rates.first.selected = true
end
unless shipping_rates.empty? || order.manual_shipping_selection
select_first_shipping_method(shipping_rates, frontend_only)
end
shipping_rates
@@ -40,6 +31,21 @@ module OrderManagement
private
# Sets the first available shipping method to "selected".
# Note: seems like a hangover from Spree, we can probably just remove this at some point.
def select_first_shipping_method(shipping_rates, frontend_only)
if frontend_only
shipping_rates.each do |rate|
if rate.shipping_method.frontend?
rate.selected = true
break
end
end
else
shipping_rates.first.selected = true
end
end
def shipping_methods(package)
shipping_methods = package.shipping_methods
shipping_methods.delete_if { |ship_method| !ship_method.calculator.available?(package) }

View File

@@ -388,6 +388,7 @@ describe Spree::Shipment do
end
it "should create adjustment when not present" do
allow(shipment).to receive_messages(fee_adjustment: nil)
allow(shipment).to receive_messages(selected_shipping_rate_id: 1)
expect(shipping_method).to receive(:create_adjustment).with(shipment.adjustment_label,
shipment, true, "open")