From f8c0edd68bef490ca0f715b170b3864c163af445 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:05:25 +0500 Subject: [PATCH 1/9] Update CompleteOrdersWithBalanceQuery --- app/controllers/spree/users_controller.rb | 2 +- ...b => complete_orders_with_balance_query.rb} | 4 ++-- app/queries/outstanding_balance.rb | 2 +- app/serializers/api/order_serializer.rb | 2 +- ...complete_orders_with_balance_query_spec.rb} | 18 ++++++++---------- 5 files changed, 13 insertions(+), 15 deletions(-) rename app/queries/{complete_orders_with_balance.rb => complete_orders_with_balance_query.rb} (88%) rename spec/queries/{complete_orders_with_balance_spec.rb => complete_orders_with_balance_query_spec.rb} (73%) diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index 60b7b33e26..4feff9bf7a 100644 --- a/app/controllers/spree/users_controller.rb +++ b/app/controllers/spree/users_controller.rb @@ -79,7 +79,7 @@ module Spree private def orders_collection - CompleteOrdersWithBalance.new(@user).query + CompleteOrdersWithBalanceQuery.new(@user).call end def load_object diff --git a/app/queries/complete_orders_with_balance.rb b/app/queries/complete_orders_with_balance_query.rb similarity index 88% rename from app/queries/complete_orders_with_balance.rb rename to app/queries/complete_orders_with_balance_query.rb index 57223e5c6e..665e2d2838 100644 --- a/app/queries/complete_orders_with_balance.rb +++ b/app/queries/complete_orders_with_balance_query.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true # Fetches complete orders of the specified user including their balance as a computed column -class CompleteOrdersWithBalance +class CompleteOrdersWithBalanceQuery def initialize(user) @user = user end - def query + def call OutstandingBalance.new(sorted_finalized_orders).query end diff --git a/app/queries/outstanding_balance.rb b/app/queries/outstanding_balance.rb index 637451b25c..b157739b29 100644 --- a/app/queries/outstanding_balance.rb +++ b/app/queries/outstanding_balance.rb @@ -7,7 +7,7 @@ # Alternatively, you can get the SQL by calling #statement, which is suitable for more complex # cases. # -# See CompleteOrdersWithBalance or CustomersWithBalance as examples. +# See CompleteOrdersWithBalanceQuery or CustomersWithBalance as examples. # # Note this query object and `app/models/concerns/balance.rb` should implement the same behavior # until we find a better way. If you change one, please, change the other too. diff --git a/app/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb index 067d23b76d..900d2aeaa0 100644 --- a/app/serializers/api/order_serializer.rb +++ b/app/serializers/api/order_serializer.rb @@ -9,7 +9,7 @@ module Api has_many :payments, serializer: Api::PaymentSerializer - # This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalance` + # This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalanceQuery` # for reference. def outstanding_balance -object.balance_value diff --git a/spec/queries/complete_orders_with_balance_spec.rb b/spec/queries/complete_orders_with_balance_query_spec.rb similarity index 73% rename from spec/queries/complete_orders_with_balance_spec.rb rename to spec/queries/complete_orders_with_balance_query_spec.rb index a587153766..f0988bc532 100644 --- a/spec/queries/complete_orders_with_balance_spec.rb +++ b/spec/queries/complete_orders_with_balance_query_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -describe CompleteOrdersWithBalance do - let(:complete_orders_with_balance) { described_class.new(user) } +describe CompleteOrdersWithBalanceQuery do + let(:result) { described_class.new(user).call } - describe '#query' do + describe '#call' do let(:user) { order.user } let(:outstanding_balance) { instance_double(OutstandingBalance) } @@ -28,17 +28,16 @@ describe CompleteOrdersWithBalance do allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:query) - complete_orders_with_balance.query + result end it 'returns complete orders including their balance' do - order = complete_orders_with_balance.query.first + order = result.first expect(order[:balance_value]).to eq(-1.0) end it 'sorts them by their completed_at with the most recent first' do - orders = complete_orders_with_balance.query - expect(orders.pluck(:id)).to eq([other_order.id, order.id]) + expect(result.pluck(:id)).to eq([other_order.id, order.id]) end end @@ -49,12 +48,11 @@ describe CompleteOrdersWithBalance do allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:query) - complete_orders_with_balance.query + result end it 'returns an empty array' do - order = complete_orders_with_balance.query - expect(order).to be_empty + expect(result).to be_empty end end end From d4f37a3daabd88a01780b4c5331226dcbb0b7389 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:08:21 +0500 Subject: [PATCH 2/9] Update CompleteVisibleOrdersQuery --- ...le_orders.rb => complete_visible_orders_query.rb} | 4 ++-- lib/reporting/line_items.rb | 2 +- lib/reporting/reports/bulk_coop/base.rb | 2 +- ...spec.rb => complete_visible_orders_query_spec.rb} | 12 ++++++------ 4 files changed, 10 insertions(+), 10 deletions(-) rename app/queries/{complete_visible_orders.rb => complete_visible_orders_query.rb} (83%) rename spec/queries/{complete_visible_orders_spec.rb => complete_visible_orders_query_spec.rb} (72%) diff --git a/app/queries/complete_visible_orders.rb b/app/queries/complete_visible_orders_query.rb similarity index 83% rename from app/queries/complete_visible_orders.rb rename to app/queries/complete_visible_orders_query.rb index 10eabe4f6d..22e427fec6 100644 --- a/app/queries/complete_visible_orders.rb +++ b/app/queries/complete_visible_orders_query.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -class CompleteVisibleOrders +class CompleteVisibleOrdersQuery def initialize(order_permissions) @order_permissions = order_permissions end - def query + def call order_permissions.visible_orders.complete end diff --git a/lib/reporting/line_items.rb b/lib/reporting/line_items.rb index a949e02dce..fe56194417 100644 --- a/lib/reporting/line_items.rb +++ b/lib/reporting/line_items.rb @@ -7,7 +7,7 @@ module Reporting @order_permissions = order_permissions @params = params complete_not_canceled_visible_orders = - CompleteVisibleOrders.new(order_permissions).query.not_state(:canceled) + CompleteVisibleOrdersQuery.new(order_permissions).call.not_state(:canceled) @orders_relation = orders_relation || complete_not_canceled_visible_orders end diff --git a/lib/reporting/reports/bulk_coop/base.rb b/lib/reporting/reports/bulk_coop/base.rb index bf48a6a9d8..5bb079d923 100644 --- a/lib/reporting/reports/bulk_coop/base.rb +++ b/lib/reporting/reports/bulk_coop/base.rb @@ -35,7 +35,7 @@ module Reporting @report_line_items ||= Reporting::LineItems.new( order_permissions, @params, - CompleteVisibleOrders.new(order_permissions).query + CompleteVisibleOrdersQuery.new(order_permissions).call ) end diff --git a/spec/queries/complete_visible_orders_spec.rb b/spec/queries/complete_visible_orders_query_spec.rb similarity index 72% rename from spec/queries/complete_visible_orders_spec.rb rename to spec/queries/complete_visible_orders_query_spec.rb index 2018674338..982c567e1f 100644 --- a/spec/queries/complete_visible_orders_spec.rb +++ b/spec/queries/complete_visible_orders_query_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -describe CompleteVisibleOrders do - subject(:complete_visible_orders) { described_class.new(order_permissions) } +describe CompleteVisibleOrdersQuery do + subject(:result) { described_class.new(order_permissions).call } let(:filter_canceled) { false } - describe '#query' do + describe '#call' do let(:user) { create(:user) } let(:enterprise) { create(:enterprise) } let(:order_permissions) { Permissions::Order.new(user, filter_canceled) } @@ -20,7 +20,7 @@ describe CompleteVisibleOrders do let(:cart_order) { create(:order, distributor: enterprise) } it 'does not return it' do - expect(complete_visible_orders.query).not_to include(cart_order) + expect(result).not_to include(cart_order) end end @@ -28,13 +28,13 @@ describe CompleteVisibleOrders do let(:complete_order) { create(:order, completed_at: 1.day.ago, distributor: enterprise) } it 'does not return it' do - expect(complete_visible_orders.query).to include(complete_order) + expect(result).to include(complete_order) end end it 'calls #visible_orders' do expect(order_permissions).to receive(:visible_orders).and_call_original - complete_visible_orders.query + result end end end From 81f40a99d9debf2c8ab65a4d12aa470f83294087 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:25:33 +0500 Subject: [PATCH 3/9] Update CustomersWithBalanceQuery --- app/controllers/admin/customers_controller.rb | 2 +- .../api/v1/customers_controller.rb | 4 +- ...nce.rb => customers_with_balance_query.rb} | 4 +- app/queries/outstanding_balance.rb | 2 +- .../admin/customer_with_balance_serializer.rb | 2 +- .../admin/customers_controller_spec.rb | 8 +- .../order_cycle_management_report_spec.rb | 2 - spec/queries/customers_with_balance_spec.rb | 92 ++++++++++--------- 8 files changed, 58 insertions(+), 58 deletions(-) rename app/queries/{customers_with_balance.rb => customers_with_balance_query.rb} (95%) diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 9751e9c6bb..ff5c43ab63 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -70,7 +70,7 @@ module Admin def collection if json_request? && params[:enterprise_id].present? - CustomersWithBalance.new(customers).query. + CustomersWithBalanceQuery.new(customers).call. includes( :enterprise, { bill_address: [:state, :country] }, diff --git a/app/controllers/api/v1/customers_controller.rb b/app/controllers/api/v1/customers_controller.rb index 3f488251e7..0fe756efc5 100644 --- a/app/controllers/api/v1/customers_controller.rb +++ b/app/controllers/api/v1/customers_controller.rb @@ -59,7 +59,7 @@ module Api def customer @customer ||= if action_name == "show" - CustomersWithBalance.new(Customer.where(id: params[:id])).query.first! + CustomersWithBalanceQuery.new(Customer.where(id: params[:id])).call.first! else Customer.find(params[:id]) end @@ -74,7 +74,7 @@ module Api customers = customers.where(enterprise_id: params[:enterprise_id]) if params[:enterprise_id] if @extra_customer_fields.include?(:balance) - customers = CustomersWithBalance.new(customers).query + customers = CustomersWithBalanceQuery.new(customers).call end customers.ransack(params[:q]).result.order(:id) diff --git a/app/queries/customers_with_balance.rb b/app/queries/customers_with_balance_query.rb similarity index 95% rename from app/queries/customers_with_balance.rb rename to app/queries/customers_with_balance_query.rb index 623190ff4c..aa02f37e65 100644 --- a/app/queries/customers_with_balance.rb +++ b/app/queries/customers_with_balance_query.rb @@ -2,12 +2,12 @@ # Adds an aggregated 'balance_value' to each customer based on their order history # -class CustomersWithBalance +class CustomersWithBalanceQuery def initialize(customers) @customers = customers end - def query + def call @customers. joins(left_join_complete_orders). group("customers.id"). diff --git a/app/queries/outstanding_balance.rb b/app/queries/outstanding_balance.rb index b157739b29..023c23a928 100644 --- a/app/queries/outstanding_balance.rb +++ b/app/queries/outstanding_balance.rb @@ -7,7 +7,7 @@ # Alternatively, you can get the SQL by calling #statement, which is suitable for more complex # cases. # -# See CompleteOrdersWithBalanceQuery or CustomersWithBalance as examples. +# See CompleteOrdersWithBalanceQuery or CustomersWithBalanceQuery as examples. # # Note this query object and `app/models/concerns/balance.rb` should implement the same behavior # until we find a better way. If you change one, please, change the other too. diff --git a/app/serializers/api/admin/customer_with_balance_serializer.rb b/app/serializers/api/admin/customer_with_balance_serializer.rb index fb043dda76..69f740ac9d 100644 --- a/app/serializers/api/admin/customer_with_balance_serializer.rb +++ b/app/serializers/api/admin/customer_with_balance_serializer.rb @@ -3,7 +3,7 @@ module Api module Admin # This serializer relies on `object` to respond to `#balance_value`. That's done in - # `CustomersWithBalance` due to the fact that ActiveRecord maps the DB result set's columns to + # `CustomersWithBalanceQuery` due to the fact that ActiveRecord maps the DB result set's columns to # instance methods. This way, the `balance_value` alias on that class ends up being # `object.balance_value` here. class CustomerWithBalanceSerializer < CustomerSerializer diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 1ef7e91e3e..96667f2d65 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -44,12 +44,12 @@ module Admin get :index, params: end - it 'calls CustomersWithBalance' do - customers_with_balance = instance_double(CustomersWithBalance) - allow(CustomersWithBalance) + it 'calls CustomersWithBalanceQuery' do + customers_with_balance = instance_double(CustomersWithBalanceQuery) + allow(CustomersWithBalanceQuery) .to receive(:new).with(customers) { customers_with_balance } - expect(customers_with_balance).to receive(:query) { Customer.none } + expect(customers_with_balance).to receive(:call) { Customer.none } get :index, params: end diff --git a/spec/lib/reports/order_cycle_management_report_spec.rb b/spec/lib/reports/order_cycle_management_report_spec.rb index 6b88cdc4e9..47c5c1fbdc 100644 --- a/spec/lib/reports/order_cycle_management_report_spec.rb +++ b/spec/lib/reports/order_cycle_management_report_spec.rb @@ -17,8 +17,6 @@ module Reporting end describe "fetching orders" do - let(:customers_with_balance) { instance_double(CustomersWithBalance) } - it 'calls the OutstandingBalance query object' do outstanding_balance = instance_double(OutstandingBalance, query: Spree::Order.none) expect(OutstandingBalance).to receive(:new).and_return(outstanding_balance) diff --git a/spec/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_spec.rb index 7a600e9412..e886350e5f 100644 --- a/spec/queries/customers_with_balance_spec.rb +++ b/spec/queries/customers_with_balance_spec.rb @@ -2,97 +2,99 @@ require 'spec_helper' -describe CustomersWithBalance do - subject(:customers_with_balance) { described_class.new(Customer.where(id: customer)) } +describe CustomersWithBalanceQuery do + subject(:result) { described_class.new(Customer.where(id: customers)).call } - describe '#query' do - let(:customer) { create(:customer) } + describe '#call' do + let(:customers) { create(:customer) } let(:total) { 200.00 } let(:order_total) { 100.00 } let(:outstanding_balance) { instance_double(OutstandingBalance) } - it 'calls CustomersWithBalance#statement' do + it 'calls OutstandingBalance#statement' do allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:statement) - customers_with_balance.query + result end describe 'arguments' do context 'with customers collection' do + let(:customers) { create_pair(:customer) } + it 'returns balance' do - customers = create_pair(:customer) - query = described_class.new(Customer.where(id: customers)).query - expect(query.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) - expect(query.map(&:balance_value)).to eq([0, 0]) + expect(result.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) + expect(result.map(&:balance_value)).to eq([0, 0]) end end context 'with empty customers collection' do + let(:customers) { Customer.none } + it 'returns empty customers collection' do - expect(described_class.new(Customer.none).query).to eq([]) + expect(result).to eq([]) end end end context 'when orders are in cart state' do before do - create(:order, customer:, total: order_total, payment_total: 0, state: 'cart') - create(:order, customer:, total: order_total, payment_total: 0, state: 'cart') + create(:order, customer: customers, total: order_total, payment_total: 0, state: 'cart') + create(:order, customer: customers, total: order_total, payment_total: 0, state: 'cart') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(0) end end context 'when orders are in address state' do before do - create(:order, customer:, total: order_total, payment_total: 0, state: 'address') - create(:order, customer:, total: order_total, payment_total: 50, state: 'address') + create(:order, customer: customers, total: order_total, payment_total: 0, state: 'address') + create(:order, customer: customers, total: order_total, payment_total: 50, state: 'address') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(0) end end context 'when orders are in delivery state' do before do - create(:order, customer:, total: order_total, payment_total: 0, state: 'delivery') - create(:order, customer:, total: order_total, payment_total: 50, state: 'delivery') + create(:order, customer: customers, total: order_total, payment_total: 0, state: 'delivery') + create(:order, customer: customers, total: order_total, payment_total: 50, state: 'delivery') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(0) end end context 'when orders are in payment state' do before do - create(:order, customer:, total: order_total, payment_total: 0, state: 'payment') - create(:order, customer:, total: order_total, payment_total: 50, state: 'payment') + create(:order, customer: customers, total: order_total, payment_total: 0, state: 'payment') + create(:order, customer: customers, total: order_total, payment_total: 50, state: 'payment') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(0) end end context 'when no orders where paid' do before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(-total) end end @@ -101,14 +103,14 @@ describe CustomersWithBalance do let(:payment_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total:) + order = create(:order, customer: customers, total: order_total, payment_total:) order.update_attribute(:state, 'complete') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -118,11 +120,11 @@ describe CustomersWithBalance do let(:non_canceled_orders_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') create( :order, - customer:, + customer: customers, total: order_total, payment_total: order_total, state: 'canceled' @@ -130,7 +132,7 @@ describe CustomersWithBalance do end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - non_canceled_orders_total) end end @@ -139,14 +141,14 @@ describe CustomersWithBalance do let(:payment_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total:) + order = create(:order, customer: customers, total: order_total, payment_total:) order.update_attribute(:state, 'resumed') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -155,14 +157,14 @@ describe CustomersWithBalance do let(:payment_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total:) + order = create(:order, customer: customers, total: order_total, payment_total:) order.update_attribute(:state, 'payment') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -171,14 +173,14 @@ describe CustomersWithBalance do let(:payment_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total:) + order = create(:order, customer: customers, total: order_total, payment_total:) order.update_attribute(:state, 'awaiting_return') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -188,21 +190,21 @@ describe CustomersWithBalance do let(:non_returned_orders_total) { order_total } before do - order = create(:order, customer:, total: order_total, payment_total: 0) + order = create(:order, customer: customers, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer:, total: order_total, payment_total:) + order = create(:order, customer: customers, total: order_total, payment_total:) order.update_attribute(:state, 'returned') end it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(payment_total - non_returned_orders_total) end end context 'when there are no orders' do it 'returns the customer balance' do - customer = customers_with_balance.query.first + customer = result.first expect(customer.balance_value).to eq(0) end end From ff6830f95486646b7f0280486c5b015e0f6d16aa Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:37:11 +0500 Subject: [PATCH 4/9] Update OutstandingBalanceQuery --- .../complete_orders_with_balance_query.rb | 2 +- app/queries/customers_with_balance_query.rb | 2 +- ...alance.rb => outstanding_balance_query.rb} | 4 +-- .../reports/order_cycle_management/base.rb | 2 +- .../spree/users_controller_spec.rb | 8 ++--- .../order_cycle_management_report_spec.rb | 6 ++-- ...complete_orders_with_balance_query_spec.rb | 14 ++++---- ...b => customers_with_balance_query_spec.rb} | 6 ++-- ...c.rb => outstanding_balance_query_spec.rb} | 33 ++++++++++--------- 9 files changed, 39 insertions(+), 38 deletions(-) rename app/queries/{outstanding_balance.rb => outstanding_balance_query.rb} (97%) rename spec/queries/{customers_with_balance_spec.rb => customers_with_balance_query_spec.rb} (97%) rename spec/queries/{outstanding_balance_spec.rb => outstanding_balance_query_spec.rb} (88%) diff --git a/app/queries/complete_orders_with_balance_query.rb b/app/queries/complete_orders_with_balance_query.rb index 665e2d2838..c6507f45bb 100644 --- a/app/queries/complete_orders_with_balance_query.rb +++ b/app/queries/complete_orders_with_balance_query.rb @@ -7,7 +7,7 @@ class CompleteOrdersWithBalanceQuery end def call - OutstandingBalance.new(sorted_finalized_orders).query + OutstandingBalanceQuery.new(sorted_finalized_orders).call end private diff --git a/app/queries/customers_with_balance_query.rb b/app/queries/customers_with_balance_query.rb index aa02f37e65..a33b109540 100644 --- a/app/queries/customers_with_balance_query.rb +++ b/app/queries/customers_with_balance_query.rb @@ -32,6 +32,6 @@ class CustomersWithBalanceQuery end def outstanding_balance_sum - "SUM(#{OutstandingBalance.new.statement})::float" + "SUM(#{OutstandingBalanceQuery.new.statement})::float" end end diff --git a/app/queries/outstanding_balance.rb b/app/queries/outstanding_balance_query.rb similarity index 97% rename from app/queries/outstanding_balance.rb rename to app/queries/outstanding_balance_query.rb index 023c23a928..a0685800fa 100644 --- a/app/queries/outstanding_balance.rb +++ b/app/queries/outstanding_balance_query.rb @@ -11,7 +11,7 @@ # # Note this query object and `app/models/concerns/balance.rb` should implement the same behavior # until we find a better way. If you change one, please, change the other too. -class OutstandingBalance +class OutstandingBalanceQuery # 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 @@ -22,7 +22,7 @@ class OutstandingBalance @relation = relation end - def query + def call relation.select("#{statement} AS balance_value") end diff --git a/lib/reporting/reports/order_cycle_management/base.rb b/lib/reporting/reports/order_cycle_management/base.rb index e20e13ab25..d62cd3de33 100644 --- a/lib/reporting/reports/order_cycle_management/base.rb +++ b/lib/reporting/reports/order_cycle_management/base.rb @@ -29,7 +29,7 @@ module Reporting def orders search_result = search.result.order(:completed_at) - orders = OutstandingBalance.new(search_result).query.select('spree_orders.*') + orders = OutstandingBalanceQuery.new(search_result).call.select('spree_orders.*') filter(orders) end diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 8f6455055c..c61d64d886 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -23,7 +23,7 @@ describe Spree::UsersController, type: :controller do let(:orders) { assigns(:orders) } let(:shops) { Enterprise.where(id: orders.pluck(:distributor_id)) } - let(:outstanding_balance) { instance_double(OutstandingBalance) } + let(:outstanding_balance_query) { instance_double(OutstandingBalanceQuery) } before do allow(controller).to receive(:spree_current_user) { u1 } @@ -47,9 +47,9 @@ describe Spree::UsersController, type: :controller do expect(orders).not_to include d1o3 end - it 'calls OutstandingBalance' do - allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:query) { Spree::Order.none } + it 'calls OutstandingBalanceQuery' do + allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance_query) + expect(outstanding_balance_query).to receive(:call) { Spree::Order.none } spree_get :show end diff --git a/spec/lib/reports/order_cycle_management_report_spec.rb b/spec/lib/reports/order_cycle_management_report_spec.rb index 47c5c1fbdc..72d6c44831 100644 --- a/spec/lib/reports/order_cycle_management_report_spec.rb +++ b/spec/lib/reports/order_cycle_management_report_spec.rb @@ -17,9 +17,9 @@ module Reporting end describe "fetching orders" do - it 'calls the OutstandingBalance query object' do - outstanding_balance = instance_double(OutstandingBalance, query: Spree::Order.none) - expect(OutstandingBalance).to receive(:new).and_return(outstanding_balance) + it 'calls the OutstandingBalanceQuery query object' do + outstanding_balance = instance_double(OutstandingBalanceQuery, call: Spree::Order.none) + expect(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) subject.orders end diff --git a/spec/queries/complete_orders_with_balance_query_spec.rb b/spec/queries/complete_orders_with_balance_query_spec.rb index f0988bc532..ba9fb6e6a2 100644 --- a/spec/queries/complete_orders_with_balance_query_spec.rb +++ b/spec/queries/complete_orders_with_balance_query_spec.rb @@ -7,7 +7,7 @@ describe CompleteOrdersWithBalanceQuery do describe '#call' do let(:user) { order.user } - let(:outstanding_balance) { instance_double(OutstandingBalance) } + let(:outstanding_balance) { instance_double(OutstandingBalanceQuery) } context 'when the user has complete orders' do let(:order) do @@ -24,9 +24,9 @@ describe CompleteOrdersWithBalanceQuery do ) end - it 'calls OutstandingBalance#query' do - allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:query) + it 'calls OutstandingBalanceQuery#call' do + allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) + expect(outstanding_balance).to receive(:call) result end @@ -44,9 +44,9 @@ describe CompleteOrdersWithBalanceQuery do context 'when the user has no complete orders' do let(:order) { create(:order) } - it 'calls OutstandingBalance' do - allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:query) + it 'calls OutstandingBalanceQuery' do + allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) + expect(outstanding_balance).to receive(:call) result end diff --git a/spec/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_query_spec.rb similarity index 97% rename from spec/queries/customers_with_balance_spec.rb rename to spec/queries/customers_with_balance_query_spec.rb index e886350e5f..76208eb32a 100644 --- a/spec/queries/customers_with_balance_spec.rb +++ b/spec/queries/customers_with_balance_query_spec.rb @@ -9,10 +9,10 @@ describe CustomersWithBalanceQuery do let(:customers) { create(:customer) } let(:total) { 200.00 } let(:order_total) { 100.00 } - let(:outstanding_balance) { instance_double(OutstandingBalance) } + let(:outstanding_balance) { instance_double(OutstandingBalanceQuery) } - it 'calls OutstandingBalance#statement' do - allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance) + it 'calls OutstandingBalanceQuery#statement' do + allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:statement) result diff --git a/spec/queries/outstanding_balance_spec.rb b/spec/queries/outstanding_balance_query_spec.rb similarity index 88% rename from spec/queries/outstanding_balance_spec.rb rename to spec/queries/outstanding_balance_query_spec.rb index 91b80f4c82..2af9a2c611 100644 --- a/spec/queries/outstanding_balance_spec.rb +++ b/spec/queries/outstanding_balance_query_spec.rb @@ -2,14 +2,15 @@ require 'spec_helper' -describe OutstandingBalance do - subject(:outstanding_balance) { described_class.new(relation) } +describe OutstandingBalanceQuery do + subject { described_class.new(relation) } + let(:result) { subject.call } describe '#statement' do let(:relation) { Spree::Order.none } it 'returns the CASE statement necessary to compute the order balance' do - normalized_sql_statement = normalize(outstanding_balance.statement) + normalized_sql_statement = normalize(subject.statement) expect(normalized_sql_statement).to eq(normalize(<<-SQL)) CASE WHEN "spree_orders"."state" IN ('canceled', 'returned') THEN "spree_orders"."payment_total" @@ -23,7 +24,7 @@ describe OutstandingBalance do end end - describe '#query' do + describe '#call' do let(:relation) { Spree::Order.all } let(:total) { 200.00 } let(:order_total) { 100.00 } @@ -35,7 +36,7 @@ describe OutstandingBalance do end it 'returns the order balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(-order_total) end end @@ -47,7 +48,7 @@ describe OutstandingBalance do end it 'returns the order balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(-order_total) end end @@ -59,7 +60,7 @@ describe OutstandingBalance do end it 'returns the order balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(-order_total) end end @@ -71,7 +72,7 @@ describe OutstandingBalance do end it 'returns the order balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(-order_total) end end @@ -85,7 +86,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(-order_total) end end @@ -101,7 +102,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total - 200.0) end end @@ -117,7 +118,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total) end end @@ -133,7 +134,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total - 200.0) end end @@ -149,7 +150,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total - 200.0) end end @@ -165,7 +166,7 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total - 200.0) end end @@ -182,14 +183,14 @@ describe OutstandingBalance do end it 'returns the customer balance' do - order = outstanding_balance.query.first + order = result.first expect(order.balance_value).to eq(payment_total) end end context 'when there are no orders' do it 'returns the order balance' do - orders = outstanding_balance.query + orders = result expect(orders).to be_empty end end From ef17fd7d8001eeec424e323378082299ed862216 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:41:48 +0500 Subject: [PATCH 5/9] Cleanup --- app/controllers/spree/users_controller.rb | 2 +- ....rb => payments_requiring_action_query.rb} | 4 +-- ...complete_orders_with_balance_query_spec.rb | 12 +++---- .../complete_visible_orders_query_spec.rb | 8 ++--- .../customers_with_balance_query_spec.rb | 34 +++++++++---------- ...> payments_requiring_action_query_spec.rb} | 10 +++--- 6 files changed, 35 insertions(+), 35 deletions(-) rename app/queries/{payments_requiring_action.rb => payments_requiring_action_query.rb} (83%) rename spec/queries/{payments_requiring_action_spec.rb => payments_requiring_action_query_spec.rb} (70%) diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index 4feff9bf7a..ad4e9082ae 100644 --- a/app/controllers/spree/users_controller.rb +++ b/app/controllers/spree/users_controller.rb @@ -16,7 +16,7 @@ module Spree before_action :set_locale def show - @payments_requiring_action = PaymentsRequiringAction.new(spree_current_user).query + @payments_requiring_action = PaymentsRequiringActionQuery.new(spree_current_user).call @orders = orders_collection.includes(:line_items) customers = spree_current_user.customers diff --git a/app/queries/payments_requiring_action.rb b/app/queries/payments_requiring_action_query.rb similarity index 83% rename from app/queries/payments_requiring_action.rb rename to app/queries/payments_requiring_action_query.rb index ffe705e955..a680b94ddc 100644 --- a/app/queries/payments_requiring_action.rb +++ b/app/queries/payments_requiring_action_query.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -class PaymentsRequiringAction +class PaymentsRequiringActionQuery def initialize(user) @user = user end - def query + def call Spree::Payment.joins(:order).where("spree_orders.user_id = ?", user.id). requires_authorization end diff --git a/spec/queries/complete_orders_with_balance_query_spec.rb b/spec/queries/complete_orders_with_balance_query_spec.rb index ba9fb6e6a2..77deb34bec 100644 --- a/spec/queries/complete_orders_with_balance_query_spec.rb +++ b/spec/queries/complete_orders_with_balance_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CompleteOrdersWithBalanceQuery do - let(:result) { described_class.new(user).call } + subject { described_class.new(user).call } describe '#call' do let(:user) { order.user } @@ -28,16 +28,16 @@ describe CompleteOrdersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:call) - result + subject end it 'returns complete orders including their balance' do - order = result.first + order = subject.first expect(order[:balance_value]).to eq(-1.0) end it 'sorts them by their completed_at with the most recent first' do - expect(result.pluck(:id)).to eq([other_order.id, order.id]) + expect(subject.pluck(:id)).to eq([other_order.id, order.id]) end end @@ -48,11 +48,11 @@ describe CompleteOrdersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:call) - result + subject end it 'returns an empty array' do - expect(result).to be_empty + expect(subject).to be_empty end end end diff --git a/spec/queries/complete_visible_orders_query_spec.rb b/spec/queries/complete_visible_orders_query_spec.rb index 982c567e1f..61ffc5f733 100644 --- a/spec/queries/complete_visible_orders_query_spec.rb +++ b/spec/queries/complete_visible_orders_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CompleteVisibleOrdersQuery do - subject(:result) { described_class.new(order_permissions).call } + subject { described_class.new(order_permissions).call } let(:filter_canceled) { false } describe '#call' do @@ -20,7 +20,7 @@ describe CompleteVisibleOrdersQuery do let(:cart_order) { create(:order, distributor: enterprise) } it 'does not return it' do - expect(result).not_to include(cart_order) + expect(subject).not_to include(cart_order) end end @@ -28,13 +28,13 @@ describe CompleteVisibleOrdersQuery do let(:complete_order) { create(:order, completed_at: 1.day.ago, distributor: enterprise) } it 'does not return it' do - expect(result).to include(complete_order) + expect(subject).to include(complete_order) end end it 'calls #visible_orders' do expect(order_permissions).to receive(:visible_orders).and_call_original - result + subject end end end diff --git a/spec/queries/customers_with_balance_query_spec.rb b/spec/queries/customers_with_balance_query_spec.rb index 76208eb32a..44d24ac14e 100644 --- a/spec/queries/customers_with_balance_query_spec.rb +++ b/spec/queries/customers_with_balance_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CustomersWithBalanceQuery do - subject(:result) { described_class.new(Customer.where(id: customers)).call } + subject { described_class.new(Customer.where(id: customers)).call } describe '#call' do let(:customers) { create(:customer) } @@ -15,7 +15,7 @@ describe CustomersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:statement) - result + subject end describe 'arguments' do @@ -23,8 +23,8 @@ describe CustomersWithBalanceQuery do let(:customers) { create_pair(:customer) } it 'returns balance' do - expect(result.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) - expect(result.map(&:balance_value)).to eq([0, 0]) + expect(subject.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) + expect(subject.map(&:balance_value)).to eq([0, 0]) end end @@ -32,7 +32,7 @@ describe CustomersWithBalanceQuery do let(:customers) { Customer.none } it 'returns empty customers collection' do - expect(result).to eq([]) + expect(subject).to eq([]) end end end @@ -44,7 +44,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(0) end end @@ -56,7 +56,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(0) end end @@ -68,7 +68,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(0) end end @@ -80,7 +80,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(0) end end @@ -94,7 +94,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(-total) end end @@ -110,7 +110,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -132,7 +132,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - non_canceled_orders_total) end end @@ -148,7 +148,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -164,7 +164,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -180,7 +180,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -197,14 +197,14 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(payment_total - non_returned_orders_total) end end context 'when there are no orders' do it 'returns the customer balance' do - customer = result.first + customer = subject.first expect(customer.balance_value).to eq(0) end end diff --git a/spec/queries/payments_requiring_action_spec.rb b/spec/queries/payments_requiring_action_query_spec.rb similarity index 70% rename from spec/queries/payments_requiring_action_spec.rb rename to spec/queries/payments_requiring_action_query_spec.rb index 77a9ca0404..8fcde56146 100644 --- a/spec/queries/payments_requiring_action_spec.rb +++ b/spec/queries/payments_requiring_action_query_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -describe PaymentsRequiringAction do +describe PaymentsRequiringActionQuery do let(:user) { create(:user) } let(:order) { create(:order, user:) } - subject(:payments_requiring_action) { described_class.new(user) } + subject { described_class.new(user).call } - describe '#query' do + describe '#call' do context "payment has a cvv_response_message" do let(:payment) do create(:payment, @@ -17,7 +17,7 @@ describe PaymentsRequiringAction do end it "finds the payment" do - expect(payments_requiring_action.query.all).to include(payment) + expect(subject.all).to include(payment) end end @@ -27,7 +27,7 @@ describe PaymentsRequiringAction do end it "does not find the payment" do - expect(payments_requiring_action.query.all).to_not include(payment) + expect(subject.all).to_not include(payment) end end end From 67cd6ea6edd52728eb09aaeb6bb5863d39541c06 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Mon, 26 Feb 2024 23:44:58 +0500 Subject: [PATCH 6/9] cops --- app/queries/customers_with_balance_query.rb | 2 +- app/queries/outstanding_balance_query.rb | 2 +- app/queries/payments_requiring_action_query.rb | 2 +- spec/queries/complete_visible_orders_query_spec.rb | 1 + spec/queries/customers_with_balance_query_spec.rb | 3 ++- spec/queries/outstanding_balance_query_spec.rb | 3 ++- spec/queries/payments_requiring_action_query_spec.rb | 5 +++-- 7 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/queries/customers_with_balance_query.rb b/app/queries/customers_with_balance_query.rb index a33b109540..47d0fabca9 100644 --- a/app/queries/customers_with_balance_query.rb +++ b/app/queries/customers_with_balance_query.rb @@ -20,7 +20,7 @@ class CustomersWithBalanceQuery # The resulting orders are in states that belong after the checkout. Only these can be considered # for a customer's balance. def left_join_complete_orders - <<~SQL + <<~SQL.squish LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id AND #{finalized_states.to_sql} SQL diff --git a/app/queries/outstanding_balance_query.rb b/app/queries/outstanding_balance_query.rb index a0685800fa..c7d37fd62f 100644 --- a/app/queries/outstanding_balance_query.rb +++ b/app/queries/outstanding_balance_query.rb @@ -29,7 +29,7 @@ class OutstandingBalanceQuery # Arel doesn't support CASE statements until v7.1.0 so we'll have to wait with SQL literals # a little longer. See https://github.com/rails/arel/pull/400 for details. def statement - <<~SQL + <<~SQL.squish CASE WHEN "spree_orders"."state" IN #{non_fulfilled_states_group.to_sql} THEN "spree_orders"."payment_total" WHEN "spree_orders"."state" IS NOT NULL THEN "spree_orders"."payment_total" - "spree_orders"."total" ELSE 0 END diff --git a/app/queries/payments_requiring_action_query.rb b/app/queries/payments_requiring_action_query.rb index a680b94ddc..c8fe5eda23 100644 --- a/app/queries/payments_requiring_action_query.rb +++ b/app/queries/payments_requiring_action_query.rb @@ -6,7 +6,7 @@ class PaymentsRequiringActionQuery end def call - Spree::Payment.joins(:order).where("spree_orders.user_id = ?", user.id). + Spree::Payment.joins(:order).where(spree_orders: { user_id: user.id }). requires_authorization end diff --git a/spec/queries/complete_visible_orders_query_spec.rb b/spec/queries/complete_visible_orders_query_spec.rb index 61ffc5f733..40b88b10c8 100644 --- a/spec/queries/complete_visible_orders_query_spec.rb +++ b/spec/queries/complete_visible_orders_query_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe CompleteVisibleOrdersQuery do subject { described_class.new(order_permissions).call } + let(:filter_canceled) { false } describe '#call' do diff --git a/spec/queries/customers_with_balance_query_spec.rb b/spec/queries/customers_with_balance_query_spec.rb index 44d24ac14e..6a5de07211 100644 --- a/spec/queries/customers_with_balance_query_spec.rb +++ b/spec/queries/customers_with_balance_query_spec.rb @@ -64,7 +64,8 @@ describe CustomersWithBalanceQuery do context 'when orders are in delivery state' do before do create(:order, customer: customers, total: order_total, payment_total: 0, state: 'delivery') - create(:order, customer: customers, total: order_total, payment_total: 50, state: 'delivery') + create(:order, customer: customers, total: order_total, payment_total: 50, + state: 'delivery') end it 'returns the customer balance' do diff --git a/spec/queries/outstanding_balance_query_spec.rb b/spec/queries/outstanding_balance_query_spec.rb index 2af9a2c611..dbad29ec06 100644 --- a/spec/queries/outstanding_balance_query_spec.rb +++ b/spec/queries/outstanding_balance_query_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe OutstandingBalanceQuery do subject { described_class.new(relation) } + let(:result) { subject.call } describe '#statement' do @@ -12,7 +13,7 @@ describe OutstandingBalanceQuery do it 'returns the CASE statement necessary to compute the order balance' do normalized_sql_statement = normalize(subject.statement) - expect(normalized_sql_statement).to eq(normalize(<<-SQL)) + expect(normalized_sql_statement).to eq(normalize(<<-SQL.squish)) CASE WHEN "spree_orders"."state" IN ('canceled', 'returned') THEN "spree_orders"."payment_total" WHEN "spree_orders"."state" IS NOT NULL THEN "spree_orders"."payment_total" - "spree_orders"."total" ELSE 0 END diff --git a/spec/queries/payments_requiring_action_query_spec.rb b/spec/queries/payments_requiring_action_query_spec.rb index 8fcde56146..ade23b5528 100644 --- a/spec/queries/payments_requiring_action_query_spec.rb +++ b/spec/queries/payments_requiring_action_query_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' describe PaymentsRequiringActionQuery do + subject { described_class.new(user).call } + let(:user) { create(:user) } let(:order) { create(:order, user:) } - subject { described_class.new(user).call } describe '#call' do context "payment has a cvv_response_message" do @@ -27,7 +28,7 @@ describe PaymentsRequiringActionQuery do end it "does not find the payment" do - expect(subject.all).to_not include(payment) + expect(subject.all).not_to include(payment) end end end From 3cf75fce72b5e7beac837f8620de369110c2ab07 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Tue, 27 Feb 2024 00:29:59 +0500 Subject: [PATCH 7/9] cops --- ...complete_orders_with_balance_query_spec.rb | 12 +++---- .../complete_visible_orders_query_spec.rb | 8 ++--- .../customers_with_balance_query_spec.rb | 34 +++++++++---------- .../queries/outstanding_balance_query_spec.rb | 6 ++-- .../payments_requiring_action_query_spec.rb | 6 ++-- 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/spec/queries/complete_orders_with_balance_query_spec.rb b/spec/queries/complete_orders_with_balance_query_spec.rb index 77deb34bec..979efb1f08 100644 --- a/spec/queries/complete_orders_with_balance_query_spec.rb +++ b/spec/queries/complete_orders_with_balance_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CompleteOrdersWithBalanceQuery do - subject { described_class.new(user).call } + subject(:result) { described_class.new(user).call } describe '#call' do let(:user) { order.user } @@ -28,16 +28,16 @@ describe CompleteOrdersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:call) - subject + result end it 'returns complete orders including their balance' do - order = subject.first + order = result.first expect(order[:balance_value]).to eq(-1.0) end it 'sorts them by their completed_at with the most recent first' do - expect(subject.pluck(:id)).to eq([other_order.id, order.id]) + expect(result.pluck(:id)).to eq([other_order.id, order.id]) end end @@ -48,11 +48,11 @@ describe CompleteOrdersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:call) - subject + result end it 'returns an empty array' do - expect(subject).to be_empty + expect(result).to be_empty end end end diff --git a/spec/queries/complete_visible_orders_query_spec.rb b/spec/queries/complete_visible_orders_query_spec.rb index 40b88b10c8..af7d54872f 100644 --- a/spec/queries/complete_visible_orders_query_spec.rb +++ b/spec/queries/complete_visible_orders_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CompleteVisibleOrdersQuery do - subject { described_class.new(order_permissions).call } + subject(:result) { described_class.new(order_permissions).call } let(:filter_canceled) { false } @@ -21,7 +21,7 @@ describe CompleteVisibleOrdersQuery do let(:cart_order) { create(:order, distributor: enterprise) } it 'does not return it' do - expect(subject).not_to include(cart_order) + expect(result).not_to include(cart_order) end end @@ -29,13 +29,13 @@ describe CompleteVisibleOrdersQuery do let(:complete_order) { create(:order, completed_at: 1.day.ago, distributor: enterprise) } it 'does not return it' do - expect(subject).to include(complete_order) + expect(result).to include(complete_order) end end it 'calls #visible_orders' do expect(order_permissions).to receive(:visible_orders).and_call_original - subject + result end end end diff --git a/spec/queries/customers_with_balance_query_spec.rb b/spec/queries/customers_with_balance_query_spec.rb index 6a5de07211..952294b0e6 100644 --- a/spec/queries/customers_with_balance_query_spec.rb +++ b/spec/queries/customers_with_balance_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CustomersWithBalanceQuery do - subject { described_class.new(Customer.where(id: customers)).call } + subject(:result) { described_class.new(Customer.where(id: customers)).call } describe '#call' do let(:customers) { create(:customer) } @@ -15,7 +15,7 @@ describe CustomersWithBalanceQuery do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) expect(outstanding_balance).to receive(:statement) - subject + result end describe 'arguments' do @@ -23,8 +23,8 @@ describe CustomersWithBalanceQuery do let(:customers) { create_pair(:customer) } it 'returns balance' do - expect(subject.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) - expect(subject.map(&:balance_value)).to eq([0, 0]) + expect(result.pluck(:id).sort).to eq([customers.first.id, customers.second.id].sort) + expect(result.map(&:balance_value)).to eq([0, 0]) end end @@ -32,7 +32,7 @@ describe CustomersWithBalanceQuery do let(:customers) { Customer.none } it 'returns empty customers collection' do - expect(subject).to eq([]) + expect(result).to eq([]) end end end @@ -44,7 +44,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(0) end end @@ -56,7 +56,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(0) end end @@ -69,7 +69,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(0) end end @@ -81,7 +81,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(0) end end @@ -95,7 +95,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(-total) end end @@ -111,7 +111,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -133,7 +133,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - non_canceled_orders_total) end end @@ -149,7 +149,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -165,7 +165,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -181,7 +181,7 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - total) end end @@ -198,14 +198,14 @@ describe CustomersWithBalanceQuery do end it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(payment_total - non_returned_orders_total) end end context 'when there are no orders' do it 'returns the customer balance' do - customer = subject.first + customer = result.first expect(customer.balance_value).to eq(0) end end diff --git a/spec/queries/outstanding_balance_query_spec.rb b/spec/queries/outstanding_balance_query_spec.rb index dbad29ec06..feba121359 100644 --- a/spec/queries/outstanding_balance_query_spec.rb +++ b/spec/queries/outstanding_balance_query_spec.rb @@ -3,15 +3,15 @@ require 'spec_helper' describe OutstandingBalanceQuery do - subject { described_class.new(relation) } + subject(:query) { described_class.new(relation) } - let(:result) { subject.call } + let(:result) { query.call } describe '#statement' do let(:relation) { Spree::Order.none } it 'returns the CASE statement necessary to compute the order balance' do - normalized_sql_statement = normalize(subject.statement) + normalized_sql_statement = normalize(query.statement) expect(normalized_sql_statement).to eq(normalize(<<-SQL.squish)) CASE WHEN "spree_orders"."state" IN ('canceled', 'returned') THEN "spree_orders"."payment_total" diff --git a/spec/queries/payments_requiring_action_query_spec.rb b/spec/queries/payments_requiring_action_query_spec.rb index ade23b5528..4101a181ca 100644 --- a/spec/queries/payments_requiring_action_query_spec.rb +++ b/spec/queries/payments_requiring_action_query_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe PaymentsRequiringActionQuery do - subject { described_class.new(user).call } + subject(:result) { described_class.new(user).call } let(:user) { create(:user) } let(:order) { create(:order, user:) } @@ -18,7 +18,7 @@ describe PaymentsRequiringActionQuery do end it "finds the payment" do - expect(subject.all).to include(payment) + expect(result.all).to include(payment) end end @@ -28,7 +28,7 @@ describe PaymentsRequiringActionQuery do end it "does not find the payment" do - expect(subject.all).not_to include(payment) + expect(result.all).not_to include(payment) end end end From c2d8bdd4147f6460a0d0ee9eec421ac246071161 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Tue, 27 Feb 2024 01:01:22 +0500 Subject: [PATCH 8/9] cops --- .../api/admin/customer_with_balance_serializer.rb | 4 ++-- app/serializers/api/order_serializer.rb | 4 ++-- spec/lib/reports/order_cycle_management_report_spec.rb | 3 ++- spec/queries/complete_orders_with_balance_query_spec.rb | 8 ++++++-- spec/queries/outstanding_balance_query_spec.rb | 3 +-- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/serializers/api/admin/customer_with_balance_serializer.rb b/app/serializers/api/admin/customer_with_balance_serializer.rb index 69f740ac9d..90f2726a10 100644 --- a/app/serializers/api/admin/customer_with_balance_serializer.rb +++ b/app/serializers/api/admin/customer_with_balance_serializer.rb @@ -3,8 +3,8 @@ module Api module Admin # This serializer relies on `object` to respond to `#balance_value`. That's done in - # `CustomersWithBalanceQuery` due to the fact that ActiveRecord maps the DB result set's columns to - # instance methods. This way, the `balance_value` alias on that class ends up being + # `CustomersWithBalanceQuery` due to the fact that ActiveRecord maps the DB result set's + # columns to instance methods. This way, the `balance_value` alias on that class ends up being # `object.balance_value` here. class CustomerWithBalanceSerializer < CustomerSerializer attributes :balance, :balance_status diff --git a/app/serializers/api/order_serializer.rb b/app/serializers/api/order_serializer.rb index 900d2aeaa0..b8fa04dff2 100644 --- a/app/serializers/api/order_serializer.rb +++ b/app/serializers/api/order_serializer.rb @@ -9,8 +9,8 @@ module Api has_many :payments, serializer: Api::PaymentSerializer - # This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalanceQuery` - # for reference. + # This method relies on `balance_value` as a computed DB column. + # See `CompleteOrdersWithBalanceQuery` for reference. def outstanding_balance -object.balance_value end diff --git a/spec/lib/reports/order_cycle_management_report_spec.rb b/spec/lib/reports/order_cycle_management_report_spec.rb index 72d6c44831..80bed21b3c 100644 --- a/spec/lib/reports/order_cycle_management_report_spec.rb +++ b/spec/lib/reports/order_cycle_management_report_spec.rb @@ -18,7 +18,8 @@ module Reporting describe "fetching orders" do it 'calls the OutstandingBalanceQuery query object' do - outstanding_balance = instance_double(OutstandingBalanceQuery, call: Spree::Order.none) + outstanding_balance = instance_double(OutstandingBalanceQuery, + call: Spree::Order.none) expect(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) subject.orders diff --git a/spec/queries/complete_orders_with_balance_query_spec.rb b/spec/queries/complete_orders_with_balance_query_spec.rb index 979efb1f08..0f7e856865 100644 --- a/spec/queries/complete_orders_with_balance_query_spec.rb +++ b/spec/queries/complete_orders_with_balance_query_spec.rb @@ -26,9 +26,11 @@ describe CompleteOrdersWithBalanceQuery do it 'calls OutstandingBalanceQuery#call' do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:call) + allow(outstanding_balance).to receive(:call) result + + expect(outstanding_balance).to have_received(:call) end it 'returns complete orders including their balance' do @@ -46,9 +48,11 @@ describe CompleteOrdersWithBalanceQuery do it 'calls OutstandingBalanceQuery' do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:call) + allow(outstanding_balance).to receive(:call) result + + expect(outstanding_balance).to have_received(:call) end it 'returns an empty array' do diff --git a/spec/queries/outstanding_balance_query_spec.rb b/spec/queries/outstanding_balance_query_spec.rb index feba121359..77e9acb307 100644 --- a/spec/queries/outstanding_balance_query_spec.rb +++ b/spec/queries/outstanding_balance_query_spec.rb @@ -9,10 +9,9 @@ describe OutstandingBalanceQuery do describe '#statement' do let(:relation) { Spree::Order.none } + let(:normalized_sql_statement) { normalize(query.statement) } it 'returns the CASE statement necessary to compute the order balance' do - normalized_sql_statement = normalize(query.statement) - expect(normalized_sql_statement).to eq(normalize(<<-SQL.squish)) CASE WHEN "spree_orders"."state" IN ('canceled', 'returned') THEN "spree_orders"."payment_total" WHEN "spree_orders"."state" IS NOT NULL THEN "spree_orders"."payment_total" - "spree_orders"."total" From 3bf76c81aafaa97b90c32e028d434f309aa0ad91 Mon Sep 17 00:00:00 2001 From: Feruz Oripov Date: Sat, 2 Mar 2024 16:11:26 +0500 Subject: [PATCH 9/9] Update specs --- .../customers_with_balance_query_spec.rb | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/spec/queries/customers_with_balance_query_spec.rb b/spec/queries/customers_with_balance_query_spec.rb index 952294b0e6..47811e21ce 100644 --- a/spec/queries/customers_with_balance_query_spec.rb +++ b/spec/queries/customers_with_balance_query_spec.rb @@ -6,16 +6,19 @@ describe CustomersWithBalanceQuery do subject(:result) { described_class.new(Customer.where(id: customers)).call } describe '#call' do - let(:customers) { create(:customer) } + let(:customer) { create(:customer) } + let(:customers) { [customer] } let(:total) { 200.00 } let(:order_total) { 100.00 } let(:outstanding_balance) { instance_double(OutstandingBalanceQuery) } it 'calls OutstandingBalanceQuery#statement' do allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance) - expect(outstanding_balance).to receive(:statement) + allow(outstanding_balance).to receive(:statement) result + + expect(outstanding_balance).to have_received(:statement) end describe 'arguments' do @@ -39,8 +42,8 @@ describe CustomersWithBalanceQuery do context 'when orders are in cart state' do before do - create(:order, customer: customers, total: order_total, payment_total: 0, state: 'cart') - create(:order, customer: customers, total: order_total, payment_total: 0, state: 'cart') + create(:order, customer:, total: order_total, payment_total: 0, state: 'cart') + create(:order, customer:, total: order_total, payment_total: 0, state: 'cart') end it 'returns the customer balance' do @@ -51,8 +54,8 @@ describe CustomersWithBalanceQuery do context 'when orders are in address state' do before do - create(:order, customer: customers, total: order_total, payment_total: 0, state: 'address') - create(:order, customer: customers, total: order_total, payment_total: 50, state: 'address') + create(:order, customer:, total: order_total, payment_total: 0, state: 'address') + create(:order, customer:, total: order_total, payment_total: 50, state: 'address') end it 'returns the customer balance' do @@ -63,8 +66,8 @@ describe CustomersWithBalanceQuery do context 'when orders are in delivery state' do before do - create(:order, customer: customers, total: order_total, payment_total: 0, state: 'delivery') - create(:order, customer: customers, total: order_total, payment_total: 50, + create(:order, customer:, total: order_total, payment_total: 0, state: 'delivery') + create(:order, customer:, total: order_total, payment_total: 50, state: 'delivery') end @@ -76,8 +79,8 @@ describe CustomersWithBalanceQuery do context 'when orders are in payment state' do before do - create(:order, customer: customers, total: order_total, payment_total: 0, state: 'payment') - create(:order, customer: customers, total: order_total, payment_total: 50, state: 'payment') + create(:order, customer:, total: order_total, payment_total: 0, state: 'payment') + create(:order, customer:, total: order_total, payment_total: 50, state: 'payment') end it 'returns the customer balance' do @@ -88,9 +91,9 @@ describe CustomersWithBalanceQuery do context 'when no orders where paid' do before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') end @@ -104,9 +107,9 @@ describe CustomersWithBalanceQuery do let(:payment_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total:) + order = create(:order, customer:, total: order_total, payment_total:) order.update_attribute(:state, 'complete') end @@ -121,11 +124,11 @@ describe CustomersWithBalanceQuery do let(:non_canceled_orders_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') create( :order, - customer: customers, + customer:, total: order_total, payment_total: order_total, state: 'canceled' @@ -142,9 +145,9 @@ describe CustomersWithBalanceQuery do let(:payment_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total:) + order = create(:order, customer:, total: order_total, payment_total:) order.update_attribute(:state, 'resumed') end @@ -158,9 +161,9 @@ describe CustomersWithBalanceQuery do let(:payment_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total:) + order = create(:order, customer:, total: order_total, payment_total:) order.update_attribute(:state, 'payment') end @@ -174,9 +177,9 @@ describe CustomersWithBalanceQuery do let(:payment_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total:) + order = create(:order, customer:, total: order_total, payment_total:) order.update_attribute(:state, 'awaiting_return') end @@ -191,9 +194,9 @@ describe CustomersWithBalanceQuery do let(:non_returned_orders_total) { order_total } before do - order = create(:order, customer: customers, total: order_total, payment_total: 0) + order = create(:order, customer:, total: order_total, payment_total: 0) order.update_attribute(:state, 'complete') - order = create(:order, customer: customers, total: order_total, payment_total:) + order = create(:order, customer:, total: order_total, payment_total:) order.update_attribute(:state, 'returned') end