raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12203 from feruzoripov/queries/naming

Browse Source 
Standardize Naming Conventions for Query-Related Services in `app/queries`
This commit is contained in:
David Cook
2024-03-04 10:15:49 +11:00
committed by GitHub
parent a692fc9d12 1f299c84bc
commit 2fa681ae8a
21 changed files with 123 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/admin/customers_controller.rb
View File

@@ -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] },

4
app/controllers/api/v1/customers_controller.rb
View File

@@ -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)

4
app/controllers/spree/users_controller.rb
View File

@@ -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
@@ -79,7 +79,7 @@ module Spree
private
def orders_collection
CompleteOrdersWithBalance.new(@user).query
CompleteOrdersWithBalanceQuery.new(@user).call
end
def load_object

6
app/queries/complete_orders_with_balance.rb → app/queries/complete_orders_with_balance_query.rb
View File

@@ -1,13 +1,13 @@
# 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
OutstandingBalance.new(sorted_finalized_orders).query
def call
OutstandingBalanceQuery.new(sorted_finalized_orders).call
end
private

4
app/queries/complete_visible_orders.rb → app/queries/complete_visible_orders_query.rb
View File

@@ -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

8
app/queries/customers_with_balance.rb → app/queries/customers_with_balance_query.rb
View File

@@ -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").
@@ -20,7 +20,7 @@ class CustomersWithBalance
# 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
@@ -32,6 +32,6 @@ class CustomersWithBalance
end
def outstanding_balance_sum
"SUM(#{OutstandingBalance.new.statement})::float"
"SUM(#{OutstandingBalanceQuery.new.statement})::float"
end
end

8
app/queries/outstanding_balance.rb → app/queries/outstanding_balance_query.rb
View File

@@ -7,11 +7,11 @@
# 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 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.
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,14 +22,14 @@ class OutstandingBalance
@relation = relation
end
def query
def call
relation.select("#{statement} AS balance_value")
end
# 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

6
app/queries/payments_requiring_action.rb → app/queries/payments_requiring_action_query.rb
View File

@@ -1,12 +1,12 @@
# frozen_string_literal: true
class PaymentsRequiringAction
class PaymentsRequiringActionQuery
def initialize(user)
@user = user
end
def query
Spree::Payment.joins(:order).where("spree_orders.user_id = ?", user.id).
def call
Spree::Payment.joins(:order).where(spree_orders: { user_id: user.id }).
requires_authorization
end

4
app/serializers/api/admin/customer_with_balance_serializer.rb
View File

@@ -3,8 +3,8 @@
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
# 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

4
app/serializers/api/order_serializer.rb
View File

@@ -9,8 +9,8 @@ module Api
has_many :payments, serializer: Api::PaymentSerializer
# This method relies on `balance_value` as a computed DB column. See `CompleteOrdersWithBalance`
# for reference.
# This method relies on `balance_value` as a computed DB column.
# See `CompleteOrdersWithBalanceQuery` for reference.
def outstanding_balance
-object.balance_value
end

2
lib/reporting/line_items.rb
View File

@@ -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

2
lib/reporting/reports/bulk_coop/base.rb
View File

@@ -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

2
lib/reporting/reports/order_cycle_management/base.rb
View File

@@ -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

8
spec/controllers/admin/customers_controller_spec.rb
View File

@@ -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

8
spec/controllers/spree/users_controller_spec.rb
View File

@@ -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

9
spec/lib/reports/order_cycle_management_report_spec.rb
View File

@@ -17,11 +17,10 @@ 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)
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

36
spec/queries/complete_orders_with_balance_spec.rb → spec/queries/complete_orders_with_balance_query_spec.rb
View File

@@ -2,12 +2,12 @@
require 'spec_helper'
describe CompleteOrdersWithBalance do
let(:complete_orders_with_balance) { described_class.new(user) }
describe CompleteOrdersWithBalanceQuery do
subject(:result) { described_class.new(user).call }
describe '#query' 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,37 +24,39 @@ describe CompleteOrdersWithBalance 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)
allow(outstanding_balance).to receive(:call)
complete_orders_with_balance.query
result
expect(outstanding_balance).to have_received(:call)
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
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)
allow(outstanding_balance).to receive(:call)
complete_orders_with_balance.query
result
expect(outstanding_balance).to have_received(:call)
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

13
spec/queries/complete_visible_orders_spec.rb → spec/queries/complete_visible_orders_query_spec.rb
View File

@@ -2,11 +2,12 @@
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 +21,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 +29,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

58
spec/queries/customers_with_balance_spec.rb → spec/queries/customers_with_balance_query_spec.rb
View File

@@ -2,35 +2,40 @@
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
describe '#call' do
let(:customer) { create(:customer) }
let(:customers) { [customer] }
let(:total) { 200.00 }
let(:order_total) { 100.00 }
let(:outstanding_balance) { instance_double(OutstandingBalance) }
let(:outstanding_balance) { instance_double(OutstandingBalanceQuery) }
it 'calls CustomersWithBalance#statement' do
allow(OutstandingBalance).to receive(:new).and_return(outstanding_balance)
expect(outstanding_balance).to receive(:statement)
it 'calls OutstandingBalanceQuery#statement' do
allow(OutstandingBalanceQuery).to receive(:new).and_return(outstanding_balance)
allow(outstanding_balance).to receive(:statement)
customers_with_balance.query
result
expect(outstanding_balance).to have_received(:statement)
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
@@ -42,7 +47,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(0)
end
end
@@ -54,7 +59,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(0)
end
end
@@ -62,11 +67,12 @@ describe CustomersWithBalance do
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:, 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
@@ -78,7 +84,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(0)
end
end
@@ -92,7 +98,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(-total)
end
end
@@ -108,7 +114,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 - total)
end
end
@@ -130,7 +136,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
@@ -146,7 +152,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 - total)
end
end
@@ -162,7 +168,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 - total)
end
end
@@ -178,7 +184,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 - total)
end
end
@@ -195,14 +201,14 @@ 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_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

37
spec/queries/outstanding_balance_spec.rb → spec/queries/outstanding_balance_query_spec.rb
View File

@@ -2,16 +2,17 @@
require 'spec_helper'
describe OutstandingBalance do
subject(:outstanding_balance) { described_class.new(relation) }
describe OutstandingBalanceQuery do
subject(:query) { described_class.new(relation) }
let(:result) { query.call }
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(outstanding_balance.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
@@ -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

11
spec/queries/payments_requiring_action_spec.rb → spec/queries/payments_requiring_action_query_spec.rb
View File

@@ -2,12 +2,13 @@
require 'spec_helper'
describe PaymentsRequiringAction do
describe PaymentsRequiringActionQuery do
subject(:result) { described_class.new(user).call }
let(:user) { create(:user) }
let(:order) { create(:order, user:) }
subject(:payments_requiring_action) { described_class.new(user) }
describe '#query' do
describe '#call' do
context "payment has a cvv_response_message" do
let(:payment) do
create(:payment,
@@ -17,7 +18,7 @@ describe PaymentsRequiringAction do
end
it "finds the payment" do
expect(payments_requiring_action.query.all).to include(payment)
expect(result.all).to include(payment)
end
end
@@ -27,7 +28,7 @@ describe PaymentsRequiringAction do
end
it "does not find the payment" do
expect(payments_requiring_action.query.all).to_not include(payment)
expect(result.all).not_to include(payment)
end
end
end