From d675037bf015b75039768fbdb9de71358b224144 Mon Sep 17 00:00:00 2001 From: Daniela Baron Date: Sat, 22 Mar 2025 05:57:17 -0400 Subject: [PATCH] [#4821] Persist profile attached documents through validation errors Use direct uploads for multiple file input so that file blobs remain available even if the model fails to save due to validation errors. - Submit hidden fields containing the signed_id of attached but unpersisted documents so that previously selected documents will be saved on the next successful form submission - Hide the native file input to prevent "No file chosen" from appearing when files were selected before a validation error - Use a custom button and file listing to display selected files during the current selection or on form re-render --- app/helpers/partners/profile_helper.rb | 20 ++++ .../controllers/file_input_controller.js | 46 +++++++-- .../step/_attached_documents_form.html.erb | 31 ++++++- .../partners/profile_edit_system_spec.rb | 93 +++++++++++++++++-- 4 files changed, 171 insertions(+), 19 deletions(-) create mode 100644 app/helpers/partners/profile_helper.rb diff --git a/app/helpers/partners/profile_helper.rb b/app/helpers/partners/profile_helper.rb new file mode 100644 index 0000000000..8f7f28326c --- /dev/null +++ b/app/helpers/partners/profile_helper.rb @@ -0,0 +1,20 @@ +module Partners + module ProfileHelper + # Returns an array of filenames that are attached to the profile but not persisted. + # This is to display to the user that the system remembers their file selections + # even if there was a form validation error. + # The method returns a JSON string (an array of filenames) to be used in a Stimulus controller. + def attached_but_not_persisted_file_names(profile) + filenames = profile.documents.attachments + .select { |att| !att.persisted? } + .map { |att| att.blob.filename.to_s } + + filenames.to_json + end + + # Returns true if at least one document attachment is actually persisted + def has_persisted_documents?(profile) + profile.documents.attachments.any?(&:persisted?) + end + end +end diff --git a/app/javascript/controllers/file_input_controller.js b/app/javascript/controllers/file_input_controller.js index 2264cd4d6d..08a40e4f77 100644 --- a/app/javascript/controllers/file_input_controller.js +++ b/app/javascript/controllers/file_input_controller.js @@ -5,8 +5,9 @@ import { Controller } from "@hotwired/stimulus"; * * This controller: * - Listens for file selection on an `` - * - Displays selected file names in a custom list **when multiple files are selected - * - Defaults to the browser’s built-in display for a single file selection + * - Displays selected file names in a custom list when multiple files are selected + * - If provided a `filenames` array, displays those file names as if user had just selected them. + * This is useful for displaying previously selected files on page load with validation errors. * * Expected HTML structure should have a placeholder div for the selected file names: * @@ -20,29 +21,60 @@ import { Controller } from "@hotwired/stimulus"; export default class extends Controller { static targets = ["input", "list"]; + static values = { + filenames: Array + } + connect() { this.inputTarget.addEventListener("change", () => this.updateFileList()); + + if (this.hasFilenamesValue && this.filenamesValue.length > 0) { + this.updateFileListFromValue(); + } + } + + // Opens the hidden file input when "Choose Files" button is clicked + triggerFileSelection() { + this.inputTarget.click(); } + // native file input selection updateFileList() { const files = this.inputTarget.files; - this.listTarget.innerHTML = ""; // Clear previous list - // If no files or only one file is selected, let the native UI handle it - if (files.length <= 1) { + if (files.length === 0) { return; } + this.renderFileList(Array.from(files).map(file => file.name)); + } + + updateFileListFromValue() { + this.renderFileList(this.filenamesValue); + } + + renderFileList(fileNames) { + // Clear previous list + this.listTarget.innerHTML = ""; + + // Create subheader + const header = document.createElement("p"); + header.textContent = "Selected files:"; + header.classList.add("font-weight-bold", "mb-1"); + + // Create file list const ul = document.createElement("ul"); ul.classList.add("list-unstyled", "mt-2"); - Array.from(files).forEach((file) => { + fileNames.forEach((name) => { const li = document.createElement("li"); li.classList.add("p-1", "rounded", "mb-1"); - li.textContent = file.name; + li.textContent = name; ul.appendChild(li); }); + // Append header and list to target container + this.listTarget.appendChild(header); this.listTarget.appendChild(ul); } } diff --git a/app/views/partners/profiles/step/_attached_documents_form.html.erb b/app/views/partners/profiles/step/_attached_documents_form.html.erb index 3952882c38..09c1708437 100644 --- a/app/views/partners/profiles/step/_attached_documents_form.html.erb +++ b/app/views/partners/profiles/step/_attached_documents_form.html.erb @@ -1,6 +1,10 @@ <%= f.fields_for :profile, profile do |pf| %> -
- <% if profile.documents.attached? %> +
+ + <%# Allow user to download and/or remove existing attachments %> + <% if has_persisted_documents?(profile) %> Attached files:
    <% profile.documents.each do |doc| %> @@ -17,8 +21,27 @@
<% end %> - <%# Native file input and placeholder for selected file names %> - <%= pf.file_field :documents, multiple: true, class: "form-control-file", data: { file_input_target: "input" } %> + <%# Submit hidden fields for attachments that are not persisted to preserve them through form validation errors %> + <% profile.documents.attachments.each do |att| %> + <% if !att.persisted? %> + <%= pf.hidden_field :documents, multiple: true, value: att.blob.signed_id %> + <% end %> + <% end %> + + <%# Hide native file input to support custom behaviour to display %> + <%# previously selected files when validation error occurs %> + <%= pf.file_field :documents, + multiple: true, + direct_upload: true, + class: "form-control-file d-none", + data: { file_input_target: "input" } %> + + <%# Custom button to trigger file selection %> + + + <%# Placeholder to display selected file(s) populated by app/javascript/controllers/file_input_controller.js %>
<% end %> diff --git a/spec/system/partners/profile_edit_system_spec.rb b/spec/system/partners/profile_edit_system_spec.rb index 2267cc6c9c..65ac489f2d 100644 --- a/spec/system/partners/profile_edit_system_spec.rb +++ b/spec/system/partners/profile_edit_system_spec.rb @@ -143,22 +143,32 @@ find("button[data-bs-target='#attached_documents']").click expect(page).to have_css("#attached_documents.accordion-collapse.collapse.show", visible: true) - # Upload two documents - needs to be done individually because Capybara doesn't have attach_files multiple support - # https://github.com/teamcapybara/capybara/issues/37 + # Upload multiple documents at once within "#attached_documents" do - attach_file("partner_profile_documents", Rails.root.join("spec/fixtures/files/document1.md"), make_visible: true) + attach_file("partner_profile_documents", [ + Rails.root.join("spec/fixtures/files/document1.md"), + Rails.root.join("spec/fixtures/files/document2.md") + ], make_visible: true) + + # Verify both documents are displayed in custom selection list + expect(page).to have_text("Selected files:") + expect(page).to have_css("[data-file-input-target='list'] li", text: "document1.md") + expect(page).to have_css("[data-file-input-target='list'] li", text: "document2.md") end + + # Save Progress all("input[type='submit'][value='Save Progress']").last.click + expect(page).to have_css(".alert-success", text: "Details were successfully updated.") + + # Verify both documents persist after page reload visit edit_partners_profile_path find("button[data-bs-target='#attached_documents']").click within "#attached_documents" do - attach_file("partner_profile_documents", Rails.root.join("spec/fixtures/files/document2.md"), make_visible: true) + expect(page).to have_link("document1.md") + expect(page).to have_link("document2.md") end - all("input[type='submit'][value='Save Progress']").last.click # Remove the first document - visit edit_partners_profile_path - find("button[data-bs-target='#attached_documents']").click within "#attached_documents" do document_name = "document1.md" document_li = find("li.attached-document", text: document_name) @@ -170,11 +180,12 @@ all("input[type='submit'][value='Save Progress']").last.click expect(page).to have_css(".alert-success", text: "Details were successfully updated.") - # Verify only one document is listed + # Verify only one document remains visit edit_partners_profile_path find("button[data-bs-target='#attached_documents']").click within "#attached_documents" do expect(page).to have_link("document2.md") + expect(page).not_to have_link("document1.md") end end @@ -225,5 +236,71 @@ expect(find("label[for='partner_profile_proof_of_partner_status']")).to have_content("irs_determination_letter.md") end end + + it "persists multiple file uploads when there are validation errors" do + # Open Pick up person section and fill in 4 email addresses which will generate a validation error + find("button[data-bs-target='#pick_up_person']").click + within "#pick_up_person" do + fill_in "Pick Up Person's Email", with: "email1@example.com, email2@example.com, email3@example.com, email4@example.com" + end + + # Open attached documents section + find("button[data-bs-target='#attached_documents']").click + expect(page).to have_css("#attached_documents.accordion-collapse.collapse.show", visible: true) + + # Upload multiple documents + within "#attached_documents" do + attach_file("partner_profile_documents", [ + Rails.root.join("spec/fixtures/files/document1.md"), + Rails.root.join("spec/fixtures/files/document2.md") + ], make_visible: true) + + # Verify both documents are displayed in custom selection list + expect(page).to have_css("[data-file-input-target='list'] li", text: "document1.md") + expect(page).to have_css("[data-file-input-target='list'] li", text: "document2.md") + end + + # Save Progress + all("input[type='submit'][value='Save Progress']").last.click + + # Expect an alert-danger message containing validation errors + expect(page).to have_css(".alert-danger", text: /There is a problem/) + + # Open attached documents section + find("button[data-bs-target='#attached_documents']").click + expect(page).to have_css("#attached_documents.accordion-collapse.collapse.show", visible: true) + + # Expect both documents are still displayed in custom list as selected, but nothing is actually attached + within "#attached_documents" do + expect(page).to have_text("Selected files:") + expect(page).to have_css("[data-file-input-target='list'] li", text: "document1.md") + expect(page).to have_css("[data-file-input-target='list'] li", text: "document2.md") + + expect(page).not_to have_text("Attached files:") + expect(page).not_to have_link("document1.md") + expect(page).not_to have_link("document2.md") + end + + # Fix validation error in Pick up person section: It's already open due to having a validation error + within "#pick_up_person" do + fill_in "Pick Up Person's Email", with: "email1@example.com, email2@example.com, email3@example.com" + end + + # Save Progress + all("input[type='submit'][value='Save Progress']").last.click + expect(page).to have_css(".alert-success", text: "Details were successfully updated.") + + # Open attached documents section + find("button[data-bs-target='#attached_documents']").click + expect(page).to have_css("#attached_documents.accordion-collapse.collapse.show", visible: true) + + # Expect both documents are now rendered as downloadable links + # i.e. they've been saved, without user having had to select them again + within "#attached_documents" do + expect(page).to have_text("Attached files:") + expect(page).to have_link("document1.md") + expect(page).to have_link("document2.md") + end + end end end