Don't create preview icon for links with no image (quotes)

ts/models/conversations.ts

@@ -80,6 +80,7 @@ import {
   take,
   repeat,
   zipObject,
+  collect,
 } from '../util/iterables';
 import * as universalExpireTimer from '../util/universalExpireTimer';
 import type { GroupNameCollisionsWithIdsByTitle } from '../util/groupMemberNameCollisions';
@@ -3640,22 +3641,11 @@ export class ConversationModel extends window.Backbone
     }
 
     if (preview && preview.length) {
-      const previewsToUse = take(preview, 1);
+      const previewImages = collect(preview, prev => prev.image);
+      const previewImagesToUse = take(previewImages, 1);
 
       return Promise.all(
-        map(previewsToUse, async attachment => {
-          const { image } = attachment;
-
-          if (!image) {
-            return {
-              contentType: IMAGE_JPEG,
-              // Our protos library complains about these fields being undefined, so we
-              //   force them to null
-              fileName: null,
-              thumbnail: null,
-            };
-          }
-
+        map(previewImagesToUse, async image => {
           const { contentType } = image;
 
           return {

ts/test-both/util/iterables_test.ts

@@ -5,6 +5,7 @@ import { assert } from 'chai';
 import * as sinon from 'sinon';
 
 import {
+  collect,
   concat,
   every,
   filter,
@@ -251,6 +252,52 @@ describe('iterable utilities', () => {
     });
   });
 
+  describe('collect', () => {
+    it('returns an empty iterable when passed an empty iterable', () => {
+      const fn = sinon.fake();
+
+      assert.deepEqual([...collect([], fn)], []);
+      assert.deepEqual([...collect(new Set(), fn)], []);
+      assert.deepEqual([...collect(new Map(), fn)], []);
+
+      sinon.assert.notCalled(fn);
+    });
+
+    it('returns a new iterator with some values removed', () => {
+      const getB = sinon.fake((v: { a: string; b?: number }) => v.b);
+      const result = collect(
+        [{ a: 'n' }, { a: 'm', b: 0 }, { a: 'o' }, { a: 'p', b: 1 }],
+        getB
+      );
+
+      sinon.assert.notCalled(getB);
+
+      assert.deepEqual([...result], [0, 1]);
+      assert.notInstanceOf(result, Array);
+
+      sinon.assert.callCount(getB, 4);
+    });
+
+    it('can collect an infinite iterable', () => {
+      const everyNumber = {
+        *[Symbol.iterator]() {
+          for (let i = 0; true; i += 1) {
+            yield { a: 'x', ...(i % 2 ? { b: i } : {}) };
+          }
+        },
+      };
+
+      const getB = sinon.fake((v: { a: string; b?: number }) => v.b);
+      const result = collect(everyNumber, getB);
+      const iterator = result[Symbol.iterator]();
+
+      assert.deepEqual(iterator.next(), { value: 1, done: false });
+      assert.deepEqual(iterator.next(), { value: 3, done: false });
+      assert.deepEqual(iterator.next(), { value: 5, done: false });
+      assert.deepEqual(iterator.next(), { value: 7, done: false });
+    });
+  });
+
   describe('find', () => {
     const isOdd = (n: number) => Boolean(n % 2);
 

ts/test-electron/models/conversations_test.ts

@@ -3,6 +3,7 @@
 
 import { assert } from 'chai';
 import { SendStatus } from '../../messages/MessageSendState';
+import { IMAGE_PNG } from '../../types/MIME';
 import { UUID } from '../../types/UUID';
 
 describe('Conversations', () => {
@@ -104,4 +105,51 @@ describe('Conversations', () => {
 
     assert.strictEqual(conversation.get('lastMessage'), '');
   });
+
+  it('only produces attachments on a quote with an image', async () => {
+    // Creating a fake conversation
+    const conversation = new window.Whisper.Conversation({
+      avatars: [],
+      id: UUID.generate().toString(),
+      e164: '+15551234567',
+      uuid: UUID.generate().toString(),
+      type: 'private',
+      inbox_position: 0,
+      isPinned: false,
+      markedUnread: false,
+      lastMessageDeletedForEveryone: false,
+      messageCount: 0,
+      sentMessageCount: 0,
+      profileSharing: true,
+      version: 0,
+    });
+
+    const resultNoImage = await conversation.getQuoteAttachment(
+      [],
+      [
+        {
+          url: 'https://sometest.signal.org/',
+        },
+      ]
+    );
+
+    assert.deepEqual(resultNoImage, []);
+
+    const [resultWithImage] = await conversation.getQuoteAttachment(
+      [],
+      [
+        {
+          url: 'https://sometest.signal.org/',
+          image: {
+            contentType: IMAGE_PNG,
+            size: 100,
+            data: new Uint8Array(),
+          },
+        },
+      ]
+    );
+
+    assert.equal(resultWithImage.contentType, 'image/png');
+    assert.equal(resultWithImage.fileName, null);
+  });
 });

ts/util/iterables.ts

@@ -101,6 +101,50 @@ class FilterIterator<T> implements Iterator<T> {
   }
 }
 
+/**
+ * Filter and transform (map) that produces a new type
+ * useful when traversing through fields that might be undefined
+ */
+export function collect<T, S>(
+  iterable: Iterable<T>,
+  fn: (value: T) => S | undefined
+): Iterable<S> {
+  return new CollectIterable(iterable, fn);
+}
+
+class CollectIterable<T, S> implements Iterable<S> {
+  constructor(
+    private readonly iterable: Iterable<T>,
+    private readonly fn: (value: T) => S | undefined
+  ) {}
+
+  [Symbol.iterator](): Iterator<S> {
+    return new CollectIterator(this.iterable[Symbol.iterator](), this.fn);
+  }
+}
+
+class CollectIterator<T, S> implements Iterator<S> {
+  constructor(
+    private readonly iterator: Iterator<T>,
+    private readonly fn: (value: T) => S | undefined
+  ) {}
+
+  next(): IteratorResult<S> {
+    // eslint-disable-next-line no-constant-condition
+    while (true) {
+      const nextIteration = this.iterator.next();
+      if (nextIteration.done) return nextIteration;
+      const nextValue = this.fn(nextIteration.value);
+      if (nextValue !== undefined) {
+        return {
+          done: false,
+          value: nextValue,
+        };
+      }
+    }
+  }
+}
+
 export function find<T>(
   iterable: Iterable<T>,
   predicate: (value: T) => unknown