From cc9e3fe69bf14ab34db0930a7eee47b791642180 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 20 Jan 2021 14:24:59 +0100 Subject: [PATCH] Replace double negation with proper list of states We rely now on the exhaustive list of states an order can be in after checkout. What made this all a bit more messy is that I made up the "checkout" order state, likely mixing it from the payment model states. This simplifies things quite a bit and gives meaningful names to things. --- app/models/spree/order.rb | 9 +++------ app/queries/complete_orders_with_balance.rb | 6 +++--- app/queries/customers_with_balance.rb | 9 ++++----- app/queries/outstanding_balance.rb | 6 +++++- spec/queries/customers_with_balance_spec.rb | 18 +++++++++--------- spec/queries/outstanding_balance_spec.rb | 18 +++++++++--------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index a908c56e1e..ac75ff2fe1 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -130,13 +130,10 @@ module Spree where("state != ?", state) } - # All the states an order can be in before completing the checkout - PRIOR_TO_COMPLETION_STATES = %w(cart address delivery payment).freeze - # All the states of a complete order but that shouldn't count towards the balance. Those that - # the customer won't enjoy. - NON_FULFILLED_STATES = %w(canceled returned).freeze + # All the states an order can be in after completing the checkout + FINALIZED_STATES = %w(complete canceled resumed awaiting_return returned).freeze - scope :in_incomplete_state, -> { where(state: PRIOR_TO_COMPLETION_STATES) } + scope :finalized, -> { where(state: FINALIZED_STATES) } def self.by_number(number) where(number: number) diff --git a/app/queries/complete_orders_with_balance.rb b/app/queries/complete_orders_with_balance.rb index 31e89d69fb..57223e5c6e 100644 --- a/app/queries/complete_orders_with_balance.rb +++ b/app/queries/complete_orders_with_balance.rb @@ -7,14 +7,14 @@ class CompleteOrdersWithBalance end def query - OutstandingBalance.new(sorted_complete_orders).query + OutstandingBalance.new(sorted_finalized_orders).query end private - def sorted_complete_orders + def sorted_finalized_orders @user.orders - .where.not(Spree::Order.in_incomplete_state.where_values_hash) + .finalized .select('spree_orders.*') .order(completed_at: :desc) end diff --git a/app/queries/customers_with_balance.rb b/app/queries/customers_with_balance.rb index 0d547f6c2c..4abd070d34 100644 --- a/app/queries/customers_with_balance.rb +++ b/app/queries/customers_with_balance.rb @@ -28,13 +28,12 @@ class CustomersWithBalance def left_join_complete_orders <<-SQL.strip_heredoc LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id - AND #{complete_orders.to_sql} + AND #{finalized_states.to_sql} SQL end - def complete_orders - states = Spree::Order::PRIOR_TO_COMPLETION_STATES - .map { |state| Arel::Nodes.build_quoted(state) } - Arel::Nodes::NotIn.new(Spree::Order.arel_table[:state], states) + def finalized_states + states = Spree::Order::FINALIZED_STATES.map { |state| Arel::Nodes.build_quoted(state) } + Arel::Nodes::In.new(Spree::Order.arel_table[:state], states) end end diff --git a/app/queries/outstanding_balance.rb b/app/queries/outstanding_balance.rb index a31bb1b2df..ce4d4d78f4 100644 --- a/app/queries/outstanding_balance.rb +++ b/app/queries/outstanding_balance.rb @@ -9,6 +9,10 @@ # # See CompleteOrdersWithBalance or CustomersWithBalance as examples. class OutstandingBalance + # All the states of a finished order but that shouldn't count towards the balance (the customer + # didn't get the order for whatever reason). Note it does not include complete + FINALIZED_NON_SUCCESSFUL_STATES = %w(canceled returned).freeze + # The relation must be an ActiveRecord::Relation object with `spree_orders` in the SQL statement # FROM for #statement to work. def initialize(relation = nil) @@ -34,7 +38,7 @@ class OutstandingBalance attr_reader :relation def non_fulfilled_states_group - states = Spree::Order::NON_FULFILLED_STATES.map { |state| Arel::Nodes.build_quoted(state) } + states = FINALIZED_NON_SUCCESSFUL_STATES.map { |state| Arel::Nodes.build_quoted(state) } Arel::Nodes::Grouping.new(states) end end diff --git a/spec/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_spec.rb index 7942e36060..479be6b410 100644 --- a/spec/queries/customers_with_balance_spec.rb +++ b/spec/queries/customers_with_balance_spec.rb @@ -69,9 +69,9 @@ describe CustomersWithBalance do context 'when no orders where paid' do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do @@ -85,9 +85,9 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: payment_total) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do @@ -102,7 +102,7 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') create( :order, customer: customer, @@ -123,7 +123,7 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'resumed') end @@ -139,7 +139,7 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'payment') end @@ -155,7 +155,7 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'awaiting_return') end @@ -172,7 +172,7 @@ describe CustomersWithBalance do before do order = create(:order, customer: customer, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, customer: customer, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'returned') end diff --git a/spec/queries/outstanding_balance_spec.rb b/spec/queries/outstanding_balance_spec.rb index a662bcccf9..af880528f7 100644 --- a/spec/queries/outstanding_balance_spec.rb +++ b/spec/queries/outstanding_balance_spec.rb @@ -79,9 +79,9 @@ describe OutstandingBalance do context 'when no orders where paid' do before do order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do @@ -95,9 +95,9 @@ describe OutstandingBalance do before do order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, total: order_total, payment_total: payment_total) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do @@ -113,7 +113,7 @@ describe OutstandingBalance do before do create(:order, total: order_total, payment_total: order_total, state: 'canceled') order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do @@ -127,7 +127,7 @@ describe OutstandingBalance do before do order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'resumed') end @@ -143,7 +143,7 @@ describe OutstandingBalance do before do order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'payment') end @@ -159,7 +159,7 @@ describe OutstandingBalance do before do order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') order = create(:order, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'awaiting_return') end @@ -178,7 +178,7 @@ describe OutstandingBalance do order = create(:order, total: order_total, payment_total: payment_total) order.update_attribute(:state, 'returned') order = create(:order, total: order_total, payment_total: 0) - order.update_attribute(:state, 'checkout') + order.update_attribute(:state, 'complete') end it 'returns the customer balance' do