[#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

Author: danielabar

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

app/javascript/controllers/file_input_controller.js

@@ -5,8 +5,9 @@ import { Controller } from "@hotwired/stimulus";
  *
  * This controller:
  * - Listens for file selection on an `<input type="file" multiple="multiple">`
- * - 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);
   }
 }

app/views/partners/profiles/step/_attached_documents_form.html.erb

@@ -1,6 +1,10 @@
 <%= f.fields_for :profile, profile do |pf| %>
-  <div class="form-group" data-controller="file-input">
-    <% if profile.documents.attached? %>
+  <div class="form-group"
+      data-controller="file-input"
+      data-file-input-filenames-value="<%= attached_but_not_persisted_file_names(profile) %>">
+
+    <%# Allow user to download and/or remove existing attachments %>
+    <% if has_persisted_documents?(profile) %>
       <strong>Attached files:</strong>
       <ul class="list-unstyled">
         <% profile.documents.each do |doc| %>
@@ -17,8 +21,27 @@
       </ul>
     <% 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 %>
+    <button type="button" class="btn btn-outline-primary" data-action="click->file-input#triggerFileSelection">
+      Choose Files
+    </button>
+
+    <%# Placeholder to display selected file(s) populated by app/javascript/controllers/file_input_controller.js %>
     <div data-file-input-target="list" class="mt-2"></div>
   </div>
 <% end %>

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: "[email protected], [email protected], [email protected], [email protected]"
+      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: "[email protected], [email protected], [email protected]"
+      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