diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb index 32325d9232..ba1c1f0832 100644 --- a/app/helpers/ui_helper.rb +++ b/app/helpers/ui_helper.rb @@ -88,8 +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 = {}) - _link_to link, { icon: "download", type: "info", text: "Download", size: "md" }.merge(options) + 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/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/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/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 35e1a96359..558fc072fa 100644 --- a/spec/helpers/ui_helper_spec.rb +++ b/spec/helpers/ui_helper_spec.rb @@ -138,6 +138,40 @@ 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 + + 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"]).to be_nil + end + 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. + context 'with remote: true' do + subject { helper.download_button_to("/reports/annual_reports/2025.csv", text: "Export Report", remote: true, data: {test: "test"}) } + + 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") + expect(button.attributes["data-action"].value).to eq("download#download") + expect(button.attributes["data-test"].value).to eq("test") + 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/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 5b26888cb2..59099dcedf 100644 --- a/spec/system/annual_reports_system_spec.rb +++ b/spec/system/annual_reports_system_spec.rb @@ -33,5 +33,19 @@ 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 + + # 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") + expect(button).to have_no_text("Please wait...") + end end end