From da71f711c0594913ebfed58cb16f3ac5ca41280f Mon Sep 17 00:00:00 2001
From: Cillian O'Ruanaidh <contact@cilliano.com>
Date: Fri, 2 Feb 2024 13:11:15 +0000
Subject: [PATCH 1/9] Make sure fees are applied when adding the first item to
 a back office order

---
 app/controllers/api/v0/shipments_controller.rb       |  2 ++
 spec/controllers/api/v0/shipments_controller_spec.rb | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb
index 05376a2666..d742f2c29e 100644
--- a/app/controllers/api/v0/shipments_controller.rb
+++ b/app/controllers/api/v0/shipments_controller.rb
@@ -23,6 +23,8 @@ module Api
 
         OrderWorkflow.new(@order).advance_to_payment if @order.line_items.any?
 
+        @order.recreate_all_fees!
+
         render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok
       end
 
diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb
index 98584c9121..9ade418913 100644
--- a/spec/controllers/api/v0/shipments_controller_spec.rb
+++ b/spec/controllers/api/v0/shipments_controller_spec.rb
@@ -91,6 +91,18 @@ describe Api::V0::ShipmentsController, type: :controller do
 
         expect_error_response
       end
+
+      it "applies any enterprise fees that are present" do
+        order_cycle = create(:simple_order_cycle,
+                             coordinator: order.distributor,
+                             coordinator_fees: [create(:enterprise_fee, amount: 20)],
+                             distributors: [order.distributor],
+                             variants: [variant])
+        order.update!(order_cycle_id: order_cycle.id)
+        spree_post :create, params
+
+        expect(order.line_item_adjustments.where(originator_type: "EnterpriseFee")).to be_present
+      end
     end
 
     it "can make a shipment ready" do

From 75355b035947f31b3ca9ca7cac83cbe992c431c7 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 6 Feb 2024 16:47:18 +1100
Subject: [PATCH 2/9] Add test to cover updating pending payments

---
 .../order_management/order/updater_spec.rb    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb
index 921c4cc18a..127b678ece 100644
--- a/engines/order_management/spec/services/order_management/order/updater_spec.rb
+++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb
@@ -128,6 +128,46 @@ module OrderManagement
           expect(updater).not_to receive(:update_shipments).with(order)
           updater.update
         end
+
+        context "with pending payments" do
+          let!(:payment) { create(:payment, order: , amount: order.total) }
+
+          context "with order in payment state" do
+            let(:order) { create(:order_with_totals, state: "payment") }
+
+            it "updates pending payments" do
+              # update order so the order total will change
+              update_order_quantity(order)
+              order.payments.reload
+
+              expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
+            end
+          end
+
+          context "with order in confirmation state" do
+            let(:order) { create(:order_with_totals, state: "confirmation") }
+
+            it "updates pending payments" do
+              # update order so the order total will change
+              update_order_quantity(order)
+              order.payments.reload
+
+              expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
+            end
+
+          end
+
+          context "with order in cart" do
+            let(:order) { create(:order_with_totals) }
+
+            it "doesn't update pending payments" do
+              # update order so the order total will change
+              update_order_quantity(order)
+
+              expect { updater.update }.not_to change { payment.reload.amount }
+            end
+          end
+        end
       end
 
       it "updates totals once" do
@@ -377,6 +417,10 @@ module OrderManagement
           order.updater.update_payment_state
         end
       end
+
+      def update_order_quantity(order)
+        order.line_items.first.update_attribute(:quantity, 2)
+      end
     end
   end
 end

From f8cb98f43e1b107f224c30721a48eb3bf701c997 Mon Sep 17 00:00:00 2001
From: cyrillefr <cyrille.vasseur@gmail.com>
Date: Tue, 6 Feb 2024 17:02:23 +0100
Subject: [PATCH 3/9] Replace remote_toggle with method inside generic
 controller

 - put former remote toggle ctrller toggle method
   in new toggleAdvancedSettings in more generic toggle ctrller
 - modified acordingly the 2 associated views
 - put former test code in more generic ctrller test file
 - deleted now useless ctrller + test files
---
 .../order_cycles/_advanced_settings.html.haml |  4 +-
 .../_order_cycle_top_buttons.html.haml        |  6 +-
 .../controllers/remote_toggle_controller.js   | 16 -----
 .../controllers/toggle_control_controller.js  | 14 ++++-
 .../stimulus/remote_toggle_controller_test.js | 59 -------------------
 .../toggle_control_controller_test.js         | 45 ++++++++++++++
 6 files changed, 62 insertions(+), 82 deletions(-)
 delete mode 100644 app/webpacker/controllers/remote_toggle_controller.js
 delete mode 100644 spec/javascripts/stimulus/remote_toggle_controller_test.js

diff --git a/app/views/admin/order_cycles/_advanced_settings.html.haml b/app/views/admin/order_cycles/_advanced_settings.html.haml
index 455329b68c..6c04ce73b9 100644
--- a/app/views/admin/order_cycles/_advanced_settings.html.haml
+++ b/app/views/admin/order_cycles/_advanced_settings.html.haml
@@ -22,7 +22,7 @@
     .omega.eight.columns
       = f.check_box :automatic_notifications
 
-  .row{ "data-controller": "remote-toggle", "data-remote-toggle-selector-value": "#advanced_settings" }
+  .row{ "data-controller": "toggle-control", "data-toggle-control-selector-value": "#advanced_settings" }
     .sixteen.columns.alpha.omega.text-center
       %input{ type: 'submit', value: t('.save_reload') }
-      %a{ href: "#", "data-action": "click->remote-toggle#toggle" }= t(:close)
+      %a{ href: "#", "data-action": "click->toggle-control#toggleAdvancedSettings" }= t(:close)
diff --git a/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml b/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml
index 2cc89117b0..8d5cd0d6ed 100644
--- a/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml
+++ b/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml
@@ -1,8 +1,8 @@
 - content_for :page_actions do
-  %li{ "data-controller": "remote-toggle", "data-remote-toggle-selector-value": "#advanced_settings" }
-    %button#toggle_settings{ "data-action": "click->remote-toggle#toggle" }
+  %li{ "data-controller": "toggle-control", "data-toggle-control-selector-value": "#advanced_settings" }
+    %button#toggle_settings{ "data-action": "click->toggle-control#toggleAdvancedSettings" }
       = t('.advanced_settings')
-      %i.icon-chevron-down{ "data-remote-toggle-target": "chevron" }
+      %i.icon-chevron-down{ "data-toggle-control-target": "chevron" }
 
 #advanced_settings{ style: "display: none" }
   = render partial: "/admin/order_cycles/advanced_settings"
diff --git a/app/webpacker/controllers/remote_toggle_controller.js b/app/webpacker/controllers/remote_toggle_controller.js
deleted file mode 100644
index ad6944f837..0000000000
--- a/app/webpacker/controllers/remote_toggle_controller.js
+++ /dev/null
@@ -1,16 +0,0 @@
-import { Controller } from "stimulus";
-
-export default class extends Controller {
-  static targets = ["chevron"];
-  static values = { selector: String };
-
-  toggle(event) {
-    if (this.hasChevronTarget) {
-      this.chevronTarget.classList.toggle("icon-chevron-down");
-      this.chevronTarget.classList.toggle("icon-chevron-up");
-    }
-
-    const element = document.querySelector(this.selectorValue);
-    element.style.display = element.style.display === "none" ? "block" : "none";
-  }
-}
diff --git a/app/webpacker/controllers/toggle_control_controller.js b/app/webpacker/controllers/toggle_control_controller.js
index d1e4a4692c..7ba2bdb85d 100644
--- a/app/webpacker/controllers/toggle_control_controller.js
+++ b/app/webpacker/controllers/toggle_control_controller.js
@@ -1,7 +1,8 @@
 import { Controller } from "stimulus";
 
 export default class extends Controller {
-  static targets = ["control", "content"];
+  static targets = ["control", "content", "chevron"];
+  static values = { selector: String };
 
   disableIfPresent(event) {
     const input = event.currentTarget;
@@ -32,7 +33,16 @@ export default class extends Controller {
       t.style.display = input.dataset.toggleShow === "true" ? "block" : "none";
     });
   }
-  //todo: can toggleDisplay with optional chevron-target replace RemoteToggleController?
+
+  toggleAdvancedSettings(event) {
+    if (this.hasChevronTarget) {
+      this.chevronTarget.classList.toggle("icon-chevron-down");
+      this.chevronTarget.classList.toggle("icon-chevron-up");
+    }
+
+    const element = document.querySelector(this.selectorValue);
+    element.style.display = element.style.display === "none" ? "block" : "none";
+  }
 
   // private
 
diff --git a/spec/javascripts/stimulus/remote_toggle_controller_test.js b/spec/javascripts/stimulus/remote_toggle_controller_test.js
deleted file mode 100644
index 507ce3827d..0000000000
--- a/spec/javascripts/stimulus/remote_toggle_controller_test.js
+++ /dev/null
@@ -1,59 +0,0 @@
-/**
- * @jest-environment jsdom
- */
-
-import { Application } from "stimulus";
-import remote_toggle_controller from "../../../app/webpacker/controllers/remote_toggle_controller";
-
-describe("RemoteToggleController", () => {
-  beforeAll(() => {
-    const application = Application.start();
-    application.register("remote-toggle", remote_toggle_controller);
-  });
-
-  describe("#toggle", () => {
-    beforeEach(() => {
-      document.body.innerHTML = `
-        <div data-controller="remote-toggle" data-remote-toggle-selector-value="#content">
-          <button id="remote-toggle" data-action="click->remote-toggle#toggle"></button>
-          <button id="remote-toggle-with-chevron" data-action="click->remote-toggle#toggle">
-            <i class="icon-chevron-down" data-remote-toggle-target="chevron"></i>
-          </button>
-        </div>
-        <div id="content">...</div>
-      `;
-    });
-
-    it("clicking a toggle switches the visibility of the :data-remote-toggle-selector element", () => {
-      const button = document.getElementById("remote-toggle");
-      const content = document.getElementById("content");
-      expect(content.style.display).toBe("");
-
-      button.click();
-
-      expect(content.style.display).toBe("none");
-
-      button.click();
-
-      expect(content.style.display).toBe("block");
-    });
-
-    it("clicking a toggle with a chevron icon switches the visibility of content and the direction of the icon", () => {
-      const button = document.getElementById("remote-toggle-with-chevron");
-      const chevron = button.querySelector("i");
-      const content = document.getElementById("content");
-      expect(content.style.display).toBe("");
-      expect(chevron.className).toBe("icon-chevron-down");
-
-      button.click();
-
-      expect(content.style.display).toBe("none");
-      expect(chevron.className).toBe("icon-chevron-up");
-
-      button.click();
-
-      expect(content.style.display).toBe("block");
-      expect(chevron.className).toBe("icon-chevron-down");
-    });
-  });
-});
diff --git a/spec/javascripts/stimulus/toggle_control_controller_test.js b/spec/javascripts/stimulus/toggle_control_controller_test.js
index 40a63f947a..412ec9e7ee 100644
--- a/spec/javascripts/stimulus/toggle_control_controller_test.js
+++ b/spec/javascripts/stimulus/toggle_control_controller_test.js
@@ -108,4 +108,49 @@ describe("ToggleControlController", () => {
       expect(content.style.display).toBe("block");
     });
   });
+  describe("#toggleAdvancedSettings", () => {
+    beforeEach(() => {
+      document.body.innerHTML = `
+        <div data-controller="toggle-control" data-toggle-control-selector-value="#content">
+        <button id="remote-toggle" data-action="click->toggle-control#toggleAdvancedSettings"></button>
+        <button id="remote-toggle-with-chevron" data-action="click->toggle-control#toggleAdvancedSettings">
+        <i class="icon-chevron-down" data-toggle-control-target="chevron"></i>
+        </button>
+        </div>
+        <div id="content">...</div>
+        `;
+    });
+
+    it("clicking a toggle switches the visibility of the :data-remote-toggle-selector element", () => {
+      const button = document.getElementById("remote-toggle");
+      const content = document.getElementById("content");
+      expect(content.style.display).toBe("");
+
+      button.click();
+
+      expect(content.style.display).toBe("none");
+
+      button.click();
+
+      expect(content.style.display).toBe("block");
+    });
+
+    it("clicking a toggle with a chevron icon switches the visibility of content and the direction of the icon", () => {
+      const button = document.getElementById("remote-toggle-with-chevron");
+      const chevron = button.querySelector("i");
+      const content = document.getElementById("content");
+      expect(content.style.display).toBe("");
+      expect(chevron.className).toBe("icon-chevron-down");
+
+      button.click();
+
+      expect(content.style.display).toBe("none");
+      expect(chevron.className).toBe("icon-chevron-up");
+
+      button.click();
+
+      expect(content.style.display).toBe("block");
+      expect(chevron.className).toBe("icon-chevron-down");
+    });
+  });
 });

From e6fba74a875ba103a22f6e129a5ced9faa168c09 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 6 Feb 2024 16:49:08 +1100
Subject: [PATCH 4/9] Make `update_pending_payment` private

It's not used anywhere in the code, there is no reason for it to be
public
---
 .../app/services/order_management/order/updater.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb
index 6c311dade7..497991d18f 100644
--- a/engines/order_management/app/services/order_management/order/updater.rb
+++ b/engines/order_management/app/services/order_management/order/updater.rb
@@ -38,13 +38,6 @@ module OrderManagement
         update_pending_payment
       end
 
-      def update_pending_payment
-        return unless order.state.in? ["payment", "confirmation"]
-        return unless order.pending_payments.any?
-
-        order.pending_payments.first.update_attribute :amount, order.total
-      end
-
       # Updates the following Order total values:
       #
       # - payment_total - total value of all finalized Payments (excludes non-finalized Payments)
@@ -239,6 +232,13 @@ module OrderManagement
       def requires_authorization?
         payments.requires_authorization.any? && payments.completed.empty?
       end
+
+      def update_pending_payment
+        return unless order.state.in? ["payment", "confirmation"]
+        return unless order.pending_payments.any?
+
+        order.pending_payments.first.update_attribute :amount, order.total
+      end
     end
   end
 end

From 7da516b637f7bee3da5ddcd2bb0e26943f798fee Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 6 Feb 2024 16:56:29 +1100
Subject: [PATCH 5/9] Update pending payment for completed order

Update pending payment for cash order, so we can take into account any
changes affecting order toral (shipments, line item quantity etc...).
This is to allow payment capture to cover the order total, not just
the amount due at checkout.
---
 .../app/services/order_management/order/updater.rb |  5 ++++-
 .../order_management/order/updater_spec.rb         | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb
index 497991d18f..97b77ace28 100644
--- a/engines/order_management/app/services/order_management/order/updater.rb
+++ b/engines/order_management/app/services/order_management/order/updater.rb
@@ -234,7 +234,10 @@ module OrderManagement
       end
 
       def update_pending_payment
-        return unless order.state.in? ["payment", "confirmation"]
+        # We only want to update complete order pending payment when it's a cash payment. We assume
+        # that if the payment was a credit card it would alread have been processed, so we don't
+        # bother checking the payment type
+        return unless order.state.in? ["payment", "confirmation", "complete"]
         return unless order.pending_payments.any?
 
         order.pending_payments.first.update_attribute :amount, order.total
diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb
index 127b678ece..3dab32c37e 100644
--- a/engines/order_management/spec/services/order_management/order/updater_spec.rb
+++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb
@@ -105,6 +105,20 @@ module OrderManagement
           expect(shipment).to receive(:update!).with(order)
           updater.update_shipments
         end
+
+        context "whith pending payments" do
+          let(:order) { create(:completed_order_with_totals) }
+
+          it "updates pending payments" do
+            payment = create(:payment, order: , amount: order.total)
+
+            # update order so the order total will change
+            update_order_quantity(order)
+            order.payments.reload
+
+            expect { updater.update }.to change { payment.reload.amount }.from(50).to(60)
+          end
+        end
       end
 
       context "incompleted order" do

From c11d30eb13ec16baa8825f29c6dc5f2cfead528a Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 7 Feb 2024 14:36:03 +1100
Subject: [PATCH 6/9] Refactor order updater spec

Re organise the specs based on the method we are testing, it makes
the spec file more readable.

Best viewed with whitespace hidden
---
 .../order_management/order/updater_spec.rb    | 317 +++++++++---------
 1 file changed, 166 insertions(+), 151 deletions(-)

diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb
index 3dab32c37e..0447e84cb8 100644
--- a/engines/order_management/spec/services/order_management/order/updater_spec.rb
+++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb
@@ -8,7 +8,7 @@ module OrderManagement
       let(:order) { create(:order) }
       let(:updater) { OrderManagement::Order::Updater.new(order) }
 
-      context "updating order totals" do
+      describe "#update_totals" do
         before do
           2.times { create(:line_item, order:, price: 10) }
         end
@@ -19,11 +19,23 @@ module OrderManagement
           updater.update_totals
           expect(order.payment_total).to eq(10)
         end
+      end
+
+      describe "#update_item_total" do
+        before do
+          2.times { create(:line_item, order:, price: 10) }
+        end
 
         it "updates item total" do
           updater.update_item_total
           expect(order.item_total).to eq(20)
         end
+      end
+
+      describe "#update_adjustment_total" do
+        before do
+          2.times { create(:line_item, order:, price: 10) }
+        end
 
         it "updates adjustment totals" do
           allow(order).to receive_message_chain(:all_adjustments, :additional, :eligible,
@@ -40,7 +52,7 @@ module OrderManagement
         end
       end
 
-      context "updating shipment state" do
+      describe "#update_shipment_state" do
         let(:shipment) { build(:shipment) }
 
         before do
@@ -85,19 +97,111 @@ module OrderManagement
         order.state_changed('shipment')
       end
 
-      context "completed order" do
+      describe "#update" do
+        it "updates totals once" do
+          expect(updater).to receive(:update_totals).once
+          updater.update
+        end
+
+        it "updates all adjustments" do
+          expect(updater).to receive(:update_all_adjustments)
+          updater.update
+        end
+
+        context "completed order" do
+          before { allow(order).to receive(:completed?) { true } }
+
+          it "updates payment state" do
+            expect(updater).to receive(:update_payment_state)
+            updater.update
+          end
+
+          it "updates shipment state" do
+            expect(updater).to receive(:update_shipment_state)
+            updater.update
+          end
+
+          context "whith pending payments" do
+            let(:order) { create(:completed_order_with_totals) }
+
+            it "updates pending payments" do
+              payment = create(:payment, order:, amount: order.total)
+
+              # update order so the order total will change
+              update_order_quantity(order)
+              order.payments.reload
+
+              expect { updater.update }.to change { payment.reload.amount }.from(50).to(60)
+            end
+          end
+        end
+
+        context "incompleted order" do
+          before { allow(order).to receive_messages completed?: false }
+
+          it "doesnt update payment state" do
+            expect(updater).not_to receive(:update_payment_state)
+            updater.update
+          end
+
+          it "doesnt update shipment state" do
+            expect(updater).not_to receive(:update_shipment_state)
+            updater.update
+          end
+
+          it "doesnt update the order shipment" do
+            shipment = build(:shipment)
+            allow(order).to receive_messages shipments: [shipment]
+
+            expect(shipment).not_to receive(:update!).with(order)
+            expect(updater).not_to receive(:update_shipments).with(order)
+            updater.update
+          end
+
+          context "with pending payments" do
+            let!(:payment) { create(:payment, order:, amount: order.total) }
+
+            context "with order in payment state" do
+              let(:order) { create(:order_with_totals, state: "payment") }
+
+              it "updates pending payments" do
+                # update order so the order total will change
+                update_order_quantity(order)
+                order.payments.reload
+
+                expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
+              end
+            end
+
+            context "with order in confirmation state" do
+              let(:order) { create(:order_with_totals, state: "confirmation") }
+
+              it "updates pending payments" do
+                # update order so the order total will change
+                update_order_quantity(order)
+                order.payments.reload
+
+                expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
+              end
+            end
+
+            context "with order in cart" do
+              let(:order) { create(:order_with_totals) }
+
+              it "doesn't update pending payments" do
+                # update order so the order total will change
+                update_order_quantity(order)
+
+                expect { updater.update }.not_to change { payment.reload.amount }
+              end
+            end
+          end
+        end
+      end
+
+      describe "#update_shipments" do
         before { allow(order).to receive(:completed?) { true } }
 
-        it "updates payment state" do
-          expect(updater).to receive(:update_payment_state)
-          updater.update
-        end
-
-        it "updates shipment state" do
-          expect(updater).to receive(:update_shipment_state)
-          updater.update
-        end
-
         it "updates the order shipment" do
           shipment = build(:shipment)
           allow(order).to receive_messages shipments: [shipment]
@@ -105,93 +209,6 @@ module OrderManagement
           expect(shipment).to receive(:update!).with(order)
           updater.update_shipments
         end
-
-        context "whith pending payments" do
-          let(:order) { create(:completed_order_with_totals) }
-
-          it "updates pending payments" do
-            payment = create(:payment, order: , amount: order.total)
-
-            # update order so the order total will change
-            update_order_quantity(order)
-            order.payments.reload
-
-            expect { updater.update }.to change { payment.reload.amount }.from(50).to(60)
-          end
-        end
-      end
-
-      context "incompleted order" do
-        before { allow(order).to receive_messages completed?: false }
-
-        it "doesnt update payment state" do
-          expect(updater).not_to receive(:update_payment_state)
-          updater.update
-        end
-
-        it "doesnt update shipment state" do
-          expect(updater).not_to receive(:update_shipment_state)
-          updater.update
-        end
-
-        it "doesnt update the order shipment" do
-          shipment = build(:shipment)
-          allow(order).to receive_messages shipments: [shipment]
-
-          expect(shipment).not_to receive(:update!).with(order)
-          expect(updater).not_to receive(:update_shipments).with(order)
-          updater.update
-        end
-
-        context "with pending payments" do
-          let!(:payment) { create(:payment, order: , amount: order.total) }
-
-          context "with order in payment state" do
-            let(:order) { create(:order_with_totals, state: "payment") }
-
-            it "updates pending payments" do
-              # update order so the order total will change
-              update_order_quantity(order)
-              order.payments.reload
-
-              expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
-            end
-          end
-
-          context "with order in confirmation state" do
-            let(:order) { create(:order_with_totals, state: "confirmation") }
-
-            it "updates pending payments" do
-              # update order so the order total will change
-              update_order_quantity(order)
-              order.payments.reload
-
-              expect { updater.update }.to change { payment.reload.amount }.from(10).to(20)
-            end
-
-          end
-
-          context "with order in cart" do
-            let(:order) { create(:order_with_totals) }
-
-            it "doesn't update pending payments" do
-              # update order so the order total will change
-              update_order_quantity(order)
-
-              expect { updater.update }.not_to change { payment.reload.amount }
-            end
-          end
-        end
-      end
-
-      it "updates totals once" do
-        expect(updater).to receive(:update_totals).once
-        updater.update
-      end
-
-      it "updates all adjustments" do
-        expect(updater).to receive(:update_all_adjustments)
-        updater.update
       end
 
       describe "#update_payment_state" do
@@ -331,9 +348,28 @@ module OrderManagement
             end
           end
         end
+
+        context "when unused payments records exist which require authorization, " \
+                "but the order is fully paid" do
+          let!(:cash_payment) {
+            build(:payment, state: "completed", amount: order.new_outstanding_balance)
+          }
+          let!(:stripe_payment) { build(:payment, state: "requires_authorization") }
+          before do
+            order.payments << cash_payment
+            order.payments << stripe_payment
+          end
+
+          it "cancels unused payments requiring authorization" do
+            expect(stripe_payment).to receive(:void_transaction!)
+            expect(cash_payment).to_not receive(:void_transaction!)
+
+            order.updater.update_payment_state
+          end
+        end
       end
 
-      context '#shipping_address_from_distributor' do
+      describe '#shipping_address_from_distributor' do
         let(:distributor) { build(:distributor_enterprise) }
         let(:shipment) {
           create(:shipment_with, :shipping_method, shipping_method:)
@@ -367,71 +403,50 @@ module OrderManagement
         end
       end
 
-      describe "updating order totals" do
-        describe "#update_totals_and_states" do
-          it "deals with legacy taxes" do
-            expect(updater).to receive(:handle_legacy_taxes)
+      describe "#update_totals_and_states" do
+        it "deals with legacy taxes" do
+          expect(updater).to receive(:handle_legacy_taxes)
 
-            updater.update_totals_and_states
+          updater.update_totals_and_states
+        end
+      end
+
+      describe "#handle_legacy_taxes" do
+        context "when the order is incomplete" do
+          it "doesn't touch taxes" do
+            allow(order).to receive(:completed?) { false }
+
+            expect(order).to_not receive(:create_tax_charge!)
+            updater.__send__(:handle_legacy_taxes)
           end
         end
 
-        describe "#handle_legacy_taxes" do
-          context "when the order is incomplete" do
-            it "doesn't touch taxes" do
-              allow(order).to receive(:completed?) { false }
+        context "when the order is complete" do
+          before { allow(order).to receive(:completed?) { true } }
+
+          context "and the order has legacy taxes" do
+            let!(:legacy_tax_adjustment) {
+              create(:adjustment, order:, adjustable: order, included: false,
+                                  originator_type: "Spree::TaxRate")
+            }
+
+            it "re-applies order taxes" do
+              expect(order).to receive(:create_tax_charge!)
 
-              expect(order).to_not receive(:create_tax_charge!)
               updater.__send__(:handle_legacy_taxes)
             end
           end
 
-          context "when the order is complete" do
-            before { allow(order).to receive(:completed?) { true } }
+          context "and the order has no legacy taxes" do
+            it "leaves taxes untouched" do
+              expect(order).to_not receive(:create_tax_charge!)
 
-            context "and the order has legacy taxes" do
-              let!(:legacy_tax_adjustment) {
-                create(:adjustment, order:, adjustable: order, included: false,
-                                    originator_type: "Spree::TaxRate")
-              }
-
-              it "re-applies order taxes" do
-                expect(order).to receive(:create_tax_charge!)
-
-                updater.__send__(:handle_legacy_taxes)
-              end
-            end
-
-            context "and the order has no legacy taxes" do
-              it "leaves taxes untouched" do
-                expect(order).to_not receive(:create_tax_charge!)
-
-                updater.__send__(:handle_legacy_taxes)
-              end
+              updater.__send__(:handle_legacy_taxes)
             end
           end
         end
       end
 
-      context "when unused payments records exist which require authorization, " \
-              "but the order is fully paid" do
-        let!(:cash_payment) {
-          build(:payment, state: "completed", amount: order.new_outstanding_balance)
-        }
-        let!(:stripe_payment) { build(:payment, state: "requires_authorization") }
-        before do
-          order.payments << cash_payment
-          order.payments << stripe_payment
-        end
-
-        it "cancels unused payments requiring authorization" do
-          expect(stripe_payment).to receive(:void_transaction!)
-          expect(cash_payment).to_not receive(:void_transaction!)
-
-          order.updater.update_payment_state
-        end
-      end
-
       def update_order_quantity(order)
         order.line_items.first.update_attribute(:quantity, 2)
       end

From 5fd36e7eedf70f1bc7a062f2faadd79b29a7a0f0 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 7 Feb 2024 15:27:51 +1100
Subject: [PATCH 7/9] Fix SubscriptionConfirmJob spec

Fix the payment stubbing, payment `process_offline!` was stubbed to
return true and payment `completed?` was also stubbed to return true.
`process_offline!` did not change the payment status in the database,
meaning that any call to 'order.pending_payments' after that would
return the payment which should have been processed offline, and
therefore completed. This created some unwanted side effect, resulting
in the test breaking.
---
 spec/jobs/subscription_confirm_job_spec.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb
index 237659e876..ff96e97a60 100644
--- a/spec/jobs/subscription_confirm_job_spec.rb
+++ b/spec/jobs/subscription_confirm_job_spec.rb
@@ -239,8 +239,11 @@ describe SubscriptionConfirmJob do
 
         context "when payments are processed without error" do
           before do
-            expect(payment).to receive(:process_offline!) { true }
-            expect(payment).to receive(:completed?) { true }
+            allow(payment).to receive(:process_offline!) do
+              # Mark payment as complete like it would be if sucessfully processed offline
+              payment.state = "complete"
+              true
+            end
           end
 
           it "sends only a subscription confirm email, no regular confirmation emails" do

From 0af3e8afcb23184a368a798f18cfe6708a0e7e99 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Wed, 14 Feb 2024 09:17:52 +0000
Subject: [PATCH 8/9] chore(deps-dev): bump letter_opener from 1.8.1 to 1.9.0

Bumps [letter_opener](https://github.com/ryanb/letter_opener) from 1.8.1 to 1.9.0.
- [Changelog](https://github.com/ryanb/letter_opener/blob/master/CHANGELOG.md)
- [Commits](https://github.com/ryanb/letter_opener/compare/v1.8.1...v1.9.0)

---
updated-dependencies:
- dependency-name: letter_opener
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
 Gemfile.lock | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index d7f53aa31c..3e772a2836 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -383,9 +383,9 @@ GEM
     knapsack_pro (6.0.4)
       rake
     language_server-protocol (3.17.0.3)
-    launchy (2.5.0)
-      addressable (~> 2.7)
-    letter_opener (1.8.1)
+    launchy (2.5.2)
+      addressable (~> 2.8)
+    letter_opener (1.9.0)
       launchy (>= 2.2, < 3)
     link_header (0.0.8)
     listen (3.8.0)

From cffe573203ef7009d0b4a02b2499078e0626dacc Mon Sep 17 00:00:00 2001
From: isidzukuri <axesigon@gmail.com>
Date: Wed, 14 Feb 2024 18:41:28 +0200
Subject: [PATCH 9/9] improve random name generation in factories related to
 tax_rate_factory

---
 spec/factories/tax_category_factory.rb | 2 +-
 spec/factories/zone_factory.rb         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/factories/tax_category_factory.rb b/spec/factories/tax_category_factory.rb
index 0cb0f3af4a..5503091437 100644
--- a/spec/factories/tax_category_factory.rb
+++ b/spec/factories/tax_category_factory.rb
@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :tax_category, class: Spree::TaxCategory do
-    name { "TaxCategory - #{rand(999_999)}" }
+    sequence(:name) { |n| "TaxCategory - #{n}" }
     description { generate(:random_string) }
   end
 end
diff --git a/spec/factories/zone_factory.rb b/spec/factories/zone_factory.rb
index 4d4ed09542..95c5ae8ddc 100644
--- a/spec/factories/zone_factory.rb
+++ b/spec/factories/zone_factory.rb
@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :zone, class: Spree::Zone do
-    name { generate(:random_string) }
+    sequence(:name) { |n| "#{generate(:random_string)}#{n}" }
     description { generate(:random_string) }
   end