From 02a9aaf1ecc01aeb6bd107872147b1d11c44202d 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 1/8] 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 2a0caca57094b3666b6d5061db57177f2e30f314 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 2/8] 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 | 39 +++++++++++++++++-------------
 2 files changed, 24 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..fac578eca2 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,28 @@ 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
+      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
+      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(adjustments).not_to include(fee1, fee2)
+      expect(adjustments).to include(fee3, fee4)
+    end
   end
 
   describe "soft-deletion" do

From d83f8ded0daa3ecff544671faaf306b642ce7427 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 3/8] 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 b5bc6b84d7c4480ec019e35514736a1abcb460cf Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 31 Mar 2025 12:55:35 +1100
Subject: [PATCH 4/8] 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    |  69 +++++++-
 .../orders/handle_fees_service_spec.rb        | 163 +++++++++++++++++-
 3 files changed, 220 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 642b57678a..8ad700f3a3 100644
--- a/app/models/spree/order.rb
+++ b/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?
diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 09ee04f1a2..16d0ffd0d1 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,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
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index f31ce21ce2..7018c662c7 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/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
 

From 223faa55698fbd84e202757fab9ea0cc285e2698 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 5/8] Move #recreate_all_fees! spec to HandleFeesService

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

diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 16d0ffd0d1..88f2595d60 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
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 7018c662c7..87663b014d 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -19,6 +19,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 3744ba9198c4ad8cd8bf2a2b42d112cbf25f2d41 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 1 Apr 2025 10:52:34 +1100
Subject: [PATCH 6/8] create_*_adjustment now return the adjustment

---
 lib/open_food_network/enterprise_fee_applicator.rb          | 1 +
 .../lib/open_food_network/enterprise_fee_applicator_spec.rb | 6 ++----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb
index 227947777c..05dbd643fa 100644
--- a/lib/open_food_network/enterprise_fee_applicator.rb
+++ b/lib/open_food_network/enterprise_fee_applicator.rb
@@ -20,6 +20,7 @@ module OpenFoodNetwork
       AdjustmentMetadata.create! adjustment:, enterprise: enterprise_fee.enterprise,
                                  fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type,
                                  enterprise_role: role
+      adjustment
     end
 
     def line_item_adjustment_label
diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb
index b16b11f124..3648ba3d5a 100644
--- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb
+++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb
@@ -19,9 +19,8 @@ module OpenFoodNetwork
     describe "#create_line_item_adjustment" do
       it "creates an adjustment for a line item" do
         allow(applicator).to receive(:line_item_adjustment_label) { 'label' }
-        applicator.create_line_item_adjustment line_item
+        adjustment = applicator.create_line_item_adjustment line_item
 
-        adjustment = Spree::Adjustment.last
         expect(adjustment.label).to eq('label')
         expect(adjustment.adjustable).to eq(line_item)
         expect(adjustment.originator).to eq(enterprise_fee)
@@ -43,9 +42,8 @@ module OpenFoodNetwork
 
       it "creates an adjustment for an order" do
         allow(applicator).to receive(:order_adjustment_label) { 'label' }
-        applicator.create_order_adjustment order
+        adjustment = applicator.create_order_adjustment order
 
-        adjustment = Spree::Adjustment.last
         expect(adjustment.label).to eq('label')
         expect(adjustment.adjustable).to eq(order)
         expect(adjustment.originator).to eq(enterprise_fee)

From 1279ab21a68e6129669e74e9c6cba02a19ad7654 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 1 Apr 2025 13:19:34 +1100
Subject: [PATCH 7/8] Fix delete fee logic

A fee can be associated to both the incoming and outgoing exchange, the
previous logic did not account for that, resulting in the fee not being
correctly removed.
Now the delete logic also check for the metadata enterprise role to see
if any additional fee need to be removed.
---
 app/models/spree/adjustment.rb                | 16 +++++
 app/services/orders/handle_fees_service.rb    | 69 +++++++++++++++----
 .../orders/handle_fees_service_spec.rb        | 56 +++++++++++++--
 3 files changed, 123 insertions(+), 18 deletions(-)

diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb
index e3165f3ca9..f9a9d9b564 100644
--- a/app/models/spree/adjustment.rb
+++ b/app/models/spree/adjustment.rb
@@ -86,6 +86,22 @@ module Spree
 
     localize_number :amount
 
+    def self.by_originator_and_enterprise_role(originator, enterprise_role)
+      joins(:metadata)
+        .find_by(
+          originator:,
+          adjustment_metadata: { enterprise_role: }
+        )
+    end
+
+    def self.by_originator_and_not_enterprise_role(originator, enterprise_role)
+      joins(:metadata)
+        .where(originator:)
+        .where.not(
+          spree_adjustments: { adjustment_metadata: { enterprise_role: } }
+        ).first
+    end
+
     # Update both the eligibility and amount of the adjustment. Adjustments
     # delegate updating of amount to their Originator when present, but only if
     # +locked+ is false. Adjustments that are +locked+ will never change their amount.
diff --git a/app/services/orders/handle_fees_service.rb b/app/services/orders/handle_fees_service.rb
index 88f2595d60..64e42f63ad 100644
--- a/app/services/orders/handle_fees_service.rb
+++ b/app/services/orders/handle_fees_service.rb
@@ -1,11 +1,13 @@
 # frozen_string_literal: true
 
 module Orders
-  class HandleFeesService
+  class HandleFeesService # rubocop:disable Metrics/ClassLength
     attr_reader :order
 
     delegate :distributor, :order_cycle, to: :order
 
+    FeeValue = Struct.new(:fee, :role, keyword_init: true)
+
     def initialize(order)
       @order = order
     end
@@ -77,7 +79,9 @@ module Orders
       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)
+        fee_adjustment = line_item.adjustments.by_originator_and_enterprise_role(
+          fee_applicator.enterprise_fee, fee_applicator.role
+        )
 
         if fee_adjustment
           fee_adjustment.update_adjustment!(line_item, force: true)
@@ -87,8 +91,11 @@ module Orders
       end
 
       # Update any fees not already processed
-      fees_to_update =
-        order_cycle_per_item_enterprise_fee_for - applicators.map(&:enterprise_fee)
+      fees_to_update = order_cycle_fees.map(&:fee) - applicators.map(&:enterprise_fee)
+      update_fee_adjustments!(line_item, fees_to_update)
+    end
+
+    def update_fee_adjustments!(line_item, fees_to_update)
       fees_to_update.each do |fee|
         fee_adjustment = line_item.adjustments.find_by(originator: fee)
 
@@ -97,25 +104,59 @@ module Orders
     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.map(&:fee)
+      )
 
-      removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)
+      # The same fee can be used in the incoming and outgoing exchange, (supplier and distributor
+      # fees), so we need an extra check to see if a fee linked to both exchanges has been removed
+      order_cycle_fees.each do |order_cycle_fee|
+        # Check if there is any fee adjustment with a role other than the one in the order cycle fee
+        fee = line_item.enterprise_fee_adjustments.by_originator_and_not_enterprise_role(
+          order_cycle_fee.fee, order_cycle_fee.role
+        )
+
+        # Check if the fee matches a fee linked to the order cycle
+        if fee.nil? || order_cycle_fees_include_fee?(fee)
+          next
+        end
+
+        # If not linked to the order cycle we add it to the list of fee to be removed
+        removed_fees = removed_fees.to_a.push(fee)
+      end
 
       removed_fees.each(&:destroy)
     end
 
-    def order_cycle_per_item_enterprise_fee_for
-      fees = []
+    def order_cycle_fees
+      return @order_cycle_fees if defined? @order_cycle_fees
 
-      return fees unless order_cycle && distributor
+      @order_cycle_fees = begin
+        fees = []
 
-      order_cycle.exchanges.supplying_to(distributor).each do |exchange|
-        fees += exchange.enterprise_fees.per_item
+        return fees unless order_cycle && distributor
+
+        order_cycle.exchanges.supplying_to(distributor).each do |exchange|
+          exchange.enterprise_fees.per_item.each do |enterprise_fee|
+            fee_value = FeeValue.new(fee: enterprise_fee, role: exchange.role)
+            fees << fee_value
+          end
+        end
+
+        order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
+          fees << FeeValue.new(fee: enterprise_fee, role: "coordinator")
+        end
+
+        fees
       end
+    end
 
-      fees += order_cycle.coordinator_fees.per_item
-
-      fees
+    def order_cycle_fees_include_fee?(fee)
+      matching = order_cycle_fees.select do |order_cycle_fee|
+        order_cycle_fee.fee == fee.originator &&
+          order_cycle_fee.role == fee.metadata.enterprise_role
+      end
+      matching.present?
     end
 
     def calculator
diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index 87663b014d..b730abb4c1 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -89,7 +89,7 @@ RSpec.describe Orders::HandleFeesService do
         allow(calculator).to receive(
           :per_item_enterprise_fee_applicators_for
         ).and_return([fee_applicator])
-        adjustment = fee.create_adjustment('foo', line_item, true)
+        adjustment = fee_applicator.create_line_item_adjustment(line_item)
 
         expect do
           service.create_or_update_line_item_fees!
@@ -101,7 +101,7 @@ RSpec.describe Orders::HandleFeesService do
           allow(calculator).to receive(
             :per_item_enterprise_fee_applicators_for
           ).and_return([])
-          adjustment = fee.create_adjustment('foo', line_item, true)
+          adjustment = fee_applicator.create_line_item_adjustment(line_item)
 
           expect do
             service.create_or_update_line_item_fees!
@@ -111,7 +111,7 @@ RSpec.describe Orders::HandleFeesService do
 
       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)
+          adjustment = fee_applicator.create_line_item_adjustment(line_item)
           order_cycle.exchanges.first.enterprise_fees.destroy(fee)
           allow(calculator).to receive(
             :per_item_enterprise_fee_applicators_for
@@ -136,6 +136,54 @@ RSpec.describe Orders::HandleFeesService do
             end.to change { line_item.adjustments.reload.enterprise_fee.count }.by(-1)
           end
         end
+
+        context "with the same fee used for both supplier an distributor" do
+          let!(:supplier_adjustment) {
+            fee_applicator.create_line_item_adjustment(line_item)
+          }
+          let!(:distributor_adjustment) {
+            distributor_applicator.create_line_item_adjustment(line_item)
+          }
+          let(:distributor_applicator) {
+            OpenFoodNetwork::EnterpriseFeeApplicator.new(fee, line_item.variant,
+                                                         distributor_exchange.role)
+          }
+          let(:supplier_exchange) { order_cycle.cached_incoming_exchanges.first }
+          let(:distributor_exchange) { order_cycle.cached_outgoing_exchanges.first }
+
+          before do
+            # Use the supplier fee for the distributor
+            distributor_exchange.enterprise_fees = [fee]
+          end
+
+          it "removes the supplier fee when removed from the order cycle" do
+            # Delete supplier fee
+            supplier_exchange.enterprise_fees.destroy(fee)
+            allow(calculator).to receive(
+              :per_item_enterprise_fee_applicators_for
+            ).and_return([distributor_applicator])
+
+            enterprise_fees = line_item.adjustments.reload.enterprise_fee
+            expect do
+              service.create_or_update_line_item_fees!
+            end.to change { enterprise_fees.count }.by(-1)
+            expect(enterprise_fees).not_to include(supplier_adjustment)
+          end
+
+          it "removes the distributor fee when removed from the order cycle" do
+            # Delete distributor fee
+            distributor_exchange.enterprise_fees.destroy(fee)
+            allow(calculator).to receive(
+              :per_item_enterprise_fee_applicators_for
+            ).and_return([fee_applicator])
+
+            enterprise_fees = line_item.adjustments.reload.enterprise_fee
+            expect do
+              service.create_or_update_line_item_fees!
+            end.to change { enterprise_fees.count }.by(-1)
+            expect(enterprise_fees).not_to include(distributor_adjustment)
+          end
+        end
       end
 
       context "when an enterprise fee is deleted" do
@@ -172,7 +220,7 @@ RSpec.describe Orders::HandleFeesService do
         let(:fee_applicator2) {
           OpenFoodNetwork::EnterpriseFeeApplicator.new(new_fee, line_item.variant, role)
         }
-        let!(:adjustment) { fee.create_adjustment('foo', line_item, true) }
+        let!(:adjustment) { fee_applicator.create_line_item_adjustment(line_item) }
 
         before do
           allow(service).to receive(:provided_by_order_cycle?) { true }

From 5c5a115daa8a046d7748148d82b32adae29952e6 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 2 Apr 2025 10:38:47 +1100
Subject: [PATCH 8/8] Fix typo

---
 spec/services/orders/handle_fees_service_spec.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/services/orders/handle_fees_service_spec.rb b/spec/services/orders/handle_fees_service_spec.rb
index b730abb4c1..97c43d842b 100644
--- a/spec/services/orders/handle_fees_service_spec.rb
+++ b/spec/services/orders/handle_fees_service_spec.rb
@@ -63,14 +63,14 @@ RSpec.describe Orders::HandleFeesService do
 
   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
+      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)
 
         service.create_or_update_line_item_fees!
       end
 
-      it "does not create fee if variant not in Order Cyle" do
+      it "does not create fee if variant not in Order Cycle" do
         allow(service).to receive(:provided_by_order_cycle?) { false }
         expect(calculator).not_to receive(:create_line_item_adjustments_for).with(line_item)
 
@@ -215,7 +215,7 @@ RSpec.describe Orders::HandleFeesService do
         end
       end
 
-      context "with a new enterprise fee added to the order cylce" do
+      context "with a new enterprise fee added to the order cycle" do
         let(:new_fee) { create(:enterprise_fee, enterprise: fee.enterprise) }
         let(:fee_applicator2) {
           OpenFoodNetwork::EnterpriseFeeApplicator.new(new_fee, line_item.variant, role)