From 7af11da90143e60d619ff47db2bded82ec8fbe2d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 13 Jun 2018 17:00:46 +1000 Subject: [PATCH] Use a SubscriptionsCount query object to provide counts to IndexOrderCycleSerializer --- .../admin/order_cycles_controller.rb | 9 ++--- .../api/admin/index_order_cycle_serializer.rb | 2 +- app/services/subscriptions_count.rb | 19 +++++++++++ .../admin/order_cycles_controller_spec.rb | 25 -------------- spec/services/subscriptions_count_spec.rb | 34 +++++++++++++++++++ 5 files changed, 56 insertions(+), 33 deletions(-) create mode 100644 app/services/subscriptions_count.rb create mode 100644 spec/services/subscriptions_count_spec.rb diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 1db8d22471..55b08de405 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -19,7 +19,7 @@ module Admin respond_to do |format| format.html format.json do - render_as_json @collection, ams_prefix: params[:ams_prefix], current_user: spree_current_user, subscriptions_counts: subscriptions_counts + render_as_json @collection, ams_prefix: params[:ams_prefix], current_user: spree_current_user, subscriptions_count: SubscriptionsCount.new(@collection) end end end @@ -81,7 +81,7 @@ module Admin def bulk_update if order_cycle_set.andand.save respond_to do |format| - format.json { render_as_json @order_cycles, ams_prefix: 'index', current_user: spree_current_user, subscriptions_counts: subscriptions_counts } + format.json { render_as_json @order_cycles, ams_prefix: 'index', current_user: spree_current_user, subscriptions_count: SubscriptionsCount.new(@collection) } end else respond_to do |format| @@ -230,11 +230,6 @@ module Admin render json: { errors: t('admin.order_cycles.bulk_update.no_data') }, status: :unprocessable_entity end - def subscriptions_counts - return [] if @collection.blank? - ProxyOrder.not_canceled.group(:order_cycle_id).where(order_cycle_id: @collection).count - end - def ams_prefix_whitelist [:basic, :index] end diff --git a/app/serializers/api/admin/index_order_cycle_serializer.rb b/app/serializers/api/admin/index_order_cycle_serializer.rb index 9a05e1616d..144bc38a7c 100644 --- a/app/serializers/api/admin/index_order_cycle_serializer.rb +++ b/app/serializers/api/admin/index_order_cycle_serializer.rb @@ -62,7 +62,7 @@ module Api end def subscriptions_count - options[:subscriptions_counts][object.id] + options[:subscriptions_count].for(object.id) end private diff --git a/app/services/subscriptions_count.rb b/app/services/subscriptions_count.rb new file mode 100644 index 0000000000..6afbe15dae --- /dev/null +++ b/app/services/subscriptions_count.rb @@ -0,0 +1,19 @@ +class SubscriptionsCount + def initialize(order_cycles) + @order_cycles = order_cycles + end + + def for(order_cycle_id) + active[order_cycle_id] || 0 + end + + private + + attr_accessor :order_cycles + + def active + return @active unless @active.nil? + return @active = [] if order_cycles.blank? + @active ||= ProxyOrder.not_canceled.group(:order_cycle_id).where(order_cycle_id: order_cycles).count + end +end diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index be835c6d98..4bfe487353 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -338,30 +338,5 @@ module Admin end end - describe "#subscriptions_counts" do - let(:oc1) { create(:simple_order_cycle) } - let(:oc2) { create(:simple_order_cycle) } - let(:collection) { OrderCycle.where(id: [oc1, oc2]) } - - context "when the collection has not been set" do - it "returns and empty array" do - expect(controller.send(:subscriptions_counts)).to eq [] - end - end - - context "when the collection has been set" do - let!(:po1) { create(:proxy_order, order_cycle: oc1) } - let!(:po2) { create(:proxy_order, order_cycle: oc1) } - let!(:po3) { create(:proxy_order, order_cycle: oc2) } - - before { controller.instance_variable_set(:@collection, collection) } - - it "returns grouped count of all active proxy orders associated each order cycle in the collection" do - result = controller.send(:subscriptions_counts) - expect(result[oc1.id]).to eq 2 - expect(result[oc2.id]).to eq 1 - end - end - end end end diff --git a/spec/services/subscriptions_count_spec.rb b/spec/services/subscriptions_count_spec.rb new file mode 100644 index 0000000000..2446bcc8bf --- /dev/null +++ b/spec/services/subscriptions_count_spec.rb @@ -0,0 +1,34 @@ +describe SubscriptionsCount do + let(:oc1) { create(:simple_order_cycle) } + let(:oc2) { create(:simple_order_cycle) } + let(:subscriptions_count) { SubscriptionsCount.new(order_cycles) } + + describe "#for" do + context "when the collection has not been set" do + let(:order_cycles) { nil } + it "returns 0" do + expect(subscriptions_count.for(oc1.id)).to eq 0 + end + end + + context "when the collection has been set" do + let(:order_cycles) { OrderCycle.where(id: [oc1]) } + let!(:po1) { create(:proxy_order, order_cycle: oc1) } + let!(:po2) { create(:proxy_order, order_cycle: oc1) } + let!(:po3) { create(:proxy_order, order_cycle: oc2) } + + context "but the requested id is not present in the list of order cycles provided" do + it "returns 0" do + # Note that po3 applies to oc2, but oc2 in not in the collection + expect(subscriptions_count.for(oc2.id)).to eq 0 + end + end + + context "and the requested id is present in the list of order cycles provided" do + it "returns a count of active proxy orders associated with the requested order cycle" do + expect(subscriptions_count.for(oc1.id)).to eq 2 + end + end + end + end +end