From 75a9653de3e23472eab7a081e00a18f28785b1d2 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Tue, 29 Oct 2024 17:02:39 -0400 Subject: [PATCH 01/10] Adds event listeners to listen for unsaved edits and alerting the user if there are unsaved edits --- app/assets/javascripts/stories.js | 42 +++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 5e7da51b..4b726ae5 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -25,8 +25,50 @@ document.addEventListener("DOMContentLoaded", () => { debounceTimer = window.setTimeout(updateMarkdown, 300); }); }); + + const form = document.querySelector('.edit_story'); + const backButton = document.getElementById('back'); + const logo = document.getElementById('logo'); + let isDirty = false; + + // Mark the form as dirty when any input changes + form.addEventListener('input', function () { + isDirty = true; + }); + + // Attach a click event to the custom back button + [backButton, logo].forEach(element => { + element.addEventListener('click', function (event) { + if (isDirty) { + const confirmLeave = confirm("You have unsaved changes. Are you sure you want to go back?"); + if (!confirmLeave) { + // Prevent navigation if the user chooses not to leave + event.preventDefault(); + } else { + // Optionally, reset isDirty if leaving + isDirty = false; + } + } + }) + }); + + // Reset isDirty on form submission + form.addEventListener('submit', function () { + isDirty = false; + }); + + if (isDirty) { + window.addEventListener("beforeunload", warnUserifUnsavedEdits); + } else { + window.removeEventListener("beforeunload", warnUserifUnsavedEdits); + } }); +function warnUserifUnsavedEdits(event) { + event.preventDefault(); + event.returnValue = ''; +} + function updateStatusButton(color, status) { const button = document.querySelector(".story-title .dropdown-wrapper > button"); button.className = `button ${color}`; From bf7f0dca08fef2c17a0f8f629917d2d504c5cfe4 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Wed, 30 Oct 2024 10:09:22 -0400 Subject: [PATCH 02/10] Adds spec to test for presence of alert when we try to navigate away from an unsaved edited story --- spec/features/stories_manage_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/features/stories_manage_spec.rb b/spec/features/stories_manage_spec.rb index 5048201f..22cf0c98 100644 --- a/spec/features/stories_manage_spec.rb +++ b/spec/features/stories_manage_spec.rb @@ -68,6 +68,17 @@ expect(page).to have_content "Story updated!" end + it "alerts me when I try to navigate away from the page without saving my edits", js: true do + visit project_path(id: project.id) + click_button "More actions" + click_link "Edit" + fill_in "story[title]", with: "As a user, I want to edit stories" + click_link "Back" + alert_text = page.driver.browser.switch_to.alert.text + + expect(alert_text).to eq "You have unsaved changes. Are you sure you want to go back?" + end + it "allows me to delete a story" do visit project_path(id: project.id) From 2dc08d00cf893ea84619d7b17d375e5842b54b32 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Thu, 7 Nov 2024 15:03:13 -0500 Subject: [PATCH 03/10] Refactored the code to only add the event listeners on the form if the form existed. Also created a function to add and remove the beforeunload event listener --- app/assets/javascripts/stories.js | 35 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 4b726ae5..f06b95e6 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -26,19 +26,28 @@ document.addEventListener("DOMContentLoaded", () => { }); }); - const form = document.querySelector('.edit_story'); - const backButton = document.getElementById('back'); - const logo = document.getElementById('logo'); + const form = document.querySelector(".edit_story"); + const backButton = document.getElementById("back"); + const logo = document.getElementById("logo"); let isDirty = false; - // Mark the form as dirty when any input changes - form.addEventListener('input', function () { - isDirty = true; - }); + if (form) { + // Mark the form as dirty when any input changes + form.addEventListener("input", function () { + isDirty = true; + addBeforeUnloadEventListener(isDirty); + }); + + // Reset isDirty on form submission + form.addEventListener("submit", function () { + isDirty = false; + addBeforeUnloadEventListener(isDirty); + }); + } // Attach a click event to the custom back button [backButton, logo].forEach(element => { - element.addEventListener('click', function (event) { + element.addEventListener("click", function (event) { if (isDirty) { const confirmLeave = confirm("You have unsaved changes. Are you sure you want to go back?"); if (!confirmLeave) { @@ -47,22 +56,20 @@ document.addEventListener("DOMContentLoaded", () => { } else { // Optionally, reset isDirty if leaving isDirty = false; + addBeforeUnloadEventListener(isDirty) } } }) }); +}); - // Reset isDirty on form submission - form.addEventListener('submit', function () { - isDirty = false; - }); - +function addBeforeUnloadEventListener(isDirty) { if (isDirty) { window.addEventListener("beforeunload", warnUserifUnsavedEdits); } else { window.removeEventListener("beforeunload", warnUserifUnsavedEdits); } -}); +} function warnUserifUnsavedEdits(event) { event.preventDefault(); From 18ade801fc27bffd0ddf9bf72d2c3235503ffd83 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Fri, 5 Sep 2025 19:47:12 -0400 Subject: [PATCH 04/10] DT-340: Edit tests so that it tests all scenarios --- spec/features/stories_manage_spec.rb | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/spec/features/stories_manage_spec.rb b/spec/features/stories_manage_spec.rb index 22cf0c98..f9f59501 100644 --- a/spec/features/stories_manage_spec.rb +++ b/spec/features/stories_manage_spec.rb @@ -72,11 +72,26 @@ visit project_path(id: project.id) click_button "More actions" click_link "Edit" - fill_in "story[title]", with: "As a user, I want to edit stories" + click_link "Back" - alert_text = page.driver.browser.switch_to.alert.text + assert_current_path project_path(id: project.id) + + click_button "More actions" + click_link "Edit" + + fill_in "story[title]", with: "As a user, I want to edit stories" + + dismiss_confirm("You have unsaved changes. Are you sure you want to go back?") do + find("#logo").click + end + + assert_current_path edit_project_story_path(project, story) + + accept_confirm("You have unsaved changes. Are you sure you want to go back?") do + click_link "Back" + end - expect(alert_text).to eq "You have unsaved changes. Are you sure you want to go back?" + assert_current_path project_path(id: project.id) end it "allows me to delete a story" do From 2fd263d96e1205db8406c48e89df02b37eddcd22 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Fri, 5 Sep 2025 20:01:39 -0400 Subject: [PATCH 05/10] Update app/assets/javascripts/stories.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Juan Vásquez --- app/assets/javascripts/stories.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index f06b95e6..6a607813 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -50,13 +50,13 @@ document.addEventListener("DOMContentLoaded", () => { element.addEventListener("click", function (event) { if (isDirty) { const confirmLeave = confirm("You have unsaved changes. Are you sure you want to go back?"); - if (!confirmLeave) { - // Prevent navigation if the user chooses not to leave - event.preventDefault(); - } else { + if (confirmLeave) { // Optionally, reset isDirty if leaving isDirty = false; addBeforeUnloadEventListener(isDirty) + } else { + // Prevent navigation if the user chooses not to leave + event.preventDefault(); } } }) From b6e51b9e317610d3d2f03852a16d863721637b86 Mon Sep 17 00:00:00 2001 From: "G. Torres" Date: Fri, 5 Sep 2025 20:01:45 -0400 Subject: [PATCH 06/10] Update app/assets/javascripts/stories.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Juan Vásquez --- app/assets/javascripts/stories.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 6a607813..3fdcc5ba 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -71,7 +71,7 @@ function addBeforeUnloadEventListener(isDirty) { } } -function warnUserifUnsavedEdits(event) { +function warnUserIfUnsavedEdits(event) { event.preventDefault(); event.returnValue = ''; } From 255df35b78cb774707a969e43b23a8d847c386a0 Mon Sep 17 00:00:00 2001 From: Juan Vasquez Date: Tue, 16 Sep 2025 09:03:16 -0600 Subject: [PATCH 07/10] Fix method name --- app/assets/javascripts/stories.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 3fdcc5ba..5d4ecd85 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -65,9 +65,9 @@ document.addEventListener("DOMContentLoaded", () => { function addBeforeUnloadEventListener(isDirty) { if (isDirty) { - window.addEventListener("beforeunload", warnUserifUnsavedEdits); + window.addEventListener("beforeunload", warnUserIfUnsavedEdits); } else { - window.removeEventListener("beforeunload", warnUserifUnsavedEdits); + window.removeEventListener("beforeunload", warnUserIfUnsavedEdits); } } From 9b2da2980ed45071a82d0c8fad31699c744ac8e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Mon, 20 Jul 2026 18:21:45 -0600 Subject: [PATCH 08/10] Fix unsaved-changes detection to compare against initial form state - Track dirtiness by comparing the current form serialization to a snapshot taken on load, so reverting an edit (typing then deleting it) correctly clears the warning instead of leaving the form permanently 'dirty'. - Also listen for 'change' so status-select edits are detected, and guard the back/logo click handlers against missing elements on non-edit pages. - Add a feature spec covering the revert-to-original case. --- app/assets/javascripts/stories.js | 22 ++++++++++++++++++---- spec/features/stories_manage_spec.rb | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 5d4ecd85..1afc0e3b 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -32,11 +32,19 @@ document.addEventListener("DOMContentLoaded", () => { let isDirty = false; if (form) { - // Mark the form as dirty when any input changes - form.addEventListener("input", function () { - isDirty = true; + // Snapshot the initial form state so we can compare against it. Tracking a + // plain "changed at least once" flag reported the form as dirty even after + // the user undid their edits (e.g. typed some text and then deleted it). + const initialState = serializeForm(form); + + const refreshDirtyState = function () { + isDirty = serializeForm(form) !== initialState; addBeforeUnloadEventListener(isDirty); - }); + }; + + // "input" covers text fields; "change" covers selects like the status. + form.addEventListener("input", refreshDirtyState); + form.addEventListener("change", refreshDirtyState); // Reset isDirty on form submission form.addEventListener("submit", function () { @@ -47,6 +55,8 @@ document.addEventListener("DOMContentLoaded", () => { // Attach a click event to the custom back button [backButton, logo].forEach(element => { + if (!element) return; + element.addEventListener("click", function (event) { if (isDirty) { const confirmLeave = confirm("You have unsaved changes. Are you sure you want to go back?"); @@ -63,6 +73,10 @@ document.addEventListener("DOMContentLoaded", () => { }); }); +function serializeForm(form) { + return new URLSearchParams(new FormData(form)).toString(); +} + function addBeforeUnloadEventListener(isDirty) { if (isDirty) { window.addEventListener("beforeunload", warnUserIfUnsavedEdits); diff --git a/spec/features/stories_manage_spec.rb b/spec/features/stories_manage_spec.rb index f9f59501..2e711a61 100644 --- a/spec/features/stories_manage_spec.rb +++ b/spec/features/stories_manage_spec.rb @@ -94,6 +94,20 @@ assert_current_path project_path(id: project.id) end + it "does not alert me when I revert my edits back to their original values", js: true do + visit edit_project_story_path(project, story) + original_title = find_field("story[title]").value + + # Make a change and then undo it, returning the form to its initial state. + fill_in "story[title]", with: "A temporary edit" + fill_in "story[title]", with: original_title + + # No unsaved-changes confirm should appear, so navigation happens directly. + click_link "Back" + + assert_current_path project_path(id: project.id) + end + it "allows me to delete a story" do visit project_path(id: project.id) From f3a4fed4b141c1585bd05d5532b40f749b4b5d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Tue, 21 Jul 2026 12:26:07 -0600 Subject: [PATCH 09/10] Unify unsaved-changes wording and init on turbolinks:load - Route every in-app link that leaves the edit page through one confirm with a single shared message constant, instead of hard-coding only Back and the logo. The native beforeunload prompt (reload / tab close) still uses the browser's own text, since browsers ignore custom strings there. - Initialize on turbolinks:load instead of DOMContentLoaded, matching project.js, so the guard (and the markdown preview) also initialize on Turbolinks visits. Module-scoped state lets the delegated click handler be added by reference and de-duplicated across visits. --- app/assets/javascripts/stories.js | 125 +++++++++++++++++---------- spec/features/stories_manage_spec.rb | 4 +- 2 files changed, 81 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/stories.js b/app/assets/javascripts/stories.js index 1afc0e3b..df0f4438 100644 --- a/app/assets/javascripts/stories.js +++ b/app/assets/javascripts/stories.js @@ -1,4 +1,20 @@ -document.addEventListener("DOMContentLoaded", () => { +// Single source of truth for the unsaved-changes wording. It is shown for +// every in-app link the user might leave through. The native beforeunload +// prompt (reload / closing the tab) always uses the browser's own generic +// text; browsers ignore any custom string there, so that one case aside, this +// keeps the wording consistent everywhere we can control it. +const UNSAVED_CHANGES_MESSAGE = + "You have unsaved changes. Are you sure you want to leave?"; + +// Init on turbolinks:load (not DOMContentLoaded) so it also runs on Turbolinks +// visits, matching project.js. reloadable state lives at module scope so the +// delegated click handler can be added by reference and de-duplicated across +// visits instead of stacking a new listener each time. +let editForm = null; +let editFormInitialState = null; +let editFormDirty = false; + +document.addEventListener("turbolinks:load", () => { document.querySelectorAll("[data-has-preview]").forEach((element) => { let debounceTimer; @@ -26,52 +42,69 @@ document.addEventListener("DOMContentLoaded", () => { }); }); - const form = document.querySelector(".edit_story"); - const backButton = document.getElementById("back"); - const logo = document.getElementById("logo"); - let isDirty = false; - - if (form) { - // Snapshot the initial form state so we can compare against it. Tracking a - // plain "changed at least once" flag reported the form as dirty even after - // the user undid their edits (e.g. typed some text and then deleted it). - const initialState = serializeForm(form); - - const refreshDirtyState = function () { - isDirty = serializeForm(form) !== initialState; - addBeforeUnloadEventListener(isDirty); - }; - - // "input" covers text fields; "change" covers selects like the status. - form.addEventListener("input", refreshDirtyState); - form.addEventListener("change", refreshDirtyState); - - // Reset isDirty on form submission - form.addEventListener("submit", function () { - isDirty = false; - addBeforeUnloadEventListener(isDirty); - }); + initUnsavedChangesGuard(); +}); + +function initUnsavedChangesGuard() { + editForm = document.querySelector(".edit_story"); + editFormDirty = false; + // Start each visit from a clean slate; drop any beforeunload guard carried + // over from a previous page. + addBeforeUnloadEventListener(false); + + if (!editForm) { + return; } - // Attach a click event to the custom back button - [backButton, logo].forEach(element => { - if (!element) return; - - element.addEventListener("click", function (event) { - if (isDirty) { - const confirmLeave = confirm("You have unsaved changes. Are you sure you want to go back?"); - if (confirmLeave) { - // Optionally, reset isDirty if leaving - isDirty = false; - addBeforeUnloadEventListener(isDirty) - } else { - // Prevent navigation if the user chooses not to leave - event.preventDefault(); - } - } - }) - }); -}); + // Snapshot the initial form state so we can compare against it. Tracking a + // plain "changed at least once" flag reported the form as dirty even after + // the user undid their edits (e.g. typed some text and then deleted it). + editFormInitialState = serializeForm(editForm); + + // "input" covers text fields; "change" covers selects like the status. + editForm.addEventListener("input", refreshEditFormDirtyState); + editForm.addEventListener("change", refreshEditFormDirtyState); + editForm.addEventListener("submit", clearEditFormDirtyState); + + // Same warning for every in-app link that leaves the page (Back, the logo, + // Sign out, ...), not just a couple of buttons. Added by reference so it is + // de-duplicated if turbolinks:load fires again. + document.addEventListener("click", confirmLeaveIfUnsavedEdits); +} + +function refreshEditFormDirtyState() { + editFormDirty = serializeForm(editForm) !== editFormInitialState; + addBeforeUnloadEventListener(editFormDirty); +} + +function clearEditFormDirtyState() { + editFormDirty = false; + addBeforeUnloadEventListener(false); +} + +function confirmLeaveIfUnsavedEdits(event) { + if (!editFormDirty) { + return; + } + + const link = event.target.closest("a[href]"); + if (!link) { + return; + } + + // Links that do not actually leave the page (new tab, in-page anchors, + // javascript: handlers) should not trigger the warning. + const href = link.getAttribute("href") || ""; + if (link.target === "_blank" || href.startsWith("#") || href.startsWith("javascript:")) { + return; + } + + if (window.confirm(UNSAVED_CHANGES_MESSAGE)) { + clearEditFormDirtyState(); + } else { + event.preventDefault(); + } +} function serializeForm(form) { return new URLSearchParams(new FormData(form)).toString(); @@ -87,7 +120,7 @@ function addBeforeUnloadEventListener(isDirty) { function warnUserIfUnsavedEdits(event) { event.preventDefault(); - event.returnValue = ''; + event.returnValue = UNSAVED_CHANGES_MESSAGE; } function updateStatusButton(color, status) { diff --git a/spec/features/stories_manage_spec.rb b/spec/features/stories_manage_spec.rb index 2e711a61..cd4fb12f 100644 --- a/spec/features/stories_manage_spec.rb +++ b/spec/features/stories_manage_spec.rb @@ -81,13 +81,13 @@ fill_in "story[title]", with: "As a user, I want to edit stories" - dismiss_confirm("You have unsaved changes. Are you sure you want to go back?") do + dismiss_confirm("You have unsaved changes. Are you sure you want to leave?") do find("#logo").click end assert_current_path edit_project_story_path(project, story) - accept_confirm("You have unsaved changes. Are you sure you want to go back?") do + accept_confirm("You have unsaved changes. Are you sure you want to leave?") do click_link "Back" end From 681a66fb3e90246fe178be9bc991305249b53d90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Tue, 21 Jul 2026 13:14:57 -0600 Subject: [PATCH 10/10] Fix flaky clone sub-projects spec ordering Assert the cloned sub-projects by set of titles (contain_exactly) instead of positional index. The projects association has no default order, so indexing into it was order-dependent and failed on the build-rails-next lane when the database returned the rows in a different order. --- spec/features/projects_manage_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/projects_manage_spec.rb b/spec/features/projects_manage_spec.rb index 38e0e992..f223ba14 100644 --- a/spec/features/projects_manage_spec.rb +++ b/spec/features/projects_manage_spec.rb @@ -266,9 +266,9 @@ last_project = Project.parents.last expect(last_project.id).not_to eq(project.id) - expect(last_project.projects.count).to eq 2 - expect(last_project.projects[0].title).to eq sub_project1.title - expect(last_project.projects[1].title).to eq sub_project3.title + expect(last_project.projects.map(&:title)).to contain_exactly( + sub_project1.title, sub_project3.title + ) end it "allows to select/unselect all sub-projects at once" do