Ensure that we resolve attachments before displaying them

stylesheets/components/StoryImage.scss

@@ -22,7 +22,7 @@
     width: 100%;
   }
 
-  &__spinner-container {
+  &__overlay-container {
     align-items: center;
     display: flex;
     height: 100%;

ts/components/StoryImage.tsx

@@ -40,7 +40,9 @@ export const StoryImage = ({
   storyId,
 }: PropsType): JSX.Element | null => {
   const shouldDownloadAttachment =
-    !isDownloaded(attachment) && !isDownloading(attachment);
+    !isDownloaded(attachment) &&
+    !isDownloading(attachment) &&
+    !hasNotResolved(attachment);
 
   useEffect(() => {
     if (shouldDownloadAttachment) {
@@ -61,7 +63,11 @@ export const StoryImage = ({
   let storyElement: JSX.Element;
   if (attachment.textAttachment) {
     storyElement = (
-      <TextAttachment i18n={i18n} textAttachment={attachment.textAttachment} />
+      <TextAttachment
+        i18n={i18n}
+        isThumbnail={isThumbnail}
+        textAttachment={attachment.textAttachment}
+      />
     );
   } else if (isNotReadyToShow) {
     storyElement = (
@@ -98,10 +104,10 @@ export const StoryImage = ({
     );
   }
 
-  let spinner: JSX.Element | undefined;
+  let overlay: JSX.Element | undefined;
   if (isPending) {
-    spinner = (
-      <div className="StoryImage__spinner-container">
+    overlay = (
+      <div className="StoryImage__overlay-container">
         <div className="StoryImage__spinner-bubble" title={i18n('loading')}>
           <Spinner moduleClassName="StoryImage__spinner" svgSize="small" />
         </div>
@@ -117,7 +123,7 @@ export const StoryImage = ({
       )}
     >
       {storyElement}
-      {spinner}
+      {overlay}
     </div>
   );
 };

ts/components/StoryViewer.tsx

@@ -198,6 +198,11 @@ export const StoryViewer = ({
   // We need to be careful about this effect refreshing, it should only run
   // every time a story changes or its duration changes.
   useEffect(() => {
+    if (!storyDuration) {
+      spring.stop();
+      return;
+    }
+
     spring.start({
       config: {
         duration: storyDuration,

ts/components/TextAttachment.stories.tsx

@@ -164,7 +164,20 @@ story.add('Link preview', () => (
       preview: {
         url: 'https://www.signal.org/workworkwork',
         title: 'Signal >> Careers',
-        // TODO add image
+      },
+    }}
+  />
+));
+
+story.add('Link preview (thumbnail)', () => (
+  <TextAttachment
+    {...getDefaultProps()}
+    isThumbnail
+    textAttachment={{
+      color: 4294951251,
+      preview: {
+        url: 'https://www.signal.org/workworkwork',
+        title: 'Signal >> Careers',
       },
     }}
   />

ts/components/TextAttachment.tsx

@@ -40,6 +40,7 @@ enum TextSize {
 
 export type PropsType = {
   i18n: LocalizerType;
+  isThumbnail?: boolean;
   textAttachment: TextAttachmentType;
 };
 
@@ -85,6 +86,7 @@ function getFont(
 
 export const TextAttachment = ({
   i18n,
+  isThumbnail,
   textAttachment,
 }: PropsType): JSX.Element | null => {
   const linkPreview = useRef<HTMLDivElement | null>(null);
@@ -149,25 +151,27 @@ export const TextAttachment = ({
             )}
             {textAttachment.preview && (
               <>
-                {linkPreviewOffsetTop && textAttachment.preview.url && (
-                  <a
-                    className="TextAttachment__preview__tooltip"
-                    href={textAttachment.preview.url}
-                    rel="noreferrer"
-                    style={{
-                      top: linkPreviewOffsetTop - 150,
-                    }}
-                    target="_blank"
-                  >
-                    <div>
-                      <div>{i18n('TextAttachment__preview__link')}</div>
-                      <div className="TextAttachment__preview__tooltip__url">
-                        {textAttachment.preview.url}
+                {linkPreviewOffsetTop &&
+                  !isThumbnail &&
+                  textAttachment.preview.url && (
+                    <a
+                      className="TextAttachment__preview__tooltip"
+                      href={textAttachment.preview.url}
+                      rel="noreferrer"
+                      style={{
+                        top: linkPreviewOffsetTop - 150,
+                      }}
+                      target="_blank"
+                    >
+                      <div>
+                        <div>{i18n('TextAttachment__preview__link')}</div>
+                        <div className="TextAttachment__preview__tooltip__url">
+                          {textAttachment.preview.url}
+                        </div>
                       </div>
-                    </div>
-                    <div className="TextAttachment__preview__tooltip__arrow" />
-                  </a>
-                )}
+                      <div className="TextAttachment__preview__tooltip__arrow" />
+                    </a>
+                  )}
                 <div
                   className={classNames('TextAttachment__preview', {
                     'TextAttachment__preview--large': Boolean(

ts/state/ducks/stories.ts

@@ -25,7 +25,11 @@ import { markViewed } from '../../services/MessageUpdater';
 import { queueAttachmentDownloads } from '../../util/queueAttachmentDownloads';
 import { replaceIndex } from '../../util/replaceIndex';
 import { showToast } from '../../util/showToast';
-import { isDownloaded, isDownloading } from '../../types/Attachment';
+import {
+  hasNotResolved,
+  isDownloaded,
+  isDownloading,
+} from '../../types/Attachment';
 import { useBoundActions } from '../../hooks/useBoundActions';
 import { viewSyncJobQueue } from '../../jobs/viewSyncJobQueue';
 import { viewedReceiptsJobQueue } from '../../jobs/viewedReceiptsJobQueue';
@@ -63,6 +67,7 @@ const LOAD_STORY_REPLIES = 'stories/LOAD_STORY_REPLIES';
 const MARK_STORY_READ = 'stories/MARK_STORY_READ';
 const REACT_TO_STORY = 'stories/REACT_TO_STORY';
 const REPLY_TO_STORY = 'stories/REPLY_TO_STORY';
+export const RESOLVE_ATTACHMENT_URL = 'stories/RESOLVE_ATTACHMENT_URL';
 const STORY_CHANGED = 'stories/STORY_CHANGED';
 const TOGGLE_VIEW = 'stories/TOGGLE_VIEW';
 
@@ -92,6 +97,14 @@ type ReplyToStoryActionType = {
   payload: MessageAttributesType;
 };
 
+type ResolveAttachmentUrlActionType = {
+  type: typeof RESOLVE_ATTACHMENT_URL;
+  payload: {
+    messageId: string;
+    attachmentUrl: string;
+  };
+};
+
 type StoryChangedActionType = {
   type: typeof STORY_CHANGED;
   payload: StoryDataType;
@@ -108,6 +121,7 @@ export type StoriesActionType =
   | MessageDeletedActionType
   | ReactToStoryActionType
   | ReplyToStoryActionType
+  | ResolveAttachmentUrlActionType
   | StoryChangedActionType
   | ToggleViewActionType;
 
@@ -206,7 +220,12 @@ function markStoryRead(
 
 function queueStoryDownload(
   storyId: string
-): ThunkAction<void, RootStateType, unknown, NoopActionType> {
+): ThunkAction<
+  void,
+  RootStateType,
+  unknown,
+  NoopActionType | ResolveAttachmentUrlActionType
+> {
   return async dispatch => {
     const story = await getMessageById(storyId);
 
@@ -226,6 +245,25 @@ function queueStoryDownload(
     }
 
     if (isDownloaded(attachment)) {
+      if (!attachment.path) {
+        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;
     }
 
@@ -500,5 +538,40 @@ 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
+      ),
+    };
+  }
+
   return state;
 }

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

@@ -0,0 +1,178 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import * as sinon from 'sinon';
+import path from 'path';
+import { assert } from 'chai';
+import { v4 as uuid } from 'uuid';
+
+import type { StoriesStateType } from '../../../state/ducks/stories';
+import type { MessageAttributesType } from '../../../model-types.d';
+import { IMAGE_JPEG } from '../../../types/MIME';
+import {
+  actions,
+  getEmptyState,
+  reducer,
+  RESOLVE_ATTACHMENT_URL,
+} from '../../../state/ducks/stories';
+import { noopAction } from '../../../state/ducks/noop';
+import { reducer as rootReducer } from '../../../state/reducer';
+
+describe('both/state/ducks/stories', () => {
+  const getEmptyRootState = () => ({
+    ...rootReducer(undefined, noopAction()),
+    stories: getEmptyState(),
+  });
+
+  function getStoryMessage(id: string): MessageAttributesType {
+    const now = Date.now();
+
+    return {
+      conversationId: uuid(),
+      id,
+      received_at: now,
+      sent_at: now,
+      timestamp: now,
+      type: 'story',
+    };
+  }
+
+  describe('queueStoryDownload', () => {
+    const { queueStoryDownload } = actions;
+
+    it('no attachment, no dispatch', async function test() {
+      const storyId = uuid();
+      const messageAttributes = getStoryMessage(storyId);
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getEmptyRootState, null);
+
+      sinon.assert.notCalled(dispatch);
+    });
+
+    it('downloading, no dispatch', async function test() {
+      const storyId = uuid();
+      const messageAttributes = {
+        ...getStoryMessage(storyId),
+        attachments: [
+          {
+            contentType: IMAGE_JPEG,
+            downloadJobId: uuid(),
+            pending: true,
+            size: 0,
+          },
+        ],
+      };
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getEmptyRootState, null);
+
+      sinon.assert.notCalled(dispatch);
+    });
+
+    it('downloaded, no dispatch', async function test() {
+      const storyId = uuid();
+      const messageAttributes = {
+        ...getStoryMessage(storyId),
+        attachments: [
+          {
+            contentType: IMAGE_JPEG,
+            path: 'image.jpg',
+            url: '/path/to/image.jpg',
+            size: 0,
+          },
+        ],
+      };
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getEmptyRootState, null);
+
+      sinon.assert.notCalled(dispatch);
+    });
+
+    it('downloaded, but unresolved, we should resolve the path', async function test() {
+      const storyId = uuid();
+      const attachment = {
+        contentType: IMAGE_JPEG,
+        path: 'image.jpg',
+        size: 0,
+      };
+      const messageAttributes = {
+        ...getStoryMessage(storyId),
+        attachments: [attachment],
+      };
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getEmptyRootState, 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,
+          },
+        ],
+      };
+
+      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();
+      const messageAttributes = {
+        ...getStoryMessage(storyId),
+        attachments: [
+          {
+            contentType: IMAGE_JPEG,
+            size: 0,
+          },
+        ],
+      };
+
+      window.MessageController.register(storyId, messageAttributes);
+
+      const dispatch = sinon.spy();
+      await queueStoryDownload(storyId)(dispatch, getEmptyRootState, null);
+
+      sinon.assert.calledWith(dispatch, {
+        type: 'NOOP',
+        payload: null,
+      });
+    });
+  });
+});

ts/types/Attachment.ts

@@ -730,7 +730,7 @@ export function isDownloaded(attachment?: AttachmentType): boolean {
 }
 
 export function hasNotResolved(attachment?: AttachmentType): boolean {
-  return Boolean(attachment && !attachment.url);
+  return Boolean(attachment && !attachment.url && !attachment.textAttachment);
 }
 
 export function isDownloading(attachment?: AttachmentType): boolean {

ts/util/getStoryDuration.ts

@@ -2,7 +2,12 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type { AttachmentType } from '../types/Attachment';
-import { isGIF, isVideo } from '../types/Attachment';
+import {
+  hasNotResolved,
+  isDownloaded,
+  isGIF,
+  isVideo,
+} from '../types/Attachment';
 import { count } from './grapheme';
 import { SECOND } from './durations';
 
@@ -12,7 +17,11 @@ const MIN_TEXT_DURATION = 3 * SECOND;
 
 export async function getStoryDuration(
   attachment: AttachmentType
-): Promise<number> {
+): Promise<number | undefined> {
+  if (!isDownloaded(attachment) || hasNotResolved(attachment)) {
+    return;
+  }
+
   if (isGIF([attachment]) || isVideo([attachment])) {
     const videoEl = document.createElement('video');
     if (!attachment.url) {