Prioritizer pulls from third stock location when there is enough in first + second

bradwbradw commented 7 years ago

Steps to reproduce

set up 3 stock locations, override sort_packages so that you have control over which one comes last. For brevity, they can be named "A", "B", and "C", and prioritized in that order.
set up the stock counts of a variant as such: A: 2, B: 4, C: 5
set up an order, and specify that a line item has 5 of the variant

Expected behavior

Tell us what should happen

there should be two shipments: 2 coming from A, and 3 coming from B
the prioritizer should be returning 2 inventory units in A, and 3 in B

Actual behavior

there are three shipments: 2 coming from A, 2 coming from B, and 1 coming from C
the prioritizer is returning 2 inventory units in A, 2 in B, and 1 in C

System configuration

Solidus Version: 2.0.0 Extensions in use: none, just common_spree_dependencies

I reproduced this by adding to the prioritizer_spec.rb in the solidus gem directory and running rspec in there (.rvm/gems/ruby-2.2.5/bundler/gems/solidus-blabla)

here is a test that is expected to fail that can be added to https://github.com/solidusio/solidus/blob/master/core/spec/models/spree/stock/prioritizer_spec.rb#L86


      it '1st has some, 2nd has remaining, 3rd is empty' do
        5.times { build_inventory_unit }

        package1 = pack do |package|
          2.times { |i| package.add inventory_units[i] }
        end
        package2 = pack do |package|
          4.times { |i| package.add inventory_units[i] }
        end
        package3 = pack do |package|
          5.times { |i| package.add inventory_units[i] }
        end

        packages = [package1, package2, package3]
        prioritizer = Prioritizer.new(inventory_units, packages)
        packages = prioritizer.prioritized_packages

        expect(packages.count).to eq 2
        expect(packages[0].quantity).to eq 2
        expect(packages[1].quantity).to eq 3
      end

jhawthorn commented 7 years ago

Thanks for the issue. I'd guess that the root cause is the same as #583

jhawthorn commented 7 years ago

I believe this will be fixed by #2199. I've added the (greatly appreciated) spec here as ec7b241c1c0a7f38adf0aa5d993c0eb2137b204b.

solidusio / solidus