From 54738fc5520feac9272145520109dd55becfa272 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 Mar 2024 15:35:00 +1100 Subject: [PATCH] Remove unnecessary method checkout_steps It allowed introspection of a dynamic state machine. But the only two usages of this method only referred to the first state which is always the same. Our complicated checkout logic needs more clarity and introducing some hardcoded state names here can only help. --- app/controllers/spree/orders_controller.rb | 2 +- app/models/spree/order/checkout.rb | 21 +-------------------- spec/models/spree/order/checkout_spec.rb | 16 ---------------- spec/models/spree/order_spec.rb | 13 ------------- 4 files changed, 2 insertions(+), 50 deletions(-) diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index f1055785c6..7ac23f8222 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -82,7 +82,7 @@ module Spree format.html do if params.key?(:checkout) @order.next_transition.run_callbacks if @order.cart? - redirect_to main_app.checkout_step_path(@order.checkout_steps.first) + redirect_to main_app.checkout_step_path("address") elsif @order.complete? redirect_to main_app.order_path(@order) else diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index e5a09f2cc3..e51ffe442a 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -8,7 +8,6 @@ module Spree class_attribute :next_event_transitions class_attribute :previous_states class_attribute :checkout_flow - class_attribute :checkout_steps def self.checkout_flow(&block) if block_given? @@ -20,7 +19,6 @@ module Spree end def self.define_state_machine! - self.checkout_steps = {} self.next_event_transitions = [] self.previous_states = [:cart] @@ -97,7 +95,6 @@ module Spree end def self.go_to_state(name, options = {}) - checkout_steps[name] = options previous_states.each do |state| add_transition({ from: state, to: name }.merge(options)) end @@ -112,30 +109,14 @@ module Spree @next_event_transitions ||= [] end - def self.checkout_steps - @checkout_steps ||= {} - end - def self.add_transition(options) next_event_transitions << { options.delete(:from) => options.delete(:to) }. merge(options) end - def checkout_steps - steps = self.class.checkout_steps. - each_with_object([]) { |(step, options), checkout_steps| - next if options.include?(:if) && !options[:if].call(self) - - checkout_steps << step - }.map(&:to_s) - # Ensure there is always a complete step - steps << "complete" unless steps.include?("complete") - steps - end - def restart_checkout_flow update_columns( - state: checkout_steps.first, + state: "address", updated_at: Time.zone.now, ) end diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index 391442e2f6..a8686917b2 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -6,22 +6,6 @@ describe Spree::Order::Checkout do let(:order) { Spree::Order.new } context "with default state machine" do - context "#checkout_steps" do - context "when payment not required" do - before { allow(order).to receive_messages payment_required?: false } - specify do - expect(order.checkout_steps).to eq %w(address delivery confirmation complete) - end - end - - context "when payment required" do - before { allow(order).to receive_messages payment_required?: true } - specify do - expect(order.checkout_steps).to eq %w(address delivery payment confirmation complete) - end - end - end - it "starts out at cart" do expect(order.state).to eq "cart" end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 480cef7cab..e3efd69a4b 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -1325,19 +1325,6 @@ describe Spree::Order do end end - describe "determining checkout steps for an order" do - let!(:enterprise) { create(:enterprise) } - let!(:order) { create(:order, distributor: enterprise) } - let!(:payment_method) { - create(:stripe_sca_payment_method, distributor_ids: [enterprise.id]) - } - let!(:payment) { create(:payment, order:, payment_method:) } - - it "does not include the :confirm step" do - expect(order.checkout_steps).not_to include "confirm" - end - end - describe "payments" do let(:payment_method) { create(:payment_method) } let(:shipping_method) { create(:shipping_method) }