feat: improved quoted message not found

consolidated quote props lookup into getMessageQuoteProps, only use the db message in the quote components

ts/components/conversation/message/message-content/MessageContent.tsx

@@ -182,7 +182,7 @@ export const MessageContent = (props: Props) => {
             <StyledMessageOpaqueContent messageDirection={direction} highlight={highlight}>
               {!isDeleted && (
                 <>
-                  <MessageQuote messageId={props.messageId} />
+                  <MessageQuote messageId={props.messageId} direction={direction} />
                   <MessageLinkPreview
                     messageId={props.messageId}
                     handleImageError={handleImageError}

ts/components/conversation/message/message-content/MessageQuote.tsx

@@ -1,7 +1,7 @@
 import React, { useCallback } from 'react';
 import { useSelector } from 'react-redux';
-import _ from 'lodash';
-import { MessageRenderingProps } from '../../../../models/messageType';
+import _, { isEmpty } from 'lodash';
+import { MessageModelType, MessageRenderingProps } from '../../../../models/messageType';
 import { PubKey } from '../../../../session/types';
 import { openConversationToSpecificMessage } from '../../../../state/ducks/conversations';
 import {
@@ -16,41 +16,35 @@ import { ToastUtils } from '../../../../session/utils';
 
 type Props = {
   messageId: string;
+  // Note: this is the direction of the quote in case the quoted message is not found
+  direction: MessageModelType;
 };
 
 export type MessageQuoteSelectorProps = Pick<MessageRenderingProps, 'quote' | 'direction'>;
 
 export const MessageQuote = (props: Props) => {
+  const selected = useSelector(state => getMessageQuoteProps(state as any, props.messageId));
   const multiSelectMode = useSelector(isMessageSelectionMode);
   const isMessageDetailViewMode = useSelector(isMessageDetailView);
 
-  const selected = useSelector(state => getMessageQuoteProps(state as any, props.messageId));
-  if (!selected) {
+  if (!selected || isEmpty(selected)) {
     return null;
   }
 
-  const { quote, direction } = selected;
-  if (!quote || !quote.sender || !quote.messageId) {
+  const quote = selected ? selected.quote : undefined;
+  const direction = selected ? selected.direction : props.direction ? props.direction : undefined;
+
+  if (!quote || isEmpty(quote)) {
     return null;
   }
 
-  const {
-    text,
-    attachment,
-    isFromMe,
-    sender: quoteAuthor,
-    authorProfileName,
-    authorName,
-    messageId: quotedMessageId,
-    referencedMessageNotFound,
-    convoId,
-  } = quote;
-
-  const quoteText = text || null;
-  const quoteNotFound = referencedMessageNotFound || false;
+  const quoteNotFound = Boolean(
+    !quote?.sender || !quote.messageId || !quote.convoId || quote.referencedMessageNotFound
+  );
 
-  const shortenedPubkey = PubKey.shorten(quoteAuthor);
-  const displayedPubkey = authorProfileName ? shortenedPubkey : quoteAuthor;
+  const quoteText = quote?.text || null;
+  const shortenedPubkey = quote?.sender ? PubKey.shorten(quote?.sender) : undefined;
+  const displayedPubkey = String(quote?.authorProfileName ? shortenedPubkey : quote?.sender);
 
   const onQuoteClick = useCallback(
     async (event: React.MouseEvent<HTMLDivElement>) => {
@@ -58,6 +52,7 @@ export const MessageQuote = (props: Props) => {
       event.stopPropagation();
 
       if (!quote) {
+        ToastUtils.pushOriginalNotFound();
         window.log.warn('onQuoteClick: quote not valid');
         return;
       }
@@ -68,40 +63,32 @@ export const MessageQuote = (props: Props) => {
       }
 
       // For simplicity's sake, we show the 'not found' toast no matter what if we were
-      // not able to find the referenced message when the quote was received.
-      if (quoteNotFound || !quotedMessageId || !quoteAuthor || !convoId) {
+      // not able to find the referenced message when the quote was received or if the conversation no longer exists.
+      if (quoteNotFound) {
         ToastUtils.pushOriginalNotFound();
         return;
+      } else {
+        void openConversationToSpecificMessage({
+          conversationKey: String(quote.convoId),
+          messageIdToNavigateTo: String(quote.messageId),
+          shouldHighlightMessage: true,
+        });
       }
-
-      void openConversationToSpecificMessage({
-        conversationKey: convoId,
-        messageIdToNavigateTo: quotedMessageId,
-        shouldHighlightMessage: true,
-      });
     },
-    [
-      convoId,
-      isMessageDetailViewMode,
-      multiSelectMode,
-      quote,
-      quoteNotFound,
-      quotedMessageId,
-      quoteAuthor,
-    ]
+    [isMessageDetailViewMode, multiSelectMode, quote, quoteNotFound]
   );
 
   return (
     <Quote
       onClick={onQuoteClick}
       text={quoteText}
-      attachment={attachment}
+      attachment={quote?.attachment}
       isIncoming={direction === 'incoming'}
       sender={displayedPubkey}
-      authorProfileName={authorProfileName}
-      authorName={authorName}
-      referencedMessageNotFound={referencedMessageNotFound || false}
-      isFromMe={isFromMe || false}
+      authorProfileName={quote?.authorProfileName}
+      authorName={quote?.authorName}
+      referencedMessageNotFound={quoteNotFound}
+      isFromMe={quote?.isFromMe || false}
     />
   );
 };

ts/components/conversation/message/message-content/quote/Quote.tsx

@@ -11,47 +11,6 @@ import { QuoteIconContainer } from './QuoteIconContainer';
 import styled from 'styled-components';
 import { isEmpty } from 'lodash';
 
-export type QuotePropsWithoutListener = {
-  attachment?: QuotedAttachmentType;
-  sender: string;
-  authorProfileName?: string;
-  authorName?: string;
-  isFromMe: boolean;
-  isIncoming: boolean;
-  text: string | null;
-  referencedMessageNotFound: boolean;
-};
-
-export type QuotePropsWithListener = QuotePropsWithoutListener & {
-  onClick?: (e: React.MouseEvent<HTMLDivElement>) => void;
-};
-
-export interface Attachment {
-  contentType: MIME.MIMEType;
-  /** Not included in protobuf, and is loaded asynchronously */
-  objectUrl?: string;
-}
-
-export interface QuotedAttachmentType {
-  contentType: MIME.MIMEType;
-  fileName: string;
-  /** Not included in protobuf */
-  isVoiceMessage: boolean;
-  thumbnail?: Attachment;
-}
-
-function validateQuote(quote: QuotePropsWithoutListener): boolean {
-  if (quote.text) {
-    return true;
-  }
-
-  if (quote.attachment) {
-    return true;
-  }
-
-  return false;
-}
-
 const StyledQuoteContainer = styled.div`
   min-width: 300px; // if the quoted content is small it doesn't look very good so we set a minimum
   padding-right: var(--margins-xs);
@@ -87,19 +46,41 @@ const StyledQuoteTextContent = styled.div`
   justify-content: center;
 `;
 
-export const Quote = (props: QuotePropsWithListener) => {
-  const [imageBroken, setImageBroken] = useState(false);
-  const handleImageErrorBound = () => {
-    setImageBroken(true);
-  };
+export type QuoteProps = {
+  attachment?: QuotedAttachmentType;
+  sender: string;
+  authorProfileName?: string;
+  authorName?: string;
+  isFromMe: boolean;
+  isIncoming: boolean;
+  text: string | null;
+  referencedMessageNotFound: boolean;
+  onClick?: (e: React.MouseEvent<HTMLDivElement>) => void;
+};
+
+export interface Attachment {
+  contentType: MIME.MIMEType;
+  /** Not included in protobuf, and is loaded asynchronously */
+  objectUrl?: string;
+}
+
+export interface QuotedAttachmentType {
+  contentType: MIME.MIMEType;
+  fileName: string;
+  /** Not included in protobuf */
+  isVoiceMessage: boolean;
+  thumbnail?: Attachment;
+}
 
+export const Quote = (props: QuoteProps) => {
   const isPublic = useSelector(isPublicGroupConversation);
 
-  if (!validateQuote(props)) {
-    return null;
-  }
+  const { isIncoming, attachment, text, referencedMessageNotFound, onClick } = props;
 
-  const { isIncoming, attachment, text, onClick } = props;
+  const [imageBroken, setImageBroken] = useState(false);
+  const handleImageErrorBound = () => {
+    setImageBroken(true);
+  };
 
   return (
     <StyledQuoteContainer>
@@ -112,17 +93,24 @@ export const Quote = (props: QuotePropsWithListener) => {
           attachment={attachment}
           handleImageErrorBound={handleImageErrorBound}
           imageBroken={imageBroken}
+          referencedMessageNotFound={referencedMessageNotFound}
         />
         <StyledQuoteTextContent>
           <QuoteAuthor
+            sender={props.sender}
             authorName={props.authorName}
-            author={props.sender}
             authorProfileName={props.authorProfileName}
             isFromMe={props.isFromMe}
-            isIncoming={props.isIncoming}
+            isIncoming={isIncoming}
             showPubkeyForAuthor={isPublic}
+            referencedMessageNotFound={referencedMessageNotFound}
+          />
+          <QuoteText
+            isIncoming={isIncoming}
+            text={text}
+            attachment={attachment}
+            referencedMessageNotFound={referencedMessageNotFound}
           />
-          <QuoteText isIncoming={isIncoming} text={text} attachment={attachment} />
         </StyledQuoteTextContent>
       </StyledQuote>
     </StyledQuoteContainer>

ts/components/conversation/message/message-content/quote/QuoteAuthor.tsx

@@ -2,6 +2,7 @@ import React = require('react');
 import { ContactName } from '../../../ContactName';
 import { PubKey } from '../../../../../session/types';
 import styled from 'styled-components';
+import { QuoteProps } from './Quote';
 
 const StyledQuoteAuthor = styled.div<{ isIncoming: boolean }>`
   color: ${props =>
@@ -20,17 +21,32 @@ const StyledQuoteAuthor = styled.div<{ isIncoming: boolean }>`
   }
 `;
 
-type QuoteAuthorProps = {
-  author: string;
-  authorProfileName?: string;
-  authorName?: string;
-  isFromMe: boolean;
-  isIncoming: boolean;
+type QuoteAuthorProps = Pick<
+  QuoteProps,
+  | 'authorName'
+  | 'authorProfileName'
+  | 'isFromMe'
+  | 'isIncoming'
+  | 'referencedMessageNotFound'
+  | 'sender'
+> & {
   showPubkeyForAuthor?: boolean;
 };
 
 export const QuoteAuthor = (props: QuoteAuthorProps) => {
-  const { authorProfileName, author, authorName, isFromMe, isIncoming } = props;
+  const {
+    authorProfileName,
+    authorName,
+    isFromMe,
+    isIncoming,
+    referencedMessageNotFound,
+    sender,
+    showPubkeyForAuthor,
+  } = props;
+
+  if (referencedMessageNotFound) {
+    return null;
+  }
 
   return (
     <StyledQuoteAuthor isIncoming={isIncoming}>
@@ -38,11 +54,11 @@ export const QuoteAuthor = (props: QuoteAuthorProps) => {
         window.i18n('you')
       ) : (
         <ContactName
-          pubkey={PubKey.shorten(author)}
+          pubkey={PubKey.shorten(sender)}
           name={authorName}
           profileName={authorProfileName}
           compact={true}
-          shouldShowPubkey={Boolean(props.showPubkeyForAuthor)}
+          shouldShowPubkey={Boolean(showPubkeyForAuthor)}
         />
       )}
     </StyledQuoteAuthor>

ts/components/conversation/message/message-content/quote/QuoteIconContainer.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { Attachment, QuotePropsWithoutListener } from './Quote';
+import { Attachment, QuoteProps } from './Quote';
 import { GoogleChrome } from '../../../../../util';
 import { MIME } from '../../../../../types';
 
@@ -82,14 +82,14 @@ export const QuoteIcon = (props: QuoteIconProps) => {
 };
 
 export const QuoteIconContainer = (
-  props: Pick<QuotePropsWithoutListener, 'attachment'> & {
+  props: Pick<QuoteProps, 'attachment' | 'referencedMessageNotFound'> & {
     handleImageErrorBound: () => void;
     imageBroken: boolean;
   }
 ) => {
-  const { attachment, imageBroken, handleImageErrorBound } = props;
+  const { attachment, imageBroken, handleImageErrorBound, referencedMessageNotFound } = props;
 
-  if (!attachment || isEmpty(attachment)) {
+  if (referencedMessageNotFound || !attachment || isEmpty(attachment)) {
     return null;
   }
 

ts/components/conversation/message/message-content/quote/QuoteText.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { QuotePropsWithoutListener } from './Quote';
+import { QuoteProps } from './Quote';
 import { useSelector } from 'react-redux';
 import { getSelectedConversationKey } from '../../../../../state/selectors/conversations';
 import { useIsPrivate } from '../../../../../hooks/useParamSelector';
@@ -66,14 +66,14 @@ function getTypeLabel({
 }
 
 export const QuoteText = (
-  props: Pick<QuotePropsWithoutListener, 'text' | 'attachment' | 'isIncoming'>
+  props: Pick<QuoteProps, 'text' | 'attachment' | 'isIncoming' | 'referencedMessageNotFound'>
 ) => {
-  const { text, attachment, isIncoming } = props;
+  const { text, attachment, isIncoming, referencedMessageNotFound } = props;
 
   const convoId = useSelector(getSelectedConversationKey);
   const isGroup = !useIsPrivate(convoId);
 
-  if (attachment && !isEmpty(attachment)) {
+  if (!referencedMessageNotFound && attachment && !isEmpty(attachment)) {
     const { contentType, isVoiceMessage } = attachment;
 
     const typeLabel = getTypeLabel({ contentType, isVoiceMessage });

ts/data/data.ts

@@ -451,8 +451,8 @@ async function filterAlreadyFetchedOpengroupMessage(
 }
 
 /**
- *
- * @param propsList An array of objects containing a source (the sender id) and timestamp of the message - not to be confused with the serverTimestamp. This is equivalent to sent_at
+ * Fetch all messages that match the sender pubkey and sent_at timestamp
+ * @param {Object[]} propsList An array of objects containing a source (the sender id) and timestamp of the message - not to be confused with the serverTimestamp. This is equivalent to sent_at
  * @returns
  */
 async function getMessagesBySenderAndSentAt(

ts/models/message.ts

@@ -43,7 +43,6 @@ import {
   PropsForGroupUpdateLeft,
   PropsForGroupUpdateName,
   PropsForMessageWithoutConvoProps,
-  PropsForQuote,
 } from '../state/ducks/conversations';
 import {
   VisibleMessage,
@@ -1390,7 +1389,7 @@ export function findAndFormatContact(pubkey: string): FindAndFormatContactType {
   };
 }
 
-function processQuoteAttachment(attachment: any) {
+export function processQuoteAttachment(attachment: any) {
   const { thumbnail } = attachment;
   const path = thumbnail && thumbnail.path && getAbsoluteAttachmentPath(thumbnail.path);
   const objectUrl = thumbnail && thumbnail.objectUrl;
@@ -1409,50 +1408,3 @@ function processQuoteAttachment(attachment: any) {
   });
   // tslint:enable: prefer-object-spread
 }
-
-// TODO rename and consolidate with getPropsForQuote
-export function overrideWithSourceMessage(
-  quote: PropsForQuote,
-  msg: MessageModelPropsWithoutConvoProps
-): PropsForQuote {
-  const msgProps = msg.propsForMessage;
-  if (!msgProps || msgProps.isDeleted) {
-    return quote;
-  }
-
-  const convo = getConversationController().getOrThrow(String(msgProps?.convoId));
-
-  const sender = msgProps.sender && isEmpty(msgProps.sender) ? msgProps.sender : quote.sender;
-  const contact = findAndFormatContact(sender);
-  const authorName = contact?.profileName || contact?.name || window.i18n('unknown');
-  const attachment =
-    msgProps.attachments && msgProps.attachments[0] ? msgProps.attachments[0] : quote.attachment;
-
-  let isFromMe = convo ? convo.id === UserUtils.getOurPubKeyStrFromCache() : false;
-
-  if (convo?.isPublic() && PubKey.hasBlindedPrefix(sender)) {
-    const room = OpenGroupData.getV2OpenGroupRoom(msgProps.convoId);
-    if (room && roomHasBlindEnabled(room)) {
-      const usFromCache = findCachedBlindedIdFromUnblinded(
-        UserUtils.getOurPubKeyStrFromCache(),
-        room.serverPublicKey
-      );
-      if (usFromCache && usFromCache === sender) {
-        isFromMe = true;
-      }
-    }
-  }
-
-  const quoteProps: PropsForQuote = {
-    text: msgProps.text || quote.text,
-    attachment: attachment ? processQuoteAttachment(attachment) : undefined,
-    isFromMe,
-    sender,
-    authorName,
-    messageId: msgProps.id || quote.messageId,
-    referencedMessageNotFound: false,
-    convoId: convo.id,
-  };
-
-  return quoteProps;
-}

ts/node/sql.ts

@@ -1089,7 +1089,7 @@ function getMessagesBySenderAndSentAt(
     rows.push(..._rows);
   }
 
-  return uniq(map(rows, row => jsonToObject(row.json)));
+  return map(rows, row => jsonToObject(row.json));
 }
 
 function filterAlreadyFetchedOpengroupMessage(
@@ -1294,7 +1294,7 @@ function getMessagesByConversation(
   }
 
   if (returnQuotes) {
-    quotes = messages.filter(message => message.quote).map(message => message.quote);
+    quotes = uniq(messages.filter(message => message.quote).map(message => message.quote));
   }
 
   return { messages, quotes };

ts/state/ducks/conversations.ts

@@ -345,6 +345,12 @@ export type MentionsMembersType = Array<{
   authorProfileName: string;
 }>;
 
+/**
+ * Fetches the messages for a conversation to put into redux.
+ * @param {string} conversationKey - the id of the conversation
+ * @param {string} messageId - the id of the message in view so we can fetch the messages around it
+ * @returns
+ */
 async function getMessages({
   conversationKey,
   messageId,
@@ -382,8 +388,8 @@ async function getMessages({
 
   if (quotesCollection?.length) {
     const quotePropsList = quotesCollection.map(quote => ({
-      timestamp: Number(quote?.id),
-      source: String(quote?.author),
+      timestamp: Number(quote.id),
+      source: String(quote.author),
     }));
 
     const quotedMessagesCollection = await Data.getMessagesBySenderAndSentAt(quotePropsList);

ts/state/selectors/conversations.ts

@@ -8,6 +8,7 @@ import {
   MessageModelPropsWithConvoProps,
   MessageModelPropsWithoutConvoProps,
   MessagePropsDetails,
+  PropsForQuote,
   ReduxConversationType,
   SortedMessageModelProps,
 } from '../ducks/conversations';
@@ -36,7 +37,11 @@ import { ConversationTypeEnum } from '../../models/conversationAttributes';
 
 import { MessageReactsSelectorProps } from '../../components/conversation/message/message-content/MessageReactions';
 import { filter, isEmpty, pick, sortBy } from 'lodash';
-import { overrideWithSourceMessage } from '../../models/message';
+import { findAndFormatContact, processQuoteAttachment } from '../../models/message';
+import { PubKey } from '../../session/types';
+import { OpenGroupData } from '../../data/opengroups';
+import { roomHasBlindEnabled } from '../../session/apis/open_group_api/sogsv3/sogsV3Capabilities';
+import { findCachedBlindedIdFromUnblinded } from '../../session/apis/open_group_api/sogsv3/knownBlindedkeys';
 
 export const getConversations = (state: StateType): ConversationsStateType => state.conversations;
 
@@ -976,21 +981,59 @@ export const getMessageQuoteProps = createSelector(
     }
 
     const direction = msgProps.propsForMessage.direction;
-    let quote = msgProps.propsForQuote;
-    if (!direction || !quote || isEmpty(quote)) {
+    if (!msgProps.propsForQuote || isEmpty(msgProps.propsForQuote)) {
       return undefined;
     }
 
-    const { messageId, sender } = quote;
+    const { messageId, sender } = msgProps.propsForQuote;
     if (!messageId || !sender) {
       return undefined;
     }
 
     const sourceMessage = convosProps.quotes[`${messageId}-${sender}`];
-    if (sourceMessage) {
-      quote = overrideWithSourceMessage(quote, sourceMessage);
+    if (!sourceMessage) {
+      return { direction, quote: { sender, referencedMessageNotFound: true } };
     }
 
+    const sourceMsgProps = sourceMessage.propsForMessage;
+    if (!sourceMsgProps || sourceMsgProps.isDeleted) {
+      return { direction, quote: { sender, referencedMessageNotFound: true } };
+    }
+
+    const convo = getConversationController().get(sourceMsgProps.convoId);
+    if (!convo) {
+      return { direction, quote: { sender, referencedMessageNotFound: true } };
+    }
+
+    const contact = findAndFormatContact(sourceMsgProps.sender);
+    const authorName = contact?.profileName || contact?.name || window.i18n('unknown');
+    const attachment = sourceMsgProps.attachments && sourceMsgProps.attachments[0];
+    let isFromMe = convo ? convo.id === UserUtils.getOurPubKeyStrFromCache() : false;
+
+    if (convo.isPublic() && PubKey.hasBlindedPrefix(sourceMsgProps.sender)) {
+      const room = OpenGroupData.getV2OpenGroupRoom(sourceMsgProps.convoId);
+      if (room && roomHasBlindEnabled(room)) {
+        const usFromCache = findCachedBlindedIdFromUnblinded(
+          UserUtils.getOurPubKeyStrFromCache(),
+          room.serverPublicKey
+        );
+        if (usFromCache && usFromCache === sourceMsgProps.sender) {
+          isFromMe = true;
+        }
+      }
+    }
+
+    const quote: PropsForQuote = {
+      text: sourceMsgProps.text,
+      attachment: attachment ? processQuoteAttachment(attachment) : undefined,
+      isFromMe,
+      sender: sourceMsgProps.sender,
+      authorName,
+      messageId: sourceMsgProps.id,
+      referencedMessageNotFound: false,
+      convoId: convo.id,
+    };
+
     return { direction, quote };
   }
 );