From 7ba8c5ace1303dacdec5bfc21c498780c5554e2c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 16 Jul 2020 16:31:13 +0100 Subject: [PATCH 1/4] Make OC advanced settings work by permitting the extra parameter --- app/services/permitted_attributes/order_cycle.rb | 1 + .../admin/order_cycles_controller_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/services/permitted_attributes/order_cycle.rb b/app/services/permitted_attributes/order_cycle.rb index 41fd82bead..b81dccef0f 100644 --- a/app/services/permitted_attributes/order_cycle.rb +++ b/app/services/permitted_attributes/order_cycle.rb @@ -11,6 +11,7 @@ module PermittedAttributes @params.require(:order_cycle).permit( :name, :orders_open_at, :orders_close_at, :coordinator_id, + :preferred_product_selection_from_coordinator_inventory_only, incoming_exchanges: permitted_exchange_attributes, outgoing_exchanges: permitted_exchange_attributes, schedule_ids: [], coordinator_fee_ids: [] diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index 2ba98c9f8e..0175cc1d28 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -178,10 +178,22 @@ module Admin it "returns an error message" do spree_put :update, params + json_response = JSON.parse(response.body) expect(json_response['errors']).to be end end + + it "can update preference product_selection_from_coordinator_inventory_only" do + expect(OrderCycleForm).to receive(:new). + with(order_cycle, + { "preferred_product_selection_from_coordinator_inventory_only" => true }, + anything) { form_mock } + allow(form_mock).to receive(:save) { true } + + spree_put :update, params. + merge(order_cycle: { preferred_product_selection_from_coordinator_inventory_only: true }) + end end end From 9b5875a7d12566a93d055e2327fd86d4f104b235 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 21 Jul 2020 19:12:50 +0100 Subject: [PATCH 2/4] Move select out of scope visible_for because it is breaking exchange_product queries and it's just not needed there. The only other use of this product's scope visible_for is the enterprise serializer so we add the select to it. --- app/models/spree/product_decorator.rb | 3 +-- .../api/admin/for_order_cycle/enterprise_serializer.rb | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 602329b5c1..554309f687 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -63,8 +63,7 @@ Spree::Product.class_eval do scope :visible_for, lambda { |enterprise| joins('LEFT OUTER JOIN spree_variants AS o_spree_variants ON (o_spree_variants.product_id = spree_products.id)'). joins('LEFT OUTER JOIN inventory_items AS o_inventory_items ON (o_spree_variants.id = o_inventory_items.variant_id)'). - where('o_inventory_items.enterprise_id = (?) AND visible = (?)', enterprise, true). - select('DISTINCT spree_products.*') + where('o_inventory_items.enterprise_id = (?) AND visible = (?)', enterprise, true) } # -- Scopes diff --git a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb index 360f135680..66ed62b7f8 100644 --- a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb +++ b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb @@ -30,7 +30,9 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer def products_scope products_relation = object.supplied_products if order_cycle.prefers_product_selection_from_coordinator_inventory_only? - products_relation = products_relation.visible_for(order_cycle.coordinator) + products_relation = products_relation. + visible_for(order_cycle.coordinator). + select('DISTINCT spree_products.*') end products_relation.order(:name) end From aadbc9ed5d2133021a2abbabd8382f7483059f7d Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 21 Jul 2020 19:20:08 +0100 Subject: [PATCH 3/4] Remove unnecessary order statement, the relation will only be used for counting products --- .../api/admin/for_order_cycle/enterprise_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb index 66ed62b7f8..7e6fed44f7 100644 --- a/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb +++ b/app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb @@ -34,7 +34,7 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer visible_for(order_cycle.coordinator). select('DISTINCT spree_products.*') end - products_relation.order(:name) + products_relation end def products From e445fc33a1aae7912a1a22b9691fa85117036aa7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 21 Jul 2020 20:48:16 +0100 Subject: [PATCH 4/4] Add spec to cover SQL query issue with OCs where the only products from the coordinator inventory are renderer --- .../exchange_products_renderer_spec.rb | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/spec/services/exchange_products_renderer_spec.rb b/spec/services/exchange_products_renderer_spec.rb index 67233906ad..b2eba1c6c3 100644 --- a/spec/services/exchange_products_renderer_spec.rb +++ b/spec/services/exchange_products_renderer_spec.rb @@ -7,8 +7,9 @@ describe ExchangeProductsRenderer do describe "#exchange_products" do describe "for an incoming exchange" do + let(:exchange) { order_cycle.exchanges.incoming.first } + it "loads products" do - exchange = order_cycle.exchanges.incoming.first products = renderer.exchange_products(true, exchange.sender) expect(products.first.supplier.name).to eq exchange.variants.first.product.supplier.name @@ -16,14 +17,34 @@ describe ExchangeProductsRenderer do end describe "for an outgoing exchange" do + let(:exchange) { order_cycle.exchanges.outgoing.first } + it "loads products" do - exchange = order_cycle.exchanges.outgoing.first products = renderer.exchange_products(false, exchange.receiver) suppliers = [exchange.variants[0].product.supplier.name, exchange.variants[1].product.supplier.name] expect(suppliers).to include products.first.supplier.name expect(suppliers).to include products.second.supplier.name end + + context "showing products from coordinator inventory only" do + before { order_cycle.update prefers_product_selection_from_coordinator_inventory_only: true } + + it "loads no products if there are no products from the coordinator inventory" do + products = renderer.exchange_products(false, exchange.receiver) + + expect(products).to be_empty + end + + it "loads products from the coordinator inventory" do + # Add variant already in the exchange to the coordinator's inventory + exchange.variants.first.inventory_items = [create(:inventory_item, enterprise: order_cycle.coordinator)] + + products = renderer.exchange_products(false, exchange.receiver) + + expect(products).to eq [exchange.variants.first.product] + end + end end end