From 7c533c63471e8a6e6db76fbc8c9b73341b708611 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 6 Mar 2019 11:17:44 +0100 Subject: [PATCH] Extract DistributedValidProducts from OrderCycle --- app/models/distributed_valid_products.rb | 35 +++++++++++ app/models/order_cycle.rb | 17 +---- .../models/distributed_valid_products_spec.rb | 63 +++++++++++++++++++ spec/models/order_cycle_spec.rb | 54 ---------------- 4 files changed, 99 insertions(+), 70 deletions(-) create mode 100644 app/models/distributed_valid_products.rb create mode 100644 spec/models/distributed_valid_products_spec.rb diff --git a/app/models/distributed_valid_products.rb b/app/models/distributed_valid_products.rb new file mode 100644 index 0000000000..2151320b91 --- /dev/null +++ b/app/models/distributed_valid_products.rb @@ -0,0 +1,35 @@ +# Finds valid products distributed by a particular distributor in an order cycle +class DistributedValidProducts + def initialize(order_cycle, distributor) + @order_cycle = order_cycle + @distributor = distributor + end + + def all + variants = order_cycle.variants_distributed_by(distributor) + products = variants.map(&:product).uniq + + valid_products = products.reject do |product| + product_has_only_obsolete_master_in_distribution?(product, variants) + end + product_ids = valid_products.map(&:id) + + Spree::Product.where(id: product_ids) + end + + private + + attr_reader :order_cycle, :distributor + + # If a product without variants is added to an order cycle, and then some variants are added + # to that product, but not the order cycle, then the master variant should not available for + # customers to purchase. + # + # This method is used by #valid_products_distributed_by to filter out such products so that + # the customer cannot purchase them. + def product_has_only_obsolete_master_in_distribution?(product, distributed_variants) + product.has_variants? && + distributed_variants.include?(product.master) && + (product.variants & distributed_variants).empty? + end +end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index ac9b7d8cb8..7122d9cee7 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -156,7 +156,6 @@ class OrderCycle < ActiveRecord::Base end def distributed_variants - # TODO: only used in DistributionChangeValidator, can we remove? self.exchanges.outgoing.map(&:variants).flatten.uniq.reject(&:deleted?) end @@ -180,10 +179,7 @@ class OrderCycle < ActiveRecord::Base # to purchase. # This method filters out such products so that the customer cannot purchase them. def valid_products_distributed_by(distributor) - variants = variants_distributed_by(distributor) - products = variants.map(&:product).uniq - product_ids = products.reject{ |p| product_has_only_obsolete_master_in_distribution?(p, variants) }.map(&:id) - Spree::Product.where(id: product_ids) + DistributedValidProducts.new(self, distributor).all end def products @@ -265,17 +261,6 @@ class OrderCycle < ActiveRecord::Base private - # If a product without variants is added to an order cycle, and then some variants are added - # to that product, but not the order cycle, then the master variant should not available for customers - # to purchase. - # This method is used by #valid_products_distributed_by to filter out such products so that - # the customer cannot purchase them. - def product_has_only_obsolete_master_in_distribution?(product, distributed_variants) - product.has_variants? && - distributed_variants.include?(product.master) && - (product.variants & distributed_variants).empty? - end - def orders_close_at_after_orders_open_at? return if orders_open_at.blank? || orders_close_at.blank? return if orders_close_at > orders_open_at diff --git a/spec/models/distributed_valid_products_spec.rb b/spec/models/distributed_valid_products_spec.rb new file mode 100644 index 0000000000..1aa06d20a2 --- /dev/null +++ b/spec/models/distributed_valid_products_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe DistributedValidProducts do + it "returns valid products but not invalid products" do + p_valid = create(:product) + p_invalid = create(:product) + v_valid = p_valid.variants.first + v_invalid = p_invalid.variants.first + + d = create(:distributor_enterprise) + oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master]) + + expect(oc.valid_products_distributed_by(d)).to eq([p_valid]) + end + + describe "checking if a product has only an obsolete master variant in a distributution" do + it "returns true when so" do + master = double(:master) + unassociated_variant = double(:variant) + product = double(:product, has_variants?: true, master: master, variants: []) + distributed_variants = [master, unassociated_variant] + + oc = OrderCycle.new + distributed_valid_products = described_class.new(oc, nil) + + expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be true + end + + it "returns false when the product doesn't have variants" do + master = double(:master) + product = double(:product, has_variants?: false, master: master, variants: []) + distributed_variants = [master] + + oc = OrderCycle.new + distributed_valid_products = described_class.new(oc, nil) + + expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false + end + + it "returns false when the master isn't distributed" do + master = double(:master) + product = double(:product, has_variants?: true, master: master, variants: []) + distributed_variants = [] + + oc = OrderCycle.new + distributed_valid_products = described_class.new(oc, nil) + + expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false + end + + it "returns false when the product has other variants distributed" do + master = double(:master) + variant = double(:variant) + product = double(:product, has_variants?: true, master: master, variants: [variant]) + distributed_variants = [master, variant] + + oc = OrderCycle.new + distributed_valid_products = described_class.new(oc, nil) + + expect(distributed_valid_products.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants)).to be false + end + end +end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 3ffd650ddd..3c17fba457 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -283,60 +283,6 @@ describe OrderCycle do end end - describe "finding valid products distributed by a particular distributor" do - it "returns valid products but not invalid products" do - p_valid = create(:product) - p_invalid = create(:product) - v_valid = p_valid.variants.first - v_invalid = p_invalid.variants.first - - d = create(:distributor_enterprise) - oc = create(:simple_order_cycle, distributors: [d], variants: [v_valid, p_invalid.master]) - - oc.valid_products_distributed_by(d).should == [p_valid] - end - - describe "checking if a product has only an obsolete master variant in a distributution" do - it "returns true when so" do - master = double(:master) - unassociated_variant = double(:variant) - product = double(:product, :has_variants? => true, :master => master, :variants => []) - distributed_variants = [master, unassociated_variant] - - oc = OrderCycle.new - oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be true - end - - it "returns false when the product doesn't have variants" do - master = double(:master) - product = double(:product, :has_variants? => false, :master => master, :variants => []) - distributed_variants = [master] - - oc = OrderCycle.new - oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false - end - - it "returns false when the master isn't distributed" do - master = double(:master) - product = double(:product, :has_variants? => true, :master => master, :variants => []) - distributed_variants = [] - - oc = OrderCycle.new - oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false - end - - it "returns false when the product has other variants distributed" do - master = double(:master) - variant = double(:variant) - product = double(:product, :has_variants? => true, :master => master, :variants => [variant]) - distributed_variants = [master, variant] - - oc = OrderCycle.new - oc.send(:product_has_only_obsolete_master_in_distribution?, product, distributed_variants).should be false - end - end - end - describe "exchanges" do before(:each) do @oc = create(:simple_order_cycle)