Skip to content

Commit

Permalink
reduce n+1 queries and fix specs
Browse files Browse the repository at this point in the history
  • Loading branch information
vinay-mittal committed Mar 28, 2016
1 parent 05c1245 commit 3dabbeb
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 7 deletions.
7 changes: 4 additions & 3 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,10 @@ def self.like_any(fields, values)
# values should not be displayed to frontend users. Otherwise it breaks the
# idea of having variants
def variants_and_option_values(current_currency = nil)
variants.includes(:option_values).active(current_currency).select do |variant|
variant.option_values.any?
end
variants.includes(:option_values, :prices, :stock_items_with_active_location).
active(current_currency).select do |variant|
variant.option_values.any?
end
end

def empty_option_values?
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/stock/quantifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ class Quantifier

def initialize(variant)
@variant = variant
@stock_items = @variant.stock_items.with_active_stock_location
@stock_items = @variant.stock_items_with_active_location
end

def total_on_hand
if variant.should_track_inventory?
stock_items.sum(:count_on_hand)
stock_items.inject(0) { |result, stock_item| result += stock_item.count_on_hand }
else
Float::INFINITY
end
Expand Down
5 changes: 4 additions & 1 deletion core/app/models/spree/stock_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ class StockItem < Spree::Base

with_options inverse_of: :stock_items do
belongs_to :stock_location, class_name: 'Spree::StockLocation'
belongs_to :active_stock_location, -> { where(active: true) },
class_name: 'Spree::StockLocation',
foreign_key: :stock_location_id
belongs_to :variant, class_name: 'Spree::Variant', counter_cache: true
end
has_many :stock_movements, inverse_of: :stock_item
Expand All @@ -24,7 +27,7 @@ class StockItem < Spree::Base

self.whitelisted_ransackable_attributes = ['count_on_hand', 'stock_location_id']

scope :with_active_stock_location, -> { joins(:stock_location).merge(Spree::StockLocation.active) }
scope :with_active_stock_location, -> { joins(:active_stock_location) }

def backordered_inventory_units
Spree::InventoryUnit.backordered_for_stock_item(self)
Expand Down
3 changes: 3 additions & 0 deletions core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class Variant < Spree::Base
with_options inverse_of: :variant do
has_many :inventory_units
has_many :stock_items, dependent: :destroy
has_many :stock_items_with_active_location, -> { with_active_stock_location },
class_name: "Spree::StockItem",
dependent: :destroy
end

has_many :line_items, dependent: :restrict_with_error
Expand Down
2 changes: 2 additions & 0 deletions core/spec/models/spree/line_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@

it "doesnt allow to increase item quantity" do
line_item = order.line_items.first
line_item.reload
line_item.quantity += 2
line_item.target_shipment = order.shipments.first

Expand Down Expand Up @@ -270,6 +271,7 @@

it "doesnt allow to increase quantity over stock availability" do
line_item = order.line_items.first
line_item.reload
line_item.quantity += 3
line_item.target_shipment = order.shipments.first

Expand Down
9 changes: 9 additions & 0 deletions core/spec/models/spree/stock_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@

subject { stock_location.stock_items.order(:id).first }

describe "associations" do
it do
is_expected.to belong_to(:active_stock_location).
conditions(active: true).
class_name("Spree::StockLocation").
with_foreign_key(:stock_location_id)
end
end

it 'maintains the count on hand for a variant' do
expect(subject.count_on_hand).to eq 10
end
Expand Down
10 changes: 10 additions & 0 deletions core/spec/models/spree/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@

it_behaves_like 'default_price'

describe 'associations' do
it do
is_expected.to have_many(:stock_items_with_active_location).
conditions(:with_active_stock_location).
class_name("Spree::StockItem").
dependent(:destroy)
end
end

describe 'validations' do
it { expect(master_variant).to_not validate_presence_of(:option_values) }
it { expect(variant).to validate_presence_of(:option_values) }
Expand Down Expand Up @@ -58,6 +67,7 @@

context 'when a variant is created' do
let!(:new_variant) { create(:variant, product: product) }
before { product.reload }

it { expect(product.master).to_not be_in_stock }
end
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/controllers/spree/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def load_product
else
@products = Product.active(current_currency)
end
@product = @products.includes(:variants_including_master).friendly.find(params[:id])
@product = @products.includes(variant_images: :viewable).friendly.find(params[:id])
end

def load_taxon
Expand Down

0 comments on commit 3dabbeb

Please sign in to comment.