From a56c4e09c011f10626c1c7e2ddccddd225d9ddb6 Mon Sep 17 00:00:00 2001 From: John Costa Date: Sat, 19 Sep 2026 21:20:38 -0700 Subject: [PATCH 1/6] Keep download buttons enabled after the file downloads download_button_to renders a GET link to a send_data endpoint, so clicking it downloads a file without loading a new page. rails-ujs' data-disable-with only re-enables the link on the next page load, which never comes, and the button stays stuck showing "Please wait...". Opt download buttons out of data-disable-with, the same way modal_button_to was fixed for #5632. This covers the annual report export and every other CSV export button rendered through the helper. Resolves #5691 --- app/helpers/ui_helper.rb | 6 ++++- spec/helpers/ui_helper_spec.rb | 32 +++++++++++++++++++++++ spec/system/annual_reports_system_spec.rb | 13 +++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb index 32325d9232..d8862ab965 100644 --- a/app/helpers/ui_helper.rb +++ b/app/helpers/ui_helper.rb @@ -89,7 +89,11 @@ def clear_filter_button(options = {}) end def download_button_to(link, options = {}) - _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options) + options = { icon: "download", type: "info", text: "Download", size: "md" }.merge(options) + # A download link serves a file instead of a new page, so rails-ujs never re-enables the + # button and it stays stuck on "Please wait..." (see #5691; same cause as #5632 below). + options[:data] = { disable_with: nil }.merge(options[:data] || {}) + _link_to link, options end def edit_button_to(link, options = {}, properties = {}) diff --git a/spec/helpers/ui_helper_spec.rb b/spec/helpers/ui_helper_spec.rb index 35e1a96359..ada2b008e1 100644 --- a/spec/helpers/ui_helper_spec.rb +++ b/spec/helpers/ui_helper_spec.rb @@ -138,6 +138,38 @@ end end + describe 'download_button_to' do + context 'with default options' do + subject { helper.download_button_to("/reports/annual_reports/2025.csv", text: "Export Report") } + + it 'renders a download link' do + button = Nokogiri::HTML(subject).css("a").first + expect(button).to_not be_nil + expect(button.attributes["href"].value).to eq("/reports/annual_reports/2025.csv") + expect(button.attributes["class"].value).to include("btn btn-info") + expect(button.text.strip).to eq("Export Report") + end + + # Regression test for #5691: a download link serves a file without a page load, so it must + # not carry rails-ujs' `data-disable-with` attribute, otherwise it stays stuck on + # "Please wait..." after the file has downloaded. + it 'does not set data-disable-with' do + button = Nokogiri::HTML(subject).css("a").first + expect(button.attributes["data-disable-with"]).to be_nil + end + end + + context 'when the caller passes custom data' do + subject { helper.download_button_to("/donations.csv", text: "Export Donations", data: {test: "test"}) } + + it 'keeps the custom data and still omits data-disable-with' do + button = Nokogiri::HTML(subject).css("a").first + expect(button.attributes["data-test"].value).to eq("test") + expect(button.attributes["data-disable-with"]).to be_nil + end + end + end + describe 'modal_button_to' do context 'with default options' do subject { helper.modal_button_to("#newRequest", text: "New Quantity Request", icon: "plus", type: "success") } diff --git a/spec/system/annual_reports_system_spec.rb b/spec/system/annual_reports_system_spec.rb index 5b26888cb2..be6c532f00 100644 --- a/spec/system/annual_reports_system_spec.rb +++ b/spec/system/annual_reports_system_spec.rb @@ -33,5 +33,18 @@ expect(page).to have_content("Year End Summary") expect(page).to have_content("Period Supplies") end + + it "keeps the Export Report button label after exporting" do + year = 1.year.ago.year + click_on(year.to_s) + expect(page).to have_selector("h1", text: "Diapers") + + click_on("Export Report") + wait_for_download + + button = find("a[href='#{reports_annual_report_path(year, format: :csv)}']") + expect(button).to have_text("Export Report") + expect(button).to have_no_text("Please wait...") + end end end From a5d049f1b4ce520c3a371cb411f5c280fcf84a55 Mon Sep 17 00:00:00 2001 From: John Costa Date: Sun, 20 Sep 2026 11:35:54 -0700 Subject: [PATCH 2/6] Retrigger CI From 384c1c5935af751fd0eff0b67c2fd0bc8d262656 Mon Sep 17 00:00:00 2001 From: John Costa Date: Sun, 20 Sep 2026 11:45:15 -0700 Subject: [PATCH 3/6] Retrigger CI From 8060d8299692735487881b044933215266dbfb59 Mon Sep 17 00:00:00 2001 From: John Costa Date: Fri, 25 Sep 2026 12:29:08 -0700 Subject: [PATCH 4/6] Keep the download debounce and re-enable the button after a delay --- app/helpers/ui_helper.rb | 7 +++--- .../controllers/download_link_controller.js | 23 +++++++++++++++++++ spec/helpers/ui_helper_spec.rb | 14 +++++------ spec/system/annual_reports_system_spec.rb | 3 ++- 4 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 app/javascript/controllers/download_link_controller.js diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb index d8862ab965..0a9c5cd0de 100644 --- a/app/helpers/ui_helper.rb +++ b/app/helpers/ui_helper.rb @@ -89,11 +89,10 @@ def clear_filter_button(options = {}) end def download_button_to(link, options = {}) - options = { icon: "download", type: "info", text: "Download", size: "md" }.merge(options) # A download link serves a file instead of a new page, so rails-ujs never re-enables the - # button and it stays stuck on "Please wait..." (see #5691; same cause as #5632 below). - options[:data] = { disable_with: nil }.merge(options[:data] || {}) - _link_to link, options + # button on its own (#5691). The download-link controller re-enables it after a delay, which + # keeps the double-click protection for slow reports without leaving it stuck. + _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options), { data: { controller: "download-link" } } end def edit_button_to(link, options = {}, properties = {}) diff --git a/app/javascript/controllers/download_link_controller.js b/app/javascript/controllers/download_link_controller.js new file mode 100644 index 0000000000..f78d4f8c63 --- /dev/null +++ b/app/javascript/controllers/download_link_controller.js @@ -0,0 +1,23 @@ +import { Controller } from "@hotwired/stimulus" +import Rails from "@rails/ujs" + +// Connects to data-controller="download-link" +// A download link serves a file instead of a new page, so rails-ujs never re-enables it after +// `data-disable-with` kicks in (#5691). Re-enable it after a delay instead: long enough to stop +// double-clicks from starting a second copy of a slow report, short enough not to stay stuck. +const REENABLE_DELAY_MS = 5000 + +export default class extends Controller { + connect() { + this.reenable = () => { + clearTimeout(this.timeout) + this.timeout = setTimeout(() => Rails.enableElement(this.element), REENABLE_DELAY_MS) + } + this.element.addEventListener("click", this.reenable) + } + + disconnect() { + this.element.removeEventListener("click", this.reenable) + clearTimeout(this.timeout) + } +} diff --git a/spec/helpers/ui_helper_spec.rb b/spec/helpers/ui_helper_spec.rb index ada2b008e1..a8b9eafd9a 100644 --- a/spec/helpers/ui_helper_spec.rb +++ b/spec/helpers/ui_helper_spec.rb @@ -150,22 +150,22 @@ expect(button.text.strip).to eq("Export Report") end - # Regression test for #5691: a download link serves a file without a page load, so it must - # not carry rails-ujs' `data-disable-with` attribute, otherwise it stays stuck on - # "Please wait..." after the file has downloaded. - it 'does not set data-disable-with' do + # Regression test for #5691: keep rails-ujs' double-click protection, and hand the link to + # the download-link controller, which re-enables it since no page load will. + it 'disables on click and uses the download-link controller' do button = Nokogiri::HTML(subject).css("a").first - expect(button.attributes["data-disable-with"]).to be_nil + expect(button.attributes["data-disable-with"].value).to eq("Please wait...") + expect(button.attributes["data-controller"].value).to eq("download-link") end end context 'when the caller passes custom data' do subject { helper.download_button_to("/donations.csv", text: "Export Donations", data: {test: "test"}) } - it 'keeps the custom data and still omits data-disable-with' do + it 'keeps the custom data alongside the download-link controller' do button = Nokogiri::HTML(subject).css("a").first expect(button.attributes["data-test"].value).to eq("test") - expect(button.attributes["data-disable-with"]).to be_nil + expect(button.attributes["data-controller"].value).to eq("download-link") end end end diff --git a/spec/system/annual_reports_system_spec.rb b/spec/system/annual_reports_system_spec.rb index be6c532f00..cfaad8f519 100644 --- a/spec/system/annual_reports_system_spec.rb +++ b/spec/system/annual_reports_system_spec.rb @@ -42,8 +42,9 @@ click_on("Export Report") wait_for_download + # The download-link controller re-enables the button a few seconds after the click. button = find("a[href='#{reports_annual_report_path(year, format: :csv)}']") - expect(button).to have_text("Export Report") + expect(button).to have_text("Export Report", wait: 10) expect(button).to have_no_text("Please wait...") end end From a2d940f489697e30ba61a20e8f3d2c44e3d8e7d8 Mon Sep 17 00:00:00 2001 From: John Costa Date: Thu, 1 Oct 2026 10:45:22 -0700 Subject: [PATCH 5/6] Use one download controller for every download button Replace csv_download_controller and download_link_controller with a single download controller. Download buttons now fetch the file in the background, stay on "Please wait..." until it arrives, then re-enable. Redirects are detected with redirect: "manual" and handed to the browser, a signed-out 401 reloads the page, and other failures show a toast. --- app/helpers/ui_helper.rb | 6 +- .../controllers/csv_download_controller.js | 37 --------- .../controllers/download_controller.js | 77 +++++++++++++++++++ .../controllers/download_link_controller.js | 23 ------ app/views/shared/_csv_download.html.erb | 4 +- spec/helpers/ui_helper_spec.rb | 11 +-- spec/requests/distributions_requests_spec.rb | 4 +- spec/system/annual_reports_system_spec.rb | 4 +- 8 files changed, 92 insertions(+), 74 deletions(-) delete mode 100644 app/javascript/controllers/csv_download_controller.js create mode 100644 app/javascript/controllers/download_controller.js delete mode 100644 app/javascript/controllers/download_link_controller.js diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb index 0a9c5cd0de..226f002f01 100644 --- a/app/helpers/ui_helper.rb +++ b/app/helpers/ui_helper.rb @@ -90,9 +90,9 @@ def clear_filter_button(options = {}) def download_button_to(link, options = {}) # A download link serves a file instead of a new page, so rails-ujs never re-enables the - # button on its own (#5691). The download-link controller re-enables it after a delay, which - # keeps the double-click protection for slow reports without leaving it stuck. - _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options), { data: { controller: "download-link" } } + # button on its own (#5691). The download controller fetches the file and re-enables the + # button once it arrives, which keeps the double-click protection without leaving it stuck. + _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options), { data: { controller: "download", action: "download#download" } } end def edit_button_to(link, options = {}, properties = {}) diff --git a/app/javascript/controllers/csv_download_controller.js b/app/javascript/controllers/csv_download_controller.js deleted file mode 100644 index 5b5051f663..0000000000 --- a/app/javascript/controllers/csv_download_controller.js +++ /dev/null @@ -1,37 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -// Connects to data-controller="csv-download" -// Fetches a CSV file in the background and triggers a browser download. -// -// Usage: -//
-// -export default class extends Controller { - static values = { url: String } - - connect() { - fetch(this.urlValue, { headers: { "X-Requested-With": "XMLHttpRequest" } }) - .then(response => { - if (!response.ok) return - const filename = this._extractFilename(response.headers.get("Content-Disposition")) - return response.blob().then(blob => ({ blob, filename })) - }) - .then(({ blob, filename }) => { - const objectUrl = URL.createObjectURL(blob) - const anchor = document.createElement("a") - anchor.href = objectUrl - anchor.download = filename || "report.csv" - anchor.click() - URL.revokeObjectURL(objectUrl) - }) - } - - _extractFilename(disposition) { - if (!disposition) return null - - const utf8Match = disposition.match(/filename\*=UTF-8''([^;\n]+)/i) - if (utf8Match) return decodeURIComponent(utf8Match[1]) - const plainMatch = disposition.match(/filename="?([^";\n]+)"?/i) - return plainMatch ? plainMatch[1] : null - } -} diff --git a/app/javascript/controllers/download_controller.js b/app/javascript/controllers/download_controller.js new file mode 100644 index 0000000000..5060f3886b --- /dev/null +++ b/app/javascript/controllers/download_controller.js @@ -0,0 +1,77 @@ +import { Controller } from "@hotwired/stimulus" +import Rails from "@rails/ujs" + +// Connects to data-controller="download" +// Fetches a file in the background and hands it to the browser as a download, so a download +// button can stay disabled until the file actually arrives (#5691). rails-ujs only re-enables +// a disabled link on the next page load, and a file download never triggers one. +// +// Usage: +// +//
(downloads on connect) +// +export default class extends Controller { + static values = { url: String } + + connect() { + if (this.hasUrlValue) this.fetchFile(this.urlValue) + } + + async download(event) { + // Let new-tab clicks through to the browser. These are the same ones rails-ujs skips, so the + // link is never left disabled. + if (event.metaKey || event.ctrlKey || event.button !== 0) return + event.preventDefault() + if (this.busy) return + + this.busy = true + Rails.disableElement(this.element) + try { + await this.fetchFile(this.element.href) + } finally { + Rails.enableElement(this.element) + this.busy = false + } + } + + async fetchFile(url) { + try { + // fetch never exposes a 3xx to JS: it either follows it or, in manual mode, returns an + // opaque response with no Location. Following it here would also use up the flash that + // export redirects (handle_csv_export) rely on, so let the browser make the request instead. + // That means a redirecting endpoint is requested twice, so it must not do work before redirecting. + const response = await fetch(url, { redirect: "manual", headers: { "X-Requested-With": "XMLHttpRequest" } }) + if (response.type === "opaqueredirect") { + window.location.href = url + return + } + // Signed out: reloading the page sends the user to the login screen. + if (response.status === 401) { + window.location.reload() + return + } + if (!response.ok) throw new Error(`Download failed with status ${response.status}`) + + const filename = this._extractFilename(response.headers.get("Content-Disposition")) + const objectUrl = URL.createObjectURL(await response.blob()) + const anchor = document.createElement("a") + anchor.href = objectUrl + anchor.download = filename || new URL(url, window.location.href).pathname.split("/").pop() + anchor.click() + // Revoking in the same tick can cancel the download in Safari. + setTimeout(() => URL.revokeObjectURL(objectUrl), 1000) + } catch (error) { + console.error(error) + window.toastr.error("Sorry, the download failed. Please try again.", "", { timeOut: 5000 }) + } + } + + _extractFilename(disposition) { + if (!disposition) return null + + const utf8Match = disposition.match(/filename\*=UTF-8''([^;\n]+)/i) + if (utf8Match) return decodeURIComponent(utf8Match[1]) + const plainMatch = disposition.match(/filename="?([^";\n]+)"?/i) + return plainMatch ? plainMatch[1] : null + } +} diff --git a/app/javascript/controllers/download_link_controller.js b/app/javascript/controllers/download_link_controller.js deleted file mode 100644 index f78d4f8c63..0000000000 --- a/app/javascript/controllers/download_link_controller.js +++ /dev/null @@ -1,23 +0,0 @@ -import { Controller } from "@hotwired/stimulus" -import Rails from "@rails/ujs" - -// Connects to data-controller="download-link" -// A download link serves a file instead of a new page, so rails-ujs never re-enables it after -// `data-disable-with` kicks in (#5691). Re-enable it after a delay instead: long enough to stop -// double-clicks from starting a second copy of a slow report, short enough not to stay stuck. -const REENABLE_DELAY_MS = 5000 - -export default class extends Controller { - connect() { - this.reenable = () => { - clearTimeout(this.timeout) - this.timeout = setTimeout(() => Rails.enableElement(this.element), REENABLE_DELAY_MS) - } - this.element.addEventListener("click", this.reenable) - } - - disconnect() { - this.element.removeEventListener("click", this.reenable) - clearTimeout(this.timeout) - } -} diff --git a/app/views/shared/_csv_download.html.erb b/app/views/shared/_csv_download.html.erb index ddf2cff4f7..f949f5142b 100644 --- a/app/views/shared/_csv_download.html.erb +++ b/app/views/shared/_csv_download.html.erb @@ -1,7 +1,7 @@ <% if flash[:trigger_csv_download] %> <% csv_url = "#{request.path}.csv#{"?#{request.query_string}" if request.query_string.present?}" %> -
<% end %> diff --git a/spec/helpers/ui_helper_spec.rb b/spec/helpers/ui_helper_spec.rb index a8b9eafd9a..09a3c48524 100644 --- a/spec/helpers/ui_helper_spec.rb +++ b/spec/helpers/ui_helper_spec.rb @@ -151,21 +151,22 @@ end # Regression test for #5691: keep rails-ujs' double-click protection, and hand the link to - # the download-link controller, which re-enables it since no page load will. - it 'disables on click and uses the download-link controller' do + # the download controller, which re-enables it once the file arrives since no page load will. + it 'disables on click and uses the download controller' do button = Nokogiri::HTML(subject).css("a").first expect(button.attributes["data-disable-with"].value).to eq("Please wait...") - expect(button.attributes["data-controller"].value).to eq("download-link") + expect(button.attributes["data-controller"].value).to eq("download") + expect(button.attributes["data-action"].value).to eq("download#download") end end context 'when the caller passes custom data' do subject { helper.download_button_to("/donations.csv", text: "Export Donations", data: {test: "test"}) } - it 'keeps the custom data alongside the download-link controller' do + it 'keeps the custom data alongside the download controller' do button = Nokogiri::HTML(subject).css("a").first expect(button.attributes["data-test"].value).to eq("test") - expect(button.attributes["data-controller"].value).to eq("download-link") + expect(button.attributes["data-controller"].value).to eq("download") end end end diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb index 8a093d1636..6a41b7f7e4 100644 --- a/spec/requests/distributions_requests_spec.rb +++ b/spec/requests/distributions_requests_spec.rb @@ -84,11 +84,11 @@ end context "with export_csv param" do - it "redirects then renders a csv-download stimulus controller to export CSV" do + it "redirects then renders a download stimulus controller to export CSV" do get distributions_path(export_csv: true, foo: "bar") expect(response).to redirect_to(distributions_path(foo: "bar")) follow_redirect! - expect(response.body).to include("data-controller=\"toast csv-download\"") + expect(response.body).to include("data-controller=\"toast download\"") expect(response.body).to include("distributions.csv?foo=bar") end end diff --git a/spec/system/annual_reports_system_spec.rb b/spec/system/annual_reports_system_spec.rb index cfaad8f519..59099dcedf 100644 --- a/spec/system/annual_reports_system_spec.rb +++ b/spec/system/annual_reports_system_spec.rb @@ -42,9 +42,9 @@ click_on("Export Report") wait_for_download - # The download-link controller re-enables the button a few seconds after the click. + # The download controller re-enables the button once the file has arrived. button = find("a[href='#{reports_annual_report_path(year, format: :csv)}']") - expect(button).to have_text("Export Report", wait: 10) + expect(button).to have_text("Export Report") expect(button).to have_no_text("Please wait...") end end From af162fb79467b53ea22c6ac6d7cdefb35feb6e41 Mon Sep 17 00:00:00 2001 From: John Costa Date: Wed, 14 Oct 2026 13:12:40 -0700 Subject: [PATCH 6/6] Make the download controller opt-in with remote: true Only the annual report export buttons from #5691 use it now; every other download_button_to keeps the default link behavior. --- app/helpers/ui_helper.rb | 10 ++++++---- .../reports/annual_reports/index.html.erb | 3 ++- .../reports/annual_reports/show.html.erb | 3 ++- spec/helpers/ui_helper_spec.rb | 19 ++++++++++--------- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb index 226f002f01..ba1c1f0832 100644 --- a/app/helpers/ui_helper.rb +++ b/app/helpers/ui_helper.rb @@ -88,11 +88,13 @@ def clear_filter_button(options = {}) cancel_button_to request.path, { size: "md", text: "Clear Filters" }.merge(options) end + # Pass remote: true to fetch the file in the background with the download controller. rails-ujs + # only re-enables a disabled link on the next page load, which a file download never triggers, so + # without it the button stays on "Please wait..." (#5691). The controller re-enables it once the + # file arrives. def download_button_to(link, options = {}) - # A download link serves a file instead of a new page, so rails-ujs never re-enables the - # button on its own (#5691). The download controller fetches the file and re-enables the - # button once it arrives, which keeps the double-click protection without leaving it stuck. - _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options), { data: { controller: "download", action: "download#download" } } + properties = options[:remote] ? { data: { controller: "download", action: "download#download" } } : {} + _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options.except(:remote)), properties end def edit_button_to(link, options = {}, properties = {}) diff --git a/app/views/reports/annual_reports/index.html.erb b/app/views/reports/annual_reports/index.html.erb index a549b96a44..3251c38b7a 100644 --- a/app/views/reports/annual_reports/index.html.erb +++ b/app/views/reports/annual_reports/index.html.erb @@ -32,7 +32,8 @@ <%= download_button_to( range_reports_annual_reports_path(year_start: @years.first, year_end: @years.last, format: :csv), - text: "Export Yearly Reports" + text: "Export Yearly Reports", + remote: true ) %> This will recalculate all the reports, and may take some time. diff --git a/app/views/reports/annual_reports/show.html.erb b/app/views/reports/annual_reports/show.html.erb index d8e254e6f8..cf20afd240 100644 --- a/app/views/reports/annual_reports/show.html.erb +++ b/app/views/reports/annual_reports/show.html.erb @@ -36,7 +36,8 @@ <%= download_button_to( reports_annual_report_path(@year, format: :csv), - text: "Export Report" + text: "Export Report", + remote: true ) %> diff --git a/spec/helpers/ui_helper_spec.rb b/spec/helpers/ui_helper_spec.rb index 09a3c48524..558fc072fa 100644 --- a/spec/helpers/ui_helper_spec.rb +++ b/spec/helpers/ui_helper_spec.rb @@ -150,23 +150,24 @@ expect(button.text.strip).to eq("Export Report") end - # Regression test for #5691: keep rails-ujs' double-click protection, and hand the link to - # the download controller, which re-enables it once the file arrives since no page load will. - it 'disables on click and uses the download controller' do + it 'disables on click and leaves the link as a normal navigation' do button = Nokogiri::HTML(subject).css("a").first expect(button.attributes["data-disable-with"].value).to eq("Please wait...") - expect(button.attributes["data-controller"].value).to eq("download") - expect(button.attributes["data-action"].value).to eq("download#download") + expect(button.attributes["data-controller"]).to be_nil end end - context 'when the caller passes custom data' do - subject { helper.download_button_to("/donations.csv", text: "Export Donations", data: {test: "test"}) } + # Regression test for #5691: keep rails-ujs' double-click protection, and hand the link to + # the download controller, which re-enables it once the file arrives since no page load will. + context 'with remote: true' do + subject { helper.download_button_to("/reports/annual_reports/2025.csv", text: "Export Report", remote: true, data: {test: "test"}) } - it 'keeps the custom data alongside the download controller' do + it 'disables on click and uses the download controller' do button = Nokogiri::HTML(subject).css("a").first - expect(button.attributes["data-test"].value).to eq("test") + expect(button.attributes["data-disable-with"].value).to eq("Please wait...") expect(button.attributes["data-controller"].value).to eq("download") + expect(button.attributes["data-action"].value).to eq("download#download") + expect(button.attributes["data-test"].value).to eq("test") end end end