Allow link-only stories, download previews

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>

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(
-          (item: LinkPreviewType) =>
-            (item.image || item.title) &&
-            urls.includes(item.url) &&
-            LinkPreview.shouldPreviewHref(item.url)
-        );
+        const preview = incomingPreview.filter((item: LinkPreviewType) => {
+          if (!item.image && !item.title) {
+            return false;
+          }
+          // 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 {
-  hasFailed,
-  hasNotResolved,
-  isDownloaded,
-  isDownloading,
-} from '../../types/Attachment';
+import { hasFailed, 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<
-    RootStateType,
-    unknown,
-    NoopActionType | ResolveAttachmentUrlActionType
-  >,
+  dispatch: ThunkDispatch<RootStateType, unknown, NoopActionType>,
   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 => {
-    if (isDownloaded(item.attachment) || isDownloading(item.attachment)) {
+  storiesByConversationId.forEach(({ attachment, messageId }) => {
+    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, [
-    'attachment',
-    'expirationStartTimestamp',
-    'expireTimer',
-    'readAt',
-    'timestamp',
-  ]);
+  } = story;
 
   const { sendStateByConversationId } = story;
   let sendState: Array<StorySendStateType> | undefined;