Delegate responsibility to estimating prices for subscriptions to dedicated service object

app/services/subscription_estimator.rb

@@ -0,0 +1,28 @@
+class SubscriptionEstimator
+  def initialize(subscription, fee_calculator)
+    @subscription = subscription
+    @fee_calculator = fee_calculator
+  end
+
+  def estimate!
+    assign_price_estimates
+  end
+
+  private
+
+  attr_accessor :subscription, :fee_calculator
+
+  delegate :subscription_line_items, to: :subscription
+
+  def assign_price_estimates
+    subscription_line_items.each do |item|
+      item.price_estimate = price_estimate_for(item.variant)
+    end
+  end
+
+  def price_estimate_for(variant)
+    return 0.0 unless fee_calculator && variant
+    fees = fee_calculator.indexed_fees_for(variant)
+    (variant.price + fees).to_d
+  end
+end

app/services/subscription_form.rb

@@ -1,7 +1,7 @@
 require 'open_food_network/proxy_order_syncer'
 
 class SubscriptionForm
-  attr_accessor :subscription, :params, :fee_calculator, :order_update_issues, :validator, :order_syncer
+  attr_accessor :subscription, :params, :order_update_issues, :validator, :order_syncer, :estimator
 
   delegate :json_errors, :valid?, to: :validator
   delegate :order_update_issues, to: :order_syncer
@@ -9,16 +9,16 @@ class SubscriptionForm
   def initialize(subscription, params = {}, fee_calculator = nil)
     @subscription = subscription
     @params = params
-    @fee_calculator = fee_calculator
+    @estimator = SubscriptionEstimator.new(subscription, fee_calculator)
     @validator = SubscriptionValidator.new(subscription)
     @order_syncer = OrderSyncer.new(subscription)
   end
 
   def save
-    validate_price_estimates
     subscription.assign_attributes(params)
     return false unless valid?
     subscription.transaction do
+      estimator.estimate!
       proxy_order_syncer.sync!
       order_syncer.sync!
       subscription.save!
@@ -30,29 +30,4 @@ class SubscriptionForm
   def proxy_order_syncer
     OpenFoodNetwork::ProxyOrderSyncer.new(subscription)
   end
-
-  def validate_price_estimates
-    return unless params[:subscription_line_items_attributes]
-    return clear_price_estimates unless fee_calculator
-    calculate_prices_from_variant_ids
-  end
-
-  def clear_price_estimates
-    params[:subscription_line_items_attributes].each do |item_attrs|
-      item_attrs.delete(:price_estimate)
-    end
-  end
-
-  def calculate_prices_from_variant_ids
-    params[:subscription_line_items_attributes].each do |item_attrs|
-      variant = Spree::Variant.find_by_id(item_attrs[:variant_id])
-      next item_attrs.delete(:price_estimate) unless variant
-      item_attrs[:price_estimate] = price_estimate_for(variant)
-    end
-  end
-
-  def price_estimate_for(variant)
-    fees = fee_calculator.indexed_fees_for(variant)
-    (variant.price + fees).to_d
-  end
 end

spec/services/subscription_estimator_spec.rb

@@ -0,0 +1,45 @@
+describe SubscriptionEstimator do
+  describe "#estimate!" do
+    let!(:subscription) { create(:subscription, with_items: true) }
+    let!(:sli1) { subscription.subscription_line_items.first }
+    let!(:sli2) { subscription.subscription_line_items.second }
+    let!(:sli3) { subscription.subscription_line_items.third }
+    let(:fee_calculator) { nil }
+    let(:estimator) { SubscriptionEstimator.new(subscription, fee_calculator) }
+
+    before do
+      sli1.update_attributes(price_estimate: 4.0)
+      sli2.update_attributes(price_estimate: 5.0)
+      sli3.update_attributes(price_estimate: 6.0)
+      sli1.variant.update_attributes(price: 1.0)
+      sli2.variant.update_attributes(price: 2.0)
+      sli3.variant.update_attributes(price: 3.0)
+    end
+
+    context "when a fee calculator is not present" do
+      it "removes price estimates from all items" do
+        estimator.estimate!
+        subscription.subscription_line_items.each do |item|
+          expect(item.price_estimate).to eq 0
+        end
+      end
+    end
+
+    context "when a fee calculator is present" do
+      let(:fee_calculator) { instance_double(OpenFoodNetwork::EnterpriseFeeCalculator) }
+
+      before do
+        allow(fee_calculator).to receive(:indexed_fees_for).with(sli1.variant) { 1.0 }
+        allow(fee_calculator).to receive(:indexed_fees_for).with(sli2.variant) { 0.0 }
+        allow(fee_calculator).to receive(:indexed_fees_for).with(sli3.variant) { 3.0 }
+      end
+
+      it "recalculates price_estimates based on variant prices and associated fees" do
+        estimator.estimate!
+        expect(sli1.price_estimate).to eq 2.0
+        expect(sli2.price_estimate).to eq 2.0
+        expect(sli3.price_estimate).to eq 6.0
+      end
+    end
+  end
+end

spec/services/subscription_form_spec.rb

@@ -21,6 +21,7 @@ describe SubscriptionForm do
     let!(:payment_method) { create(:payment_method, distributors: [shop]) }
     let!(:shipping_method) { create(:shipping_method, distributors: [shop]) }
     let!(:address) { create(:address) }
+    let!(:fee_calculator) { OpenFoodNetwork::EnterpriseFeeCalculator.new(shop, order_cycle2) }
     let(:subscription) { Subscription.new }
 
     let!(:params) {
@@ -35,19 +36,23 @@ describe SubscriptionForm do
         begins_at: 4.days.ago,
         ends_at: 14.days.from_now,
         subscription_line_items_attributes: [
-          {variant_id: variant1.id, quantity: 1},
-          {variant_id: variant2.id, quantity: 2},
-          {variant_id: variant3.id, quantity: 3}
+          {variant_id: variant1.id, quantity: 1, price_estimate: 7.0},
+          {variant_id: variant2.id, quantity: 2, price_estimate: 8.0},
+          {variant_id: variant3.id, quantity: 3, price_estimate: 9.0}
         ]
       } }
 
-    let(:form) { SubscriptionForm.new(subscription, params) }
+    let(:form) { SubscriptionForm.new(subscription, params, fee_calculator) }
 
     it "creates orders for each order cycle in the schedule" do
       Spree::Config.set allow_backorders: false
       expect(form.save).to be true
 
       expect(subscription.proxy_orders.count).to be 2
+      expect(subscription.subscription_line_items.count).to be 3
+      expect(subscription.subscription_line_items[0].price_estimate).to eq 13.75
+      expect(subscription.subscription_line_items[1].price_estimate).to eq 7.75
+      expect(subscription.subscription_line_items[2].price_estimate).to eq 4.25
 
       # This order cycle has already closed, so no order is initialized
       proxy_order1 = subscription.proxy_orders.find_by_order_cycle_id(order_cycle1.id)
@@ -88,56 +93,4 @@ describe SubscriptionForm do
       expect(proxy_order4).to be nil
     end
   end
-
-  describe "validating price_estimates on subscription line items" do
-    let(:params) { { } }
-    let(:form) { SubscriptionForm.new(nil, params) }
-
-    context "when line_item params are present" do
-      before { allow(form).to receive(:price_estimate_for) }
-
-      it "does nothing" do
-        form.send(:validate_price_estimates)
-        expect(form.params[:subscription_line_items_attributes]).to be nil
-      end
-    end
-
-    context "when line_item params are present" do
-      before do
-        params[:subscription_line_items_attributes] = [{ id: 1, price_estimate: 2.50 }, { id: 2, price_estimate: 3.50 }]
-      end
-
-      context "when no fee calculator is present" do
-        before { allow(form).to receive(:price_estimate_for) }
-
-        it "clears price estimates on all subscription line item attributes" do
-          form.send(:validate_price_estimates)
-          attrs = form.params[:subscription_line_items_attributes]
-          expect(attrs.first.keys).to_not include :price_estimate
-          expect(attrs.last.keys).to_not include :price_estimate
-          expect(form).to_not have_received(:price_estimate_for)
-        end
-      end
-
-      context "when a fee calculator is present" do
-        let(:variant) { create(:variant) }
-        let(:fee_calculator) { double(:fee_calculator) }
-
-        before do
-          allow(form).to receive(:fee_calculator) { fee_calculator }
-          allow(form).to receive(:price_estimate_for) { 5.30 }
-          params[:subscription_line_items_attributes].first[:variant_id] = variant.id
-        end
-
-        it "clears price estimates on subscription line item attributes without variant ids" do
-          form.send(:validate_price_estimates)
-          attrs = form.params[:subscription_line_items_attributes]
-          expect(attrs.first.keys).to include :price_estimate
-          expect(attrs.last.keys).to_not include :price_estimate
-          expect(attrs.first[:price_estimate]).to eq 5.30
-          expect(form).to have_received(:price_estimate_for).with(variant)
-        end
-      end
-    end
-  end
 end