Files
openfoodnetwork/app/controllers/admin/customers_controller.rb
Pau Perez ead1ab31b4 Fix multiple N+1 on /admin/customers.json
This (should) considerably improve traces like
https://app.datadoghq.com/apm/trace/917632173599137280?spanID=3163385094622710144&env=production&sort=time&colorBy=service&spanViewType=metadata&graphType=flamegraph&shouldShowLegend=true
by fixing the following 3 N+1s

```
user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Customer => [:enterprise]
  Add to your query: .includes([:enterprise])
Call stack
  /usr/src/app/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb:24:in `balance_value'
  /usr/src/app/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb:9:in `balance'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'

user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Spree::Address => [:state]
  Add to your query: .includes([:state])
Call stack
  /usr/src/app/app/serializers/api/address_serializer.rb:14:in `state_name'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'

user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Spree::Address => [:country]
  Add to your query: .includes([:country])
Call stack
  /usr/src/app/app/serializers/api/address_serializer.rb:10:in `country_name'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'
```

This popped up after improving the balances calculation. Now, that it's
fast, it's clear that are more performance problems on that endpoint.
We'll see if there are any left after this.
2021-03-25 11:19:00 +01:00

151 lines
4.5 KiB
Ruby

require 'open_food_network/address_finder'
module Admin
class CustomersController < Admin::ResourceController
before_action :load_managed_shops, only: :index, if: :html_request?
respond_to :json
respond_override update: { json: {
success: lambda {
tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: @customer.enterprise))
render_as_json @customer, tag_rule_mapping: tag_rule_mapping
},
failure: lambda { render json: { errors: @customer.errors.full_messages }, status: :unprocessable_entity }
} }
def index
respond_to do |format|
format.html
format.json do
render json: @collection,
each_serializer: index_each_serializer,
tag_rule_mapping: tag_rule_mapping,
customer_tags: customer_tags_by_id
end
end
end
def index_each_serializer
if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, spree_current_user)
::Api::Admin::CustomerWithBalanceSerializer
else
::Api::Admin::CustomerWithCalculatedBalanceSerializer
end
end
def show
render_as_json @customer, ams_prefix: params[:ams_prefix]
end
def create
@customer = Customer.new(customer_params)
if user_can_create_customer?
if @customer.save
tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: @customer.enterprise))
render_as_json @customer, tag_rule_mapping: tag_rule_mapping
else
render json: { errors: @customer.errors.full_messages }, status: :bad_request
end
else
redirect_to '/unauthorized'
end
end
# copy of Admin::ResourceController without flash notice
def destroy
invoke_callbacks(:destroy, :before)
if @object.destroy
invoke_callbacks(:destroy, :after)
respond_with(@object) do |format|
format.html { redirect_to location_after_destroy }
format.js { render partial: "spree/admin/shared/destroy" }
end
else
invoke_callbacks(:destroy, :fails)
respond_with(@object) do |format|
format.html { redirect_to location_after_destroy }
format.json { render json: { errors: @object.errors.full_messages }, status: :conflict }
end
end
end
private
def collection
if json_request? && params[:enterprise_id].present?
customers_relation.
includes(
:enterprise,
{ bill_address: [:state, :country] },
{ ship_address: [:state, :country] },
user: :credit_cards
)
else
Customer.where('1=0')
end
end
def customers_relation
if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, spree_current_user)
CustomersWithBalance.new(managed_enterprise_id).query
else
Customer.of(managed_enterprise_id)
end
end
def managed_enterprise_id
@managed_enterprise_id ||= Enterprise.managed_by(spree_current_user).
select('enterprises.id').find_by(id: params[:enterprise_id])
end
def load_managed_shops
@shops = Enterprise.managed_by(spree_current_user).is_distributor
end
def user_can_create_customer?
spree_current_user.admin? ||
spree_current_user.enterprises.include?(@customer.enterprise)
end
def ams_prefix_whitelist
[:subscription]
end
def customer_params
params.require(:customer).permit(
:enterprise_id, :name, :email, :code, :tag_list,
ship_address_attributes: PermittedAttributes::Address.attributes,
bill_address_attributes: PermittedAttributes::Address.attributes,
)
end
# Used in Admin::ResourceController#update
def permitted_resource_params
customer_params
end
def tag_rule_mapping
TagRule.mapping_for(Enterprise.where(id: managed_enterprise_id))
end
# Fetches tags for all customers of the enterprise and returns a hash indexed by customer_id
def customer_tags_by_id
customer_tags = ::ActsAsTaggableOn::Tag.
joins(:taggings).
includes(:taggings).
where(taggings:
{ taggable_type: 'Customer',
taggable_id: Customer.of(managed_enterprise_id),
context: 'tags' })
customer_tags.each_with_object({}) do |tag, indexed_hash|
tag.taggings.each do |tagging|
customer_id = tagging.taggable_id
indexed_hash[customer_id] ||= []
indexed_hash[customer_id] << tag.name
end
end
end
end
end