Allow link-only stories, download previews

ts/models/messages.ts

@@ -2530,12 +2530,19 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
 
         const urls = LinkPreview.findLinks(dataMessage.body || '');
         const incomingPreview = dataMessage.preview || [];
-        const preview = incomingPreview.filter(
+        const preview = incomingPreview.filter((item: LinkPreviewType) => {
-          (item: LinkPreviewType) =>
+          if (!item.image && !item.title) {
-            (item.image || item.title) &&
+            return false;
-            urls.includes(item.url) &&
+          }
-            LinkPreview.shouldPreviewHref(item.url)
+          // Story link previews don't have to correspond to links in the
-        );
+          // message body.
+          if (isStory(message.attributes)) {
+            return true;
+          }
+          return (
+            urls.includes(item.url) && LinkPreview.shouldPreviewHref(item.url)
+          );
+        });
         if (preview.length < incomingPreview.length) {
           log.info(
             `${message.idForLogging()}: Eliminated ${

ts/services/storyLoader.ts

@@ -7,7 +7,11 @@ import type { StoryDataType } from '../state/ducks/stories';
 import * as durations from '../util/durations';
 import * as log from '../logging/log';
 import dataInterface from '../sql/Client';
-import { getAttachmentsForMessage } from '../state/selectors/message';
+import {
+  getAttachmentsForMessage,
+  getPropsForAttachment,
+} from '../state/selectors/message';
+import type { LinkPreviewType } from '../types/message/LinkPreviews';
 import { isNotNil } from '../util/isNotNil';
 import { strictAssert } from '../util/assert';
 import { dropNull } from '../util/dropNull';
@@ -66,11 +70,38 @@ export function getStoryDataFromMessageAttributes(
     return;
   }
 
-  const [attachment] =
+  let [attachment] =
     unresolvedAttachment && unresolvedAttachment.path
       ? getAttachmentsForMessage(message)
       : [unresolvedAttachment];
 
+  let preview: LinkPreviewType | undefined;
+  if (message.preview?.length) {
+    strictAssert(
+      message.preview.length === 1,
+      'getStoryDataFromMessageAttributes: story can have only one preview'
+    );
+    [preview] = message.preview;
+
+    strictAssert(
+      attachment?.textAttachment,
+      'getStoryDataFromMessageAttributes: story must have a ' +
+        'textAttachment with preview'
+    );
+    attachment = {
+      ...attachment,
+      textAttachment: {
+        ...attachment.textAttachment,
+        preview: {
+          ...preview,
+          image: preview.image && getPropsForAttachment(preview.image),
+        },
+      },
+    };
+  } else if (attachment) {
+    attachment = getPropsForAttachment(attachment);
+  }
+
   return {
     attachment,
     messageId: message.id,

ts/state/ducks/stories.ts

@@ -32,17 +32,15 @@ import { markViewed } from '../../services/MessageUpdater';
 import { queueAttachmentDownloads } from '../../util/queueAttachmentDownloads';
 import { replaceIndex } from '../../util/replaceIndex';
 import { showToast } from '../../util/showToast';
-import {
+import { hasFailed, isDownloaded, isDownloading } from '../../types/Attachment';
-  hasFailed,
-  hasNotResolved,
-  isDownloaded,
-  isDownloading,
-} from '../../types/Attachment';
 import {
   getConversationSelector,
   getHideStoryConversationIds,
 } from '../selectors/conversations';
-import { getStories } from '../selectors/stories';
+import {
+  getStories,
+  getStoryDownloadableAttachment,
+} from '../selectors/stories';
 import { getStoryDataFromMessageAttributes } from '../../services/storyLoader';
 import { isGroup } from '../../util/whatTypeOfConversation';
 import { isNotNil } from '../../util/isNotNil';
@@ -113,7 +111,6 @@ const LIST_MEMBERS_VERIFIED = 'stories/LIST_MEMBERS_VERIFIED';
 const LOAD_STORY_REPLIES = 'stories/LOAD_STORY_REPLIES';
 const MARK_STORY_READ = 'stories/MARK_STORY_READ';
 const QUEUE_STORY_DOWNLOAD = 'stories/QUEUE_STORY_DOWNLOAD';
-export const RESOLVE_ATTACHMENT_URL = 'stories/RESOLVE_ATTACHMENT_URL';
 const SEND_STORY_MODAL_OPEN_STATE_CHANGED =
   'stories/SEND_STORY_MODAL_OPEN_STATE_CHANGED';
 const STORY_CHANGED = 'stories/STORY_CHANGED';
@@ -155,14 +152,6 @@ type QueueStoryDownloadActionType = {
   payload: string;
 };
 
-type ResolveAttachmentUrlActionType = {
-  type: typeof RESOLVE_ATTACHMENT_URL;
-  payload: {
-    messageId: string;
-    attachmentUrl: string;
-  };
-};
-
 type SendStoryModalOpenStateChanged = {
   type: typeof SEND_STORY_MODAL_OPEN_STATE_CHANGED;
   payload: number | undefined;
@@ -195,7 +184,6 @@ export type StoriesActionType =
   | MessageDeletedActionType
   | MessagesAddedActionType
   | QueueStoryDownloadActionType
-  | ResolveAttachmentUrlActionType
   | SendStoryModalOpenStateChanged
   | StoryChangedActionType
   | ToggleViewActionType
@@ -337,7 +325,7 @@ function queueStoryDownload(
   void,
   RootStateType,
   unknown,
-  NoopActionType | QueueStoryDownloadActionType | ResolveAttachmentUrlActionType
+  NoopActionType | QueueStoryDownloadActionType
 > {
   return async (dispatch, getState) => {
     const { stories } = getState().stories;
@@ -347,7 +335,7 @@ function queueStoryDownload(
       return;
     }
 
-    const { attachment } = story;
+    const attachment = getStoryDownloadableAttachment(story);
 
     if (!attachment) {
       log.warn('queueStoryDownload: No attachment found for story', {
@@ -365,21 +353,6 @@ function queueStoryDownload(
         return;
       }
 
-      // This function also resolves the attachment's URL in case we've already
-      // downloaded the attachment but haven't pointed its path to an absolute
-      // location on disk.
-      if (hasNotResolved(attachment)) {
-        dispatch({
-          type: RESOLVE_ATTACHMENT_URL,
-          payload: {
-            messageId: storyId,
-            attachmentUrl: window.Signal.Migrations.getAbsoluteAttachmentPath(
-              attachment.path
-            ),
-          },
-        });
-      }
-
       return;
     }
 
@@ -403,7 +376,10 @@ function queueStoryDownload(
         payload: storyId,
       });
 
-      await queueAttachmentDownloads(message.attributes);
+      const updatedFields = await queueAttachmentDownloads(message.attributes);
+      if (updatedFields) {
+        message.set(updatedFields);
+      }
       return;
     }
 
@@ -627,11 +603,7 @@ const getSelectedStoryDataForDistributionListId = (
 };
 
 const getSelectedStoryDataForConversationId = (
-  dispatch: ThunkDispatch<
+  dispatch: ThunkDispatch<RootStateType, unknown, NoopActionType>,
-    RootStateType,
-    unknown,
-    NoopActionType | ResolveAttachmentUrlActionType
-  >,
   getState: () => RootStateType,
   conversationId: string,
   selectedStoryId?: string
@@ -671,12 +643,12 @@ const getSelectedStoryDataForConversationId = (
   const numStories = storiesByConversationId.length;
 
   // Queue all undownloaded stories once we're viewing someone's stories
-  storiesByConversationId.forEach(item => {
+  storiesByConversationId.forEach(({ attachment, messageId }) => {
-    if (isDownloaded(item.attachment) || isDownloading(item.attachment)) {
+    if (isDownloaded(attachment) || isDownloading(attachment)) {
       return;
     }
 
-    queueStoryDownload(item.messageId)(dispatch, getState, null);
+    queueStoryDownload(messageId)(dispatch, getState, null);
   });
 
   return {
@@ -1416,41 +1388,6 @@ export function reducer(
     };
   }
 
-  if (action.type === RESOLVE_ATTACHMENT_URL) {
-    const { messageId, attachmentUrl } = action.payload;
-
-    const storyIndex = state.stories.findIndex(
-      existingStory => existingStory.messageId === messageId
-    );
-
-    if (storyIndex < 0) {
-      return state;
-    }
-
-    const story = state.stories[storyIndex];
-
-    if (!story.attachment) {
-      return state;
-    }
-
-    const storyWithResolvedAttachment = {
-      ...story,
-      attachment: {
-        ...story.attachment,
-        url: attachmentUrl,
-      },
-    };
-
-    return {
-      ...state,
-      stories: replaceIndex(
-        state.stories,
-        storyIndex,
-        storyWithResolvedAttachment
-      ),
-    };
-  }
-
   if (action.type === DOE_STORY) {
     return {
       ...state,

ts/state/selectors/stories.ts

@@ -7,6 +7,7 @@ import { pick } from 'lodash';
 import type { GetConversationByIdType } from './conversations';
 import type { ConversationType } from '../ducks/conversations';
 import type { MessageReactionType } from '../../model-types.d';
+import type { AttachmentType } from '../../types/Attachment';
 import type {
   ConversationStoryType,
   MyStoryType,
@@ -133,6 +134,13 @@ function getAvatarData(
   ]);
 }
 
+export function getStoryDownloadableAttachment({
+  attachment,
+}: StoryDataType): AttachmentType | undefined {
+  // See: getStoryDataFromMessageAttributes for how preview gets populated.
+  return attachment?.textAttachment?.preview?.image ?? attachment;
+}
+
 export function getStoryView(
   conversationSelector: GetConversationByIdType,
   ourConversationId: string | undefined,
@@ -159,13 +167,7 @@ export function getStoryView(
     expireTimer,
     readAt,
     timestamp,
-  } = pick(story, [
+  } = story;
-    'attachment',
-    'expirationStartTimestamp',
-    'expireTimer',
-    'readAt',
-    'timestamp',
-  ]);
 
   const { sendStateByConversationId } = story;
   let sendState: Array<StorySendStateType> | undefined;

ts/test-electron/state/ducks/stories_test.ts

@@ -3,12 +3,9 @@
 
 import * as sinon from 'sinon';
 import casual from 'casual';
-import path from 'path';
-import { assert } from 'chai';
 
 import type {
   DispatchableViewStoryType,
-  StoriesStateType,
   StoryDataType,
 } from '../../../state/ducks/stories';
 import type { ConversationType } from '../../../state/ducks/conversations';
@@ -16,19 +13,14 @@ import type { MessageAttributesType } from '../../../model-types.d';
 import type { StateType as RootStateType } from '../../../state/reducer';
 import type { UUIDStringType } from '../../../types/UUID';
 import { DAY } from '../../../util/durations';
-import { IMAGE_JPEG } from '../../../types/MIME';
+import { TEXT_ATTACHMENT, IMAGE_JPEG } from '../../../types/MIME';
 import { ReadStatus } from '../../../messages/MessageReadStatus';
 import {
   StoryViewDirectionType,
   StoryViewModeType,
 } from '../../../types/Stories';
 import { UUID } from '../../../types/UUID';
-import {
+import { actions, getEmptyState } from '../../../state/ducks/stories';
-  actions,
-  getEmptyState,
-  reducer,
-  RESOLVE_ATTACHMENT_URL,
-} from '../../../state/ducks/stories';
 import { noopAction } from '../../../state/ducks/noop';
 import { reducer as rootReducer } from '../../../state/reducer';
 import { dropNull } from '../../../util/dropNull';
@@ -917,86 +909,6 @@ describe('both/state/ducks/stories', () => {
       sinon.assert.notCalled(dispatch);
     });
 
-    it('downloaded, but unresolved, we should resolve the path', async function test() {
-      const storyId = UUID.generate().toString();
-      const attachment = {
-        contentType: IMAGE_JPEG,
-        path: 'image.jpg',
-        size: 0,
-      };
-      const messageAttributes = {
-        ...getStoryMessage(storyId),
-        attachments: [attachment],
-      };
-
-      const rootState = getEmptyRootState();
-
-      const getState = () => ({
-        ...rootState,
-        stories: {
-          ...rootState.stories,
-          stories: [
-            {
-              ...messageAttributes,
-              attachment: messageAttributes.attachments[0],
-              messageId: messageAttributes.id,
-              expireTimer: messageAttributes.expireTimer,
-              expirationStartTimestamp: dropNull(
-                messageAttributes.expirationStartTimestamp
-              ),
-            },
-          ],
-        },
-      });
-
-      window.MessageController.register(storyId, messageAttributes);
-
-      const dispatch = sinon.spy();
-      await queueStoryDownload(storyId)(dispatch, getState, null);
-
-      const action = dispatch.getCall(0).args[0];
-
-      sinon.assert.calledWith(dispatch, {
-        type: RESOLVE_ATTACHMENT_URL,
-        payload: {
-          messageId: storyId,
-          attachmentUrl: action.payload.attachmentUrl,
-        },
-      });
-      assert.equal(
-        attachment.path,
-        path.basename(action.payload.attachmentUrl)
-      );
-
-      const stateWithStory: StoriesStateType = {
-        ...getEmptyRootState().stories,
-        stories: [
-          {
-            ...messageAttributes,
-            messageId: storyId,
-            attachment,
-            expireTimer: messageAttributes.expireTimer,
-            expirationStartTimestamp: dropNull(
-              messageAttributes.expirationStartTimestamp
-            ),
-          },
-        ],
-      };
-
-      const nextState = reducer(stateWithStory, action);
-      assert.isDefined(nextState.stories);
-      assert.equal(
-        nextState.stories[0].attachment?.url,
-        action.payload.attachmentUrl
-      );
-
-      const state = getEmptyRootState().stories;
-
-      const sameState = reducer(state, action);
-      assert.isDefined(sameState.stories);
-      assert.equal(sameState, state);
-    });
-
     it('not downloaded, queued for download', async function test() {
       const storyId = UUID.generate().toString();
       const messageAttributes = {
@@ -1039,5 +951,59 @@ describe('both/state/ducks/stories', () => {
         payload: storyId,
       });
     });
+
+    it('preview not downloaded, queued for download', async function test() {
+      const storyId = UUID.generate().toString();
+      const preview = {
+        url: 'https://signal.org',
+        image: {
+          contentType: IMAGE_JPEG,
+          size: 0,
+        },
+      };
+      const messageAttributes = {
+        ...getStoryMessage(storyId),
+        attachments: [
+          {
+            contentType: TEXT_ATTACHMENT,
+            size: 0,
+            textAttachment: {
+              preview,
+            },
+          },
+        ],
+        preview: [preview],
+      };
+
+      const rootState = getEmptyRootState();
+
+      const getState = () => ({
+        ...rootState,
+        stories: {
+          ...rootState.stories,
+          stories: [
+            {
+              ...messageAttributes,
+              attachment: messageAttributes.attachments[0],
+              messageId: messageAttributes.id,
+              expireTimer: messageAttributes.expireTimer,
+              expirationStartTimestamp: dropNull(
+                messageAttributes.expirationStartTimestamp
+              ),
+            },
+          ],
+        },
+      });
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getState, null);
+
+      sinon.assert.calledWith(dispatch, {
+        type: 'stories/QUEUE_STORY_DOWNLOAD',
+        payload: storyId,
+      });
+    });
   });
 });

ts/textsecure/MessageReceiver.ts

@@ -3,7 +3,7 @@
 
 /* eslint-disable no-bitwise */
 
-import { isBoolean, isNumber } from 'lodash';
+import { isBoolean, isNumber, omit } from 'lodash';
 import PQueue from 'p-queue';
 import { v4 as getGuid } from 'uuid';
 
@@ -60,6 +60,7 @@ import createTaskWithTimeout from './TaskWithTimeout';
 import {
   processAttachment,
   processDataMessage,
+  processPreview,
   processGroupV2Context,
 } from './processDataMessage';
 import { processSyncMessage } from './processSyncMessage';
@@ -75,6 +76,7 @@ import * as Bytes from '../Bytes';
 import type {
   ProcessedAttachment,
   ProcessedDataMessage,
+  ProcessedPreview,
   ProcessedSyncMessage,
   ProcessedSent,
   ProcessedEnvelope,
@@ -1993,6 +1995,7 @@ export default class MessageReceiver
     log.info('MessageReceiver.handleStoryMessage', logId);
 
     const attachments: Array<ProcessedAttachment> = [];
+    let preview: ReadonlyArray<ProcessedPreview> | undefined;
 
     if (msg.fileAttachment) {
       const attachment = processAttachment(msg.fileAttachment);
@@ -2000,16 +2003,17 @@ export default class MessageReceiver
     }
 
     if (msg.textAttachment) {
-      const { text } = msg.textAttachment;
+      const { text, preview: unprocessedPreview } = msg.textAttachment;
-      if (!text) {
+      if (unprocessedPreview) {
-        throw new Error('Text attachments must have text!');
+        preview = processPreview([unprocessedPreview]);
+      } else if (!text) {
+        throw new Error('Text attachments must have text or link preview!');
       }
 
-      // TODO DESKTOP-3714 we should download the story link preview image
       attachments.push({
-        size: text.length,
+        size: text?.length ?? 0,
         contentType: TEXT_ATTACHMENT,
-        textAttachment: msg.textAttachment,
+        textAttachment: omit(msg.textAttachment, 'preview'),
         blurHash: generateBlurHash(
           (msg.textAttachment.color ||
             msg.textAttachment.gradient?.startColor) ??
@@ -2045,6 +2049,7 @@ export default class MessageReceiver
 
     const message: ProcessedDataMessage = {
       attachments,
+      preview,
       canReplyToStory: Boolean(msg.allowsReplies),
       expireTimer: durations.DAY / 1000,
       flags: 0,

ts/types/Attachment.ts

@@ -697,20 +697,33 @@ export function isGIF(attachments?: ReadonlyArray<AttachmentType>): boolean {
   return hasFlag && isVideoAttachment(attachment);
 }
 
+function resolveNestedAttachment(
+  attachment?: AttachmentType
+): AttachmentType | undefined {
+  if (attachment?.textAttachment?.preview?.image) {
+    return attachment.textAttachment.preview.image;
+  }
+  return attachment;
+}
+
 export function isDownloaded(attachment?: AttachmentType): boolean {
-  return Boolean(attachment && (attachment.path || attachment.textAttachment));
+  const resolved = resolveNestedAttachment(attachment);
+  return Boolean(resolved && (resolved.path || resolved.textAttachment));
 }
 
 export function hasNotResolved(attachment?: AttachmentType): boolean {
-  return Boolean(attachment && !attachment.url && !attachment.textAttachment);
+  const resolved = resolveNestedAttachment(attachment);
+  return Boolean(resolved && !resolved.url && !resolved.textAttachment);
 }
 
 export function isDownloading(attachment?: AttachmentType): boolean {
-  return Boolean(attachment && attachment.downloadJobId && attachment.pending);
+  const resolved = resolveNestedAttachment(attachment);
+  return Boolean(resolved && resolved.downloadJobId && resolved.pending);
 }
 
 export function hasFailed(attachment?: AttachmentType): boolean {
-  return Boolean(attachment && attachment.error);
+  const resolved = resolveNestedAttachment(attachment);
+  return Boolean(resolved && resolved.error);
 }
 
 export function hasVideoBlurHash(attachments?: Array<AttachmentType>): boolean {