Fix undownloadable attachment check to require download error

Co-authored-by: ayumi-signal <143036029+ayumi-signal@users.noreply.github.com>

ts/components/conversation/Image.tsx

@@ -14,8 +14,8 @@ import type {
 } from '../../types/Attachment';
 import {
   defaultBlurHash,
-  isDownloadable,
   isIncremental,
+  isPermanentlyUndownloadable,
   isReadyToView,
 } from '../../types/Attachment';
 import { ProgressCircle } from '../ProgressCircle';
@@ -191,10 +191,7 @@ export function Image({
   );
 
   const startDownloadButton =
-    startDownload &&
-    !attachment.path &&
-    !attachment.pending &&
-    !isIncremental(attachment) ? (
+    !attachment.path && !attachment.pending && !isIncremental(attachment) ? (
       <button
         type="button"
         className="module-image__overlay-circle"
@@ -207,6 +204,25 @@ export function Image({
       </button>
     ) : undefined;
 
+  const isUndownloadable = isPermanentlyUndownloadable(attachment);
+
+  // eslint-disable-next-line no-nested-ternary
+  const startDownloadOrUnavailableButton = startDownload ? (
+    isUndownloadable ? (
+      <button
+        type="button"
+        className="module-image__overlay-circle module-image__overlay-circle--undownloadable"
+        aria-label={i18n('icu:mediaNoLongerAvailable')}
+        onClick={undownloadableClick}
+        tabIndex={tabIndex}
+      >
+        <div className="module-image__undownloadable-icon" />
+      </button>
+    ) : (
+      startDownloadButton
+    )
+  ) : null;
+
   const spinner =
     isIncremental(attachment) || !cancelDownload
       ? undefined
@@ -218,8 +234,6 @@ export function Image({
           tabIndex,
         });
 
-  const isMediaDownloadable = isDownloadable(attachment);
-
   return (
     <div
       className={classNames(
@@ -235,19 +249,7 @@ export function Image({
       }}
     >
       {imageOrBlurHash}
-      {isMediaDownloadable ? (
-        startDownloadButton
-      ) : (
-        <button
-          type="button"
-          className="module-image__overlay-circle module-image__overlay-circle--undownloadable"
-          aria-label={i18n('icu:mediaNoLongerAvailable')}
-          onClick={undownloadableClick}
-          tabIndex={tabIndex}
-        >
-          <div className="module-image__undownloadable-icon" />
-        </button>
-      )}
+      {startDownloadOrUnavailableButton}
       {spinner}
 
       {attachment.caption ? (
@@ -267,7 +269,7 @@ export function Image({
         />
       ) : null}
       {(attachment.path || isIncremental(attachment)) &&
-      isMediaDownloadable &&
+      !isUndownloadable &&
       playIconOverlay ? (
         <div className="module-image__overlay-circle">
           <div className="module-image__play-icon" />
@@ -290,9 +292,7 @@ export function Image({
           style={curveStyles}
         />
       ) : null}
-      {showVisualAttachment &&
-      isReadyToView(attachment) &&
-      isMediaDownloadable ? (
+      {showVisualAttachment && isReadyToView(attachment) ? (
         <button
           type="button"
           className={classNames('module-image__border-overlay', {

ts/components/conversation/ImageGrid.tsx

@@ -16,6 +16,7 @@ import {
   getUrl,
   isDownloadable,
   isIncremental,
+  isPermanentlyUndownloadable,
   isVideoAttachment,
 } from '../../types/Attachment';
 
@@ -157,6 +158,14 @@ export function ImageGrid({
     [startDownload]
   );
 
+  const showAttachmentOrNoLongerAvailableToast = React.useCallback(
+    attachmentIndex =>
+      isPermanentlyUndownloadable(attachments[attachmentIndex])
+        ? showMediaNoLongerAvailableToast
+        : showVisualAttachment,
+    [attachments, showVisualAttachment, showMediaNoLongerAvailableToast]
+  );
+
   if (!attachments || !attachments.length) {
     return null;
   }
@@ -215,7 +224,7 @@ export function ImageGrid({
           }
           tabIndex={tabIndex}
           showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-          showVisualAttachment={showVisualAttachment}
+          showVisualAttachment={showAttachmentOrNoLongerAvailableToast(0)}
           cancelDownload={cancelDownload}
           startDownload={startDownload}
           onError={onError}
@@ -244,7 +253,7 @@ export function ImageGrid({
           cropWidth={GAP}
           url={getThumbnailUrl(attachments[0])}
           showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-          showVisualAttachment={showVisualAttachment}
+          showVisualAttachment={showAttachmentOrNoLongerAvailableToast(0)}
           cancelDownload={cancelDownload}
           startDownload={downloadPill ? undefined : startDownload}
           onError={onError}
@@ -264,7 +273,7 @@ export function ImageGrid({
           attachment={attachments[1]}
           url={getThumbnailUrl(attachments[1])}
           showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-          showVisualAttachment={showVisualAttachment}
+          showVisualAttachment={showAttachmentOrNoLongerAvailableToast(1)}
           cancelDownload={cancelDownload}
           startDownload={downloadPill ? undefined : startDownload}
           onError={onError}
@@ -294,7 +303,7 @@ export function ImageGrid({
           cropWidth={GAP}
           url={getUrl(attachments[0])}
           showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-          showVisualAttachment={showVisualAttachment}
+          showVisualAttachment={showAttachmentOrNoLongerAvailableToast(0)}
           cancelDownload={cancelDownload}
           startDownload={downloadPill ? undefined : startDownload}
           onError={onError}
@@ -313,7 +322,7 @@ export function ImageGrid({
             playIconOverlay={isVideoAttachment(attachments[1])}
             url={getThumbnailUrl(attachments[1])}
             showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-            showVisualAttachment={showVisualAttachment}
+            showVisualAttachment={showAttachmentOrNoLongerAvailableToast(1)}
             cancelDownload={cancelDownload}
             startDownload={downloadPill ? undefined : startDownload}
             onError={onError}
@@ -332,7 +341,7 @@ export function ImageGrid({
             playIconOverlay={isVideoAttachment(attachments[2])}
             url={getThumbnailUrl(attachments[2])}
             showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-            showVisualAttachment={showVisualAttachment}
+            showVisualAttachment={showAttachmentOrNoLongerAvailableToast(2)}
             cancelDownload={cancelDownload}
             startDownload={downloadPill ? undefined : startDownload}
             onError={onError}
@@ -364,7 +373,7 @@ export function ImageGrid({
               cropWidth={GAP}
               url={getThumbnailUrl(attachments[0])}
               showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-              showVisualAttachment={showVisualAttachment}
+              showVisualAttachment={showAttachmentOrNoLongerAvailableToast(0)}
               cancelDownload={cancelDownload}
               startDownload={downloadPill ? undefined : startDownload}
               onError={onError}
@@ -383,7 +392,7 @@ export function ImageGrid({
               attachment={attachments[1]}
               url={getThumbnailUrl(attachments[1])}
               showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-              showVisualAttachment={showVisualAttachment}
+              showVisualAttachment={showAttachmentOrNoLongerAvailableToast(1)}
               cancelDownload={cancelDownload}
               startDownload={downloadPill ? undefined : startDownload}
               onError={onError}
@@ -405,7 +414,7 @@ export function ImageGrid({
               attachment={attachments[2]}
               url={getThumbnailUrl(attachments[2])}
               showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-              showVisualAttachment={showVisualAttachment}
+              showVisualAttachment={showAttachmentOrNoLongerAvailableToast(2)}
               cancelDownload={cancelDownload}
               startDownload={downloadPill ? undefined : startDownload}
               onError={onError}
@@ -424,7 +433,7 @@ export function ImageGrid({
               attachment={attachments[3]}
               url={getThumbnailUrl(attachments[3])}
               showMediaNoLongerAvailableToast={showMediaNoLongerAvailableToast}
-              showVisualAttachment={showVisualAttachment}
+              showVisualAttachment={showAttachmentOrNoLongerAvailableToast(3)}
               cancelDownload={cancelDownload}
               startDownload={downloadPill ? undefined : startDownload}
               onError={onError}

ts/components/conversation/Message.tsx

@@ -70,7 +70,7 @@ import {
   isVideo,
   isGIF,
   isPlayed,
-  isDownloadable,
+  isPermanentlyUndownloadable,
 } from '../../types/Attachment';
 import type { EmbeddedContactType } from '../../types/EmbeddedContact';
 
@@ -2563,6 +2563,7 @@ export class Message extends React.PureComponent<Props, State> {
       showLightbox,
       showExpiredIncomingTapToViewToast,
       showExpiredOutgoingTapToViewToast,
+      showMediaNoLongerAvailableToast,
     } = this.props;
     const { imageBroken } = this.state;
 
@@ -2573,6 +2574,18 @@ export class Message extends React.PureComponent<Props, State> {
       return;
     }
 
+    if (
+      attachments &&
+      attachments.length > 0 &&
+      isPermanentlyUndownloadable(attachments[0])
+    ) {
+      event.preventDefault();
+      event.stopPropagation();
+
+      showMediaNoLongerAvailableToast();
+      return;
+    }
+
     if (isTapToView) {
       if (isAttachmentPending) {
         log.info(
@@ -2610,8 +2623,7 @@ export class Message extends React.PureComponent<Props, State> {
       attachments &&
       attachments.length > 0 &&
       !isAttachmentPending &&
-      !isDownloaded(attachments[0]) &&
-      isDownloadable(attachments[0])
+      !isDownloaded(attachments[0])
     ) {
       event.preventDefault();
       event.stopPropagation();

ts/types/Attachment.ts

@@ -1268,6 +1268,12 @@ export function isDownloadable(attachment: AttachmentType): boolean {
   );
 }
 
+export function isPermanentlyUndownloadable(
+  attachment: AttachmentType
+): boolean {
+  return Boolean(!isDownloadable(attachment) && attachment.error);
+}
+
 export function isAttachmentLocallySaved(
   attachment: AttachmentType
 ): attachment is LocallySavedAttachment {