From e90e548715d46405281942ab75ba5371e7f9d7a3 Mon Sep 17 00:00:00 2001 From: William Grant Date: Mon, 15 May 2023 13:54:18 +1000 Subject: [PATCH] feat: improved quoted message not found consolidated quote props lookup into getMessageQuoteProps, only use the db message in the quote components --- .../message-content/MessageContent.tsx | 2 +- .../message/message-content/MessageQuote.tsx | 75 ++++++--------- .../message/message-content/quote/Quote.tsx | 94 ++++++++----------- .../message-content/quote/QuoteAuthor.tsx | 34 +++++-- .../quote/QuoteIconContainer.tsx | 8 +- .../message-content/quote/QuoteText.tsx | 8 +- ts/data/data.ts | 4 +- ts/models/message.ts | 50 +--------- ts/node/sql.ts | 4 +- ts/state/ducks/conversations.ts | 10 +- ts/state/selectors/conversations.ts | 55 +++++++++-- 11 files changed, 168 insertions(+), 176 deletions(-) diff --git a/ts/components/conversation/message/message-content/MessageContent.tsx b/ts/components/conversation/message/message-content/MessageContent.tsx index ad5ec2a51..b3dda4912 100644 --- a/ts/components/conversation/message/message-content/MessageContent.tsx +++ b/ts/components/conversation/message/message-content/MessageContent.tsx @@ -182,7 +182,7 @@ export const MessageContent = (props: Props) => { {!isDeleted && ( <> - + ; 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) => { @@ -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 ( ); }; diff --git a/ts/components/conversation/message/message-content/quote/Quote.tsx b/ts/components/conversation/message/message-content/quote/Quote.tsx index fd3d9ce6f..157fa2c02 100644 --- a/ts/components/conversation/message/message-content/quote/Quote.tsx +++ b/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) => 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) => 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 ( @@ -112,17 +93,24 @@ export const Quote = (props: QuotePropsWithListener) => { attachment={attachment} handleImageErrorBound={handleImageErrorBound} imageBroken={imageBroken} + referencedMessageNotFound={referencedMessageNotFound} /> + - diff --git a/ts/components/conversation/message/message-content/quote/QuoteAuthor.tsx b/ts/components/conversation/message/message-content/quote/QuoteAuthor.tsx index 434b23559..252fc0483 100644 --- a/ts/components/conversation/message/message-content/quote/QuoteAuthor.tsx +++ b/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 ( @@ -38,11 +54,11 @@ export const QuoteAuthor = (props: QuoteAuthorProps) => { window.i18n('you') ) : ( )} diff --git a/ts/components/conversation/message/message-content/quote/QuoteIconContainer.tsx b/ts/components/conversation/message/message-content/quote/QuoteIconContainer.tsx index cae16fe56..b0a77989f 100644 --- a/ts/components/conversation/message/message-content/quote/QuoteIconContainer.tsx +++ b/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 & { + props: Pick & { 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; } diff --git a/ts/components/conversation/message/message-content/quote/QuoteText.tsx b/ts/components/conversation/message/message-content/quote/QuoteText.tsx index 6ef3fb0b1..b6a1efd2b 100644 --- a/ts/components/conversation/message/message-content/quote/QuoteText.tsx +++ b/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 + props: Pick ) => { - 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 }); diff --git a/ts/data/data.ts b/ts/data/data.ts index f6051429c..d6397158a 100644 --- a/ts/data/data.ts +++ b/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( diff --git a/ts/models/message.ts b/ts/models/message.ts index 16f94ccc9..3adf923c3 100644 --- a/ts/models/message.ts +++ b/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; -} diff --git a/ts/node/sql.ts b/ts/node/sql.ts index f0b6d1714..2faebc858 100644 --- a/ts/node/sql.ts +++ b/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 }; diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 594b35b63..b9beb1c17 100644 --- a/ts/state/ducks/conversations.ts +++ b/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); diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index 3aa52a556..6b4f66205 100644 --- a/ts/state/selectors/conversations.ts +++ b/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 }; } );