Update Orders::HandleFeesService#recreate_all_fees!

We now update or create line item fees instead of deleting them and
recreating them. This is to cover the case when a product has been
removed from an Order Cycle but we want to keep the fee already applied
on existing order. This was an issue only if the existing order got
updated after the product was removed.

app/models/spree/order.rb

@@ -88,7 +88,7 @@ module Spree
 
     delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher
     delegate :update_totals, :update_totals_and_states, to: :updater
-    delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!,
+    delegate :create_order_fees!, :update_order_fees!,
              :update_line_item_fees!, :recreate_all_fees!, to: :fee_handler
 
     validates :customer, presence: true, if: :require_customer?

app/services/orders/handle_fees_service.rb

@@ -16,9 +16,9 @@ module Orders
       # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69
       # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE
       order.with_lock do
-        EnterpriseFee.clear_all_adjustments order
+        EnterpriseFee.clear_order_adjustments order
 
-        create_line_item_fees!
+        create_or_update_line_item_fees!
         create_order_fees!
       end
 
@@ -26,11 +26,17 @@ module Orders
       order.update_order!
     end
 
-    def create_line_item_fees!
-      order.line_items.includes(variant: :product).each do |line_item|
-        if provided_by_order_cycle? line_item
-          calculator.create_line_item_adjustments_for line_item
+    def create_or_update_line_item_fees!
+      order.line_items.includes(:variant).each do |line_item|
+        # No fee associated with the line item so we just create them
+        if line_item.enterprise_fee_adjustments.blank?
+          create_line_item_fees!(line_item)
+          next
         end
+        create_or_update_line_item_fee!(line_item)
+
+        # delete any fees removed from the Order Cycle
+        delete_removed_fees!(line_item)
       end
     end
 
@@ -58,6 +64,57 @@ module Orders
 
     private
 
+    def create_line_item_fees!(line_item)
+      return unless provided_by_order_cycle? line_item
+
+      calculator.create_line_item_adjustments_for(line_item)
+    end
+
+    def create_or_update_line_item_fee!(line_item)
+      applicators = calculator.per_item_enterprise_fee_applicators_for(line_item.variant)
+
+      applicators.each do |fee_applicator|
+        fee_adjustment = line_item.adjustments.find_by(originator: fee_applicator.enterprise_fee)
+
+        if fee_adjustment
+          fee_adjustment.update_adjustment!(line_item, force: true)
+        elsif provided_by_order_cycle? line_item
+          fee_applicator.create_line_item_adjustment(line_item)
+        end
+      end
+
+      # Update any fees not already processed
+      fees_to_update =
+        order_cycle_per_item_enterprise_fee_for - applicators.map(&:enterprise_fee)
+      fees_to_update.each do |fee|
+        fee_adjustment = line_item.adjustments.find_by(originator: fee)
+
+        fee_adjustment&.update_adjustment!(line_item, force: true)
+      end
+    end
+
+    def delete_removed_fees!(line_item)
+      order_cycle_fees = order_cycle_per_item_enterprise_fee_for
+
+      removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)
+
+      removed_fees.each(&:destroy)
+    end
+
+    def order_cycle_per_item_enterprise_fee_for
+      fees = []
+
+      return fees unless order_cycle && distributor
+
+      order_cycle.exchanges.supplying_to(distributor).each do |exchange|
+        fees += exchange.enterprise_fees.per_item
+      end
+
+      fees += order_cycle.coordinator_fees.per_item
+
+      fees
+    end
+
     def calculator
       @calculator ||= OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
     end

spec/services/orders/handle_fees_service_spec.rb

@@ -4,24 +4,173 @@ require 'spec_helper'
 
 RSpec.describe Orders::HandleFeesService do
   let(:order_cycle) { create(:order_cycle) }
-  let(:order) { create(:order_with_line_items, line_items_count: 1, order_cycle:) }
+  let(:order) {
+    create(:order_with_line_items, line_items_count: 1, order_cycle:,
+                                   distributor: order_cycle.distributors.first)
+  }
   let(:line_item) { order.line_items.first }
 
   let(:service) { Orders::HandleFeesService.new(order) }
   let(:calculator) {
-    double(OpenFoodNetwork::EnterpriseFeeCalculator, create_order_adjustments_for: true)
+    instance_double(OpenFoodNetwork::EnterpriseFeeCalculator, create_order_adjustments_for: true)
   }
 
   before do
     allow(service).to receive(:calculator) { calculator }
   end
 
-  describe "#create_line_item_fees!" do
-    it "creates per-line-item fee adjustments for line items in the order cycle" do
-      allow(service).to receive(:provided_by_order_cycle?) { true }
-      expect(calculator).to receive(:create_line_item_adjustments_for).with(line_item)
+  describe "#create_or_update_line_item_fees!" do
+    context "with no existing fee" do
+      it "creates per line item fee adjustments for line items in the order cylce" do
+        allow(service).to receive(:provided_by_order_cycle?) { true }
+        expect(calculator).to receive(:create_line_item_adjustments_for).with(line_item)
 
-      service.create_line_item_fees!
+        service.create_or_update_line_item_fees!
+      end
+
+      it "does not create fee if variant not in Order Cyle" do
+        allow(service).to receive(:provided_by_order_cycle?) { false }
+        expect(calculator).not_to receive(:create_line_item_adjustments_for).with(line_item)
+
+        service.create_or_update_line_item_fees!
+      end
+    end
+
+    context "with existing line item fee" do
+      let(:fee) { order_cycle.exchanges.first.enterprise_fees.first }
+      let(:role) { order_cycle.exchanges.first.role }
+      let(:fee_applicator) {
+        OpenFoodNetwork::EnterpriseFeeApplicator.new(fee, line_item.variant, role)
+      }
+
+      it "updates the line item fee" do
+        allow(calculator).to receive(
+          :per_item_enterprise_fee_applicators_for
+        ).and_return([fee_applicator])
+        adjustment = fee.create_adjustment('foo', line_item, true)
+
+        expect do
+          service.create_or_update_line_item_fees!
+        end.to change { adjustment.reload.updated_at }
+      end
+
+      context "when the variant has been removed from the order cycle" do
+        it "updates the line item fee" do
+          allow(calculator).to receive(
+            :per_item_enterprise_fee_applicators_for
+          ).and_return([])
+          adjustment = fee.create_adjustment('foo', line_item, true)
+
+          expect do
+            service.create_or_update_line_item_fees!
+          end.to change { adjustment.reload.updated_at }
+        end
+      end
+
+      context "when enterprise fee is removed from the order cycle" do
+        it "removes the line item fee" do
+          adjustment = fee.create_adjustment('foo', line_item, true)
+          order_cycle.exchanges.first.enterprise_fees.destroy(fee)
+          allow(calculator).to receive(
+            :per_item_enterprise_fee_applicators_for
+          ).and_return([])
+
+          expect do
+            service.create_or_update_line_item_fees!
+          end.to change { line_item.adjustments.reload.enterprise_fee.count }.by(-1)
+        end
+
+        context "with coordinator fee" do
+          it "removes the coordinator fee" do
+            coordinator_fee = order_cycle.coordinator_fees.per_item.first
+            adjustment = coordinator_fee.create_adjustment('foo', line_item, true)
+            order_cycle.coordinator_fees.destroy(coordinator_fee)
+            allow(calculator).to receive(
+              :per_item_enterprise_fee_applicators_for
+            ).and_return([])
+
+            expect do
+              service.create_or_update_line_item_fees!
+            end.to change { line_item.adjustments.reload.enterprise_fee.count }.by(-1)
+          end
+        end
+      end
+
+      context "when an enterprise fee is deleted" do
+        before do
+          fee.create_adjustment('foo', line_item, true)
+          allow(calculator).to receive(
+            :per_item_enterprise_fee_applicators_for
+          ).and_return([])
+        end
+
+        context "soft delete" do
+          it "deletes the line item fee" do
+            fee.destroy
+
+            expect do
+              service.create_or_update_line_item_fees!
+            end.to change { line_item.adjustments.enterprise_fee.count }.by(-1)
+          end
+        end
+
+        context "hard delete" do
+          it "deletes the line item fee" do
+            fee.really_destroy!
+
+            expect do
+              service.create_or_update_line_item_fees!
+            end.to change { line_item.adjustments.enterprise_fee.count }.by(-1)
+          end
+        end
+      end
+
+      context "with a new enterprise fee added to the order cylce" do
+        let(:new_fee) { create(:enterprise_fee, enterprise: fee.enterprise) }
+        let(:fee_applicator2) {
+          OpenFoodNetwork::EnterpriseFeeApplicator.new(new_fee, line_item.variant, role)
+        }
+        let!(:adjustment) { fee.create_adjustment('foo', line_item, true) }
+
+        before do
+          allow(service).to receive(:provided_by_order_cycle?) { true }
+          # add the new fee to the order cycle
+          order_cycle.cached_outgoing_exchanges.first.enterprise_fees << new_fee
+        end
+
+        it "creates a line item fee for the new fee" do
+          allow(calculator).to receive(
+            :per_item_enterprise_fee_applicators_for
+          ).and_return([fee_applicator, fee_applicator2])
+
+          expect(fee_applicator2).to receive(:create_line_item_adjustment).with(line_item)
+
+          service.create_or_update_line_item_fees!
+        end
+
+        it "updates existing line item fee" do
+          allow(calculator).to receive(
+            :per_item_enterprise_fee_applicators_for
+          ).and_return([fee_applicator, fee_applicator2])
+
+          expect do
+            service.create_or_update_line_item_fees!
+          end.to change { adjustment.reload.updated_at }
+        end
+
+        context "with variant not included in the order cycle" do
+          it "doesn't create a new line item fee" do
+            allow(service).to receive(:provided_by_order_cycle?) { false }
+            allow(calculator).to receive(
+              :per_item_enterprise_fee_applicators_for
+            ).and_return([])
+
+            expect(fee_applicator2).not_to receive(:create_line_item_adjustment).with(line_item)
+
+            service.create_or_update_line_item_fees!
+          end
+        end
+      end
     end
   end