From 0f1b81cc3ecae7237940462ffb834442f1825625 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 10 Feb 2025 16:10:13 +1100
Subject: [PATCH 01/11] Refactor spec

---
 spec/models/enterprise_fee_spec.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb
index 769ab93fb0..8db2a94ec3 100644
--- a/spec/models/enterprise_fee_spec.rb
+++ b/spec/models/enterprise_fee_spec.rb
@@ -142,10 +142,11 @@ RSpec.describe EnterpriseFee do
     end
   end
 
-  describe "clearing all enterprise fee adjustments on an order" do
+  describe ".clear_all_adjustments" do
+    let(:order_cycle) { create(:order_cycle) }
+    let(:order) { create(:order, order_cycle:) }
+
     it "clears adjustments from many fees and on all line items" do
-      order_cycle = create(:order_cycle)
-      order = create(:order, order_cycle:)
       line_item1 = create(:line_item, order:, variant: order_cycle.variants.first)
       line_item2 = create(:line_item, order:, variant: order_cycle.variants.second)
 
@@ -160,7 +161,6 @@ RSpec.describe EnterpriseFee do
     end
 
     it "clears adjustments from per-order fees" do
-      order = create(:order)
       enterprise_fee = create(:enterprise_fee)
       enterprise_fee_aplicator = OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil,
                                                                               'coordinator')
@@ -172,7 +172,6 @@ RSpec.describe EnterpriseFee do
     end
 
     it "does not clear adjustments from another originator" do
-      order = create(:order)
       tax_rate = create(:tax_rate, calculator: build(:calculator))
       order.adjustments.create({ amount: 12.34,
                                  originator: tax_rate,

From e87477b6798cfe1f4490be3ee25a231f792e91fb Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 10 Feb 2025 16:28:19 +1100
Subject: [PATCH 02/11] Refactor EnterpriseFee.clear_all_adjustments

Renamed to clear_order_adjustments, it doesn't clear line item
adjustment
---
 app/models/enterprise_fee.rb       |  4 ++--
 spec/models/enterprise_fee_spec.rb | 37 ++++++++++++++++--------------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb
index 07eab69415..64f1c84bca 100644
--- a/app/models/enterprise_fee.rb
+++ b/app/models/enterprise_fee.rb
@@ -42,8 +42,8 @@ class EnterpriseFee < ApplicationRecord
     joins(:calculator).where(spree_calculators: { type: PER_ORDER_CALCULATORS })
   }
 
-  def self.clear_all_adjustments(order)
-    order.all_adjustments.enterprise_fee.destroy_all
+  def self.clear_order_adjustments(order)
+    order.all_adjustments.enterprise_fee.where.not(adjustable_type: "Spree::LineItem").destroy_all
   end
 
   private
diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb
index 8db2a94ec3..91babfd966 100644
--- a/spec/models/enterprise_fee_spec.rb
+++ b/spec/models/enterprise_fee_spec.rb
@@ -142,24 +142,10 @@ RSpec.describe EnterpriseFee do
     end
   end
 
-  describe ".clear_all_adjustments" do
+  describe ".clear_order_adjustments" do
     let(:order_cycle) { create(:order_cycle) }
     let(:order) { create(:order, order_cycle:) }
 
-    it "clears adjustments from many fees and on all line items" do
-      line_item1 = create(:line_item, order:, variant: order_cycle.variants.first)
-      line_item2 = create(:line_item, order:, variant: order_cycle.variants.second)
-
-      order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true)
-      order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true)
-      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true)
-      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true)
-
-      expect do
-        EnterpriseFee.clear_all_adjustments order
-      end.to change { order.all_adjustments.count }.by(-4)
-    end
-
     it "clears adjustments from per-order fees" do
       enterprise_fee = create(:enterprise_fee)
       enterprise_fee_aplicator = OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil,
@@ -167,7 +153,7 @@ RSpec.describe EnterpriseFee do
       enterprise_fee_aplicator.create_order_adjustment(order)
 
       expect do
-        EnterpriseFee.clear_all_adjustments order
+        described_class.clear_order_adjustments order
       end.to change { order.adjustments.count }.by(-1)
     end
 
@@ -179,9 +165,26 @@ RSpec.describe EnterpriseFee do
                                  label: 'hello' })
 
       expect do
-        EnterpriseFee.clear_all_adjustments order
+        described_class.clear_order_adjustments order
       end.to change { order.adjustments.count }.by(0)
     end
+
+    it "doesn't clear adjustments from many fees and on all line items" do
+      line_item1 = create(:line_item, order:, variant: order_cycle.variants.first)
+      line_item2 = create(:line_item, order:, variant: order_cycle.variants.second)
+
+      # Order adjustment
+      order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true)
+      order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true)
+      # Line item adjustment
+      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true)
+      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true)
+
+      # does not clear line item adjustments
+      expect do
+        described_class.clear_order_adjustments order
+      end.to change { order.all_adjustments.count }.by(-2)
+    end
   end
 
   describe "soft-deletion" do

From c6fab57827923cdaed6280199b2c7bf3b60522c4 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 19 Feb 2025 13:57:38 +1100
Subject: [PATCH 03/11] Remove Module from spec and some stylying

Also remove unnecessary use of `__send__`
---
 .../enterprise_fee_calculator_spec.rb         | 616 +++++++++---------
 1 file changed, 307 insertions(+), 309 deletions(-)

diff --git a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
index 58da6263ed..61ff704128 100644
--- a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
+++ b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
@@ -3,374 +3,372 @@
 require 'spec_helper'
 require 'open_food_network/enterprise_fee_calculator'
 
-module OpenFoodNetwork
-  RSpec.describe EnterpriseFeeCalculator do
-    describe "integration" do
-      let(:supplier1)    { create(:supplier_enterprise) }
-      let(:supplier2)    { create(:supplier_enterprise) }
-      let(:coordinator) { create(:distributor_enterprise) }
-      let(:distributor) { create(:distributor_enterprise) }
-      let(:order_cycle) { create(:simple_order_cycle) }
-      let(:product1) { create(:simple_product, supplier_id: supplier1.id, price: 10.00) }
-      let(:product2) { create(:simple_product, supplier_id: supplier2.id, price: 20.00) }
+RSpec.describe OpenFoodNetwork::EnterpriseFeeCalculator do
+  describe "integration" do
+    let(:supplier1)    { create(:supplier_enterprise) }
+    let(:supplier2)    { create(:supplier_enterprise) }
+    let(:coordinator) { create(:distributor_enterprise) }
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:order_cycle) { create(:simple_order_cycle) }
+    let(:product1) { create(:simple_product, supplier_id: supplier1.id, price: 10.00) }
+    let(:product2) { create(:simple_product, supplier_id: supplier2.id, price: 20.00) }
 
-      describe "calculating fees for a variant" do
-        describe "summing all the per-item fees for variant in the specified hub + order cycle" do
-          let(:enterprise_fee1) { create(:enterprise_fee, amount: 20) }
-          let(:enterprise_fee2) { create(:enterprise_fee, amount: 3) }
-          let(:enterprise_fee3) {
-            create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2))
+    describe "calculating fees for a variant" do
+      describe "summing all the per-item fees for variant in the specified hub + order cycle" do
+        let(:enterprise_fee1) { create(:enterprise_fee, amount: 20) }
+        let(:enterprise_fee2) { create(:enterprise_fee, amount: 3) }
+        let(:enterprise_fee3) {
+          create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2))
+        }
+
+        describe "supplier fees" do
+          let!(:exchange1) {
+            create(:exchange, order_cycle:, sender: supplier1, receiver: coordinator,
+                              incoming: true, enterprise_fees: [enterprise_fee1],
+                              variants: [product1.variants.first])
+          }
+          let!(:exchange2) {
+            create(:exchange, order_cycle:, sender: supplier2, receiver: coordinator,
+                              incoming: true, enterprise_fees: [enterprise_fee2],
+                              variants: [product2.variants.first])
           }
 
-          describe "supplier fees" do
-            let!(:exchange1) {
-              create(:exchange, order_cycle:, sender: supplier1, receiver: coordinator,
-                                incoming: true, enterprise_fees: [enterprise_fee1],
-                                variants: [product1.variants.first])
-            }
-            let!(:exchange2) {
-              create(:exchange, order_cycle:, sender: supplier2, receiver: coordinator,
-                                incoming: true, enterprise_fees: [enterprise_fee2],
-                                variants: [product2.variants.first])
-            }
-
-            it "calculates via regular computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .fees_for(product1.variants.first)).to eq(20)
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .fees_for(product2.variants.first)).to eq(3)
-            end
-
-            it "calculates via indexed computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .indexed_fees_for(product1.variants.first)).to eq(20)
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .indexed_fees_for(product2.variants.first)).to eq(3)
-            end
+          it "calculates via regular computation" do
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .fees_for(product1.variants.first)).to eq(20)
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .fees_for(product2.variants.first)).to eq(3)
           end
 
-          describe "coordinator fees" do
-            let!(:exchange) {
-              create(:exchange, order_cycle:, sender: coordinator,
-                                receiver: distributor, incoming: false, enterprise_fees: [],
-                                variants: [product1.variants.first])
-            }
-
-            before do
-              order_cycle.coordinator_fees = [enterprise_fee1, enterprise_fee2, enterprise_fee3]
-            end
-
-            it "sums via regular computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .fees_for(product1.variants.first)).to eq(23)
-            end
-
-            it "sums via indexed computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .indexed_fees_for(product1.variants.first)).to eq(23)
-            end
-          end
-
-          describe "distributor fees" do
-            let!(:exchange) {
-              create(:exchange, order_cycle:, sender: coordinator,
-                                receiver: distributor, incoming: false,
-                                enterprise_fees: [enterprise_fee1,
-                                                  enterprise_fee2,
-                                                  enterprise_fee3],
-                                variants: [product1.variants.first])
-            }
-
-            it "sums via regular computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .fees_for(product1.variants.first)).to eq(23)
-            end
-
-            it "sums via indexed computation" do
-              expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-                .indexed_fees_for(product1.variants.first)).to eq(23)
-            end
+          it "calculates via indexed computation" do
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .indexed_fees_for(product1.variants.first)).to eq(20)
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .indexed_fees_for(product2.variants.first)).to eq(3)
           end
         end
 
-        describe "summing percentage fees for the variant" do
-          let!(:enterprise_fee1) {
-            create(
-              :enterprise_fee,
-              amount: 20,
-              fee_type: "admin",
-              calculator: ::Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20)
-            )
-          }
+        describe "coordinator fees" do
           let!(:exchange) {
-            create(:exchange, order_cycle:, sender: coordinator, receiver: distributor,
-                              incoming: false, enterprise_fees: [enterprise_fee1],
+            create(:exchange, order_cycle:, sender: coordinator,
+                              receiver: distributor, incoming: false, enterprise_fees: [],
+                              variants: [product1.variants.first])
+          }
+
+          before do
+            order_cycle.coordinator_fees = [enterprise_fee1, enterprise_fee2, enterprise_fee3]
+          end
+
+          it "sums via regular computation" do
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .fees_for(product1.variants.first)).to eq(23)
+          end
+
+          it "sums via indexed computation" do
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .indexed_fees_for(product1.variants.first)).to eq(23)
+          end
+        end
+
+        describe "distributor fees" do
+          let!(:exchange) {
+            create(:exchange, order_cycle:, sender: coordinator,
+                              receiver: distributor, incoming: false,
+                              enterprise_fees: [enterprise_fee1,
+                                                enterprise_fee2,
+                                                enterprise_fee3],
                               variants: [product1.variants.first])
           }
 
           it "sums via regular computation" do
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .fees_for(product1.variants.first)).to eq(2.00)
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .fees_for(product1.variants.first)).to eq(23)
           end
 
           it "sums via indexed computation" do
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .indexed_fees_for(product1.variants.first)).to eq(2.00)
+            expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+              .indexed_fees_for(product1.variants.first)).to eq(23)
           end
         end
       end
 
-      describe "calculating fees by type" do
-        let!(:ef_admin) { create(:enterprise_fee, fee_type: 'admin', amount: 1.23, name: "Admin") }
-        let!(:ef_sales) { create(:enterprise_fee, fee_type: 'sales', amount: 4.56, name: "Sales") }
-        let!(:ef_packing) {
-          create(:enterprise_fee, fee_type: 'packing', amount: 7.89, name: "Packing")
-        }
-        let!(:ef_transport) {
-          create(:enterprise_fee, fee_type: 'transport', amount: 0.12, name: "Transport")
-        }
-        let!(:ef_fundraising) {
-          create(:enterprise_fee, fee_type: 'fundraising', amount: 3.45, name: "Fundraising")
-        }
-        let!(:exchange) {
-          create(:exchange, order_cycle:,
-                            sender: coordinator, receiver: distributor, incoming: false,
-                            enterprise_fees: [
-                              ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising
-                            ],
-                            variants: [product1.variants.first])
-        }
-
-        describe "regular computation" do
-          it "returns the fees names" do
-            expect(EnterpriseFeeCalculator
-              .new(distributor, order_cycle).fees_name_by_type_for(product1.variants.first))
-              .to eq({ admin: "Admin", fundraising: "Fundraising", packing: "Packing",
-                       sales: "Sales", transport: "Transport" })
-          end
-
-          it "returns a breakdown of fees" do
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .fees_by_type_for(product1.variants.first))
-              .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
-          end
-
-          it "filters out zero fees" do
-            ef_admin.calculator.update_attribute :preferred_amount, 0
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .fees_by_type_for(product1.variants.first))
-              .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
-          end
-        end
-
-        describe "indexed computation" do
-          it "returns a breakdown of fees" do
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .indexed_fees_by_type_for(product1.variants.first))
-              .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
-          end
-
-          it "filters out zero fees" do
-            ef_admin.calculator.update_attribute :preferred_amount, 0
-            expect(EnterpriseFeeCalculator.new(distributor, order_cycle)
-              .indexed_fees_by_type_for(product1.variants.first))
-              .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
-          end
-        end
-      end
-
-      describe "creating adjustments" do
-        let(:order) { create(:order, distributor:, order_cycle:) }
-        let!(:line_item) { create(:line_item, order:, variant: product1.variants.first) }
-        let(:enterprise_fee_line_item) { create(:enterprise_fee) }
-        let(:enterprise_fee_order) {
-          create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2))
+      describe "summing percentage fees for the variant" do
+        let!(:enterprise_fee1) {
+          create(
+            :enterprise_fee,
+            amount: 20,
+            fee_type: "admin",
+            calculator: Calculator::FlatPercentPerItem.new(preferred_flat_percent: 20)
+          )
         }
         let!(:exchange) {
           create(:exchange, order_cycle:, sender: coordinator, receiver: distributor,
-                            incoming: false, variants: [product1.variants.first])
+                            incoming: false, enterprise_fees: [enterprise_fee1],
+                            variants: [product1.variants.first])
         }
 
-        before { order.reload }
-
-        it "creates adjustments for a line item" do
-          exchange.enterprise_fees << enterprise_fee_line_item
-
-          EnterpriseFeeCalculator.new(distributor,
-                                      order_cycle).create_line_item_adjustments_for line_item
-
-          a = Spree::Adjustment.last
-          expect(a.metadata.fee_name).to eq(enterprise_fee_line_item.name)
+        it "sums via regular computation" do
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .fees_for(product1.variants.first)).to eq(2.00)
         end
 
-        it "creates adjustments for an order" do
-          exchange.enterprise_fees << enterprise_fee_order
-
-          EnterpriseFeeCalculator.new(distributor, order_cycle).create_order_adjustments_for order
-
-          a = Spree::Adjustment.last
-          expect(a.metadata.fee_name).to eq(enterprise_fee_order.name)
+        it "sums via indexed computation" do
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .indexed_fees_for(product1.variants.first)).to eq(2.00)
         end
       end
     end
 
-    describe "indexed fee retrieval" do
-      subject { EnterpriseFeeCalculator.new distributor, order_cycle }
-      let(:order_cycle) { create(:simple_order_cycle, coordinator_fees: [ef_coordinator]) }
-      let(:distributor) { create(:distributor_enterprise) }
-      let(:distributor_other) { create(:distributor_enterprise) }
-      let!(:ef_absent) { create(:enterprise_fee) }
-      let!(:ef_exchange) { create(:enterprise_fee) }
-      let!(:ef_coordinator) { create(:enterprise_fee) }
-      let!(:ef_other_distributor) { create(:enterprise_fee) }
-      let!(:exchange) {
-        create(:exchange, sender: order_cycle.coordinator, receiver: distributor,
-                          order_cycle:, enterprise_fees: [ef_exchange], variants: [v])
+    describe "calculating fees by type" do
+      let!(:ef_admin) { create(:enterprise_fee, fee_type: 'admin', amount: 1.23, name: "Admin") }
+      let!(:ef_sales) { create(:enterprise_fee, fee_type: 'sales', amount: 4.56, name: "Sales") }
+      let!(:ef_packing) {
+        create(:enterprise_fee, fee_type: 'packing', amount: 7.89, name: "Packing")
+      }
+      let!(:ef_transport) {
+        create(:enterprise_fee, fee_type: 'transport', amount: 0.12, name: "Transport")
+      }
+      let!(:ef_fundraising) {
+        create(:enterprise_fee, fee_type: 'fundraising', amount: 3.45, name: "Fundraising")
+      }
+      let!(:exchange) {
+        create(:exchange, order_cycle:,
+                          sender: coordinator, receiver: distributor, incoming: false,
+                          enterprise_fees: [
+                            ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising
+                          ],
+                          variants: [product1.variants.first])
       }
-      let(:v) { create(:variant) }
-      let(:indexed_variants) { { v.id => v } }
-      let(:indexed_enterprise_fees) { subject.instance_variable_get(:@indexed_enterprise_fees) }
 
-      before { subject.instance_variable_set(:@indexed_enterprise_fees, {}) }
-
-      describe "fetching enterprise fees with pre-loaded exchange details" do
-        it "scopes enterprise fees to those on exchanges for the current order cycle" do
-          expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq(
-            [ef_exchange]
-          )
+      describe "regular computation" do
+        it "returns the fees names" do
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .fees_name_by_type_for(product1.variants.first))
+            .to eq({ admin: "Admin", fundraising: "Fundraising", packing: "Packing",
+                     sales: "Sales", transport: "Transport" })
         end
 
-        it "includes the exchange variant id" do
-          expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)
-            .first.variant_id.to_i).to eq(v.id)
+        it "returns a breakdown of fees" do
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .fees_by_type_for(product1.variants.first))
+            .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
         end
 
-        it "does not include outgoing exchanges to other distributors" do
-          create(:exchange, order_cycle:, sender: order_cycle.coordinator,
-                            receiver: distributor_other, enterprise_fees: [ef_other_distributor],
-                            variants: [v])
-
-          expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq(
-            [ef_exchange]
-          )
+        it "filters out zero fees" do
+          ef_admin.calculator.update_attribute :preferred_amount, 0
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .fees_by_type_for(product1.variants.first))
+            .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
         end
       end
 
-      describe "loading exchange fees" do
-        let(:exchange_fees) { subject.__send__(:per_item_enterprise_fees_with_exchange_details) }
-
-        it "loads exchange fees" do
-          subject.__send__(:load_exchange_fees, exchange_fees)
-          expect(indexed_enterprise_fees).to eq(v.id => [ef_exchange])
+      describe "indexed computation" do
+        it "returns a breakdown of fees" do
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .indexed_fees_by_type_for(product1.variants.first))
+            .to eq(admin: 1.23, sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
         end
-      end
 
-      describe "loading coordinator fees" do
-        it "loads coordinator fees" do
-          subject.__send__(:load_coordinator_fees)
-          expect(indexed_enterprise_fees).to eq(v.id => [ef_coordinator])
+        it "filters out zero fees" do
+          ef_admin.calculator.update_attribute :preferred_amount, 0
+          expect(OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+            .indexed_fees_by_type_for(product1.variants.first))
+            .to eq(sales: 4.56, packing: 7.89, transport: 0.12, fundraising: 3.45)
         end
       end
     end
 
     describe "creating adjustments" do
-      let(:oc) { OrderCycle.new }
-      let(:distributor) { double(:distributor) }
-      let(:ef1) { double(:enterprise_fee) }
-      let(:ef2) { double(:enterprise_fee) }
-      let(:ef3) { double(:enterprise_fee) }
-      let(:incoming_exchange) { double(:exchange, role: 'supplier') }
-      let(:outgoing_exchange) { double(:exchange, role: 'distributor') }
-      let(:applicator) { double(:enterprise_fee_applicator) }
+      let(:order) { create(:order, distributor:, order_cycle:) }
+      let!(:line_item) { create(:line_item, order:, variant: product1.variants.first) }
+      let(:enterprise_fee_line_item) { create(:enterprise_fee) }
+      let(:enterprise_fee_order) {
+        create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2))
+      }
+      let!(:exchange) {
+        create(:exchange, order_cycle:, sender: coordinator, receiver: distributor,
+                          incoming: false, variants: [product1.variants.first])
+      }
 
-      describe "for a line item" do
-        let(:variant) { double(:variant) }
-        let(:line_item) { double(:line_item, variant:, order:) }
+      before { order.reload }
 
-        before do
-          allow(incoming_exchange).to receive(:enterprise_fees) {
-                                        double(:enterprise_fees, per_item: [ef1])
-                                      }
-          allow(outgoing_exchange).to receive(:enterprise_fees) {
-                                        double(:enterprise_fees, per_item: [ef2])
-                                      }
-          allow(oc).to receive(:exchanges_carrying) { [incoming_exchange, outgoing_exchange] }
-          allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_item: [ef3]) }
+      it "creates adjustments for a line item" do
+        exchange.enterprise_fees << enterprise_fee_line_item
+
+        OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle)
+          .create_line_item_adjustments_for line_item
+
+        a = Spree::Adjustment.last
+        expect(a.metadata.fee_name).to eq(enterprise_fee_line_item.name)
+      end
+
+      it "creates adjustments for an order" do
+        exchange.enterprise_fees << enterprise_fee_order
+
+        OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor,
+                                                     order_cycle).create_order_adjustments_for order
+
+        a = Spree::Adjustment.last
+        expect(a.metadata.fee_name).to eq(enterprise_fee_order.name)
+      end
+    end
+  end
+
+  describe "indexed fee retrieval" do
+    subject { OpenFoodNetwork::EnterpriseFeeCalculator.new distributor, order_cycle }
+    let(:order_cycle) { create(:simple_order_cycle, coordinator_fees: [ef_coordinator]) }
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:distributor_other) { create(:distributor_enterprise) }
+    let!(:ef_absent) { create(:enterprise_fee) }
+    let!(:ef_exchange) { create(:enterprise_fee) }
+    let!(:ef_coordinator) { create(:enterprise_fee) }
+    let!(:ef_other_distributor) { create(:enterprise_fee) }
+    let!(:exchange) {
+      create(:exchange, sender: order_cycle.coordinator, receiver: distributor,
+                        order_cycle:, enterprise_fees: [ef_exchange], variants: [v])
+    }
+    let(:v) { create(:variant) }
+    let(:indexed_variants) { { v.id => v } }
+    let(:indexed_enterprise_fees) { subject.instance_variable_get(:@indexed_enterprise_fees) }
+
+    before { subject.instance_variable_set(:@indexed_enterprise_fees, {}) }
+
+    describe "fetching enterprise fees with pre-loaded exchange details" do
+      it "scopes enterprise fees to those on exchanges for the current order cycle" do
+        expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq(
+          [ef_exchange]
+        )
+      end
+
+      it "includes the exchange variant id" do
+        expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)
+          .first.variant_id.to_i).to eq(v.id)
+      end
+
+      it "does not include outgoing exchanges to other distributors" do
+        create(:exchange, order_cycle:, sender: order_cycle.coordinator,
+                          receiver: distributor_other, enterprise_fees: [ef_other_distributor],
+                          variants: [v])
+
+        expect(subject.__send__(:per_item_enterprise_fees_with_exchange_details)).to eq(
+          [ef_exchange]
+        )
+      end
+    end
+
+    describe "loading exchange fees" do
+      let(:exchange_fees) { subject.__send__(:per_item_enterprise_fees_with_exchange_details) }
+
+      it "loads exchange fees" do
+        subject.__send__(:load_exchange_fees, exchange_fees)
+        expect(indexed_enterprise_fees).to eq(v.id => [ef_exchange])
+      end
+    end
+
+    describe "loading coordinator fees" do
+      it "loads coordinator fees" do
+        subject.__send__(:load_coordinator_fees)
+        expect(indexed_enterprise_fees).to eq(v.id => [ef_coordinator])
+      end
+    end
+  end
+
+  describe "creating adjustments" do
+    let(:oc) { OrderCycle.new }
+    let(:distributor) { double(:distributor) }
+    let(:ef1) { double(:enterprise_fee) }
+    let(:ef2) { double(:enterprise_fee) }
+    let(:ef3) { double(:enterprise_fee) }
+    let(:incoming_exchange) { double(:exchange, role: 'supplier') }
+    let(:outgoing_exchange) { double(:exchange, role: 'distributor') }
+    let(:applicator) { double(:enterprise_fee_applicator) }
+
+    describe "for a line item" do
+      let(:variant) { double(:variant) }
+      let(:line_item) { double(:line_item, variant:, order:) }
+
+      before do
+        allow(incoming_exchange).to receive(:enterprise_fees) {
+                                      double(:enterprise_fees, per_item: [ef1])
+                                    }
+        allow(outgoing_exchange).to receive(:enterprise_fees) {
+                                      double(:enterprise_fees, per_item: [ef2])
+                                    }
+        allow(oc).to receive(:exchanges_carrying) { [incoming_exchange, outgoing_exchange] }
+        allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_item: [ef3]) }
+      end
+
+      context "with order_cycle and distributor set" do
+        let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, oc) }
+        let(:order) { double(:order, distributor:, order_cycle: oc) }
+
+        it "creates an adjustment for each fee" do
+          expect(efc).to receive(:per_item_enterprise_fee_applicators_for).with(variant) {
+                           [applicator]
+                         }
+          expect(applicator).to receive(:create_line_item_adjustment).with(line_item)
+          efc.create_line_item_adjustments_for line_item
         end
 
-        context "with order_cycle and distributor set" do
-          let(:efc) { EnterpriseFeeCalculator.new(distributor, oc) }
-          let(:order) { double(:order, distributor:, order_cycle: oc) }
-
-          it "creates an adjustment for each fee" do
-            expect(efc).to receive(:per_item_enterprise_fee_applicators_for).with(variant) {
-                             [applicator]
-                           }
-            expect(applicator).to receive(:create_line_item_adjustment).with(line_item)
-            efc.create_line_item_adjustments_for line_item
-          end
-
-          it "makes fee applicators for a line item" do
-            expect(efc.__send__(:per_item_enterprise_fee_applicators_for, line_item.variant))
-              .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, line_item.variant,
-                                                                   'supplier'),
-                      OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, line_item.variant,
-                                                                   'distributor'),
-                      OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, line_item.variant,
-                                                                   'coordinator')]
-          end
-        end
-
-        context "with no order_cycle or distributor set" do
-          let(:efc) { EnterpriseFeeCalculator.new }
-          let(:order) { double(:order, distributor: nil, order_cycle: nil) }
-
-          it "does not make applicators for an order" do
-            expect(efc.__send__(:per_item_enterprise_fee_applicators_for,
-                                line_item.variant)).to eq []
-          end
+        it "makes fee applicators for a line item" do
+          expect(efc.per_item_enterprise_fee_applicators_for(line_item.variant))
+            .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, line_item.variant,
+                                                                 'supplier'),
+                    OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, line_item.variant,
+                                                                 'distributor'),
+                    OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, line_item.variant,
+                                                                 'coordinator')]
         end
       end
 
-      describe "for an order" do
-        before do
-          allow(incoming_exchange).to receive(:enterprise_fees) {
-                                        double(:enterprise_fees, per_order: [ef1])
-                                      }
-          allow(outgoing_exchange).to receive(:enterprise_fees) {
-                                        double(:enterprise_fees, per_order: [ef2])
-                                      }
-          allow(oc).to receive(:exchanges_supplying) { [incoming_exchange, outgoing_exchange] }
-          allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_order: [ef3]) }
+      context "with no order_cycle or distributor set" do
+        let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new }
+        let(:order) { double(:order, distributor: nil, order_cycle: nil) }
+
+        it "does not make applicators for an order" do
+          expect(efc.per_item_enterprise_fee_applicators_for(line_item.variant)).to eq []
+        end
+      end
+    end
+
+    describe "for an order" do
+      before do
+        allow(incoming_exchange).to receive(:enterprise_fees) {
+                                      double(:enterprise_fees, per_order: [ef1])
+                                    }
+        allow(outgoing_exchange).to receive(:enterprise_fees) {
+                                      double(:enterprise_fees, per_order: [ef2])
+                                    }
+        allow(oc).to receive(:exchanges_supplying) { [incoming_exchange, outgoing_exchange] }
+        allow(oc).to receive(:coordinator_fees) { double(:coodinator_fees, per_order: [ef3]) }
+      end
+
+      context "with order_cycle and distributor set" do
+        let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, oc) }
+        let(:order) { double(:order, distributor:, order_cycle: oc) }
+
+        it "creates an adjustment for each fee" do
+          expect(efc).to receive(:per_order_enterprise_fee_applicators_for).with(order) {
+                           [applicator]
+                         }
+          expect(applicator).to receive(:create_order_adjustment).with(order)
+          efc.create_order_adjustments_for order
         end
 
-        context "with order_cycle and distributor set" do
-          let(:efc) { EnterpriseFeeCalculator.new(distributor, oc) }
-          let(:order) { double(:order, distributor:, order_cycle: oc) }
-
-          it "creates an adjustment for each fee" do
-            expect(efc).to receive(:per_order_enterprise_fee_applicators_for).with(order) {
-                             [applicator]
-                           }
-            expect(applicator).to receive(:create_order_adjustment).with(order)
-            efc.create_order_adjustments_for order
-          end
-
-          it "makes fee applicators for an order" do
-            expect(efc.__send__(:per_order_enterprise_fee_applicators_for, order))
-              .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, nil, 'supplier'),
-                      OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, nil, 'distributor'),
-                      OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, nil, 'coordinator')]
-          end
+        it "makes fee applicators for an order" do
+          expect(efc.per_order_enterprise_fee_applicators_for(order))
+            .to eq [OpenFoodNetwork::EnterpriseFeeApplicator.new(ef1, nil, 'supplier'),
+                    OpenFoodNetwork::EnterpriseFeeApplicator.new(ef2, nil, 'distributor'),
+                    OpenFoodNetwork::EnterpriseFeeApplicator.new(ef3, nil, 'coordinator')]
         end
+      end
 
-        context "with no order_cycle or distributor set" do
-          let(:efc) { EnterpriseFeeCalculator.new }
-          let(:order) { double(:order, distributor: nil, order_cycle: nil) }
+      context "with no order_cycle or distributor set" do
+        let(:efc) { OpenFoodNetwork::EnterpriseFeeCalculator.new }
+        let(:order) { double(:order, distributor: nil, order_cycle: nil) }
 
-          it "does not make applicators for an order" do
-            expect(efc.__send__(:per_order_enterprise_fee_applicators_for, order)).to eq []
-          end
+        it "does not make applicators for an order" do
+          expect(efc.per_order_enterprise_fee_applicators_for(order)).to eq []
         end
       end
     end

From 2fc393037a734db515e1e19f5e94d486cf45b8ab Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 19 Feb 2025 15:05:57 +1100
Subject: [PATCH 04/11] Add order_cycle_per_item_enterprise_fee_applicators_for

It retrieves all the per item fees associated with an order cycle and
create the appropriate Fee Applicator.
---
 .../enterprise_fee_calculator.rb              | 19 +++++++++
 .../enterprise_fee_calculator_spec.rb         | 40 +++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/lib/open_food_network/enterprise_fee_calculator.rb b/lib/open_food_network/enterprise_fee_calculator.rb
index 739725ce79..a4c0fb631a 100644
--- a/lib/open_food_network/enterprise_fee_calculator.rb
+++ b/lib/open_food_network/enterprise_fee_calculator.rb
@@ -97,6 +97,25 @@ module OpenFoodNetwork
       fees
     end
 
+    def order_cycle_per_item_enterprise_fee_applicators_for(variant)
+      fees = []
+
+      return fees unless @order_cycle && @distributor
+
+      @order_cycle.exchanges.supplying_to(@distributor).each do |exchange|
+        exchange.enterprise_fees.per_item.each do |enterprise_fee|
+          fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant,
+                                                               exchange.role)
+        end
+      end
+
+      @order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
+        fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
+      end
+
+      fees
+    end
+
     private
 
     def load_enterprise_fees
diff --git a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
index 61ff704128..2a011f1a66 100644
--- a/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
+++ b/spec/lib/open_food_network/enterprise_fee_calculator_spec.rb
@@ -373,4 +373,44 @@ RSpec.describe OpenFoodNetwork::EnterpriseFeeCalculator do
       end
     end
   end
+
+  describe "#order_cycle_per_item_enterprise_fee_applicators_for" do
+    subject { calculator.order_cycle_per_item_enterprise_fee_applicators_for(variant) }
+
+    let(:calculator) { described_class.new(distributor, order_cycle) }
+    let(:distributor) { create(:distributor_enterprise) }
+    let(:order_cycle) { create(:order_cycle, distributors: [distributor]) }
+    let(:variant) { instance_double(Spree::Variant) }
+
+    context "with no distributor" do
+      let(:order_cycle) { create(:order_cycle) }
+      let(:distributor) { nil }
+
+      it "return an empty array" do
+        expect(subject).to eq([])
+      end
+    end
+
+    context "with no order cycle" do
+      let(:order_cycle) { nil }
+
+      it "return an empty array" do
+        expect(subject).to eq([])
+      end
+    end
+
+    it "returns enterprise fee applicators including per item exchange fees" do
+      applicators = subject
+      exchanges = order_cycle.exchanges.supplying_to(distributor)
+      fees = exchanges.map { |ex| ex.enterprise_fees.per_item }.flatten
+
+      expect(applicators.map(&:enterprise_fee)).to include(*fees)
+    end
+
+    it "returns enterprise fee applicators including per item coordinator fees" do
+      applicators = subject
+
+      expect(applicators.map(&:enterprise_fee)).to include(*order_cycle.coordinator_fees.per_item)
+    end
+  end
 end

From 46315c40457cef52ee3da7af50eb0f9ebb4fa037 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Feb 2025 13:55:42 +1100
Subject: [PATCH 05/11] 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                     |  2 +-
 app/services/orders/handle_fees_service.rb    | 48 +++++++--
 .../orders/handle_fees_service_spec.rb        | 99 +++++++++++++++++--
 3 files changed, 135 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 66bfdb3db3..9fd75f2a8f 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -86,7 +86,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?
diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 09ee04f1a2..ab46d95d16 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/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,15 @@ 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
-        end
+    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
+        next create_line_item_fees!(line_item) if line_item.enterprise_fee_adjustments.blank?
+
+        create_or_update_line_item_fee!(line_item)
+
+        # update any fees removed from the Order Cycle
+        update_removed_fees!(line_item)
       end
     end
 
@@ -66,5 +70,35 @@ module Orders
       @order_cycle_variant_ids ||= order_cycle&.variants&.map(&:id) || []
       @order_cycle_variant_ids.include? line_item.variant_id
     end
+
+    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)
+      fee_applicators(line_item.variant).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
+    end
+
+    def update_removed_fees!(line_item)
+      order_cycle_fees = fee_applicators(line_item.variant).map(&:enterprise_fee)
+      removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)
+      removed_fees.each do |removed_fee|
+        removed_fee.update_adjustment!(line_item, force: true)
+      end
+    end
+
+    def fee_applicators(variant)
+      calculator.order_cycle_per_item_enterprise_fee_applicators_for(variant)
+    end
   end
 end
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index f31ce21ce2..8bcf8d4273 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -9,19 +9,106 @@ RSpec.describe Orders::HandleFeesService do
 
   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 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[0].enterprise_fees.first }
+      let(:role) { order_cycle.exchanges[0].role }
+      let(:fee_applicator) {
+        OpenFoodNetwork::EnterpriseFeeApplicator.new(fee, line_item.variant, role)
+      }
+
+      it "updates the line item fee" do
+        allow(calculator).to receive(
+          :order_cycle_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 enterprise fee is removed from the order cycle" do
+        it "updates 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(
+            :order_cycle_per_item_enterprise_fee_applicators_for
+          ).and_return([])
+
+          expect do
+            service.create_or_update_line_item_fees!
+          end.to change { adjustment.reload.updated_at }
+        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 }
+        end
+
+        it "creates a line item fee for the new fee" do
+          allow(calculator).to receive(
+            :order_cycle_per_item_enterprise_fee_applicators_for
+          ).and_return([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(
+            :order_cycle_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(
+              :order_cycle_per_item_enterprise_fee_applicators_for
+            ).and_return([fee_applicator2])
+
+            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
 

From 12a54dd8f053582c6dd43673499c2ad9b971918a Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Feb 2025 15:31:10 +1100
Subject: [PATCH 06/11] Move #recreate_all_fees! spec to HandleFeesService

Spree::Order just delegate Orders::HandleFeesService so there is no
point testing fees in the order spec
---
 spec/models/spree/order_spec.rb               | 58 -------------------
 .../orders/handle_fees_service_spec.rb        | 42 ++++++++++++++
 2 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 72326e8dfe..b3f26520ef 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -614,64 +614,6 @@ RSpec.describe Spree::Order do
     end
   end
 
-  describe "applying enterprise fees" do
-    subject { create(:order) }
-    let(:fee_handler) { Orders::HandleFeesService.new(subject) }
-
-    before do
-      allow(subject).to receive(:fee_handler) { fee_handler }
-      allow(subject).to receive(:update_order!)
-    end
-
-    it "clears all enterprise fee adjustments on the order" do
-      expect(EnterpriseFee).to receive(:clear_all_adjustments).with(subject)
-      subject.recreate_all_fees!
-    end
-
-    it "creates line item and order fee adjustments via Orders::HandleFeesService" do
-      expect(fee_handler).to receive(:create_line_item_fees!)
-      expect(fee_handler).to receive(:create_order_fees!)
-      subject.recreate_all_fees!
-    end
-
-    it "skips order cycle per-order adjustments for orders that don't have an order cycle" do
-      allow(EnterpriseFee).to receive(:clear_all_adjustments)
-
-      allow(subject).to receive(:order_cycle) { nil }
-
-      subject.recreate_all_fees!
-    end
-
-    it "ensures the correct adjustment(s) are created for order cycles" do
-      allow(EnterpriseFee).to receive(:clear_all_adjustments)
-      line_item = create(:line_item, order: subject)
-      allow(fee_handler).to receive(:provided_by_order_cycle?) { true }
-
-      order_cycle = double(:order_cycle)
-      expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator).
-        to receive(:create_line_item_adjustments_for).
-        with(line_item)
-      allow_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator)
-        .to receive(:create_order_adjustments_for)
-      allow(subject).to receive(:order_cycle) { order_cycle }
-
-      subject.recreate_all_fees!
-    end
-
-    it "ensures the correct per-order adjustment(s) are created for order cycles" do
-      allow(EnterpriseFee).to receive(:clear_all_adjustments)
-
-      order_cycle = double(:order_cycle)
-      expect_any_instance_of(OpenFoodNetwork::EnterpriseFeeCalculator).
-        to receive(:create_order_adjustments_for).
-        with(subject)
-
-      allow(subject).to receive(:order_cycle) { order_cycle }
-
-      subject.recreate_all_fees!
-    end
-  end
-
   describe "getting the admin and handling charge" do
     let(:o) { create(:order) }
     let(:li) { create(:line_item, order: o) }
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index 8bcf8d4273..2121c77314 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -16,6 +16,48 @@ RSpec.describe Orders::HandleFeesService do
     allow(service).to receive(:calculator) { calculator }
   end
 
+  describe "#recreate_all_fees!" do
+    before do
+      allow(order).to receive(:update_order!)
+    end
+
+    it "clears order enterprise fee adjustments on the order" do
+      expect(EnterpriseFee).to receive(:clear_order_adjustments).with(order)
+
+      service.recreate_all_fees!
+    end
+
+    # both create_or_update_line_item_fees! and create_order_fees! are tested below,
+    # so it's enough to check they get called
+    it "creates line item and order fee adjustments" do
+      expect(service).to receive(:create_or_update_line_item_fees!)
+      expect(service).to receive(:create_order_fees!)
+
+      service.recreate_all_fees!
+    end
+
+    it "updates the order" do
+      expect(order).to receive(:update_order!)
+
+      service.recreate_all_fees!
+    end
+
+    it "doesn't create tax adjustment" do
+      expect(service).not_to receive(:tax_enterprise_fees!)
+
+      service.recreate_all_fees!
+    end
+
+    context "when after payment state" do
+      it "creates the tax adjustment for the fees" do
+        expect(service).to receive(:tax_enterprise_fees!)
+
+        order.update(state: "confirmation")
+        service.recreate_all_fees!
+      end
+    end
+  end
+
   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

From 67ad53290846cf58adbb74145b83318148afbdff Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Feb 2025 16:26:57 +1100
Subject: [PATCH 07/11] Handle scenario where the enterprise fee has been
 deleted

---
 app/services/orders/handle_fees_service.rb    |  7 ++++-
 .../orders/handle_fees_service_spec.rb        | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index ab46d95d16..48dacc13f4 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/app/services/orders/handle_fees_service.rb
@@ -92,8 +92,13 @@ module Orders
     def update_removed_fees!(line_item)
       order_cycle_fees = fee_applicators(line_item.variant).map(&:enterprise_fee)
       removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)
+
       removed_fees.each do |removed_fee|
-        removed_fee.update_adjustment!(line_item, force: true)
+        if removed_fee.originator.nil? || removed_fee.originator.deleted?
+          removed_fee.destroy
+        else
+          removed_fee.update_adjustment!(line_item, force: true)
+        end
       end
     end
 
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index 2121c77314..d75bd09fdf 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -107,6 +107,35 @@ RSpec.describe Orders::HandleFeesService do
         end
       end
 
+      context "when an enterprise fee is deleted" do
+        before do
+          fee.create_adjustment('foo', line_item, true)
+          allow(calculator).to receive(
+            :order_cycle_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) {

From 8116ad986e3a9754acc40a367747e60a03b112cd Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 19 Feb 2025 10:03:19 +1100
Subject: [PATCH 08/11] Delete fees when fee are removed from the Order Cycle

This is to be consistent with the current behavior
---
 app/services/orders/handle_fees_service.rb       | 13 +++----------
 spec/services/orders/handle_fees_service_spec.rb |  4 ++--
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 48dacc13f4..aaaa1636a9 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/app/services/orders/handle_fees_service.rb
@@ -33,8 +33,7 @@ module Orders
 
         create_or_update_line_item_fee!(line_item)
 
-        # update any fees removed from the Order Cycle
-        update_removed_fees!(line_item)
+        delete_removed_fees!(line_item)
       end
     end
 
@@ -89,17 +88,11 @@ module Orders
       end
     end
 
-    def update_removed_fees!(line_item)
+    def delete_removed_fees!(line_item)
       order_cycle_fees = fee_applicators(line_item.variant).map(&:enterprise_fee)
       removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)
 
-      removed_fees.each do |removed_fee|
-        if removed_fee.originator.nil? || removed_fee.originator.deleted?
-          removed_fee.destroy
-        else
-          removed_fee.update_adjustment!(line_item, force: true)
-        end
-      end
+      removed_fees.each(&:destroy)
     end
 
     def fee_applicators(variant)
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index d75bd09fdf..3acbae250c 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -94,7 +94,7 @@ RSpec.describe Orders::HandleFeesService do
       end
 
       context "when enterprise fee is removed from the order cycle" do
-        it "updates the line item fee" 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(
@@ -103,7 +103,7 @@ RSpec.describe Orders::HandleFeesService do
 
           expect do
             service.create_or_update_line_item_fees!
-          end.to change { adjustment.reload.updated_at }
+          end.to change { line_item.adjustments.reload.enterprise_fee.count }.by(-1)
         end
       end
 

From 9c45801e379f89097e278ab7063f8ec71024d96d Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 10:31:28 +1100
Subject: [PATCH 09/11] Code formatting

---
 app/services/orders/handle_fees_service.rb         |  5 ++++-
 lib/open_food_network/enterprise_fee_calculator.rb | 14 ++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index aaaa1636a9..20baf0887d 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/app/services/orders/handle_fees_service.rb
@@ -29,7 +29,10 @@ module Orders
     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
-        next create_line_item_fees!(line_item) if line_item.enterprise_fee_adjustments.blank?
+        if line_item.enterprise_fee_adjustments.blank?
+          create_line_item_fees!(line_item)
+          next
+        end
 
         create_or_update_line_item_fee!(line_item)
 
diff --git a/lib/open_food_network/enterprise_fee_calculator.rb b/lib/open_food_network/enterprise_fee_calculator.rb
index a4c0fb631a..2412e82637 100644
--- a/lib/open_food_network/enterprise_fee_calculator.rb
+++ b/lib/open_food_network/enterprise_fee_calculator.rb
@@ -67,13 +67,12 @@ module OpenFoodNetwork
 
       @order_cycle.exchanges_carrying(variant, @distributor).each do |exchange|
         exchange.enterprise_fees.per_item.each do |enterprise_fee|
-          fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant,
-                                                               exchange.role)
+          fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role)
         end
       end
 
       @order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
-        fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
+        fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
       end
 
       fees
@@ -86,12 +85,12 @@ module OpenFoodNetwork
 
       @order_cycle.exchanges_supplying(order).each do |exchange|
         exchange.enterprise_fees.per_order.each do |enterprise_fee|
-          fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role)
+          fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role)
         end
       end
 
       @order_cycle.coordinator_fees.per_order.each do |enterprise_fee|
-        fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator')
+        fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator')
       end
 
       fees
@@ -104,13 +103,12 @@ module OpenFoodNetwork
 
       @order_cycle.exchanges.supplying_to(@distributor).each do |exchange|
         exchange.enterprise_fees.per_item.each do |enterprise_fee|
-          fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant,
-                                                               exchange.role)
+          fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role)
         end
       end
 
       @order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
-        fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
+        fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
       end
 
       fees

From f2c3ab8b03a3155cf737bacad9362c7215faec7f Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 10:37:49 +1100
Subject: [PATCH 10/11] Per review, improve test expectation

---
 spec/models/enterprise_fee_spec.rb | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb
index 91babfd966..fac578eca2 100644
--- a/spec/models/enterprise_fee_spec.rb
+++ b/spec/models/enterprise_fee_spec.rb
@@ -174,16 +174,18 @@ RSpec.describe EnterpriseFee do
       line_item2 = create(:line_item, order:, variant: order_cycle.variants.second)
 
       # Order adjustment
-      order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true)
-      order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true)
+      fee1 = order_cycle.coordinator_fees[0].create_adjustment('foo1', line_item1.order, true)
+      fee2 = order_cycle.coordinator_fees[0].create_adjustment('foo2', line_item2.order, true)
       # Line item adjustment
-      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true)
-      order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true)
+      fee3 = order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo3', line_item1, true)
+      fee4 = order_cycle.exchanges[0].enterprise_fees[0].create_adjustment('foo4', line_item2, true)
 
+      described_class.clear_order_adjustments order
+
+      adjustments = order.all_adjustments
       # does not clear line item adjustments
-      expect do
-        described_class.clear_order_adjustments order
-      end.to change { order.all_adjustments.count }.by(-2)
+      expect(adjustments).not_to include(fee1, fee2)
+      expect(adjustments).to include(fee3, fee4)
     end
   end
 

From 0480ddb59cea253ce11e0324350ff0af11856737 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 10:41:32 +1100
Subject: [PATCH 11/11] Add comment

---
 app/services/orders/handle_fees_service.rb | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 20baf0887d..46d3967566 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/app/services/orders/handle_fees_service.rb
@@ -18,6 +18,9 @@ module Orders
       order.with_lock do
         EnterpriseFee.clear_order_adjustments order
 
+        # To prevent issue with fee being removed when a product is not linked to the order cycle
+        # anymore, we now create or update line item fees.
+        # Previously fees were deleted and recreated, like we still do for order fees.
         create_or_update_line_item_fees!
         create_order_fees!
       end