From 055c6b78afe324731d910296bc0f88b7f1441c9f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 1 Aug 2021 14:27:47 +0100 Subject: [PATCH 01/21] Add order confirmation step to checkout flow --- app/models/spree/order.rb | 3 +++ config/locales/en.yml | 1 + 2 files changed, 4 insertions(+) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 90811c2eec..9d7429a42d 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -20,6 +20,9 @@ module Spree order.update_totals order.payment_required? } + go_to_state :confirmation, if: ->(order) { + Flipper.enabled? :split_checkout, order.user + } go_to_state :complete end diff --git a/config/locales/en.yml b/config/locales/en.yml index f8f58c92da..4512f6f868 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3929,6 +3929,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using awaiting_return: awaiting return canceled: cancelled cart: cart + confirmation: "confirmation" complete: complete confirm: confirm delivery: delivery From 4d77d4df77a7dc6823b37a61344358a83554ddab Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 10 Aug 2021 18:41:48 +0100 Subject: [PATCH 02/21] Add confirm event and update workflow helpers --- app/models/spree/order/checkout.rb | 4 ++++ app/services/order_workflow.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index 9a8a0511c9..148348ce1e 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -67,6 +67,10 @@ module Spree transition to: :cart, unless: :completed? end + event :confirm do + transition to: :complete, from: :confirmation + end + before_transition from: :cart, do: :ensure_line_items_present before_transition to: :delivery, do: :create_proposed_shipments diff --git a/app/services/order_workflow.rb b/app/services/order_workflow.rb index e0b0bb9d1e..526f1de6ac 100644 --- a/app/services/order_workflow.rb +++ b/app/services/order_workflow.rb @@ -27,6 +27,10 @@ class OrderWorkflow advance_to_state("payment", advance_order_options) end + def advance_to_confirmation + advance_to_state("confirmation", advance_order_options) + end + private def advance_order_options From e91a8d603fce30fae53a7109ae4d517cdda62d6d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 10 Aug 2021 19:01:00 +0100 Subject: [PATCH 03/21] Return a boolean in OrderWorkflow#advance_to_state Returns true if advancing was successful, which is helpful in control flows that use the method in a conditional --- app/services/order_workflow.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/services/order_workflow.rb b/app/services/order_workflow.rb index 526f1de6ac..1451454416 100644 --- a/app/services/order_workflow.rb +++ b/app/services/order_workflow.rb @@ -44,6 +44,8 @@ class OrderWorkflow after_transition_hook(options) end + + order.state == target_state end def advance_order!(options) From 070cb1abc1370e472981248fc5d60c1b0e2628fb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Aug 2021 13:50:03 +0100 Subject: [PATCH 04/21] Improve checkout flow logic --- app/controllers/split_checkout_controller.rb | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 47b8728a76..d6974fb27e 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -49,16 +49,11 @@ class SplitCheckoutController < ::BaseController def update if @order.update(order_params) - OrderWorkflow.new(@order).advance_to_payment + OrderWorkflow.new(@order).advance_to_confirmation - if @order.state == "payment" - redirect_to checkout_step_path(:payment) - else - render :edit - end + redirect_to_step else flash[:error] = "Saving failed, please update the highlighted fields" - render :edit end end @@ -87,14 +82,15 @@ class SplitCheckoutController < ::BaseController end def redirect_to_step - if @order.state == "payment" - if true# order.has_no_payment_method_chosen? - redirect_to checkout_step_path(:payment) - else - redirect_to checkout_step_path(:summary) - end - else + case @order.state + when "cart", "address", "delivery" redirect_to checkout_step_path(:details) + when "payment" + redirect_to checkout_step_path(:payment) + when "confirmation" + redirect_to checkout_step_path(:summary) + else + redirect_to order_path(@order) end end From 3c2c3a18019726cf515fe45ea86a4452d5f79b9d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Aug 2021 16:17:29 +0100 Subject: [PATCH 05/21] Extract helper and update tabs --- app/helpers/checkout_helper.rb | 4 ++++ app/views/split_checkout/_tabs.html.haml | 12 ++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index 02bc03e152..3aefb48f4b 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -122,4 +122,8 @@ module CheckoutHelper "{{ #{price} | localizeCurrency }}" end end + + def checkout_step?(step) + params[:step] == step.to_s + end end diff --git a/app/views/split_checkout/_tabs.html.haml b/app/views/split_checkout/_tabs.html.haml index 9f08b7522d..b285563784 100644 --- a/app/views/split_checkout/_tabs.html.haml +++ b/app/views/split_checkout/_tabs.html.haml @@ -1,12 +1,12 @@ %div.flex - %div.columns.three.text-center.checkout-tab{"class": [("selected" if @checkout_step == "details"), ("success" unless @checkout_step == "details")]} + %div.columns.three.text-center.checkout-tab{"class": [("selected" if checkout_step?(:details)), ("success" unless checkout_step?(:details))]} %span - = link_to_if (@checkout_step != "details"), t("split_checkout.your_details"), main_app.checkout_step_path(:details), {} do + = link_to_unless checkout_step?(:details), t("split_checkout.your_details"), main_app.checkout_step_path(:details) do = t("split_checkout.your_details") - %div.columns.three.text-center.checkout-tab{"class": [("selected" if @checkout_step == "payment"), ("success" if @checkout_step == "summary")]} + %div.columns.three.text-center.checkout-tab{"class": [("selected" if checkout_step?(:payment)), ("success" if checkout_step?(:summary))]} %span - = link_to_if (@checkout_step != "payment"), t("split_checkout.payment_method"), main_app.checkout_step_path(:payment), {} do + = link_to_if checkout_step?(:summary), t("split_checkout.payment_method"), main_app.checkout_step_path(:payment) do = t("split_checkout.payment_method") - %div.columns.three.text-center.checkout-tab{"class": ("selected" if @checkout_step == "summary")} + %div.columns.three.text-center.checkout-tab{"class": ("selected" if checkout_step?(:summary))} %span - = t("split_checkout.order_summary") + = t("split_checkout.order_summary") From 41ffffe17083c274d2944e7ea99d16815c2e5510 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Aug 2021 16:19:42 +0100 Subject: [PATCH 06/21] Tidy up use of @checkout_step --- app/controllers/split_checkout_controller.rb | 7 +++---- app/helpers/checkout_helper.rb | 6 +++++- app/views/split_checkout/_form.html.haml | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index d6974fb27e..b5fb482a49 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -15,7 +15,6 @@ class SplitCheckoutController < ::BaseController # Otherwise we fail on duplicate indexes or end up with negative stock. prepend_around_action CurrentOrderLocker, only: [:edit, :update] - prepend_before_action :set_checkout_step prepend_before_action :check_hub_ready_for_checkout prepend_before_action :check_order_cycle_expiry prepend_before_action :require_order_cycle @@ -37,7 +36,7 @@ class SplitCheckoutController < ::BaseController def edit return handle_redirect_from_stripe if valid_payment_intent_provided? - redirect_to_step unless @checkout_step + redirect_to_step unless checkout_step # This is only required because of spree_paypal_express. If we implement # a version of paypal that uses this controller, and more specifically @@ -68,8 +67,8 @@ class SplitCheckoutController < ::BaseController private - def set_checkout_step - @checkout_step = params[:step] + def checkout_step + @checkout_step ||= params[:step] end def order_params diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index 3aefb48f4b..ad28194fae 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -123,7 +123,11 @@ module CheckoutHelper end end + def checkout_step + params[:step] + end + def checkout_step?(step) - params[:step] == step.to_s + checkout_step == step.to_s end end diff --git a/app/views/split_checkout/_form.html.haml b/app/views/split_checkout/_form.html.haml index 8aa242ae48..610fabdcf6 100644 --- a/app/views/split_checkout/_form.html.haml +++ b/app/views/split_checkout/_form.html.haml @@ -2,5 +2,5 @@ = inject_saved_credit_cards %div.checkout-step.medium-6 - = form_with url: checkout_update_path(@checkout_step), model: @order, method: :put do |f| - = render "split_checkout/#{@checkout_step}", f: f + = form_with url: checkout_update_path(checkout_step), model: @order, method: :put do |f| + = render "split_checkout/#{checkout_step}", f: f From ffc3736f4bbf06cc312f46d2341e76ab7e5bbd52 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Aug 2021 17:01:18 +0100 Subject: [PATCH 07/21] Update fields on confirmation page --- app/views/split_checkout/_summary.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml index e22a27d720..320b9b1bf7 100644 --- a/app/views/split_checkout/_summary.html.haml +++ b/app/views/split_checkout/_summary.html.haml @@ -103,13 +103,13 @@ %div.checkout-substep %div.checkout-input - = f.check_box :accept_terms, {id: 'accept_terms', "checked": "#{all_terms_and_conditions_already_accepted?}"} + = 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"} %div.checkout-input = t("split_checkout.step3.agree") %div.checkout-submit - = f.submit t("split_checkout.step3.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false + = f.submit t("split_checkout.step3.submit"), name: "confirm_order", class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false %a.button.cancel{href: main_app.cart_path} = t("split_checkout.step3.cancel") From 4916f823aaaee49fbf78a0fe43833a2cdfde5b8c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 11 Aug 2021 17:22:24 +0100 Subject: [PATCH 08/21] Handle order confirmation --- app/controllers/split_checkout_controller.rb | 23 +++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index b5fb482a49..e7e7221528 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -47,9 +47,8 @@ class SplitCheckoutController < ::BaseController end def update - if @order.update(order_params) - OrderWorkflow.new(@order).advance_to_confirmation - + if confirm_order || update_order + advance_order_state redirect_to_step else flash[:error] = "Saving failed, please update the highlighted fields" @@ -67,6 +66,24 @@ class SplitCheckoutController < ::BaseController private + def confirm_order + return unless @order.confirmation? && params[:confirm_order] + + @order.confirm! + end + + def update_order + return unless params[:order] + + @order.update(order_params) + end + + def advance_order_state + return if @order.confirmation? || @order.complete? + + OrderWorkflow.new(@order).advance_to_confirmation + end + def checkout_step @checkout_step ||= params[:step] end From 7e8daea2336d07a5532c03a7fe16b07328bf6335 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:04:13 +0100 Subject: [PATCH 09/21] Reduce data loading on steps --- app/controllers/split_checkout_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index e7e7221528..fa03a74bba 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -20,7 +20,8 @@ class SplitCheckoutController < ::BaseController prepend_before_action :require_order_cycle prepend_before_action :require_distributor_chosen - before_action :load_order, :load_shipping_methods, :load_countries + before_action :load_order + before_action :load_shipping_methods, :load_countries, if: -> { checkout_step == "details"} before_action :ensure_order_not_completed before_action :ensure_checkout_allowed From 598e81ed32e76f5d87556332770bd16205d93df6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:04:33 +0100 Subject: [PATCH 10/21] Progress order to address at start of checkout --- app/controllers/split_checkout_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index fa03a74bba..78686668c4 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -39,6 +39,8 @@ class SplitCheckoutController < ::BaseController 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 From 99f0672f9d69977ed5219ea9f80224b4183a83bc Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:05:49 +0100 Subject: [PATCH 11/21] Set order amount when creating payment object --- app/controllers/split_checkout_controller.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 78686668c4..fb9f5ce459 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -92,12 +92,21 @@ class SplitCheckoutController < ::BaseController end def order_params - params.require(:order).permit( + 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 end def redirect_to_step From 5d9c4315d8c91852b84cc8796ec59737bf6cd3fb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:06:32 +0100 Subject: [PATCH 12/21] Remove dead code --- app/controllers/split_checkout_controller.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index fb9f5ce459..7c49428640 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -150,7 +150,6 @@ class SplitCheckoutController < ::BaseController redirect_to_cart_path && return unless valid_order_line_items? before_address - setup_for_current_state end def redirect_to_shop? @@ -177,11 +176,6 @@ class SplitCheckoutController < ::BaseController end end - def setup_for_current_state - method_name = :"before_#{@order.state}" - __send__(method_name) if respond_to?(method_name, true) - end - def before_address associate_user @@ -191,10 +185,6 @@ class SplitCheckoutController < ::BaseController @order.ship_address = finder.ship_address end - def before_payment - current_order.payments.destroy_all if request.put? - end - def valid_payment_intent_provided? return false unless params["payment_intent"]&.starts_with?("pi_") From 6c885f0f421fedeb62f1b8ee172cb333c58ce17a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:07:40 +0100 Subject: [PATCH 13/21] Declare validations together --- app/models/spree/order.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 9d7429a42d..a81264f0c4 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -81,17 +81,16 @@ module Spree before_validation :associate_customer, unless: :customer_id? before_validation :ensure_customer, unless: :customer_is_valid? - validates :customer, presence: true, if: :require_customer? - validate :products_available_from_new_distribution, if: lambda { - distributor_id_changed? || order_cycle_id_changed? - } - validate :disallow_guest_order - attr_accessor :use_billing before_create :link_by_email after_create :create_tax_charge! + validates :customer, presence: true, if: :require_customer? + validate :products_available_from_new_distribution, if: lambda { + distributor_id_changed? || order_cycle_id_changed? + } + validate :disallow_guest_order validates :email, presence: true, format: /\A([\w.%+\-']+)@([\w\-]+\.)+(\w{2,})\z/i, if: :require_email From 5fb3943634f3a35438f541515cde206829119afc Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 10:08:01 +0100 Subject: [PATCH 14/21] Validate payment object exists before advancing from payment to confirmation --- app/models/spree/order.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index a81264f0c4..ca5b45dcfa 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -94,6 +94,7 @@ 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 From dbefd95b5e83ad5eed960334672c499a701b90fb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 12:01:25 +0100 Subject: [PATCH 15/21] Improve order workflow shipping method selection --- app/controllers/split_checkout_controller.rb | 6 ++++-- app/services/order_workflow.rb | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 7c49428640..fbd4f98837 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -82,9 +82,11 @@ class SplitCheckoutController < ::BaseController end def advance_order_state - return if @order.confirmation? || @order.complete? + return if @order.complete? - OrderWorkflow.new(@order).advance_to_confirmation + workflow_options = raw_params.slice(:shipping_method_id) + + OrderWorkflow.new(@order).advance_to_confirmation(workflow_options) end def checkout_step diff --git a/app/services/order_workflow.rb b/app/services/order_workflow.rb index 1451454416..39571a6545 100644 --- a/app/services/order_workflow.rb +++ b/app/services/order_workflow.rb @@ -27,8 +27,12 @@ class OrderWorkflow advance_to_state("payment", advance_order_options) end - def advance_to_confirmation - advance_to_state("confirmation", advance_order_options) + def advance_to_confirmation(options = {}) + if options[:shipping_method_id] + order.select_shipping_method(options[:shipping_method_id]) + end + + advance_to_state("confirmation") end private @@ -38,7 +42,7 @@ class OrderWorkflow { shipping_method_id: shipping_method_id } end - def advance_to_state(target_state, options) + def advance_to_state(target_state, options = {}) until order.state == target_state break unless order.next From 41c62b97d521bff7e79838eeec65df02a0e4859e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 12:01:58 +0100 Subject: [PATCH 16/21] Use flash.now unless redirecting --- app/controllers/split_checkout_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index fbd4f98837..e1c9950bef 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -54,7 +54,7 @@ class SplitCheckoutController < ::BaseController advance_order_state redirect_to_step else - flash[:error] = "Saving failed, please update the highlighted fields" + flash.now[:error] = "Saving failed, please update the highlighted fields" render :edit end end From 98ae82147cca26ee3ed799354374b2681b3c24c0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 12:08:13 +0100 Subject: [PATCH 17/21] Avoid using Angular in shipping and payment method display --- app/helpers/checkout_helper.rb | 9 +++++++++ app/views/split_checkout/_details.html.haml | 2 +- app/views/split_checkout/_payment.html.haml | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index ad28194fae..1285ce297c 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -123,6 +123,15 @@ module CheckoutHelper end end + def payment_or_shipping_price(method, order) + price = method.compute_amount(order) + if price.zero? + t('checkout_method_free') + else + Spree::Money.new(price, currency: order.currency) + end + end + def checkout_step params[:step] end diff --git a/app/views/split_checkout/_details.html.haml b/app/views/split_checkout/_details.html.haml index 121cf6513a..c032f96917 100644 --- a/app/views/split_checkout/_details.html.haml +++ b/app/views/split_checkout/_details.html.haml @@ -80,7 +80,7 @@ "data-toggle-show": shipping_method.require_ship_address = shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s } %em.light - = payment_method_price(shipping_method, @order) + = payment_or_shipping_price(shipping_method, @order) %div.checkout-input{ "data-toggle-target": "content", style: "display: none" } diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index d9c6b9d8a6..b90d17aec5 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -11,7 +11,7 @@ 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_method_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.to_s } %div .panel{"data-paymentmethod-target": "panel", style: "display: none"} From 026e51081c5006ef3b510c012e9e5bea24f4b8a3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 12:23:55 +0100 Subject: [PATCH 18/21] Clear invalid payments When going back and changing the payment option, the previous payment gets invalidated. --- app/controllers/split_checkout_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index e1c9950bef..1d1648ab11 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -51,6 +51,7 @@ class SplitCheckoutController < ::BaseController def update if confirm_order || update_order + clear_invalid_payments advance_order_state redirect_to_step else @@ -69,6 +70,10 @@ class SplitCheckoutController < ::BaseController private + def clear_invalid_payments + @order.payments.with_state(:invalid).delete_all + end + def confirm_order return unless @order.confirmation? && params[:confirm_order] From 1147c5b2ca1a06bcf74042f70d82fc172e9b9d8f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 12:45:07 +0100 Subject: [PATCH 19/21] Update saved address loading --- app/controllers/split_checkout_controller.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 1d1648ab11..bb8ea42c21 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -20,14 +20,13 @@ class SplitCheckoutController < ::BaseController prepend_before_action :require_order_cycle prepend_before_action :require_distributor_chosen - before_action :load_order + before_action :load_order, :associate_user, :load_saved_addresses before_action :load_shipping_methods, :load_countries, if: -> { checkout_step == "details"} before_action :ensure_order_not_completed before_action :ensure_checkout_allowed before_action :handle_insufficient_stock - before_action :associate_user before_action :check_authorization before_action :enable_embedded_shopfront @@ -155,8 +154,6 @@ class SplitCheckoutController < ::BaseController redirect_to(main_app.shop_path) && return if redirect_to_shop? redirect_to_cart_path && return unless valid_order_line_items? - - before_address end def redirect_to_shop? @@ -183,9 +180,7 @@ class SplitCheckoutController < ::BaseController end end - def before_address - associate_user - + def load_saved_addresses finder = OpenFoodNetwork::AddressFinder.new(@order.email, @order.customer, spree_current_user) @order.bill_address = finder.bill_address From 1d074c2151289e880b4971e9e490f29fcdeda095 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 13:05:51 +0100 Subject: [PATCH 20/21] Extract callbacks to a concern --- .../concerns/checkout_callbacks.rb | 86 +++++++++++++++++++ app/controllers/split_checkout_controller.rb | 80 +---------------- 2 files changed, 87 insertions(+), 79 deletions(-) create mode 100644 app/controllers/concerns/checkout_callbacks.rb diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb new file mode 100644 index 0000000000..08a063ee89 --- /dev/null +++ b/app/controllers/concerns/checkout_callbacks.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +module CheckoutCallbacks + extend ActiveSupport::Concern + + included do + # We need pessimistic locking to avoid race conditions. + # Otherwise we fail on duplicate indexes or end up with negative stock. + prepend_around_action CurrentOrderLocker, only: [:edit, :update] + + prepend_before_action :check_hub_ready_for_checkout + prepend_before_action :check_order_cycle_expiry + prepend_before_action :require_order_cycle + 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 :ensure_order_not_completed + before_action :ensure_checkout_allowed + before_action :handle_insufficient_stock + before_action :check_authorization + before_action :enable_embedded_shopfront + end + + private + + def load_order + @order = current_order + + redirect_to(main_app.shop_path) && return if redirect_to_shop? + redirect_to_cart_path && return unless valid_order_line_items? + end + + def load_saved_addresses + finder = OpenFoodNetwork::AddressFinder.new(@order.email, @order.customer, spree_current_user) + + @order.bill_address = finder.bill_address + @order.ship_address = finder.ship_address + end + + def load_shipping_methods + @shipping_methods = Spree::ShippingMethod.for_distributor(@order.distributor).order(:name) + end + + def load_countries + @countries = available_countries.map { |c| [c.name, c.id] } + @countries_with_states = available_countries.map { |c| [c.id, c.states.map { |s| [s.name, s.id] }] } + end + + def redirect_to_shop? + !@order || + !@order.checkout_allowed? || + @order.completed? + end + + def redirect_to_cart_path + respond_to do |format| + format.html do + redirect_to main_app.cart_path + end + + format.json do + render json: { path: main_app.cart_path }, status: :bad_request + end + end + end + + def valid_order_line_items? + @order.insufficient_stock_lines.empty? && + OrderCycleDistributedVariants.new(@order.order_cycle, @order.distributor). + distributes_order_variants?(@order) + end + + def ensure_order_not_completed + redirect_to main_app.cart_path if @order.completed? + end + + def ensure_checkout_allowed + redirect_to main_app.cart_path unless @order.checkout_allowed? + end + + def check_authorization + authorize!(:edit, current_order, session[:access_token]) + end +end diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index bb8ea42c21..be36987689 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -7,29 +7,10 @@ class SplitCheckoutController < ::BaseController include OrderStockCheck include Spree::BaseHelper + include CheckoutCallbacks helper 'terms_and_conditions' helper 'checkout' - - # We need pessimistic locking to avoid race conditions. - # Otherwise we fail on duplicate indexes or end up with negative stock. - prepend_around_action CurrentOrderLocker, only: [:edit, :update] - - prepend_before_action :check_hub_ready_for_checkout - prepend_before_action :check_order_cycle_expiry - prepend_before_action :require_order_cycle - 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 :ensure_order_not_completed - before_action :ensure_checkout_allowed - before_action :handle_insufficient_stock - - before_action :check_authorization - before_action :enable_embedded_shopfront - helper 'spree/orders' helper OrderHelper @@ -128,65 +109,6 @@ class SplitCheckoutController < ::BaseController end end - def check_authorization - authorize!(:edit, current_order, session[:access_token]) - end - - def ensure_checkout_allowed - redirect_to main_app.cart_path unless @order.checkout_allowed? - end - - def ensure_order_not_completed - redirect_to main_app.cart_path if @order.completed? - end - - def load_shipping_methods - @shipping_methods = Spree::ShippingMethod.for_distributor(@order.distributor).order(:name) - end - - def load_countries - @countries = available_countries.map { |c| [c.name, c.id] } - @countries_with_states = available_countries.map { |c| [c.id, c.states.map { |s| [s.name, s.id] }] } - end - - def load_order - @order = current_order - - redirect_to(main_app.shop_path) && return if redirect_to_shop? - redirect_to_cart_path && return unless valid_order_line_items? - end - - def redirect_to_shop? - !@order || - !@order.checkout_allowed? || - @order.completed? - end - - def valid_order_line_items? - @order.insufficient_stock_lines.empty? && - OrderCycleDistributedVariants.new(@order.order_cycle, @order.distributor). - distributes_order_variants?(@order) - end - - def redirect_to_cart_path - respond_to do |format| - format.html do - redirect_to main_app.cart_path - end - - format.json do - render json: { path: main_app.cart_path }, status: :bad_request - end - end - end - - def load_saved_addresses - finder = OpenFoodNetwork::AddressFinder.new(@order.email, @order.customer, spree_current_user) - - @order.bill_address = finder.bill_address - @order.ship_address = finder.ship_address - end - def valid_payment_intent_provided? return false unless params["payment_intent"]&.starts_with?("pi_") From fb6ee658654f1bec59749947258039d863e81804 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 12 Aug 2021 13:08:31 +0100 Subject: [PATCH 21/21] Remove more (currently) unused code --- app/controllers/split_checkout_controller.rb | 103 ------------------- 1 file changed, 103 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index be36987689..dfc979d46a 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -40,14 +40,6 @@ class SplitCheckoutController < ::BaseController end end - # Clears the cached order. Required for #current_order to return a new order - # to serve as cart. See https://github.com/spree/spree/blob/1-3-stable/core/lib/spree/core/controller_helpers/order.rb#L14 - # for details. - def expire_current_order - session[:order_id] = nil - @current_order = nil - end - private def clear_invalid_payments @@ -129,107 +121,12 @@ class SplitCheckoutController < ::BaseController end end - def checkout_workflow(shipping_method_id) - while @order.state != "complete" - if @order.state == "payment" - return if redirect_to_payment_gateway - - return action_failed unless @order.process_payments! - end - - next if OrderWorkflow.new(@order).next({ shipping_method_id: shipping_method_id }) - - return action_failed - end - - update_response - end - - def redirect_to_payment_gateway - return unless params&.dig(:order)&.dig(:payments_attributes)&.first&.dig(:payments_attributes) - - redirect_path = Checkout::PaypalRedirect.new(params).path - redirect_path = Checkout::StripeRedirect.new(params, @order).path if redirect_path.blank? - return if redirect_path.blank? - - render json: { path: redirect_path }, status: :ok - true - end - - def order_error - if @order.errors.present? - @order.errors.full_messages.to_sentence - else - t(:payment_processing_failed) - end - end - - def update_response - if order_complete? - checkout_succeeded - update_succeeded_response - else - action_failed(RuntimeError.new("Order not complete after the checkout workflow")) - end - end - def order_complete? @order.state == "complete" || @order.completed? end - def checkout_succeeded - Checkout::PostCheckoutActions.new(@order).success(self, params, spree_current_user) - - session[:access_token] = current_order.token - flash[:notice] = t(:order_processed_successfully) - end - - def update_succeeded_response - respond_to do |format| - format.html do - respond_with(@order, location: order_path(@order)) - end - format.json do - render json: { path: order_path(@order) }, status: :ok - end - end - end - - def action_failed(error = RuntimeError.new(order_error)) - checkout_failed(error) - action_failed_response - end - - def checkout_failed(error = RuntimeError.new(order_error)) - Bugsnag.notify(error) - Checkout::PostCheckoutActions.new(@order).failure - end - - def action_failed_response - respond_to do |format| - format.html do - render :edit - end - format.json do - discard_flash_errors - render json: { errors: @order.errors, flash: flash.to_hash }.to_json, status: :bad_request - end - end - end - def rescue_from_spree_gateway_error(error) flash[:error] = t(:spree_gateway_error_flash_for_checkout, error: error.message) action_failed(error) end - - def permitted_params - PermittedAttributes::Checkout.new(params).call - end - - def discard_flash_errors - # Marks flash errors for deletion after the current action has completed. - # This ensures flash errors generated during XHR requests are not persisted in the - # session for longer than expected. - flash.discard(:error) - end end