From b072da142e665a3b76da86071dea8b33ebdcd18a Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Mon, 19 Jun 2023 12:12:39 +0100 Subject: [PATCH 1/4] filter distributors before listing on checkout options --- app/helpers/order_cycles_helper.rb | 9 +++++ .../order_cycles/checkout_options.html.haml | 4 +- spec/helpers/order_cycles_helper_spec.rb | 37 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index a68a4d5906..c3cfe8438d 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -36,6 +36,15 @@ module OrderCyclesHelper shipping_and_payment_methods: true end + def distributors_with_editable_shipping_and_payment_methods(order_cycle) + return order_cycle.distributors if order_cycle.coordinator + .in? Enterprise.managed_by(spree_current_user) + + order_cycle.distributors.select do |distributor| + distributor.in? Enterprise.managed_by(spree_current_user) + end + end + def order_cycle_status_class(order_cycle) if order_cycle.undated? 'undated' diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml index cc43c1323e..e43d322b09 100644 --- a/app/views/admin/order_cycles/checkout_options.html.haml +++ b/app/views/admin/order_cycles/checkout_options.html.haml @@ -19,7 +19,7 @@ %th{ colspan: 2 } = t('.shipping_methods') = hidden_field_tag "order_cycle[selected_distributor_shipping_method_ids][]", "" - - @order_cycle.distributors.each do |distributor| + - distributors_with_editable_shipping_and_payment_methods(@order_cycle).each do |distributor| - distributor_shipping_methods = @order_cycle.attachable_distributor_shipping_methods.where("distributor_id = ?", distributor.id).includes(:shipping_method) %tr{ class: "distributor-#{distributor.id}-shipping-methods", "data-controller": "checked" } %td.text-center @@ -50,7 +50,7 @@ %th{ colspan: 2 } = t('.payment_methods') = hidden_field_tag "order_cycle[selected_distributor_payment_method_ids][]", "" - - @order_cycle.distributors.each do |distributor| + - distributors_with_editable_shipping_and_payment_methods(@order_cycle).each do |distributor| - distributor_payment_methods = @order_cycle.attachable_distributor_payment_methods.where("distributor_id = ?", distributor.id).includes(:payment_method) %tr{ class: "distributor-#{distributor.id}-payment-methods", "data-controller": "checked" } %td.text-center diff --git a/spec/helpers/order_cycles_helper_spec.rb b/spec/helpers/order_cycles_helper_spec.rb index a5fa043c12..2fcbfd36ff 100644 --- a/spec/helpers/order_cycles_helper_spec.rb +++ b/spec/helpers/order_cycles_helper_spec.rb @@ -92,4 +92,41 @@ describe OrderCyclesHelper, type: :helper do expect(helper.pickup_time(oc2)).to eq("turtles") end end + + describe "distibutors that have editable shipping/payment methods" do + let(:hub1) { create(:distributor_enterprise, name: 'hub1') } + let(:hub2) { create(:distributor_enterprise, name: 'hub2') } + let(:supplier){ create(:supplier_enterprise, name: 'supplier') } + let(:coordinator){ create(:distributor_enterprise, name: 'coordinator') } + before do + allow(oc).to receive(:coordinator).and_return coordinator + allow(oc).to receive(:suppliers).and_return [supplier] + allow(oc).to receive(:distributors).and_return [hub1, hub2] + end + context 'current user is a coordinator' do + before do + allow(helper).to receive(:spree_current_user).and_return coordinator.owner + end + + it 'returns all distributors' do + expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1, hub2] + end + end + context 'current user is a producer' do + before do + allow(helper).to receive(:spree_current_user).and_return supplier.owner + end + it "doesn't return any distributors" do + expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [] + end + end + context 'current user is a hub' do + before do + allow(helper).to receive(:spree_current_user).and_return hub1.owner + end + it "returns only the hubs of the current user" do + expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1] + end + end + end end From 7f94fbc085eccec25eeb9ad9fa74d92fa7b8e22d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 26 Jun 2023 12:39:41 +1000 Subject: [PATCH 2/4] Add common whitespace to spec --- spec/helpers/order_cycles_helper_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/helpers/order_cycles_helper_spec.rb b/spec/helpers/order_cycles_helper_spec.rb index 2fcbfd36ff..32e641bf06 100644 --- a/spec/helpers/order_cycles_helper_spec.rb +++ b/spec/helpers/order_cycles_helper_spec.rb @@ -98,11 +98,13 @@ describe OrderCyclesHelper, type: :helper do let(:hub2) { create(:distributor_enterprise, name: 'hub2') } let(:supplier){ create(:supplier_enterprise, name: 'supplier') } let(:coordinator){ create(:distributor_enterprise, name: 'coordinator') } + before do allow(oc).to receive(:coordinator).and_return coordinator allow(oc).to receive(:suppliers).and_return [supplier] allow(oc).to receive(:distributors).and_return [hub1, hub2] end + context 'current user is a coordinator' do before do allow(helper).to receive(:spree_current_user).and_return coordinator.owner @@ -112,18 +114,22 @@ describe OrderCyclesHelper, type: :helper do expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1, hub2] end end + context 'current user is a producer' do before do allow(helper).to receive(:spree_current_user).and_return supplier.owner end + it "doesn't return any distributors" do expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [] end end + context 'current user is a hub' do before do allow(helper).to receive(:spree_current_user).and_return hub1.owner end + it "returns only the hubs of the current user" do expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1] end From 61c4fb936abf2d815d3d142c70ce69a22c2cc1f8 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 26 Jun 2023 12:49:56 +1000 Subject: [PATCH 3/4] Test OrderCycleHelper with real data, not mocks It's more realistic and doesn't break with the next change. --- spec/helpers/order_cycles_helper_spec.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/spec/helpers/order_cycles_helper_spec.rb b/spec/helpers/order_cycles_helper_spec.rb index 32e641bf06..8ebd62df2f 100644 --- a/spec/helpers/order_cycles_helper_spec.rb +++ b/spec/helpers/order_cycles_helper_spec.rb @@ -94,24 +94,27 @@ describe OrderCyclesHelper, type: :helper do end describe "distibutors that have editable shipping/payment methods" do + let(:result) { + helper.distributors_with_editable_shipping_and_payment_methods(order_cycle) + } + let(:order_cycle) { + create( + :simple_order_cycle, + coordinator:, suppliers: [supplier], distributors: [hub1, hub2], + ) + } let(:hub1) { create(:distributor_enterprise, name: 'hub1') } let(:hub2) { create(:distributor_enterprise, name: 'hub2') } let(:supplier){ create(:supplier_enterprise, name: 'supplier') } let(:coordinator){ create(:distributor_enterprise, name: 'coordinator') } - before do - allow(oc).to receive(:coordinator).and_return coordinator - allow(oc).to receive(:suppliers).and_return [supplier] - allow(oc).to receive(:distributors).and_return [hub1, hub2] - end - context 'current user is a coordinator' do before do allow(helper).to receive(:spree_current_user).and_return coordinator.owner end it 'returns all distributors' do - expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1, hub2] + expect(result).to match_array [hub1, hub2] end end @@ -121,7 +124,7 @@ describe OrderCyclesHelper, type: :helper do end it "doesn't return any distributors" do - expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [] + expect(result).to eq [] end end @@ -131,7 +134,7 @@ describe OrderCyclesHelper, type: :helper do end it "returns only the hubs of the current user" do - expect(helper.distributors_with_editable_shipping_and_payment_methods(oc)).to eq [hub1] + expect(result).to eq [hub1] end end end From c3d49bf31af5a99112894bdb25123433d529c431 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 26 Jun 2023 12:51:14 +1000 Subject: [PATCH 4/4] Speed up, simplify helper with ActiveRecord scopes --- app/helpers/order_cycles_helper.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index c3cfe8438d..1e04dba995 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -37,12 +37,10 @@ module OrderCyclesHelper end def distributors_with_editable_shipping_and_payment_methods(order_cycle) - return order_cycle.distributors if order_cycle.coordinator - .in? Enterprise.managed_by(spree_current_user) + return order_cycle.distributors if Enterprise + .managed_by(spree_current_user).exists?(order_cycle.coordinator.id) - order_cycle.distributors.select do |distributor| - distributor.in? Enterprise.managed_by(spree_current_user) - end + order_cycle.distributors.managed_by(spree_current_user) end def order_cycle_status_class(order_cycle)