From de97c596325593955ffa6c684e8fca5d1c30a415 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 25 Jan 2023 16:01:43 +1100
Subject: [PATCH 1/7] Order cycle exchange form, only set red border on
 pickup_time field if empty

The red border is set by setting pickup_time as $dirty, it then blocks next button and add leave page warning when it is not necessary, this is a fix for it.
---
 .../controllers/order_cycle_exchanges_controller.js.coffee  | 6 +++++-
 app/views/admin/order_cycles/_exchange_form.html.haml       | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/admin/order_cycles/controllers/order_cycle_exchanges_controller.js.coffee b/app/assets/javascripts/admin/order_cycles/controllers/order_cycle_exchanges_controller.js.coffee
index ee7c1aa5cf..db2cb94213 100644
--- a/app/assets/javascripts/admin/order_cycles/controllers/order_cycle_exchanges_controller.js.coffee
+++ b/app/assets/javascripts/admin/order_cycles/controllers/order_cycle_exchanges_controller.js.coffee
@@ -35,7 +35,11 @@ angular.module('admin.orderCycles')
       OrderCycle.removeExchangeFee(exchange, index)
       $scope.order_cycle_form.$dirty = true
 
-    $scope.setPickupTimeFieldDirty = (index) ->
+    $scope.setPickupTimeFieldDirty = (index, pickup_time) ->
+      # if the pickup_time is already set we are in edit mode, so no need to set pickup_time field as dirty
+      # to show it is required (it has a red border when set to dirty)
+      return if pickup_time
+
       $timeout ->
         pickup_time_field_name = "order_cycle_outgoing_exchange_" + index + "_pickup_time"
         $scope.order_cycle_form[pickup_time_field_name].$setDirty()
diff --git a/app/views/admin/order_cycles/_exchange_form.html.haml b/app/views/admin/order_cycles/_exchange_form.html.haml
index e5aa662c60..d92099cd8c 100644
--- a/app/views/admin/order_cycles/_exchange_form.html.haml
+++ b/app/views/admin/order_cycles/_exchange_form.html.haml
@@ -10,7 +10,7 @@
     %td.tags.panel-toggle.text-center{ name: "tags", ng: { if: 'enterprises[exchange.enterprise_id].managed || order_cycle.viewing_as_coordinator' } }
       {{ exchange.tags.length }}
     %td.collection-details
-      = text_field_tag 'order_cycle_outgoing_exchange_{{ $index }}_pickup_time', '', 'ng-init' => 'setPickupTimeFieldDirty($index)', 'id' => 'order_cycle_outgoing_exchange_{{ $index }}_pickup_time', 'required' => 'required', 'placeholder' => t('.pickup_time_placeholder'), 'ng-model' => 'exchange.pickup_time', 'ng-disabled' => '!enterprises[exchange.enterprise_id].managed && !order_cycle.viewing_as_coordinator', 'maxlength' => 35
+      = text_field_tag 'order_cycle_outgoing_exchange_{{ $index }}_pickup_time', '', 'ng-init' => 'setPickupTimeFieldDirty($index, exchange.pickup_time)', 'id' => 'order_cycle_outgoing_exchange_{{ $index }}_pickup_time', 'required' => 'required', 'placeholder' => t('.pickup_time_placeholder'), 'ng-model' => 'exchange.pickup_time', 'ng-disabled' => '!enterprises[exchange.enterprise_id].managed && !order_cycle.viewing_as_coordinator', 'maxlength' => 35
       %span.icon-question-sign{'ofn-with-tip' => t('.pickup_time_tip')}
       %br/
       = text_field_tag 'order_cycle_outgoing_exchange_{{ $index }}_pickup_instructions', '', 'id' => 'order_cycle_outgoing_exchange_{{ $index }}_pickup_instructions', 'placeholder' => t('.pickup_instructions_placeholder'), 'ng-model' => 'exchange.pickup_instructions', 'ng-disabled' => '!enterprises[exchange.enterprise_id].managed && !order_cycle.viewing_as_coordinator'

From ef309c0fd04d242f408bb7eb9ff16dc2c615ee32 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 31 Jan 2023 15:01:55 +1100
Subject: [PATCH 2/7] Order cycle form, checkout options steps add user warning
 when leaving page and form has been changed

Add UnsavedChanges stimulus controller, it should be generic enough so
that it can reused somewhere else. It works with both 'beforeunload' event
and 'turbolinks:before-visit' when using turbo links.
---
 .../order_cycles/checkout_options.html.haml   |  10 +-
 .../controllers/unsaved_changes_controller.js |  54 +++++++++
 .../unsaved_changes_controller_test.js        | 109 ++++++++++++++++++
 3 files changed, 168 insertions(+), 5 deletions(-)
 create mode 100644 app/webpacker/controllers/unsaved_changes_controller.js
 create mode 100644 spec/javascripts/stimulus/unsaved_changes_controller_test.js

diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml
index 42c3213301..9138c14565 100644
--- a/app/views/admin/order_cycles/checkout_options.html.haml
+++ b/app/views/admin/order_cycles/checkout_options.html.haml
@@ -3,7 +3,7 @@
 - content_for :page_title do
   = t :edit_order_cycle
 
-= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" } do |f|
+= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
 
   = render 'wizard_progress'
 
@@ -26,7 +26,7 @@
               %td.text-center
                 - if distributor_shipping_methods.many?
                   %label
-                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" }
+                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll change->unsaved-changes#formIsChanged", "data-select-all-target": "all" }
                     = t(".select_all")
               %td
                 %em= distributor.name
@@ -37,7 +37,7 @@
                           distributor_shipping_method.id,
                           @order_cycle.distributor_shipping_methods.include?(distributor_shipping_method),
                           id: "order_cycle_selected_distributor_shipping_method_ids_#{distributor_shipping_method.id}",
-                          data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if distributor_shipping_method.shipping_method.frontend?)
+                          data: ({ "action" => "change->select-all#toggleCheckbox change->unsaved-changes#formIsChanged", "select-all-target" => "checkbox" } if distributor_shipping_method.shipping_method.frontend?)
                       = distributor_shipping_method.shipping_method.name
                 - distributor.shipping_methods.backend.each do |shipping_method|
                   %label.disabled
@@ -57,7 +57,7 @@
               %td.text-center
                 - if distributor_payment_methods.many?
                   %label
-                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" }
+                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll change->unsaved-changes#formIsChanged", "data-select-all-target": "all" }
                     = t(".select_all")
               %td
                 %em= distributor.name
@@ -68,7 +68,7 @@
                           distributor_payment_method.id,
                           @order_cycle.distributor_payment_methods.include?(distributor_payment_method),
                           id: "order_cycle_selected_distributor_payment_method_ids_#{distributor_payment_method.id}",
-                          data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if distributor_payment_method.payment_method.frontend?)
+                          data: ({ "action" => "change->select-all#toggleCheckbox change->unsaved-changes#formIsChanged", "select-all-target" => "checkbox" } if distributor_payment_method.payment_method.frontend?)
                       = distributor_payment_method.payment_method.name
                 - distributor.payment_methods.inactive_or_backend.each do |payment_method|
                   %label.disabled
diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
new file mode 100644
index 0000000000..fca6da9f45
--- /dev/null
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -0,0 +1,54 @@
+import { Controller } from "stimulus";
+
+// UnsavedChanges allows you to promp the user about unsaved changes when trying to leave the page
+//
+// Usage :
+// - with beforeunload event :
+//    <form data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="true">
+//      <input data-action="change->unsaved-changes#formIsChanged">
+//    </form>
+//
+// - with turbolinks :
+//    <form data-controller="unsaved-changes" data-action="turbolinks:before-visit@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="true">
+//      <input data-action="change->unsaved-changes#formIsChanged">
+//    </form>
+//
+// You can also combine the two actions
+//
+export default class extends Controller {
+  formIsChanged(event) {
+    this.setChanged("true");
+  }
+
+  leavingPage(event) {
+    const LEAVING_PAGE_MESSAGE = I18n.t("admin.unsaved_confirm_leave");
+
+    if (this.isFormChanged()) {
+      if (event.type == "turbolinks:before-visit") {
+        if (!window.confirm(LEAVING_PAGE_MESSAGE)) {
+          event.preventDefault();
+        }
+      } else {
+        // We cover our bases, according to the documentation we should be able to prompt the user
+        // by calling event.preventDefault(), but it's not really supported yet.
+        // Instead we set the value of event.returnValue, and return a string, both of them
+        // should prompt the user.
+        // Note, in most modern browser a generic string not under the control of the webpage is shown
+        // instead of the returned string.
+        //
+        // More info : https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event
+        //
+        event.returnValue = LEAVING_PAGE_MESSAGE;
+        return event.returnValue;
+      }
+    }
+  }
+
+  setChanged(changed) {
+    this.data.set("changed", changed);
+  }
+
+  isFormChanged() {
+    return this.data.get("changed") == "true";
+  }
+}
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
new file mode 100644
index 0000000000..6b5ca0c707
--- /dev/null
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -0,0 +1,109 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { Application } from "stimulus"
+import unsaved_changes_controller from "../../../app/webpacker/controllers/unsaved_changes_controller"
+
+describe("UnsavedChangesController", () => {
+  beforeAll(() => {
+    const application = Application.start()
+    application.register("unsaved-changes", unsaved_changes_controller)
+  })
+
+  beforeEach(() => {
+    document.body.innerHTML = `
+      <form id="test-form" data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="false">
+        <input
+          id="test-checkbox"
+          type="checkbox"
+          data-action="change->unsaved-changes#formIsChanged">
+      </form>
+    `
+  })
+
+  describe("#formIsChanged", () => {
+    it("changed is set to true", () => {
+      const form = document.getElementById("test-form")
+      const checkbox = document.getElementById("test-checkbox")
+
+      checkbox.click()
+
+      expect(form.dataset.unsavedChangesChanged).toBe('true')
+
+    })
+  })
+
+  describe('#leavingPage', () => {
+    beforeEach(() => {
+      // Add a mock I18n object to
+      const mockedT = jest.fn()
+      mockedT.mockImplementation((string) => (string))
+
+      global.I18n =  {
+        t: mockedT
+      }
+    })
+
+    afterEach(() => {
+      delete global.I18n
+    })
+
+    describe('when triggering a beforeunload event', () => {
+      it("triggers leave page pop up when leaving page and form has been interacted with", () => {
+        const checkbox = document.getElementById("test-checkbox")
+
+        // interact with the form
+        checkbox.click()
+
+        // trigger beforeunload to simulate leaving the page
+        const beforeunloadEvent = new Event("beforeunload")
+        window.dispatchEvent(beforeunloadEvent)
+
+        // Test the event returnValue has been set, we don't really care about the value as
+        // the brower will ignore it 
+        expect(beforeunloadEvent.returnValue).toBeTruthy()
+      })
+    })
+
+    describe('when triggering a turbolinks:before-visit event', () => {
+      it("triggers a confirm popup up when leaving page and form has been interacted with", () => {
+        const checkbox = document.getElementById("test-checkbox")
+        const confirmSpy = jest.spyOn(window, 'confirm')
+        confirmSpy.mockImplementation((msg) => {})
+
+        // interact with the form
+        checkbox.click()
+
+        // trigger turbolinks:before-visit to simulate leaving the page
+        const turbolinkEv = new Event("turbolinks:before-visit")
+        window.dispatchEvent(turbolinkEv)
+
+        expect(confirmSpy).toHaveBeenCalled()
+
+      })
+
+      it("stays on the page if user clicks cancel on the confirm popup", () => {
+        const checkbox = document.getElementById("test-checkbox")
+        const confirmSpy = jest.spyOn(window, 'confirm')
+        // return false to simulate a user clicking on cancel
+        confirmSpy.mockImplementation((msg) => (false)) 
+
+        // interact with the form
+        checkbox.click()
+
+        // trigger turbolinks:before-visit to simulate leaving the page
+        const turbolinkEv = new Event("turbolinks:before-visit")
+        const preventDefaultSpy = jest.spyOn(turbolinkEv, 'preventDefault')
+
+        window.dispatchEvent(turbolinkEv)
+
+        expect(confirmSpy).toHaveBeenCalled()
+        expect(preventDefaultSpy).toHaveBeenCalled()
+
+        // cleanup
+        confirmSpy.mockRestore()
+      })
+    })
+  })
+})

From dd876dfd8dd6fb8726dbb214ef7ac4b424973cdb Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 1 Feb 2023 09:24:38 +1100
Subject: [PATCH 3/7] Order cycle form, disable the save and save and back
 button on page load

Buttons will be enabled once the form has been interacted with.
Update unsavedChanges stimulus controller to handle this. It should
still be generic enought that it can be reused.
---
 .../controllers/unsaved_changes_controller.js | 29 ++++++++++++++++++-
 .../unsaved_changes_controller_test.js        | 26 +++++++++++++----
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
index fca6da9f45..c458921065 100644
--- a/app/webpacker/controllers/unsaved_changes_controller.js
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -16,8 +16,19 @@ import { Controller } from "stimulus";
 // You can also combine the two actions
 //
 export default class extends Controller {
+  connect() {
+    // disable submit button when first loading the page
+    if (!this.isFormChanged()) {
+      this.disableButtons();
+    }
+  }
+
   formIsChanged(event) {
-    this.setChanged("true");
+    // We only do something if the form hasn't already been changed
+    if (!this.isFormChanged()) {
+      this.setChanged("true");
+      this.enableButtons();
+    }
   }
 
   leavingPage(event) {
@@ -51,4 +62,20 @@ export default class extends Controller {
   isFormChanged() {
     return this.data.get("changed") == "true";
   }
+
+  enableButtons() {
+    this.submitButtons().forEach((button) => {
+      button.disabled = false;
+    });
+  }
+
+  disableButtons() {
+    this.submitButtons().forEach((button) => {
+      button.disabled = true;
+    });
+  }
+
+  submitButtons() {
+    return this.element.querySelectorAll("input[type='submit']");
+  }
 }
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
index 6b5ca0c707..b5748066c9 100644
--- a/spec/javascripts/stimulus/unsaved_changes_controller_test.js
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -14,23 +14,37 @@ describe("UnsavedChangesController", () => {
   beforeEach(() => {
     document.body.innerHTML = `
       <form id="test-form" data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="false">
-        <input
-          id="test-checkbox"
-          type="checkbox"
-          data-action="change->unsaved-changes#formIsChanged">
+        <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+        <input id="test-submit" type="submit"/>
       </form>
     `
   })
 
+  describe("#connect", () => {
+    it("disables any submit button", () => {
+      const submit = document.getElementById("test-submit")
+
+      expect(submit.disabled).toBe(true)
+    })  
+  })
+
   describe("#formIsChanged", () => {
     it("changed is set to true", () => {
-      const form = document.getElementById("test-form")
       const checkbox = document.getElementById("test-checkbox")
+      const form = document.getElementById("test-form")
 
       checkbox.click()
 
-      expect(form.dataset.unsavedChangesChanged).toBe('true')
+      expect(form.dataset.unsavedChangesChanged).toBe("true")
+    })
 
+    it("enables any submit button", () => {
+      const checkbox = document.getElementById("test-checkbox")
+      const submit = document.getElementById("test-submit")
+
+      checkbox.click()
+
+      expect(submit.disabled).toBe(false)
     })
   })
 

From 5cfedddba45a4210a393f477bc4820590b85592e Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 1 Feb 2023 13:46:30 +1100
Subject: [PATCH 4/7] UnsavedChanges controller, disabling submit button is now
 optional

---
 .../controllers/unsaved_changes_controller.js |  36 +++++-
 .../unsaved_changes_controller_test.js        | 112 ++++++++++++++----
 2 files changed, 122 insertions(+), 26 deletions(-)

diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
index c458921065..7e2bc5055f 100644
--- a/app/webpacker/controllers/unsaved_changes_controller.js
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -4,21 +4,34 @@ import { Controller } from "stimulus";
 //
 // Usage :
 // - with beforeunload event :
-//    <form data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="true">
-//      <input data-action="change->unsaved-changes#formIsChanged">
+//    <form
+//      data-controller="unsaved-changes"
+//      data-action="beforeunload@window->unsaved-changes#leavingPage"
+//      data-unsaved-changes-changed="true"
+//    >
+//      <input data-action="change->unsaved-changes#formIsChanged" />
 //    </form>
 //
 // - with turbolinks :
-//    <form data-controller="unsaved-changes" data-action="turbolinks:before-visit@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="true">
-//      <input data-action="change->unsaved-changes#formIsChanged">
+//    <form
+//      data-controller="unsaved-changes"
+//      data-action="turbolinks:before-visit@window->unsaved-changes#leavingPage"
+//      data-unsaved-changes-changed="true"
+//    >
+//      <input data-action="change->unsaved-changes#formIsChanged" />
 //    </form>
 //
 // You can also combine the two actions
+// You also need to add 'data-action="change->unsaved-changes#formIsChanged"' on all the form element
+// that can be interacted with
+//
+// Optional, you can add 'data-unsaved-changes-changed="true"' if you want to disable all
+// submit buttons when the form hasn't been interacted with
 //
 export default class extends Controller {
   connect() {
     // disable submit button when first loading the page
-    if (!this.isFormChanged()) {
+    if (!this.isFormChanged() && this.isSubmitButtonDisabled()) {
       this.disableButtons();
     }
   }
@@ -27,7 +40,10 @@ export default class extends Controller {
     // We only do something if the form hasn't already been changed
     if (!this.isFormChanged()) {
       this.setChanged("true");
-      this.enableButtons();
+
+      if (this.isSubmitButtonDisabled()) {
+        this.enableButtons();
+      }
     }
   }
 
@@ -63,6 +79,14 @@ export default class extends Controller {
     return this.data.get("changed") == "true";
   }
 
+  isSubmitButtonDisabled() {
+    if (this.data.has("disable-submit-button")) {
+      return this.data.get("disable-submit-button") == "true";
+    }
+
+    return false;
+  }
+
   enableButtons() {
     this.submitButtons().forEach((button) => {
       button.disabled = false;
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
index b5748066c9..f6d8544ff7 100644
--- a/spec/javascripts/stimulus/unsaved_changes_controller_test.js
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -13,7 +13,11 @@ describe("UnsavedChangesController", () => {
 
   beforeEach(() => {
     document.body.innerHTML = `
-      <form id="test-form" data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" data-unsaved-changes-changed="false">
+      <form 
+        id="test-form" 
+        data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+        data-unsaved-changes-changed="false"
+      >
         <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
         <input id="test-submit" type="submit"/>
       </form>
@@ -21,16 +25,70 @@ describe("UnsavedChangesController", () => {
   })
 
   describe("#connect", () => {
-    it("disables any submit button", () => {
-      const submit = document.getElementById("test-submit")
+    describe("when disable-submit-button is true", () => {
+      beforeEach(() => {
+        document.body.innerHTML = `
+          <form 
+            id="test-form" data-controller="unsaved-changes" 
+            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            data-unsaved-changes-changed="false" 
+            data-unsaved-changes-disable-submit-button="true"
+          >
+            <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+            <input id="test-submit" type="submit"/>
+          </form>
+        `
+      })
 
-      expect(submit.disabled).toBe(true)
+      it("disables any submit button", () => {
+        const submit = document.getElementById("test-submit")
+
+        expect(submit.disabled).toBe(true)
+      })  
+    })  
+
+    describe("when disable-submit-button is false", () => {
+      beforeEach(() => {
+        document.body.innerHTML = `
+          <form 
+            id="test-form" 
+            data-controller="unsaved-changes" 
+            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            data-unsaved-changes-changed="false" 
+            data-unsaved-changes-disable-submit-button="false"
+          >
+            <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+            <input id="test-submit" type="submit"/>
+          </form>
+        `
+      })
+
+      it("doesn't disable any submit button", () => {
+        const submit = document.getElementById("test-submit")
+
+        expect(submit.disabled).toBe(false)
+      })  
+    })  
+
+    describe("when disable-submit-button is not set", () => {
+      it("doesn't disable any submit button", () => {
+        const submit = document.getElementById("test-submit")
+
+        expect(submit.disabled).toBe(false)
+      })  
     })  
   })
 
   describe("#formIsChanged", () => {
+    let checkbox
+    let submit 
+
+    beforeEach(() => {
+      checkbox = document.getElementById("test-checkbox")
+      submit = document.getElementById("test-submit")
+    })
+
     it("changed is set to true", () => {
-      const checkbox = document.getElementById("test-checkbox")
       const form = document.getElementById("test-form")
 
       checkbox.click()
@@ -38,17 +96,28 @@ describe("UnsavedChangesController", () => {
       expect(form.dataset.unsavedChangesChanged).toBe("true")
     })
 
-    it("enables any submit button", () => {
-      const checkbox = document.getElementById("test-checkbox")
-      const submit = document.getElementById("test-submit")
+    describe("when disable-submit-button is true", () => {
+      it("enables any submit button", () => {
+        checkbox.click()
 
-      checkbox.click()
+        expect(submit.disabled).toBe(false)
+      })
+    })
 
-      expect(submit.disabled).toBe(false)
+    describe("when disable-submit-button is false", () => {
+      it("does nothing", () => {
+        expect(submit.disabled).toBe(false)
+
+        checkbox.click()
+
+        expect(submit.disabled).toBe(false)
+      })
     })
   })
 
   describe('#leavingPage', () => {
+    let checkbox
+
     beforeEach(() => {
       // Add a mock I18n object to
       const mockedT = jest.fn()
@@ -57,6 +126,8 @@ describe("UnsavedChangesController", () => {
       global.I18n =  {
         t: mockedT
       }
+
+      checkbox = document.getElementById("test-checkbox")
     })
 
     afterEach(() => {
@@ -65,8 +136,6 @@ describe("UnsavedChangesController", () => {
 
     describe('when triggering a beforeunload event', () => {
       it("triggers leave page pop up when leaving page and form has been interacted with", () => {
-        const checkbox = document.getElementById("test-checkbox")
-
         // interact with the form
         checkbox.click()
 
@@ -81,9 +150,18 @@ describe("UnsavedChangesController", () => {
     })
 
     describe('when triggering a turbolinks:before-visit event', () => {
+      let confirmSpy
+
+      beforeEach(() => {
+        confirmSpy = jest.spyOn(window, 'confirm')
+      })
+
+      afterEach(() => {
+        // cleanup
+        confirmSpy.mockRestore()
+      })
+
       it("triggers a confirm popup up when leaving page and form has been interacted with", () => {
-        const checkbox = document.getElementById("test-checkbox")
-        const confirmSpy = jest.spyOn(window, 'confirm')
         confirmSpy.mockImplementation((msg) => {})
 
         // interact with the form
@@ -94,12 +172,9 @@ describe("UnsavedChangesController", () => {
         window.dispatchEvent(turbolinkEv)
 
         expect(confirmSpy).toHaveBeenCalled()
-
       })
 
       it("stays on the page if user clicks cancel on the confirm popup", () => {
-        const checkbox = document.getElementById("test-checkbox")
-        const confirmSpy = jest.spyOn(window, 'confirm')
         // return false to simulate a user clicking on cancel
         confirmSpy.mockImplementation((msg) => (false)) 
 
@@ -114,9 +189,6 @@ describe("UnsavedChangesController", () => {
 
         expect(confirmSpy).toHaveBeenCalled()
         expect(preventDefaultSpy).toHaveBeenCalled()
-
-        // cleanup
-        confirmSpy.mockRestore()
       })
     })
   })

From fd278e00865b44f754e2587161cd75fc54520eef Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 8 Feb 2023 16:28:41 +1100
Subject: [PATCH 5/7] Fix bug when submitting form triggered a warning and
 potentially left submit button disable

jquery-ujs automatically disable submit button when submitting the form.
If one choose cancel on the leaving page warning, then the submit buttons
end up in a disable state, with no way to re enable them. This fix
prevent the warning from being triggered when submitting the form, so
we can't end up in the scenario described.
---
 .../order_cycles/checkout_options.html.haml   |  2 +-
 .../controllers/unsaved_changes_controller.js | 18 ++++++--
 .../unsaved_changes_controller_test.js        | 45 +++++++++++++++++--
 3 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml
index 9138c14565..cfd7926990 100644
--- a/app/views/admin/order_cycles/checkout_options.html.haml
+++ b/app/views/admin/order_cycles/checkout_options.html.haml
@@ -3,7 +3,7 @@
 - content_for :page_title do
   = t :edit_order_cycle
 
-= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
+= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
 
   = render 'wizard_progress'
 
diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
index 7e2bc5055f..b3e042a84a 100644
--- a/app/webpacker/controllers/unsaved_changes_controller.js
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -6,7 +6,7 @@ import { Controller } from "stimulus";
 // - with beforeunload event :
 //    <form
 //      data-controller="unsaved-changes"
-//      data-action="beforeunload@window->unsaved-changes#leavingPage"
+//      data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
 //      <input data-action="change->unsaved-changes#formIsChanged" />
@@ -15,14 +15,19 @@ import { Controller } from "stimulus";
 // - with turbolinks :
 //    <form
 //      data-controller="unsaved-changes"
-//      data-action="turbolinks:before-visit@window->unsaved-changes#leavingPage"
+//      data-action="unsaved-changes#submit turbolinks:before-visit@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
 //      <input data-action="change->unsaved-changes#formIsChanged" />
 //    </form>
 //
-// You can also combine the two actions
-// You also need to add 'data-action="change->unsaved-changes#formIsChanged"' on all the form element
+// You can also combine the two event trigger ie : 
+//    <form
+//      data-controller="unsaved-changes"
+//      data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage"
+//      data-unsaved-changes-changed="true"
+//    >
+// You then need to add 'data-action="change->unsaved-changes#formIsChanged"' on all the form element
 // that can be interacted with
 //
 // Optional, you can add 'data-unsaved-changes-changed="true"' if you want to disable all
@@ -71,6 +76,11 @@ export default class extends Controller {
     }
   }
 
+  submit(event) {
+    // if we are submitting the form, we don't want to trigger a warning so set changed to false 
+    this.setChanged("false")
+  }
+
   setChanged(changed) {
     this.data.set("changed", changed);
   }
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
index f6d8544ff7..061f876db2 100644
--- a/spec/javascripts/stimulus/unsaved_changes_controller_test.js
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -15,7 +15,8 @@ describe("UnsavedChangesController", () => {
     document.body.innerHTML = `
       <form 
         id="test-form" 
-        data-controller="unsaved-changes" data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+        data-controller="unsaved-changes" 
+        data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
         data-unsaved-changes-changed="false"
       >
         <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
@@ -29,8 +30,9 @@ describe("UnsavedChangesController", () => {
       beforeEach(() => {
         document.body.innerHTML = `
           <form 
-            id="test-form" data-controller="unsaved-changes" 
-            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            id="test-form" 
+            data-controller="unsaved-changes" 
+            data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="true"
           >
@@ -53,7 +55,7 @@ describe("UnsavedChangesController", () => {
           <form 
             id="test-form" 
             data-controller="unsaved-changes" 
-            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="false"
           >
@@ -192,4 +194,39 @@ describe("UnsavedChangesController", () => {
       })
     })
   })
+
+  describe('#submit', () => {
+    let checkbox
+
+    beforeEach(() => {
+      // Add a mock I18n object to
+      const mockedT = jest.fn()
+      mockedT.mockImplementation((string) => (string))
+
+      global.I18n =  {
+        t: mockedT
+      }
+
+      checkbox = document.getElementById("test-checkbox")
+    })
+
+    afterEach(() => {
+      delete global.I18n
+    })
+
+    describe('when submiting the form', () => {
+      it("changed is set to true", () => {
+        const form = document.getElementById("test-form")
+
+        // interact with the form
+        checkbox.click()
+
+        // submit the form 
+        const submitEvent = new Event("submit")
+        form.dispatchEvent(submitEvent)
+
+        expect(form.dataset.unsavedChangesChanged).toBe("false")
+      })
+    })
+  })
 })

From 75ed68c9cbae4e256b4ec39e02a4d5698538c862 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 15 Feb 2023 10:30:52 +1100
Subject: [PATCH 6/7] UnsavedChangesController, automatically add onChange
 event handler to form elements

---
 .../admin/order_cycles/checkout_options.html.haml |  9 ++++-----
 .../controllers/unsaved_changes_controller.js     | 15 ++++++++++-----
 .../stimulus/unsaved_changes_controller_test.js   |  6 +++---
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml
index cfd7926990..4bbe6d59d8 100644
--- a/app/views/admin/order_cycles/checkout_options.html.haml
+++ b/app/views/admin/order_cycles/checkout_options.html.haml
@@ -4,7 +4,6 @@
   = t :edit_order_cycle
 
 = form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
-
   = render 'wizard_progress'
 
   %fieldset.no-border-bottom
@@ -26,7 +25,7 @@
               %td.text-center
                 - if distributor_shipping_methods.many?
                   %label
-                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll change->unsaved-changes#formIsChanged", "data-select-all-target": "all" }
+                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" }
                     = t(".select_all")
               %td
                 %em= distributor.name
@@ -37,7 +36,7 @@
                           distributor_shipping_method.id,
                           @order_cycle.distributor_shipping_methods.include?(distributor_shipping_method),
                           id: "order_cycle_selected_distributor_shipping_method_ids_#{distributor_shipping_method.id}",
-                          data: ({ "action" => "change->select-all#toggleCheckbox change->unsaved-changes#formIsChanged", "select-all-target" => "checkbox" } if distributor_shipping_method.shipping_method.frontend?)
+                          data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if distributor_shipping_method.shipping_method.frontend?)
                       = distributor_shipping_method.shipping_method.name
                 - distributor.shipping_methods.backend.each do |shipping_method|
                   %label.disabled
@@ -57,7 +56,7 @@
               %td.text-center
                 - if distributor_payment_methods.many?
                   %label
-                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll change->unsaved-changes#formIsChanged", "data-select-all-target": "all" }
+                    = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" }
                     = t(".select_all")
               %td
                 %em= distributor.name
@@ -68,7 +67,7 @@
                           distributor_payment_method.id,
                           @order_cycle.distributor_payment_methods.include?(distributor_payment_method),
                           id: "order_cycle_selected_distributor_payment_method_ids_#{distributor_payment_method.id}",
-                          data: ({ "action" => "change->select-all#toggleCheckbox change->unsaved-changes#formIsChanged", "select-all-target" => "checkbox" } if distributor_payment_method.payment_method.frontend?)
+                          data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if distributor_payment_method.payment_method.frontend?)
                       = distributor_payment_method.payment_method.name
                 - distributor.payment_methods.inactive_or_backend.each do |payment_method|
                   %label.disabled
diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
index b3e042a84a..5381c06368 100644
--- a/app/webpacker/controllers/unsaved_changes_controller.js
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -21,20 +21,25 @@ import { Controller } from "stimulus";
 //      <input data-action="change->unsaved-changes#formIsChanged" />
 //    </form>
 //
-// You can also combine the two event trigger ie : 
+// You can also combine the two event trigger ie :
 //    <form
 //      data-controller="unsaved-changes"
 //      data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
-// You then need to add 'data-action="change->unsaved-changes#formIsChanged"' on all the form element
-// that can be interacted with
 //
 // Optional, you can add 'data-unsaved-changes-changed="true"' if you want to disable all
 // submit buttons when the form hasn't been interacted with
 //
 export default class extends Controller {
   connect() {
+    // add onChange event to all form element
+    this.element
+      .querySelectorAll("input, select, textarea")
+      .forEach((input) => {
+        input.addEventListener("change", this.formIsChanged.bind(this));
+      });
+
     // disable submit button when first loading the page
     if (!this.isFormChanged() && this.isSubmitButtonDisabled()) {
       this.disableButtons();
@@ -77,8 +82,8 @@ export default class extends Controller {
   }
 
   submit(event) {
-    // if we are submitting the form, we don't want to trigger a warning so set changed to false 
-    this.setChanged("false")
+    // if we are submitting the form, we don't want to trigger a warning so set changed to false
+    this.setChanged("false");
   }
 
   setChanged(changed) {
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
index 061f876db2..c70b77a9e9 100644
--- a/spec/javascripts/stimulus/unsaved_changes_controller_test.js
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -19,7 +19,7 @@ describe("UnsavedChangesController", () => {
         data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
         data-unsaved-changes-changed="false"
       >
-        <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+        <input id="test-checkbox" type="checkbox" />
         <input id="test-submit" type="submit"/>
       </form>
     `
@@ -36,7 +36,7 @@ describe("UnsavedChangesController", () => {
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="true"
           >
-            <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+            <input id="test-checkbox" type="checkbox" />
             <input id="test-submit" type="submit"/>
           </form>
         `
@@ -59,7 +59,7 @@ describe("UnsavedChangesController", () => {
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="false"
           >
-            <input id="test-checkbox" type="checkbox" data-action="change->unsaved-changes#formIsChanged"/>
+            <input id="test-checkbox" type="checkbox" />
             <input id="test-submit" type="submit"/>
           </form>
         `

From eb67340c52432104c97b857887a3c8772b8e8637 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 15 Feb 2023 10:51:23 +1100
Subject: [PATCH 7/7] Simplify usage of UnsavedChangesController

Remove the need to manually bind handleSubmit to onSubmit event
---
 .../admin/order_cycles/checkout_options.html.haml    |  2 +-
 .../controllers/unsaved_changes_controller.js        | 12 +++++++-----
 .../stimulus/unsaved_changes_controller_test.js      |  8 ++++----
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml
index 4bbe6d59d8..80d58be8c6 100644
--- a/app/views/admin/order_cycles/checkout_options.html.haml
+++ b/app/views/admin/order_cycles/checkout_options.html.haml
@@ -3,7 +3,7 @@
 - content_for :page_title do
   = t :edit_order_cycle
 
-= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
+= form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" , data: { controller: 'unsaved-changes', action: 'beforeunload@window->unsaved-changes#leavingPage', 'unsaved-changes-changed': "false" } } do |f|
   = render 'wizard_progress'
 
   %fieldset.no-border-bottom
diff --git a/app/webpacker/controllers/unsaved_changes_controller.js b/app/webpacker/controllers/unsaved_changes_controller.js
index 5381c06368..afdb7ea8eb 100644
--- a/app/webpacker/controllers/unsaved_changes_controller.js
+++ b/app/webpacker/controllers/unsaved_changes_controller.js
@@ -6,7 +6,7 @@ import { Controller } from "stimulus";
 // - with beforeunload event :
 //    <form
 //      data-controller="unsaved-changes"
-//      data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage"
+//      data-action="beforeunload@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
 //      <input data-action="change->unsaved-changes#formIsChanged" />
@@ -15,7 +15,7 @@ import { Controller } from "stimulus";
 // - with turbolinks :
 //    <form
 //      data-controller="unsaved-changes"
-//      data-action="unsaved-changes#submit turbolinks:before-visit@window->unsaved-changes#leavingPage"
+//      data-action="turbolinks:before-visit@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
 //      <input data-action="change->unsaved-changes#formIsChanged" />
@@ -24,11 +24,11 @@ import { Controller } from "stimulus";
 // You can also combine the two event trigger ie :
 //    <form
 //      data-controller="unsaved-changes"
-//      data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage"
+//      data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage"
 //      data-unsaved-changes-changed="true"
 //    >
 //
-// Optional, you can add 'data-unsaved-changes-changed="true"' if you want to disable all
+// Optional, you can add 'data-unsaved-changes-disable-submit-button="true"' if you want to disable all
 // submit buttons when the form hasn't been interacted with
 //
 export default class extends Controller {
@@ -40,6 +40,8 @@ export default class extends Controller {
         input.addEventListener("change", this.formIsChanged.bind(this));
       });
 
+    this.element.addEventListener("submit", this.handleSubmit.bind(this));
+
     // disable submit button when first loading the page
     if (!this.isFormChanged() && this.isSubmitButtonDisabled()) {
       this.disableButtons();
@@ -81,7 +83,7 @@ export default class extends Controller {
     }
   }
 
-  submit(event) {
+  handleSubmit(event) {
     // if we are submitting the form, we don't want to trigger a warning so set changed to false
     this.setChanged("false");
   }
diff --git a/spec/javascripts/stimulus/unsaved_changes_controller_test.js b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
index c70b77a9e9..9379c3745d 100644
--- a/spec/javascripts/stimulus/unsaved_changes_controller_test.js
+++ b/spec/javascripts/stimulus/unsaved_changes_controller_test.js
@@ -16,7 +16,7 @@ describe("UnsavedChangesController", () => {
       <form 
         id="test-form" 
         data-controller="unsaved-changes" 
-        data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+        data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
         data-unsaved-changes-changed="false"
       >
         <input id="test-checkbox" type="checkbox" />
@@ -32,7 +32,7 @@ describe("UnsavedChangesController", () => {
           <form 
             id="test-form" 
             data-controller="unsaved-changes" 
-            data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="true"
           >
@@ -55,7 +55,7 @@ describe("UnsavedChangesController", () => {
           <form 
             id="test-form" 
             data-controller="unsaved-changes" 
-            data-action="unsaved-changes#submit beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
+            data-action="beforeunload@window->unsaved-changes#leavingPage turbolinks:before-visit@window->unsaved-changes#leavingPage" 
             data-unsaved-changes-changed="false" 
             data-unsaved-changes-disable-submit-button="false"
           >
@@ -195,7 +195,7 @@ describe("UnsavedChangesController", () => {
     })
   })
 
-  describe('#submit', () => {
+  describe('#handleSubmit', () => {
     let checkbox
 
     beforeEach(() => {