From aa9daed66e1d77831b09a629a8d6c24f8d87045a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 17 Jan 2025 15:03:39 +1100 Subject: [PATCH] Remove unused StockLocation#stock_items And the reverse association. --- app/models/spree/stock_item.rb | 5 ++--- app/models/spree/stock_location.rb | 8 -------- app/models/spree/variant.rb | 1 - app/queries/product_scope_query.rb | 2 +- app/services/products_renderer.rb | 2 +- spec/controllers/api/v0/products_controller_spec.rb | 6 ++---- spec/models/concerns/variant_stock_spec.rb | 6 ------ spec/models/spree/inventory_unit_spec.rb | 4 ++-- spec/models/spree/stock_item_spec.rb | 6 ++---- spec/models/spree/stock_location_spec.rb | 6 +----- spec/models/spree/stock_movement_spec.rb | 4 ++-- spec/models/spree/variant_spec.rb | 1 - 12 files changed, 13 insertions(+), 38 deletions(-) diff --git a/app/models/spree/stock_item.rb b/app/models/spree/stock_item.rb index fc13d27b5c..fa586a0741 100644 --- a/app/models/spree/stock_item.rb +++ b/app/models/spree/stock_item.rb @@ -4,9 +4,6 @@ module Spree class StockItem < ApplicationRecord acts_as_paranoid - # WIP: phasing out stock location, it's not used. - belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items, - optional: true belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant' has_many :stock_movements, dependent: :destroy @@ -48,6 +45,8 @@ module Spree @stock_location ||= DefaultStockLocation.find_or_create end + attr_writer :stock_location + private def process_backorders diff --git a/app/models/spree/stock_location.rb b/app/models/spree/stock_location.rb index ad1bd359a3..b7b0547328 100644 --- a/app/models/spree/stock_location.rb +++ b/app/models/spree/stock_location.rb @@ -5,8 +5,6 @@ module Spree self.belongs_to_required_by_default = false self.ignored_columns += [:backorderable_default, :active] - has_many :stock_items, dependent: :delete_all, inverse_of: :stock_location - belongs_to :state, class_name: 'Spree::State' belongs_to :country, class_name: 'Spree::Country' @@ -16,11 +14,5 @@ module Spree def stock_item(variant) StockItem.where(variant_id: variant).order(:id).first end - - # We have only one default stock location and it may be unpersisted. - # So all stock items belong to any unpersisted stock location. - def stock_items - StockItem.all - end end end diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index eda45daa17..acf0e78408 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -39,7 +39,6 @@ module Spree has_many :line_items, inverse_of: :variant, dependent: nil has_many :stock_items, dependent: :destroy, inverse_of: :variant - has_many :stock_locations, through: :stock_items has_many :images, -> { order(:position) }, as: :viewable, dependent: :destroy, class_name: "Spree::Image" diff --git a/app/queries/product_scope_query.rb b/app/queries/product_scope_query.rb index 678104695c..bdadbc3401 100644 --- a/app/queries/product_scope_query.rb +++ b/app/queries/product_scope_query.rb @@ -61,7 +61,7 @@ class ProductScopeQuery def product_query_includes [ image: { attachment_attachment: :blob }, - variants: [:default_price, :stock_locations, :stock_items, :variant_overrides] + variants: [:default_price, :stock_items, :variant_overrides] ] end diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb index 339d16c925..90b9e8e029 100644 --- a/app/services/products_renderer.rb +++ b/app/services/products_renderer.rb @@ -117,7 +117,7 @@ class ProductsRenderer # rubocop:disable Rails/FindEach # .each returns an array, .find_each returns nil distributed_products.variants_relation. - includes(:default_price, :stock_locations, :product). + includes(:default_price, :product). where(product_id: products). each { |v| scoper.scope(v) } # Scope results with variant_overrides # rubocop:enable Rails/FindEach diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index 4c45845b8d..9e961e9ae0 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -42,10 +42,8 @@ RSpec.describe Api::V0::ProductsController, type: :controller do api_get :show, id: product.to_param - expect(all_attributes.all?{ |attr| json_response.keys.include? attr }).to eq(true) - expect(variants_attributes.all?{ |attr| - json_response['variants'].first.keys.include? attr - } ).to eq(true) + expect(json_response.keys).to include(*all_attributes) + expect(json_response["variants"].first.keys).to include(*variants_attributes) end it "returns a 404 error when it cannot find a product" do diff --git a/spec/models/concerns/variant_stock_spec.rb b/spec/models/concerns/variant_stock_spec.rb index eeae48fcab..26f650d86c 100644 --- a/spec/models/concerns/variant_stock_spec.rb +++ b/spec/models/concerns/variant_stock_spec.rb @@ -83,7 +83,6 @@ RSpec.describe VariantStock do it 'returns false' do variant = build_stubbed( :variant, - stock_locations: [build_stubbed(:stock_location)] ) expect(variant.on_demand).to be_falsy end @@ -94,9 +93,6 @@ RSpec.describe VariantStock do let(:variant) do build_stubbed( :variant, - stock_locations: [ - build_stubbed(:stock_location) - ] ) end @@ -148,7 +144,6 @@ RSpec.describe VariantStock do build_stubbed( :variant, on_demand: true, - stock_locations: [build_stubbed(:stock_location)] ) end let(:stock_item) { Spree::StockItem.new(backorderable: true) } @@ -172,7 +167,6 @@ RSpec.describe VariantStock do build_stubbed( :variant, on_demand: false, - stock_locations: [build_stubbed(:stock_location)] ) end diff --git a/spec/models/spree/inventory_unit_spec.rb b/spec/models/spree/inventory_unit_spec.rb index e4743b652e..d4ea6b3622 100644 --- a/spec/models/spree/inventory_unit_spec.rb +++ b/spec/models/spree/inventory_unit_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Spree::InventoryUnit do - let(:stock_location) { create(:stock_location_with_items) } - let(:stock_item) { stock_location.stock_items.order(:id).first } + let!(:stock_location) { create(:stock_location_with_items) } + let(:stock_item) { Spree::StockItem.order(:id).first } context "#backordered_for_stock_item" do let(:order) { create(:order) } diff --git a/spec/models/spree/stock_item_spec.rb b/spec/models/spree/stock_item_spec.rb index c3f94ef71a..fd3f74965f 100644 --- a/spec/models/spree/stock_item_spec.rb +++ b/spec/models/spree/stock_item_spec.rb @@ -3,13 +3,11 @@ require 'spec_helper' RSpec.describe Spree::StockItem do - let(:stock_location) { create(:stock_location_with_items) } + let!(:stock_location) { create(:stock_location_with_items) } - subject { stock_location.stock_items.order(:id).first } + subject(:stock_item) { Spree::StockItem.order(:id).first } describe "validation" do - let(:stock_item) { stock_location.stock_items.first } - it "requires count_on_hand to be positive if not backorderable" do stock_item.backorderable = false diff --git a/spec/models/spree/stock_location_spec.rb b/spec/models/spree/stock_location_spec.rb index 1e76d1f334..a9189045cc 100644 --- a/spec/models/spree/stock_location_spec.rb +++ b/spec/models/spree/stock_location_spec.rb @@ -5,13 +5,9 @@ require 'spec_helper' module Spree RSpec.describe StockLocation do subject { create(:stock_location_with_items) } - let(:stock_item) { subject.stock_items.order(:id).first } + let(:stock_item) { StockItem.order(:id).first } let(:variant) { stock_item.variant } - it 'creates stock_items for all variants' do - expect(subject.stock_items.count).to eq Variant.count - end - it 'finds a stock_item for a variant' do stock_item = subject.stock_item(variant) expect(stock_item.count_on_hand).to eq 15 diff --git a/spec/models/spree/stock_movement_spec.rb b/spec/models/spree/stock_movement_spec.rb index 19ff09341c..235f542db2 100644 --- a/spec/models/spree/stock_movement_spec.rb +++ b/spec/models/spree/stock_movement_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Spree::StockMovement do - let(:stock_location) { create(:stock_location_with_items) } - let(:stock_item) { stock_location.stock_items.order(:id).first } + let!(:stock_location) { create(:stock_location_with_items) } + let(:stock_item) { Spree::StockItem.order(:id).first } subject { build(:stock_movement, stock_item:) } it 'should belong to a stock item' do diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 70068b4164..97a3d4695e 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -12,7 +12,6 @@ RSpec.describe Spree::Variant do it { is_expected.to have_many(:inventory_units) } it { is_expected.to have_many(:line_items) } it { is_expected.to have_many(:stock_items) } - it { is_expected.to have_many(:stock_locations).through(:stock_items) } it { is_expected.to have_many(:images) } it { is_expected.to have_one(:default_price) } it { is_expected.to have_many(:prices) }