Keep download buttons enabled after the file downloads (Resolves #5691)

[costajohnt]

Resolves #5691

Description

Download buttons got stuck on "Please wait..." after the file downloaded. rails-ujs disables a link with data-disable-with on click and only re-enables it on the next page load, and a file download never causes one.

This adds a download Stimulus controller that every download_button_to button uses (wired in the helper, so no view changes). On click it disables the button, fetches the file, saves it using the filename from Content-Disposition, then re-enables the button. So double-clicks stay blocked for exactly as long as the report takes to generate.

It replaces csv_download_controller.js, which the distributions export used to auto-download after its redirect. That partial now uses the same controller.

Redirects: fetch never exposes a 3xx to JS, so the request uses redirect: "manual", and when it gets an opaque redirect back it navigates to the link and lets the browser follow it. Following it inside fetch would use up the flash that handle_csv_export sets. A 401 (signed out) reloads the page so the user lands on the login screen, and any other error shows a toastr message.

The print_button_to PDF buttons are unchanged since those open inline in the tab.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

• Updated spec/helpers/ui_helper_spec.rb for the new controller and action attributes.
• spec/system/annual_reports_system_spec.rb checks the button label returns to "Export Report" after the download.
• Ran the system specs that download files (annual reports, distributions including the export redirect, requests, partners, donation sites) plus the helper and distributions request specs locally, all passing.

[dorner]

This fixes the bug by introducing a worse bug, I'm afraid. The debouncing is important because some of these reports can be really beefy. If we have to choose between debouncing (and forcing a page reload) or not (and going without) I'd stick with debouncing.

Can you see if there's a way to keep the debounce but also fix the stuck behavior?

[costajohnt]

Good point, I hadn't thought through the double-click case on the heavy reports. I put the debounce back: the button still switches to "Please wait..." on click, and a small Stimulus controller turns it back on 5 seconds later. rails-ujs only re-enables on the next page load, and a file download never triggers one. So a double-click during generation is still blocked, and the button doesn't stay stuck. If you'd rather re-enable when the file actually arrives, I can switch to a fetch-and-blob download like csv_download_controller.js. That's a bigger change across the ~20 download buttons, since a few endpoints redirect instead of sending a file.

[dorner]

I think I'd like to see the download-controller approach to see how much of a change it is. Ideally we'd be able to reuse that download controller across all download buttons, not just the CSV ones. Handling redirects shouldn't be too bad (if the request returns a 302 then do a location.href change, otherwise continue with download behavior).

[costajohnt]

Pushed the download controller version. Every download_button_to now fetches the file in the background, stays on "Please wait..." until the file actually arrives, then re-enables, so the debounce lasts exactly as long as the report takes. I folded csv_download_controller into it, so there's one controller for all of them.

On redirects: fetch never actually exposes the 302 to JS. It either follows it silently, or with redirect: "manual" you get an opaque response with no status or Location. So it uses manual mode, and when it sees a redirect it navigates to the original link and lets the browser follow it. Following it inside fetch would also eat the flash the distributions export relies on. Distributions is the only download button that redirects right now. The rest send a file or return an error status, and a 401 when signed out reloads the page to the login screen.

I left the print buttons alone since those PDFs open inline in the tab. Overall it's 8 files, mostly the new controller replacing the two old ones.

[dorner]

Have you validated that this is the case across the board? Nothing ever does work before redirecting?

If this is not a straight no-risk change, then we need to change this so it doesn't alter the existing download_button_to but adds a new version (e.g. with remote: true) that we can opt into rather than doing an across-the-board change.

[costajohnt]

I went through all 20 download_button_to call sites and the action behind each one. 19 of them answer with send_data/send_file directly (no redirect), so there's only ever one request. Distributions is the only one that redirects, and handle_csv_export only sets flash[:trigger_csv_download] before redirecting, so hitting it twice is harmless. The annual report show/range endpoints do write through Reports.retrieve_report, but they also respond with send_data, so they still run once.

That said, it isn't a straight no-risk change: every export button moves from a normal navigation to fetch + blob, which means the whole file sits in browser memory before it's saved (that matters most for the bigger annual reports), and I haven't tested very large files in a browser. So I'm happy to switch to the opt-in approach: leave download_button_to as it was, add an opt-in version, and use it only on the annual report buttons from #5691. Want me to do that?

[costajohnt]

Went ahead with the opt-in version since it isn't a straight no-risk change. download_button_to is back to its original behavior, and it only wires up the download controller when you pass remote: true. Right now that's just the two annual report export buttons from #5691. I also merged in main.