Skip to content

Commit

Permalink
Merge pull request #3398 from softr8/more-eager-loading-in-ddmin
Browse files Browse the repository at this point in the history
More eager loading in admin and api
  • Loading branch information
spaghetticode authored Nov 15, 2019
2 parents 9119d6c + 234a36d commit a8bef20
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 14 deletions.
15 changes: 10 additions & 5 deletions api/app/controllers/spree/api/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ def empty
def index
authorize! :index, Order
orders_includes = [
:user,
:payments,
:adjustments,
:line_items
{ user: :store_credits },
:line_items,
:valid_store_credit_payments
]
@orders = paginate(
Spree::Order
Expand Down Expand Up @@ -168,7 +167,13 @@ def permitted_payment_attributes
end

def find_order(_lock = false)
@order = Spree::Order.find_by!(number: params[:id])
@order = Spree::Order.
includes(line_items: [:adjustments, { variant: :images }],
payments: :payment_method,
shipments: {
shipping_rates: { shipping_method: :zones, taxes: :tax_rate }
}).
find_by!(number: params[:id])
end

def order_id
Expand Down
4 changes: 3 additions & 1 deletion api/app/controllers/spree/api/taxonomies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ def taxonomies
end

def taxonomy
@taxonomy ||= Spree::Taxonomy.accessible_by(current_ability, :read).find(params[:id])
@taxonomy ||= Spree::Taxonomy.accessible_by(current_ability, :read).
includes(root: :children).
find(params[:id])
end

def taxonomy_params
Expand Down
2 changes: 1 addition & 1 deletion api/app/views/spree/api/orders/_order.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
json.cache! [I18n.locale, order] do
json.(order, *order_attributes)
json.display_item_total(order.display_item_total.to_s)
json.total_quantity(order.line_items.sum(:quantity))
json.total_quantity(order.line_items.to_a.sum(&:quantity))
json.display_total(order.display_total.to_s)
json.display_ship_total(order.display_ship_total)
json.display_tax_total(order.display_tax_total)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Spree
module Admin
class ProductPropertiesController < ResourceController
belongs_to 'spree/product', find_by: :slug
belongs_to 'spree/product', find_by: :slug, includes: { product_properties: :property }
before_action :find_properties
before_action :setup_property, only: :index, if: -> { can?(:create, model_class) }
before_action :setup_variant_property_rules, only: :index
Expand Down
2 changes: 1 addition & 1 deletion backend/app/controllers/spree/admin/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def update_before
end

def product_includes
[{ variants: [:images], master: [:images, :default_price] }]
[:variant_images, { variants: [:images], master: [:images, :default_price] }]
end

def clone_object_url(resource)
Expand Down
5 changes: 4 additions & 1 deletion backend/app/controllers/spree/admin/resource_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ def belongs_to(model_name, options = {})
@parent_data[:model_name] = model_name
@parent_data[:model_class] = (options[:model_class] || model_name.to_s.classify.constantize)
@parent_data[:find_by] = options[:find_by] || :id
@parent_data[:includes] = options[:includes]
end
end

Expand Down Expand Up @@ -183,7 +184,9 @@ def parent_data

def parent
if parent?
@parent ||= self.class.parent_data[:model_class].find_by(self.class.parent_data[:find_by] => params["#{parent_model_name}_id"])
@parent ||= self.class.parent_data[:model_class]
.includes(self.class.parent_data[:includes])
.find_by(self.class.parent_data[:find_by] => params["#{parent_model_name}_id"])
instance_variable_set("@#{parent_model_name}", @parent)
else
Spree::Deprecation.warn "Calling #parent is deprecated on a ResourceController which has not defined a belongs_to"
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/concerns/spree/user_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def total_available_store_credit
deprecate total_available_store_credit: :available_store_credit_total, deprecator: Spree::Deprecation

def available_store_credit_total(currency:)
store_credits.reload.to_a.
store_credits.to_a.
select { |credit| credit.currency == currency }.
sum(&:amount_remaining)
end
Expand Down
3 changes: 2 additions & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def states

# Payments
has_many :payments, dependent: :destroy, inverse_of: :order
has_many :valid_store_credit_payments, -> { store_credits.valid }, inverse_of: :order, class_name: 'Spree::Payment', foreign_key: :order_id

# Returns
has_many :return_authorizations, dependent: :destroy, inverse_of: :order
Expand Down Expand Up @@ -686,7 +687,7 @@ def order_total_after_store_credit

def total_applicable_store_credit
if can_complete? || complete?
payments.store_credits.valid.sum(:amount)
valid_store_credit_payments.to_a.sum(&:amount)
else
[total, (user.try(:available_store_credit_total, currency: currency) || 0.0)].min
end
Expand Down
3 changes: 2 additions & 1 deletion core/spec/models/spree/concerns/user_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@

describe '#available_store_credit_total' do
subject do
test_user.available_store_credit_total(currency: 'USD')
test_user.reload.available_store_credit_total(currency: 'USD')
end

context 'when the user does not have any credit' do
Expand Down Expand Up @@ -95,6 +95,7 @@

context 'with credits of multiple currencies' do
let!(:credit_3) { create(:store_credit, user: test_user, amount: 400, currency: 'GBP') }
before { test_user.reload }

it 'separates the currencies' do
expect(test_user.available_store_credit_total(currency: 'USD')).to eq(100 + 200)
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/store_credit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@
let!(:store_credits) do
[
create(:store_credit, user: user, amount: store_credit_amount),
create(:store_credit, user: user, amount: additional_store_credit_amount)
create(:store_credit, user: user.reload, amount: additional_store_credit_amount)
]
end

Expand Down

0 comments on commit a8bef20

Please sign in to comment.