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: <% 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