diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0af8b68e13..9e9a44cb6e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1864,16 +1864,9 @@ Style/HashSyntax: - 'app/models/spree/product_decorator.rb' - 'app/models/spree/taxon_decorator.rb' - 'app/models/spree/user_decorator.rb' - - 'app/overrides/add_distributor_details_js_to_product.rb' - - 'app/overrides/add_distributor_details_to_product.rb' - - 'app/overrides/add_distributor_to_add_to_cart_form.rb' - 'app/overrides/add_enterprise_fees_to_admin_configurations_menu.rb' - - 'app/overrides/add_source_to_product.rb' - 'app/overrides/remove_search_bar.rb' - 'app/overrides/remove_side_bar.rb' - - 'app/overrides/replace_checkout_payment_button.rb' - - 'app/overrides/replace_payment_name_with_description.rb' - - 'app/overrides/replace_shipping_address_form_with_distributor_details.rb' - 'app/overrides/set_auth_token_in_backend.rb' - 'app/overrides/set_auth_token_in_frontend.rb' - 'app/presenters/variant_presenter.rb' diff --git a/app/helpers/add_to_cart_helper.rb b/app/helpers/add_to_cart_helper.rb deleted file mode 100644 index 6a1eec813f..0000000000 --- a/app/helpers/add_to_cart_helper.rb +++ /dev/null @@ -1,21 +0,0 @@ -module AddToCartHelper - def product_out_of_stock - !@product.has_stock? && !Spree::Config[:allow_backorders] - end - - def distributor_available_for?(order, product) - DistributionChangeValidator.new(order).distributor_available_for?(product) - end - - def order_cycle_available_for?(order, product) - DistributionChangeValidator.new(order).order_cycle_available_for?(product) - end - - def available_distributors_for(order, product) - DistributionChangeValidator.new(order).available_distributors_for(product) - end - - def available_order_cycles_for(order, product) - DistributionChangeValidator.new(order).available_order_cycles_for(product) - end -end diff --git a/app/helpers/spree/orders_helper.rb b/app/helpers/spree/orders_helper.rb index 99540f6ed4..b7495daafc 100644 --- a/app/helpers/spree/orders_helper.rb +++ b/app/helpers/spree/orders_helper.rb @@ -5,10 +5,6 @@ module Spree order.nil? || order.line_items.empty? end - def alternative_available_distributors(order) - DistributionChangeValidator.new(order).available_distributors(Enterprise.all) - [order.distributor] - end - def last_completed_order spree_current_user.orders.complete.last end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index ac9b7d8cb8..2790b23b67 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -39,13 +39,6 @@ class OrderCycle < ActiveRecord::Base scope :by_name, order('name') - scope :distributing_product, lambda { |product| - joins(:exchanges). - merge(Exchange.outgoing). - merge(Exchange.with_product(product)). - select('DISTINCT order_cycles.*') - } - scope :with_distributor, lambda { |distributor| joins(:exchanges).merge(Exchange.outgoing).merge(Exchange.to_enterprise(distributor)) } @@ -156,7 +149,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 diff --git a/app/services/cart_service.rb b/app/services/cart_service.rb index c239c938cd..46b1309d49 100644 --- a/app/services/cart_service.rb +++ b/app/services/cart_service.rb @@ -13,12 +13,6 @@ class CartService def populate(from_hash, overwrite = false) @distributor, @order_cycle = distributor_and_order_cycle - # Refactor: We may not need this validation - we can't change distribution here, so - # this validation probably can't fail - if !distribution_can_supply_products_in_cart(@distributor, @order_cycle) - errors.add(:base, I18n.t(:spree_order_populator_error)) - return false - end @order.with_lock do variants = read_variants from_hash @@ -116,10 +110,6 @@ class CartService [@order.distributor, @order.order_cycle] end - def distribution_can_supply_products_in_cart(distributor, order_cycle) - DistributionChangeValidator.new(@order).can_change_to_distribution?(distributor, order_cycle) - end - def varies_from_cart(variant_data) li = line_item_for_variant_id variant_data[:variant_id] diff --git a/app/views/spree/orders/_form.html.haml b/app/views/spree/orders/_form.html.haml index 2d25df71e2..9b40c2e811 100644 --- a/app/views/spree/orders/_form.html.haml +++ b/app/views/spree/orders/_form.html.haml @@ -25,10 +25,6 @@ %tfoot#edit-cart = render 'spree/orders/form/cart_actions_row' if @order.cart? - / This is the fees row which we want to replace with the pop-over - -# - unless @order.adjustments.eligible.blank? - -# = render "spree/orders/adjustments" - %tr %td.text-right{colspan:"3"} = t :orders_form_subtotal diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index c81399130f..b92b013507 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -1,12 +1,4 @@ %tr.line-item{class: "variant-#{variant.id}"} - - / removed image thumbnail on shopping cart & checkout to simplify - / %td.cart-item-image{"data-hook" => "cart_item_image"} - / - if variant.images.length == 0 - / = link_to small_image(variant.product), variant.product - / - else - / = link_to image_tag(variant.images.first.attachment.url(:small)), variant.product - %td.cart-item-description{'data-hook' => "cart_item_description"} %div.item-thumb-image{"data-hook" => "cart_item_image"} @@ -24,11 +16,6 @@ %td.text-right.cart-item-price{"data-hook" => "cart_item_price"} = line_item.single_display_amount_with_adjustments.to_html - -# Now in a template in app/assets/javascripts/templates ! - -# %price-breakdown{"price-breakdown" => "_", variant: "variant", - -# "price-breakdown-append-to-body" => "true", - -# "price-breakdown-placement" => "left", - -# "price-breakdown-animation" => true} %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"} = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => variant.on_hand, "ng-model" => "line_item_#{line_item.id}", :class => "line_item_quantity", :size => 5 %td.cart-item-total.text-right{"data-hook" => "cart_item_total"} diff --git a/lib/open_food_network/distribution_change_validator.rb b/lib/open_food_network/distribution_change_validator.rb index ba6d21b247..3f77eada7d 100644 --- a/lib/open_food_network/distribution_change_validator.rb +++ b/lib/open_food_network/distribution_change_validator.rb @@ -4,11 +4,6 @@ class DistributionChangeValidator @order = order end - def can_change_to_distributor?(distributor) - # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor - @order.line_items.empty? || all_available_distributors.include?(distributor) - end - def can_change_to_distribution?(distributor, order_cycle) (@order.line_item_variants - variants_available_for_distribution(distributor, order_cycle)).empty? end @@ -19,52 +14,4 @@ class DistributionChangeValidator product_distribution_variants + order_cycle_variants end - - def distributor_available_for?(product) - @order.nil? || available_distributors_for(product).present? - end - - def order_cycle_available_for?(product) - @order.nil? || !product_requires_order_cycle(product) || available_order_cycles_for(product).present? - end - - def available_distributors_for(product) - distributors = Enterprise.distributing_products(product) - - if @order.andand.line_items.present? - distributors = available_distributors(distributors) - end - - distributors - end - - def available_order_cycles_for(product) - order_cycles = OrderCycle.distributing_product(product) - - if @order.andand.line_items.present? - order_cycles = available_order_cycles(order_cycles) - end - - order_cycles - end - - def product_requires_order_cycle(product) - product.product_distributions.blank? - end - - def all_available_distributors - @all_available_distributors ||= (available_distributors(Enterprise.all) || []) - end - - def available_distributors enterprises - enterprises.select do |e| - (@order.line_item_variants - e.distributed_variants).empty? - end - end - - def available_order_cycles order_cycles - order_cycles.select do |oc| - (@order.line_item_variants - oc.distributed_variants).empty? - end - end end diff --git a/spec/lib/open_food_network/distribution_change_validator_spec.rb b/spec/lib/open_food_network/distribution_change_validator_spec.rb index 5a0858fe2f..cf6229c37c 100644 --- a/spec/lib/open_food_network/distribution_change_validator_spec.rb +++ b/spec/lib/open_food_network/distribution_change_validator_spec.rb @@ -54,211 +54,4 @@ describe DistributionChangeValidator do subject.variants_available_for_distribution(nil, nil).should == [] end end - - describe "finding distributors which have the same variants" do - let(:variant1) { double(:variant) } - let(:variant2) { double(:variant) } - let(:variant3) { double(:variant) } - let(:variant4) { double(:variant) } - let(:variant5) { double(:variant) } - - it "matches enterprises which offer all products within the order" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - enterprise = double(:enterprise) - enterprise.stub(:distributed_variants) { line_item_variants } # Exactly the same variants as the order - - subject.available_distributors([enterprise]).should == [enterprise] - end - - it "does not match enterprises with no products available" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - enterprise = double(:enterprise) - enterprise.stub(:distributed_variants) { [] } # No variants - - subject.available_distributors([enterprise]).should_not include enterprise - end - - it "does not match enterprises with only some of the same variants in the order available" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - enterprise_with_some_variants = double(:enterprise) - enterprise_with_some_variants.stub(:distributed_variants) { [variant1, variant3] } # Only some variants - enterprise_with_some_plus_extras = double(:enterprise) - enterprise_with_some_plus_extras.stub(:distributed_variants) { [variant1, variant2, variant3, variant4] } # Only some variants, plus extras - - subject.available_distributors([enterprise_with_some_variants]).should_not include enterprise_with_some_variants - subject.available_distributors([enterprise_with_some_plus_extras]).should_not include enterprise_with_some_plus_extras - end - - it "matches enterprises which offer all products in the order, plus additional products" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - enterprise = double(:enterprise) - enterprise.stub(:distributed_variants) { [variant1, variant2, variant3, variant4, variant5] } # Excess variants - - subject.available_distributors([enterprise]).should == [enterprise] - end - - it "matches no enterprises when none are provided" do - subject.available_distributors([]).should == [] - end - end - - describe "finding order cycles which have the same variants" do - let(:variant1) { double(:variant) } - let(:variant2) { double(:variant) } - let(:variant3) { double(:variant) } - let(:variant4) { double(:variant) } - let(:variant5) { double(:variant) } - - it "matches order cycles which offer all products within the order" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - order_cycle = double(:order_cycle) - order_cycle.stub(:distributed_variants) { line_item_variants } # Exactly the same variants as the order - - subject.available_order_cycles([order_cycle]).should == [order_cycle] - end - - it "does not match order cycles with no products available" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - order_cycle = double(:order_cycle) - order_cycle.stub(:distributed_variants) { [] } # No variants - - subject.available_order_cycles([order_cycle]).should_not include order_cycle - end - - it "does not match order cycles with only some of the same variants in the order available" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - order_cycle_with_some_variants = double(:order_cycle) - order_cycle_with_some_variants.stub(:distributed_variants) { [variant1, variant3] } # Only some variants - order_cycle_with_some_plus_extras = double(:order_cycle) - order_cycle_with_some_plus_extras.stub(:distributed_variants) { [variant1, variant2, variant3, variant4] } # Only some variants, plus extras - - subject.available_order_cycles([order_cycle_with_some_variants]).should_not include order_cycle_with_some_variants - subject.available_order_cycles([order_cycle_with_some_plus_extras]).should_not include order_cycle_with_some_plus_extras - end - - it "matches order cycles which offer all products in the order, plus additional products" do - line_item_variants = [variant1, variant3, variant5] - order.stub(:line_item_variants) { line_item_variants } - order_cycle = double(:order_cycle) - order_cycle.stub(:distributed_variants) { [variant1, variant2, variant3, variant4, variant5] } # Excess variants - - subject.available_order_cycles([order_cycle]).should == [order_cycle] - end - - it "matches no order cycles when none are provided" do - subject.available_order_cycles([]).should == [] - end - end - - describe "checking if a distributor is available for a product" do - it "returns true when order is nil" do - subject = DistributionChangeValidator.new(nil) - subject.distributor_available_for?(product).should be true - end - - it "returns true when there's an distributor that can cover the new product" do - subject.stub(:available_distributors_for).and_return([1]) - subject.distributor_available_for?(product).should be true - end - - it "returns false when there's no distributor that can cover the new product" do - subject.stub(:available_distributors_for).and_return([]) - subject.distributor_available_for?(product).should be false - end - end - - describe "checking if an order cycle is available for a product" do - it "returns true when the order is nil" do - subject = DistributionChangeValidator.new(nil) - subject.order_cycle_available_for?(product).should be true - end - - it "returns true when the product doesn't require an order cycle" do - subject.stub(:product_requires_order_cycle).and_return(false) - subject.order_cycle_available_for?(product).should be true - end - - it "returns true when there's an order cycle that can cover the product" do - subject.stub(:product_requires_order_cycle).and_return(true) - subject.stub(:available_order_cycles_for).and_return([1]) - subject.order_cycle_available_for?(product).should be true - end - - it "returns false otherwise" do - subject.stub(:product_requires_order_cycle).and_return(true) - subject.stub(:available_order_cycles_for).and_return([]) - subject.order_cycle_available_for?(product).should be false - end - end - - describe "finding available distributors for a product" do - it "returns enterprises distributing the product when there's no order" do - subject = DistributionChangeValidator.new(nil) - Enterprise.stub(:distributing_products).and_return([1, 2, 3]) - subject.should_receive(:available_distributors).never - - subject.available_distributors_for(product).should == [1, 2, 3] - end - - it "returns enterprises distributing the product when there's no order items" do - order.stub(:line_items) { [] } - Enterprise.stub(:distributing_products).and_return([1, 2, 3]) - subject.should_receive(:available_distributors).never - - subject.available_distributors_for(product).should == [1, 2, 3] - end - - it "filters by available distributors when there are order items" do - order.stub(:line_items) { [1, 2, 3] } - Enterprise.stub(:distributing_products).and_return([1, 2, 3]) - subject.should_receive(:available_distributors).and_return([2]) - - subject.available_distributors_for(product).should == [2] - end - end - - describe "finding available order cycles for a product" do - it "returns order cycles distributing the product when there's no order" do - subject = DistributionChangeValidator.new(nil) - OrderCycle.stub(:distributing_product).and_return([1, 2, 3]) - subject.should_receive(:available_order_cycles).never - - subject.available_order_cycles_for(product).should == [1, 2, 3] - end - - it "returns order cycles distributing the product when there's no order items" do - order.stub(:line_items) { [] } - OrderCycle.stub(:distributing_product).and_return([1, 2, 3]) - subject.should_receive(:available_order_cycles).never - - subject.available_order_cycles_for(product).should == [1, 2, 3] - end - - it "filters by available order cycles when there are order items" do - order.stub(:line_items) { [1, 2, 3] } - OrderCycle.stub(:distributing_product).and_return([1, 2, 3]) - subject.should_receive(:available_order_cycles).and_return([2]) - - subject.available_order_cycles_for(product).should == [2] - end - end - - describe "determining if a product requires an order cycle" do - it "returns true when the product does not have any product distributions" do - product.stub(:product_distributions).and_return([]) - subject.product_requires_order_cycle(product).should be true - end - - it "returns false otherwise" do - product.stub(:product_distributions).and_return([1]) - subject.product_requires_order_cycle(product).should be false - end - end end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 3ffd650ddd..5f46a40985 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -81,38 +81,6 @@ describe OrderCycle do OrderCycle.accessible_by(user).should_not include(oc_not_accessible) end - describe "finding order cycles distributing a product" do - it "returns order cycles distributing the product's master variant" do - p = create(:product) - d = create(:distributor_enterprise) - oc = create(:simple_order_cycle, distributors: [d], variants: [p.master]) - p.reload - - OrderCycle.distributing_product(p).should == [oc] - end - - it "returns order cycles distributing another variant" do - p = create(:product) - v = create(:variant, product: p) - d = create(:distributor_enterprise) - oc = create(:simple_order_cycle, distributors: [d], variants: [v]) - p.reload - - OrderCycle.distributing_product(p).should == [oc] - end - - it "does not return order cycles supplying but not distributing a product" do - p = create(:product) - s = create(:supplier_enterprise) - oc = create(:simple_order_cycle) - ex = create(:exchange, order_cycle: oc, sender: s, receiver: oc.coordinator, incoming: true) - ex.variants << p.master - p.reload - - OrderCycle.distributing_product(p).should == [] - end - end - it "finds the most recently closed order cycles" do oc1 = create(:simple_order_cycle, orders_close_at: 2.hours.ago) oc2 = create(:simple_order_cycle, orders_close_at: 1.hour.ago) diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index 5c5bec131a..8a3be75e31 100644 --- a/spec/services/cart_service_spec.rb +++ b/spec/services/cart_service_spec.rb @@ -51,37 +51,6 @@ describe CartService do end end - describe "populate" do - before do - expect(cart_service).to receive(:distributor_and_order_cycle). - and_return([distributor, order_cycle]) - end - - it "checks that distribution can supply all products in the cart" do - expect(cart_service).to receive(:distribution_can_supply_products_in_cart). - with(distributor, order_cycle).and_return(false) - - expect(cart_service.populate(params)).to be false - expect(cart_service.errors.to_a).to eq(["That distributor or order cycle can't supply all the products in your cart. Please choose another."]) - end - - it "locks the order" do - allow(cart_service).to receive(:distribution_can_supply_products_in_cart).and_return(true) - expect(order).to receive(:with_lock) - cart_service.populate(params, true) - end - - it "attempts cart add with max_quantity" do - allow(cart_service).to receive(:distribution_can_supply_products_in_cart).and_return true - params = { variants: { "1" => { quantity: 1, max_quantity: 2 } } } - allow(order).to receive(:with_lock).and_yield - allow(cart_service).to receive(:varies_from_cart) { true } - allow(cart_service).to receive(:variants_removed) { [] } - expect(cart_service).to receive(:attempt_cart_add).with("1", 1, 2).and_return true - cart_service.populate(params, true) - end - end - describe "varies_from_cart" do let(:variant) { double(:variant, id: 123) } @@ -236,15 +205,6 @@ describe CartService do end describe "validations" do - describe "determining if distributor can supply products in cart" do - it "delegates to DistributionChangeValidator" do - dcv = double(:dcv) - expect(dcv).to receive(:can_change_to_distribution?).with(distributor, order_cycle).and_return(true) - expect(DistributionChangeValidator).to receive(:new).with(order).and_return(dcv) - expect(cart_service.send(:distribution_can_supply_products_in_cart, distributor, order_cycle)).to be true - end - end - describe "checking order cycle is provided for a variant, OR is not needed" do let(:variant) { double(:variant) }