Merge pull request #5115 from danielabar/4821-part3-partner-profile-persist-docs-validation-errors

[#4821] Persist profile attached documents through validation errors

Author: cielf

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