Fix/attach show size (#2741)

* fix: add file size to file rows

* feat: display total file size in attachments widget

* chore: add tests

* fix: update file normalization and adjust total size display in tests

* chore: formatting

* chore: formatting

* fix: rename file size attribute for consistency in attachments

* fix: normalize file_size values to ensure consistent number format across attachments

* fix: display file size in attachment list and update tests for size labels

Author: andrewleith

app/assets/javascripts/attachments/AttachFilesModal.js

@@ -2,6 +2,7 @@ import React, { useEffect, useRef, useState } from "react";
 // Render inline as an expanding panel instead of a portal/modal
 import { ACCEPT_ATTRIBUTE } from "./useAttachments";
 import { getAttachmentTranslations } from "./localization";
+import { formatFileSize } from "./fileSize";
 
 const DEFAULT_COPY = getAttachmentTranslations("en");
 
@@ -274,33 +275,47 @@ export const AttachFilesModal = ({
       </div>
       {selectedFiles.length > 0 && (
         <ul className="space-y-2 mb-4" data-testid="pending-files-list">
-          {selectedFiles.map((pendingFile) => (
-            <li
-              key={pendingFile.id}
-              className="border border-gray-300 p-3 flex justify-between items-start align-top"
-            >
-              <div className="min-w-0 pr-4">
-                <span
-                  className="attachment-file-name-truncate mb-0 block"
-                  title={pendingFile.file.name}
-                >
-                  {pendingFile.file.name}
-                </span>
-              </div>
-              <button
-                className="link text-red-700 self-start"
-                type="button"
-                data-testid="attachments-pending-remove"
-                aria-label={`Remove ${pendingFile.file.name}`}
-                onClick={() => onRemovePending(pendingFile.id)}
+          {selectedFiles.map((pendingFile) => {
+            const selectedFileSize = formatFileSize(pendingFile.file.size);
+
+            return (
+              <li
+                key={pendingFile.id}
+                className="border border-gray-300 p-3 flex justify-between items-start align-top"
               >
-                <span className="font-bold underline">{copy.remove}</span>
-                <span className="text-[24px]" aria-hidden="true">
-                  &nbsp;×
-                </span>
-              </button>
-            </li>
-          ))}
+                <div className="min-w-0 pr-4">
+                  <p className="min-w-0 mb-0">
+                    <span
+                      className="attachment-file-name-truncate"
+                      title={pendingFile.file.name}
+                    >
+                      {pendingFile.file.name}
+                    </span>
+                    {selectedFileSize ? (
+                      <span
+                        className="attachment-size"
+                        data-testid="attachment-file-size"
+                      >
+                        {` (${selectedFileSize})`}
+                      </span>
+                    ) : null}
+                  </p>
+                </div>
+                <button
+                  className="link text-red-700 self-start"
+                  type="button"
+                  data-testid="attachments-pending-remove"
+                  aria-label={`Remove ${pendingFile.file.name}`}
+                  onClick={() => onRemovePending(pendingFile.id)}
+                >
+                  <span className="font-bold underline">{copy.remove}</span>
+                  <span className="text-[24px]" aria-hidden="true">
+                    &nbsp;×
+                  </span>
+                </button>
+              </li>
+            );
+          })}
         </ul>
       )}
 

app/assets/javascripts/attachments/AttachedFileRow.js

@@ -1,6 +1,7 @@
 import React, { useEffect, useRef } from "react";
 import { ATTACHMENT_STATUSES } from "./useAttachments";
 import { getAttachmentTranslations } from "./localization";
+import { formatFileSize } from "./fileSize";
 
 const DEFAULT_COPY = getAttachmentTranslations("en");
 
@@ -33,6 +34,7 @@ export const AttachedFileRow = ({
     ? `${downloadEndpoint}/${encodeURIComponent(file.id)}`
     : null;
   const canDownload = !isInProgress && !isMalware && Boolean(downloadHref);
+  const fileSizeLabel = formatFileSize(file.file_size);
 
   const handleDownload = async (e) => {
     e.preventDefault();
@@ -127,7 +129,17 @@ export const AttachedFileRow = ({
                   data-testid="attachment-row-spinner"
                 ></div>
               ) : null}
-              <p className="min-w-0 mb-0">{fileNameNode}</p>
+              <p className="min-w-0 mb-0">
+                {fileNameNode}
+                {fileSizeLabel ? (
+                  <span
+                    className="attachment-size"
+                    data-testid="attachment-file-size"
+                  >
+                    {` (${fileSizeLabel})`}
+                  </span>
+                ) : null}
+              </p>
             </div>
           </div>
           <button

app/assets/javascripts/attachments/AttachmentsWidget.js

@@ -6,6 +6,7 @@ import {
   useAttachments,
   validateFiles,
 } from "./useAttachments";
+import { formatFileSize, sumAttachmentFileSizes } from "./fileSize";
 import { getAttachmentTranslations } from "./localization";
 
 export const AttachmentsWidget = ({
@@ -130,6 +131,15 @@ export const AttachmentsWidget = ({
     () => summarizeStatuses(files, copy),
     [files, copy],
   );
+  const totalFileSizeLabel = useMemo(() => {
+    const countableFiles = files.filter(
+      (file) =>
+        file.status !== "virus_scan_failed" && file.status !== "deleted",
+    );
+    const totalBytes = sumAttachmentFileSizes(countableFiles);
+
+    return totalBytes > 0 ? formatFileSize(totalBytes) : null;
+  }, [files]);
 
   const handleAttach = async (selectedFiles) => {
     const result = validateFiles(selectedFiles, files, copy);
@@ -157,7 +167,17 @@ export const AttachmentsWidget = ({
 
   return (
     <section className="mb-16" data-testid="attachments-widget">
-      <h2 className="heading-medium">{copy.attachedFilesHeading}</h2>
+      <h2 className="heading-medium" data-testid="attachments-heading">
+        {copy.attachedFilesHeading}
+        {totalFileSizeLabel ? (
+          <span
+            className="hint text-xs inline ml-2"
+            data-testid="attachments-total-size"
+          >
+            ({totalFileSizeLabel})
+          </span>
+        ) : null}
+      </h2>
 
       {downloadError && (
         <div

app/assets/javascripts/attachments/fileSize.js

@@ -0,0 +1,38 @@
+const toFiniteFileSize = (value) => {
+  const parsed = Number(value);
+
+  if (!Number.isFinite(parsed) || parsed < 0) {
+    return null;
+  }
+
+  return parsed;
+};
+
+export const formatFileSize = (sizeInBytes) => {
+  const normalizedSize = toFiniteFileSize(sizeInBytes);
+
+  if (normalizedSize === null) {
+    return null;
+  }
+
+  if (normalizedSize < 1024) {
+    return `${normalizedSize} B`;
+  }
+
+  if (normalizedSize < 1024 * 1024) {
+    return `${(normalizedSize / 1024).toFixed(1)} KB`;
+  }
+
+  return `${(normalizedSize / (1024 * 1024)).toFixed(1)} MB`;
+};
+
+export const sumAttachmentFileSizes = (files = []) =>
+  files.reduce((sum, file) => {
+    const fileSize = toFiniteFileSize(file?.file_size);
+
+    if (fileSize === null) {
+      return sum;
+    }
+
+    return sum + fileSize;
+  }, 0);

app/assets/javascripts/attachments/useAttachments.js

@@ -46,6 +46,15 @@ const getExtension = (fileName) => {
   return fileName.slice(lastDot).toLowerCase();
 };
 
+const toFiniteFileSize = (value) => {
+  const parsed = Number(value);
+  if (!Number.isFinite(parsed) || parsed < 0) {
+    return undefined;
+  }
+
+  return parsed;
+};
+
 export const validateFiles = (
   selectedFiles,
   existingFiles,
@@ -60,7 +69,7 @@ export const validateFiles = (
   }
 
   const existingTotalBytes = existingFiles.reduce(
-    (sum, file) => sum + (file.size || 0),
+    (sum, file) => sum + (toFiniteFileSize(file.file_size) || 0),
     0,
   );
   const selectedTotalBytes = selectedFiles.reduce(
@@ -109,11 +118,6 @@ const nextId = () => {
 const parseApiStatus = (status, fallbackStatus) =>
   VALID_ATTACHMENT_STATUSES.has(status) ? status : fallbackStatus;
 
-const normalizeFile = (file) => ({
-  ...file,
-  status: parseApiStatus(file.status, ATTACHMENT_STATUSES.DELETED),
-});
-
 export const summarizeStatuses = (files, copy = DEFAULT_COPY) => {
   const counts = {
     scanning: 0,
@@ -160,7 +164,12 @@ export const useAttachments = (
   copy = DEFAULT_COPY,
   fetchFileStatus = null,
 ) => {
-  const [files, setFiles] = useState(() => initialFiles.map(normalizeFile));
+  const [files, setFiles] = useState(() =>
+    initialFiles.map((file) => ({
+      ...file,
+      file_size: toFiniteFileSize(file.file_size),
+    })),
+  );
   const timeoutIdsRef = useRef([]);
   const pollTimeoutIdsRef = useRef(new Map());
   const isMountedRef = useRef(true);
@@ -297,7 +306,9 @@ export const useAttachments = (
             id: fileId,
             name:
               sourceFile?.name || itemData?.name || `attachment-${nextId()}`,
-            size: sourceFile?.size || itemData?.file_size || 0,
+            file_size: toFiniteFileSize(
+              sourceFile?.size ?? itemData?.file_size,
+            ),
             status: parseApiStatus(
               itemData?.status,
               ATTACHMENT_STATUSES.PENDING_VIRUS_SCAN,

app/assets/stylesheets/tailwind/components/attachments.css

@@ -61,3 +61,8 @@
   transform: translateY(0);
   overflow: visible;
 }
+
+.attachment-size {
+  @apply text-gray-700;
+  font-size: 18px;
+}

app/templates/partials/template-attachments-list.html

@@ -29,7 +29,14 @@
                 data-testid="attachment-row-spinner"
               ></div>
             {% endif %}
-            <span class="attachment-file-name-truncate">{{ attachment.filename }}</span>
+            <p class="min-w-0 mb-0">
+              <span class="attachment-file-name-truncate">{{ attachment.filename }}</span>
+              {% if attachment.file_size is not none %}
+                <span class="attachment-size" data-testid="attachment-file-size">
+                  ({{ attachment.file_size | filesizeformat }})
+                </span>
+              {% endif %}
+            </p>
           </div>
         </li>
       {% endfor %}

app/templates/views/storybook/attachments.html

@@ -32,13 +32,13 @@ <h3 class="heading-small mb-4">Mixed in-progress and attached</h3>
           {
             "id": "Filename.pdf",
             "name": "Filename.pdf",
-            "size": 320000,
+            "file_size": 320000,
             "status": "pending_virus_scan"
           },
           {
             "id": "Sometimes_you_need_a_very_very_very_very_long_file_name_final_v1_final_final_3_hello.pdf",
             "name": "Sometimes_you_need_a_very_very_very_very_long_file_name_final_v1_final_final_3_hello.pdf",
-            "size": 170000,
+            "file_size": 170000,
             "status": "uploaded"
           }
         ],
@@ -62,13 +62,13 @@ <h3 class="heading-small mb-4">Unsafe file state</h3>
           {
             "id": "document_with_malware.pdf",
             "name": "document_with_malware.pdf",
-            "size": 470000,
+            "file_size": 470000,
             "status": "virus_scan_failed"
           },
           {
             "id": "safe_permit.pdf",
             "name": "safe_permit.pdf",
-            "size": 260000,
+            "file_size": 260000,
             "status": "uploaded"
           }
         ],
@@ -93,7 +93,7 @@ <h3 class="heading-small mb-4">Interactive status simulation</h3>
           {
             "id": "already_attached_notice.pdf",
             "name": "already_attached_notice.pdf",
-            "size": 220000,
+            "file_size": 220000,
             "status": "uploaded"
           }
         ],

tests/app/main/views/test_send.py

@@ -808,6 +808,8 @@ def test_check_messages_ok_shows_template_attachments_and_scanning_warning(
     assert page.select_one("[data-testid='template-attachments-warning']") is not None
     assert "guide.pdf" in attachments_section.text
     assert "pending.pdf" in attachments_section.text
+    size_labels = attachments_section.select("[data-testid='attachment-file-size']")
+    assert len(size_labels) == 2
 
 
 def test_check_messages_ok_hides_attachment_warning_when_all_uploaded(