Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce n+1 queries and fix specs #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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