From 685abccb61cf5394370a11fa18a5b1806a714174 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 24 Jan 2020 10:22:32 +0000 Subject: [PATCH 1/4] Make variant count consider oc config and not count variants that are hidden in the inventory of the coordinator of the OC --- app/controllers/admin/enterprises_controller.rb | 1 + .../api/exchange_products_controller.rb | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 5b2744d718..970c09a7ee 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -1,5 +1,6 @@ require 'open_food_network/referer_parser' require 'open_food_network/permissions' +require 'open_food_network/order_cycle_permissions' module Admin class EnterprisesController < ResourceController diff --git a/app/controllers/api/exchange_products_controller.rb b/app/controllers/api/exchange_products_controller.rb index e9e3ec537b..a4daa17c1c 100644 --- a/app/controllers/api/exchange_products_controller.rb +++ b/app/controllers/api/exchange_products_controller.rb @@ -28,11 +28,17 @@ module Api private def render_variant_count - render text: { - count: Spree::Variant. + variants_relation = Spree::Variant. not_master. - where(product_id: products). - count + where(product_id: products.select(&:id)) + + if @order_cycle.present? && + @order_cycle.prefers_product_selection_from_coordinator_inventory_only? + variants_relation = variants_relation.visible_for(@order_cycle.coordinator) + end + + render text: { + count: variants_relation.count }.to_json end From 4596399bc280fdc3032a23d2c1430be4d8380bab Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 24 Jan 2020 10:31:10 +0000 Subject: [PATCH 2/4] Extract logic from controller to renderer service Re-using the filter_visible method for both products and variants --- .../api/exchange_products_controller.rb | 24 +++++++++---------- app/services/exchange_products_renderer.rb | 16 +++++++++++-- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/app/controllers/api/exchange_products_controller.rb b/app/controllers/api/exchange_products_controller.rb index a4daa17c1c..362937ff89 100644 --- a/app/controllers/api/exchange_products_controller.rb +++ b/app/controllers/api/exchange_products_controller.rb @@ -28,24 +28,22 @@ module Api private def render_variant_count - variants_relation = Spree::Variant. - not_master. - where(product_id: products.select(&:id)) - - if @order_cycle.present? && - @order_cycle.prefers_product_selection_from_coordinator_inventory_only? - variants_relation = variants_relation.visible_for(@order_cycle.coordinator) - end - render text: { - count: variants_relation.count + count: variants.count }.to_json end + def variants + renderer.exchange_variants(@incoming, @enterprise) + end + def products - ExchangeProductsRenderer. - new(@order_cycle, spree_current_user). - exchange_products(@incoming, @enterprise) + renderer.exchange_products(@incoming, @enterprise) + end + + def renderer + @renderer ||= ExchangeProductsRenderer. + new(@order_cycle, spree_current_user) end def paginated_products diff --git a/app/services/exchange_products_renderer.rb b/app/services/exchange_products_renderer.rb index faadba34f1..bffb9b8cbd 100644 --- a/app/services/exchange_products_renderer.rb +++ b/app/services/exchange_products_renderer.rb @@ -12,6 +12,14 @@ class ExchangeProductsRenderer end end + def exchange_variants(incoming, enterprise) + variants_relation = Spree::Variant. + not_master. + where(product_id: exchange_products(incoming, enterprise).select(&:id)) + + filter_visible(variants_relation) + end + private def products_for_incoming_exchange(enterprise) @@ -21,12 +29,16 @@ class ExchangeProductsRenderer def supplied_products(enterprises_query_matcher) products_relation = Spree::Product.where(supplier_id: enterprises_query_matcher) + filter_visible(products_relation) + end + + def filter_visible(relation) if @order_cycle.present? && @order_cycle.prefers_product_selection_from_coordinator_inventory_only? - products_relation = products_relation.visible_for(@order_cycle.coordinator) + relation = relation.visible_for(@order_cycle.coordinator) end - products_relation + relation end def products_for_outgoing_exchange From b5004f1cbf84b05c9dc79c678f724983fcf540c7 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 24 Jan 2020 11:05:08 +0000 Subject: [PATCH 3/4] Add specs for ExchangeProductsRenderer#exchange_variants --- app/services/exchange_products_renderer.rb | 2 ++ .../exchange_products_renderer_spec.rb | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/app/services/exchange_products_renderer.rb b/app/services/exchange_products_renderer.rb index bffb9b8cbd..313016e663 100644 --- a/app/services/exchange_products_renderer.rb +++ b/app/services/exchange_products_renderer.rb @@ -1,3 +1,5 @@ +require 'open_food_network/order_cycle_permissions' + class ExchangeProductsRenderer def initialize(order_cycle, user) @order_cycle = order_cycle diff --git a/spec/services/exchange_products_renderer_spec.rb b/spec/services/exchange_products_renderer_spec.rb index 2af3636e3d..3293d27709 100644 --- a/spec/services/exchange_products_renderer_spec.rb +++ b/spec/services/exchange_products_renderer_spec.rb @@ -26,4 +26,38 @@ describe ExchangeProductsRenderer do end end end + + describe "#exchange_variants" do + describe "for an incoming exchange" do + it "loads variants" do + exchange = order_cycle.exchanges.incoming.first + variants = renderer.exchange_variants(true, exchange.sender) + + expect(variants.first.product.supplier.name).to eq exchange.variants.first.product.supplier.name + end + + describe "when OC is showing only the coordinators inventory" do + let(:exchange_with_visible_variant) { order_cycle.exchanges.incoming.second } + let(:exchange_with_hidden_variant) { order_cycle.exchanges.incoming.first } + let!(:visible_inventory_item) { create(:inventory_item, enterprise: order_cycle.coordinator, variant: exchange_with_visible_variant.variants.first, visible: true) } + let!(:hidden_inventory_item) { create(:inventory_item, enterprise: order_cycle.coordinator, variant: exchange_with_hidden_variant.variants.first, visible: false) } + + before do + order_cycle.prefers_product_selection_from_coordinator_inventory_only = true + end + + it "renders visible inventory variants" do + variants = renderer.exchange_variants(true, exchange_with_visible_variant.sender) + + expect(variants.size).to eq 1 + end + + it "does not render hidden inventory variants" do + variants = renderer.exchange_variants(true, exchange_with_hidden_variant.sender) + + expect(variants.size).to eq 0 + end + end + end + end end From 947914724a6bde50363628523c9642b82244b94b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 24 Jan 2020 12:16:37 +0000 Subject: [PATCH 4/4] Add frozen string literal magic comment --- app/controllers/api/exchange_products_controller.rb | 2 ++ app/services/exchange_products_renderer.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/controllers/api/exchange_products_controller.rb b/app/controllers/api/exchange_products_controller.rb index 362937ff89..3c20c7c764 100644 --- a/app/controllers/api/exchange_products_controller.rb +++ b/app/controllers/api/exchange_products_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This controller lists products that can be added to an exchange module Api class ExchangeProductsController < Api::BaseController diff --git a/app/services/exchange_products_renderer.rb b/app/services/exchange_products_renderer.rb index 313016e663..d1a0b0a225 100644 --- a/app/services/exchange_products_renderer.rb +++ b/app/services/exchange_products_renderer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'open_food_network/order_cycle_permissions' class ExchangeProductsRenderer