From ca489f8b98a0e38bc7ed35f6fc5c900f966bf36e Mon Sep 17 00:00:00 2001
From: automated-signal <37887102+automated-signal@users.noreply.github.com>
Date: Mon, 11 Jul 2022 10:40:42 -0700
Subject: [PATCH] Attachment downloads: Use filename if we have it, ignore
 index = 1

Co-authored-by: Scott Nonnenberg <scott@signal.org>
---
 ts/test-node/types/Attachment_test.ts | 37 ++++++++++++++++++++++++++-
 ts/types/Attachment.ts                | 10 +++++---
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/ts/test-node/types/Attachment_test.ts b/ts/test-node/types/Attachment_test.ts
index 005d0485a..a84d06cb5 100644
--- a/ts/test-node/types/Attachment_test.ts
+++ b/ts/test-node/types/Attachment_test.ts
@@ -62,7 +62,25 @@ describe('Attachment', () => {
       });
     });
     context('for attachment with index', () => {
-      it('should generate a filename based on timestamp', () => {
+      it('should use filename if provided', () => {
+        const attachment: Attachment.AttachmentType = fakeAttachment({
+          fileName: 'funny-cat.mov',
+          data: Bytes.fromString('foo'),
+          contentType: MIME.VIDEO_QUICKTIME,
+        });
+        const timestamp = new Date(
+          DAY + new Date(DAY).getTimezoneOffset() * 60 * 1000
+        );
+        const actual = Attachment.getSuggestedFilename({
+          attachment,
+          timestamp,
+          index: 3,
+        });
+        const expected = 'funny-cat.mov';
+        assert.strictEqual(actual, expected);
+      });
+
+      it('should use provided index if > 1 and filename not provided', () => {
         const attachment: Attachment.AttachmentType = fakeAttachment({
           data: Bytes.fromString('foo'),
           contentType: MIME.VIDEO_QUICKTIME,
@@ -78,6 +96,23 @@ describe('Attachment', () => {
         const expected = 'signal-1970-01-02-000000_003.mov';
         assert.strictEqual(actual, expected);
       });
+
+      it('should not use provided index == 1 if filename not provided', () => {
+        const attachment: Attachment.AttachmentType = fakeAttachment({
+          data: Bytes.fromString('foo'),
+          contentType: MIME.VIDEO_QUICKTIME,
+        });
+        const timestamp = new Date(
+          DAY + new Date(DAY).getTimezoneOffset() * 60 * 1000
+        );
+        const actual = Attachment.getSuggestedFilename({
+          attachment,
+          timestamp,
+          index: 1,
+        });
+        const expected = 'signal-1970-01-02-000000.mov';
+        assert.strictEqual(actual, expected);
+      });
     });
   });
 
diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts
index 108783382..ad21d1e9b 100644
--- a/ts/types/Attachment.ts
+++ b/ts/types/Attachment.ts
@@ -996,8 +996,9 @@ export const getSuggestedFilename = ({
   timestamp?: number | Date;
   index?: number;
 }): string => {
-  if (!isNumber(index) && attachment.fileName) {
-    return attachment.fileName;
+  const { fileName } = attachment;
+  if (fileName) {
+    return fileName;
   }
 
   const prefix = 'signal';
@@ -1006,7 +1007,10 @@ export const getSuggestedFilename = ({
     : '';
   const fileType = getFileExtension(attachment);
   const extension = fileType ? `.${fileType}` : '';
-  const indexSuffix = index ? `_${padStart(index.toString(), 3, '0')}` : '';
+  const indexSuffix =
+    isNumber(index) && index > 1
+      ? `_${padStart(index.toString(), 3, '0')}`
+      : '';
 
   return `${prefix}${suffix}${indexSuffix}${extension}`;
 };