From 5dff1768bda644d6cd551ad697c7e7ff4f7ca51b Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Fri, 3 Mar 2023 19:03:15 -0800 Subject: [PATCH] Page media in Lightbox --- ts/components/AvatarLightbox.tsx | 4 + ts/components/Lightbox.stories.tsx | 36 ++- ts/components/Lightbox.tsx | 33 ++- ts/models/conversations.ts | 32 +- ts/models/messages.ts | 14 +- ts/sql/Client.ts | 71 +---- ts/sql/Interface.ts | 91 ++---- ts/sql/Server.ts | 292 +++++++++---------- ts/sql/migrations/79-paging-lightbox.ts | 32 ++ ts/sql/migrations/index.ts | 2 + ts/state/ducks/lightbox.ts | 250 ++++++++++++++-- ts/state/ducks/stories.ts | 12 +- ts/state/selectors/lightbox.ts | 10 + ts/state/smart/Lightbox.tsx | 57 +++- ts/test-electron/sql/timelineFetches_test.ts | 48 +-- ts/util/cleanup.ts | 14 +- 16 files changed, 603 insertions(+), 395 deletions(-) create mode 100644 ts/sql/migrations/79-paging-lightbox.ts diff --git a/ts/components/AvatarLightbox.tsx b/ts/components/AvatarLightbox.tsx index 58b20558390b..282c3defc14b 100644 --- a/ts/components/AvatarLightbox.tsx +++ b/ts/components/AvatarLightbox.tsx @@ -35,6 +35,10 @@ export function AvatarLightbox({ saveAttachment={noop} toggleForwardMessageModal={noop} onMediaPlaybackStart={noop} + onNextAttachment={noop} + onPrevAttachment={noop} + onSelectAttachment={noop} + selectedIndex={0} > = {}): PropsType => ({ - closeLightbox: action('closeLightbox'), - i18n, - isViewOnce: Boolean(overrideProps.isViewOnce), - media: overrideProps.media || [], - saveAttachment: action('saveAttachment'), - selectedIndex: number('selectedIndex', overrideProps.selectedIndex || 0), - toggleForwardMessageModal: action('toggleForwardMessageModal'), - onMediaPlaybackStart: noop, -}); +const createProps = (overrideProps: Partial = {}): PropsType => { + // eslint-disable-next-line react-hooks/rules-of-hooks + const [selectedIndex, setSelectedIndex] = useState( + number('selectedIndex', overrideProps.selectedIndex || 0) + ); + const media = overrideProps.media || []; + return { + closeLightbox: action('closeLightbox'), + i18n, + isViewOnce: Boolean(overrideProps.isViewOnce), + media, + saveAttachment: action('saveAttachment'), + selectedIndex, + toggleForwardMessageModal: action('toggleForwardMessageModal'), + onMediaPlaybackStart: noop, + onPrevAttachment: () => { + setSelectedIndex(Math.max(0, selectedIndex - 1)); + }, + onNextAttachment: () => { + setSelectedIndex(Math.min(media.length - 1, selectedIndex + 1)); + }, + onSelectAttachment: setSelectedIndex, + }; +}; export function Multimedia(): JSX.Element { const props = createProps({ diff --git a/ts/components/Lightbox.tsx b/ts/components/Lightbox.tsx index d4fd872de0b1..4045fcce2f3d 100644 --- a/ts/components/Lightbox.tsx +++ b/ts/components/Lightbox.tsx @@ -32,9 +32,14 @@ export type PropsType = { isViewOnce?: boolean; media: ReadonlyArray>; saveAttachment: SaveAttachmentActionCreatorType; - selectedIndex?: number; + selectedIndex: number; toggleForwardMessageModal: (messageId: string) => unknown; onMediaPlaybackStart: () => void; + onNextAttachment: () => void; + onPrevAttachment: () => void; + onSelectAttachment: (index: number) => void; + hasPrevMessage?: boolean; + hasNextMessage?: boolean; }; const ZOOM_SCALE = 3; @@ -59,13 +64,16 @@ export function Lightbox({ i18n, isViewOnce = false, saveAttachment, - selectedIndex: initialSelectedIndex = 0, + selectedIndex, toggleForwardMessageModal, onMediaPlaybackStart, + onNextAttachment, + onPrevAttachment, + onSelectAttachment, + hasNextMessage, + hasPrevMessage, }: PropsType): JSX.Element | null { const [root, setRoot] = React.useState(); - const [selectedIndex, setSelectedIndex] = - useState(initialSelectedIndex); const [videoElement, setVideoElement] = useState( null @@ -106,9 +114,9 @@ export function Lightbox({ return; } - setSelectedIndex(prevSelectedIndex => Math.max(prevSelectedIndex - 1, 0)); + onPrevAttachment(); }, - [isZoomed] + [isZoomed, onPrevAttachment] ); const onNext = useCallback( @@ -122,11 +130,9 @@ export function Lightbox({ return; } - setSelectedIndex(prevSelectedIndex => - Math.min(prevSelectedIndex + 1, media.length - 1) - ); + onNextAttachment(); }, - [isZoomed, media] + [isZoomed, onNextAttachment] ); const onTimeUpdate = useCallback(() => { @@ -521,8 +527,9 @@ export function Lightbox({ } } - const hasNext = !isZoomed && selectedIndex < media.length - 1; - const hasPrevious = !isZoomed && selectedIndex > 0; + const hasNext = + !isZoomed && (selectedIndex < media.length - 1 || hasNextMessage); + const hasPrevious = !isZoomed && (selectedIndex > 0 || hasPrevMessage); return root ? createPortal( @@ -663,7 +670,7 @@ export function Lightbox({ event.stopPropagation(); event.preventDefault(); - setSelectedIndex(index); + onSelectAttachment(index); }} > {item.thumbnailObjectUrl ? ( diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 6343c2dc13c0..f7d303784c88 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -1491,7 +1491,8 @@ export class ConversationModel extends window.Backbone } } - const metrics = await getMessageMetricsForConversation(conversationId, { + const metrics = await getMessageMetricsForConversation({ + conversationId, includeStoryReplies: !isGroup(this.attributes), }); @@ -1511,7 +1512,8 @@ export class ConversationModel extends window.Backbone return; } - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, storyId: undefined, @@ -1564,7 +1566,8 @@ export class ConversationModel extends window.Backbone const receivedAt = message.received_at; const sentAt = message.sent_at; - const models = await getOlderMessagesByConversation(conversationId, { + const models = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, messageId: oldestMessageId, @@ -1619,7 +1622,8 @@ export class ConversationModel extends window.Backbone const receivedAt = message.received_at; const sentAt = message.sent_at; - const models = await getNewerMessagesByConversation(conversationId, { + const models = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, receivedAt, @@ -2188,17 +2192,15 @@ export class ConversationModel extends window.Backbone const first = messages ? messages[0] : undefined; // eslint-disable-next-line no-await-in-loop - messages = await window.Signal.Data.getOlderMessagesByConversation( - this.get('id'), - { - includeStoryReplies: !isGroup(this.attributes), - limit: 100, - messageId: first ? first.id : undefined, - receivedAt: first ? first.received_at : undefined, - sentAt: first ? first.sent_at : undefined, - storyId: undefined, - } - ); + messages = await window.Signal.Data.getOlderMessagesByConversation({ + conversationId: this.get('id'), + includeStoryReplies: !isGroup(this.attributes), + limit: 100, + messageId: first ? first.id : undefined, + receivedAt: first ? first.received_at : undefined, + sentAt: first ? first.sent_at : undefined, + storyId: undefined, + }); if (!messages.length) { return; diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 9293a8e83d6e..e5bfa6c322be 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -235,14 +235,12 @@ async function shouldReplyNotifyUser( // If the story is from a different user, only notify if the user has // replied or reacted to the story - const replies = await dataInterface.getOlderMessagesByConversation( - conversation.id, - { - limit: 9000, - storyId, - includeStoryReplies: true, - } - ); + const replies = await dataInterface.getOlderMessagesByConversation({ + conversationId: conversation.id, + limit: 9000, + storyId, + includeStoryReplies: true, + }); const prevCurrentUserReply = replies.find(replyMessage => { return replyMessage.type === 'outgoing'; diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 78b436231d11..56d0e1d197de 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -40,6 +40,7 @@ import { cleanupMessage } from '../util/cleanup'; import { drop } from '../util/drop'; import type { + AdjacentMessagesByConversationOptionsType, AllItemsType, AttachmentDownloadJobType, ClientInterface, @@ -674,77 +675,24 @@ function handleMessageJSON( } async function getNewerMessagesByConversation( - conversationId: string, - { - includeStoryReplies, - limit = 100, - receivedAt = 0, - sentAt = 0, - storyId, - }: { - includeStoryReplies: boolean; - limit?: number; - receivedAt?: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - } + options: AdjacentMessagesByConversationOptionsType ): Promise> { - const messages = await channels.getNewerMessagesByConversation( - conversationId, - { - includeStoryReplies, - limit, - receivedAt, - sentAt, - storyId, - } - ); + const messages = await channels.getNewerMessagesByConversation(options); return handleMessageJSON(messages); } async function getOlderMessagesByConversation( - conversationId: string, - { - includeStoryReplies, - limit = 100, - messageId, - receivedAt = Number.MAX_VALUE, - sentAt = Number.MAX_VALUE, - storyId, - }: { - includeStoryReplies: boolean; - limit?: number; - messageId?: string; - receivedAt?: number; - sentAt?: number; - storyId: string | undefined; - } + options: AdjacentMessagesByConversationOptionsType ): Promise> { - const messages = await channels.getOlderMessagesByConversation( - conversationId, - { - includeStoryReplies, - limit, - receivedAt, - sentAt, - messageId, - storyId, - } - ); + const messages = await channels.getOlderMessagesByConversation(options); return handleMessageJSON(messages); } -async function getConversationRangeCenteredOnMessage(options: { - conversationId: string; - includeStoryReplies: boolean; - limit?: number; - messageId: string; - receivedAt: number; - sentAt?: number; - storyId: UUIDStringType | undefined; -}): Promise> { +async function getConversationRangeCenteredOnMessage( + options: AdjacentMessagesByConversationOptionsType +): Promise> { const result = await channels.getConversationRangeCenteredOnMessage(options); return { @@ -771,7 +719,8 @@ async function removeAllMessagesInConversation( // Yes, we really want the await in the loop. We're deleting a chunk at a // time so we don't use too much memory. // eslint-disable-next-line no-await-in-loop - messages = await getOlderMessagesByConversation(conversationId, { + messages = await getOlderMessagesByConversation({ + conversationId, limit: chunkSize, includeStoryReplies: true, storyId: undefined, diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index a07986169eae..6ca2983d9330 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -19,6 +19,17 @@ import type { RemoveAllConfiguration } from '../types/RemoveAllConfiguration'; import type { LoggerType } from '../types/Logging'; import type { ReadStatus } from '../messages/MessageReadStatus'; +export type AdjacentMessagesByConversationOptionsType = Readonly<{ + conversationId: string; + messageId?: string; + includeStoryReplies: boolean; + limit?: number; + receivedAt?: number; + sentAt?: number; + storyId: string | undefined; + requireVisualMediaAttachments?: boolean; +}>; + export type AttachmentDownloadJobTypeType = | 'long-message' | 'attachment' @@ -481,7 +492,7 @@ export type DataInterface = { getTotalUnreadForConversation: ( conversationId: string, options: { - storyId: UUIDStringType | undefined; + storyId: string | undefined; includeStoryReplies: boolean; } ) => Promise; @@ -491,12 +502,12 @@ export type DataInterface = { newestUnreadAt: number; now?: number; readAt?: number; - storyId?: UUIDStringType; + storyId?: string; }) => Promise; getUnreadReactionsAndMarkRead: (options: { conversationId: string; newestUnreadAt: number; - storyId?: UUIDStringType; + storyId?: string; }) => Promise>; markReactionAsRead: ( targetAuthorUuid: string, @@ -536,13 +547,11 @@ export type DataInterface = { sourceUuid?: UUIDStringType; }) => Promise; // getNewerMessagesByConversation is JSON on server, full message on Client - getMessageMetricsForConversation: ( - conversationId: string, - options: { - storyId?: UUIDStringType; - includeStoryReplies: boolean; - } - ) => Promise; + getMessageMetricsForConversation: (options: { + conversationId: string; + storyId?: string; + includeStoryReplies: boolean; + }) => Promise; // getConversationRangeCenteredOnMessage is JSON on server, full message on client getConversationMessageStats: (options: { conversationId: string; @@ -738,35 +747,14 @@ export type ServerInterface = DataInterface & { ) => Promise>; getOlderMessagesByConversation: ( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - messageId?: string; - receivedAt?: number; - sentAt?: number; - storyId: string | undefined; - } + options: AdjacentMessagesByConversationOptionsType ) => Promise>; getNewerMessagesByConversation: ( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - receivedAt?: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - } + options: AdjacentMessagesByConversationOptionsType ) => Promise>; - getConversationRangeCenteredOnMessage: (options: { - conversationId: string; - includeStoryReplies: boolean; - limit?: number; - messageId: string; - receivedAt: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - }) => Promise< + getConversationRangeCenteredOnMessage: ( + options: AdjacentMessagesByConversationOptionsType + ) => Promise< GetConversationRangeCenteredOnMessageResultType >; @@ -843,35 +831,14 @@ export type ClientExclusiveInterface = { ) => Promise>; getOlderMessagesByConversation: ( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - messageId?: string; - receivedAt?: number; - sentAt?: number; - storyId: string | undefined; - } + options: AdjacentMessagesByConversationOptionsType ) => Promise>; getNewerMessagesByConversation: ( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - receivedAt?: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - } + options: AdjacentMessagesByConversationOptionsType ) => Promise>; - getConversationRangeCenteredOnMessage: (options: { - conversationId: string; - includeStoryReplies: boolean; - limit?: number; - messageId: string; - receivedAt: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - }) => Promise>; + getConversationRangeCenteredOnMessage: ( + options: AdjacentMessagesByConversationOptionsType + ) => Promise>; createOrUpdateIdentityKey: (data: IdentityKeyType) => Promise; getIdentityKeyById: ( diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index f5d8a3f9f750..7ae75585b1c7 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -71,6 +71,7 @@ import { import { updateSchema } from './migrations'; import type { + AdjacentMessagesByConversationOptionsType, StoredAllItemsType, AttachmentDownloadJobType, ConversationMetricsType, @@ -2212,7 +2213,7 @@ async function getUnreadByConversationAndMarkRead({ conversationId: string; includeStoryReplies: boolean; newestUnreadAt: number; - storyId?: UUIDStringType; + storyId?: string; readAt?: number; now?: number; }): Promise { @@ -2315,7 +2316,7 @@ async function getUnreadReactionsAndMarkRead({ }: { conversationId: string; newestUnreadAt: number; - storyId?: UUIDStringType; + storyId?: string; }): Promise> { const db = getInstance(); @@ -2477,64 +2478,106 @@ async function _removeAllReactions(): Promise { db.prepare('DELETE from reactions;').run(); } -async function getOlderMessagesByConversation( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - messageId?: string; - receivedAt?: number; - sentAt?: number; - storyId: string | undefined; - } -): Promise> { - return getOlderMessagesByConversationSync(conversationId, options); +enum AdjacentDirection { + Older = 'Older', + Newer = 'Newer', } -function getOlderMessagesByConversationSync( - conversationId: string, + +function getAdjacentMessagesByConversationSync( + direction: AdjacentDirection, { + conversationId, includeStoryReplies, limit = 100, messageId, - receivedAt = Number.MAX_VALUE, - sentAt = Number.MAX_VALUE, + receivedAt = direction === AdjacentDirection.Older ? Number.MAX_VALUE : 0, + sentAt = direction === AdjacentDirection.Older ? Number.MAX_VALUE : 0, + requireVisualMediaAttachments, storyId, - }: { - includeStoryReplies: boolean; - limit?: number; - messageId?: string; - receivedAt?: number; - sentAt?: number; - storyId: string | undefined; - } + }: AdjacentMessagesByConversationOptionsType ): Array { const db = getInstance(); - return db - .prepare( - ` - SELECT json FROM messages WHERE - conversationId = $conversationId AND - ($messageId IS NULL OR id IS NOT $messageId) AND - isStory IS 0 AND - (${_storyIdPredicate(storyId, includeStoryReplies)}) AND + const timeFilter = + direction === AdjacentDirection.Older + ? ` + (received_at = $received_at AND sent_at < $sent_at) OR + received_at < $received_at + ` + : ` + (received_at = $received_at AND sent_at > $sent_at) OR + received_at > $received_at + `; + + const timeOrder = direction === AdjacentDirection.Older ? 'DESC' : 'ASC'; + + const requireDifferentMessage = + direction === AdjacentDirection.Older || requireVisualMediaAttachments; + + let query = ` + SELECT json FROM messages WHERE + conversationId = $conversationId AND + ${ + requireDifferentMessage + ? '($messageId IS NULL OR id IS NOT $messageId) AND' + : '' + } + ${ + requireVisualMediaAttachments + ? 'hasVisualMediaAttachments IS 1 AND' + : '' + } + isStory IS 0 AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND + ( + ${timeFilter} + ) + ORDER BY received_at ${timeOrder}, sent_at ${timeOrder} + `; + + // See `filterValidAttachments` in ts/state/ducks/lightbox.ts + if (requireVisualMediaAttachments) { + query = ` + SELECT json + FROM (${query}) as messages + WHERE ( - (received_at = $received_at AND sent_at < $sent_at) OR - received_at < $received_at - ) - ORDER BY received_at DESC, sent_at DESC + SELECT COUNT(*) + FROM json_each(messages.json ->> 'attachments') AS attachment + WHERE + attachment.value ->> 'thumbnail' IS NOT NULL AND + attachment.value ->> 'pending' IS NOT 1 AND + attachment.value ->> 'error' IS NULL + ) > 0 LIMIT $limit; - ` - ) - .all({ - conversationId, - limit, - messageId: messageId || null, - received_at: receivedAt, - sent_at: sentAt, - storyId: storyId || null, - }) - .reverse(); + `; + } else { + query = `${query} LIMIT $limit`; + } + + const results = db.prepare(query).all({ + conversationId, + limit, + messageId: messageId || null, + received_at: receivedAt, + sent_at: sentAt, + storyId: storyId || null, + }); + + if (direction === AdjacentDirection.Older) { + results.reverse(); + } + + return results; +} + +async function getOlderMessagesByConversation( + options: AdjacentMessagesByConversationOptionsType +): Promise> { + return getAdjacentMessagesByConversationSync( + AdjacentDirection.Older, + options + ); } async function getAllStories({ @@ -2587,58 +2630,12 @@ async function getAllStories({ } async function getNewerMessagesByConversation( - conversationId: string, - options: { - includeStoryReplies: boolean; - limit?: number; - receivedAt?: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - } + options: AdjacentMessagesByConversationOptionsType ): Promise> { - return getNewerMessagesByConversationSync(conversationId, options); -} -function getNewerMessagesByConversationSync( - conversationId: string, - { - includeStoryReplies, - limit = 100, - receivedAt = 0, - sentAt = 0, - storyId, - }: { - includeStoryReplies: boolean; - limit?: number; - receivedAt?: number; - sentAt?: number; - storyId: UUIDStringType | undefined; - } -): Array { - const db = getInstance(); - const rows: JSONRows = db - .prepare( - ` - SELECT json FROM messages WHERE - conversationId = $conversationId AND - isStory IS 0 AND - (${_storyIdPredicate(storyId, includeStoryReplies)}) AND - ( - (received_at = $received_at AND sent_at > $sent_at) OR - received_at > $received_at - ) - ORDER BY received_at ASC, sent_at ASC - LIMIT $limit; - ` - ) - .all({ - conversationId, - limit, - received_at: receivedAt, - sent_at: sentAt, - storyId: storyId || null, - }); - - return rows; + return getAdjacentMessagesByConversationSync( + AdjacentDirection.Newer, + options + ); } function getOldestMessageForConversation( conversationId: string, @@ -2646,7 +2643,7 @@ function getOldestMessageForConversation( storyId, includeStoryReplies, }: { - storyId?: UUIDStringType; + storyId?: string; includeStoryReplies: boolean; } ): MessageMetricsType | undefined { @@ -2679,7 +2676,7 @@ function getNewestMessageForConversation( storyId, includeStoryReplies, }: { - storyId?: UUIDStringType; + storyId?: string; includeStoryReplies: boolean; } ): MessageMetricsType | undefined { @@ -2842,7 +2839,7 @@ function getOldestUnseenMessageForConversation( storyId, includeStoryReplies, }: { - storyId?: UUIDStringType; + storyId?: string; includeStoryReplies: boolean; } ): MessageMetricsType | undefined { @@ -2874,7 +2871,7 @@ function getOldestUnseenMessageForConversation( async function getTotalUnreadForConversation( conversationId: string, options: { - storyId: UUIDStringType | undefined; + storyId: string | undefined; includeStoryReplies: boolean; } ): Promise { @@ -2886,7 +2883,7 @@ function getTotalUnreadForConversationSync( storyId, includeStoryReplies, }: { - storyId: UUIDStringType | undefined; + storyId: string | undefined; includeStoryReplies: boolean; } ): number { @@ -2917,7 +2914,7 @@ function getTotalUnseenForConversationSync( storyId, includeStoryReplies, }: { - storyId?: UUIDStringType; + storyId?: string; includeStoryReplies: boolean; } ): number { @@ -2943,22 +2940,19 @@ function getTotalUnseenForConversationSync( return row; } -async function getMessageMetricsForConversation( - conversationId: string, - options: { - storyId?: UUIDStringType; - includeStoryReplies: boolean; - } -): Promise { - return getMessageMetricsForConversationSync(conversationId, options); +async function getMessageMetricsForConversation(options: { + conversationId: string; + storyId?: string; + includeStoryReplies: boolean; +}): Promise { + return getMessageMetricsForConversationSync(options); } -function getMessageMetricsForConversationSync( - conversationId: string, - options: { - storyId?: UUIDStringType; - includeStoryReplies: boolean; - } -): ConversationMetricsType { +function getMessageMetricsForConversationSync(options: { + conversationId: string; + storyId?: string; + includeStoryReplies: boolean; +}): ConversationMetricsType { + const { conversationId } = options; const oldest = getOldestMessageForConversation(conversationId, options); const newest = getNewestMessageForConversation(conversationId, options); const oldestUnseen = getOldestUnseenMessageForConversation( @@ -2980,48 +2974,24 @@ function getMessageMetricsForConversationSync( }; } -async function getConversationRangeCenteredOnMessage({ - conversationId, - includeStoryReplies, - limit, - messageId, - receivedAt, - sentAt, - storyId, -}: { - conversationId: string; - includeStoryReplies: boolean; - limit?: number; - messageId: string; - receivedAt: number; - sentAt?: number; - storyId: UUIDStringType | undefined; -}): Promise< +async function getConversationRangeCenteredOnMessage( + options: AdjacentMessagesByConversationOptionsType +): Promise< GetConversationRangeCenteredOnMessageResultType > { const db = getInstance(); return db.transaction(() => { return { - older: getOlderMessagesByConversationSync(conversationId, { - includeStoryReplies, - limit, - messageId, - receivedAt, - sentAt, - storyId, - }), - newer: getNewerMessagesByConversationSync(conversationId, { - includeStoryReplies, - limit, - receivedAt, - sentAt, - storyId, - }), - metrics: getMessageMetricsForConversationSync(conversationId, { - storyId, - includeStoryReplies, - }), + older: getAdjacentMessagesByConversationSync( + AdjacentDirection.Older, + options + ), + newer: getAdjacentMessagesByConversationSync( + AdjacentDirection.Newer, + options + ), + metrics: getMessageMetricsForConversationSync(options), }; })(); } @@ -4998,11 +4968,15 @@ async function getMessagesWithVisualMediaAttachments( const rows: JSONRows = db .prepare( ` - SELECT json FROM messages WHERE + SELECT json FROM messages + INDEXED BY messages_hasVisualMediaAttachments + WHERE isStory IS 0 AND storyId IS NULL AND conversationId = $conversationId AND - hasVisualMediaAttachments = 1 + -- Note that this check has to use 'IS' to utilize + -- 'messages_hasVisualMediaAttachments' INDEX + hasVisualMediaAttachments IS 1 ORDER BY received_at DESC, sent_at DESC LIMIT $limit; ` diff --git a/ts/sql/migrations/79-paging-lightbox.ts b/ts/sql/migrations/79-paging-lightbox.ts new file mode 100644 index 000000000000..d404014fff7a --- /dev/null +++ b/ts/sql/migrations/79-paging-lightbox.ts @@ -0,0 +1,32 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Database } from '@signalapp/better-sqlite3'; + +import type { LoggerType } from '../../types/Logging'; + +export default function updateToSchemaVersion79( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 79) { + return; + } + + db.transaction(() => { + db.exec(` + DROP INDEX messages_hasVisualMediaAttachments; + CREATE INDEX messages_hasVisualMediaAttachments + ON messages ( + conversationId, isStory, storyId, + hasVisualMediaAttachments, received_at, sent_at + ) + WHERE hasVisualMediaAttachments IS 1; + `); + + db.pragma('user_version = 79'); + })(); + + logger.info('updateToSchemaVersion79: success!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index bc9e0989b13f..40636eda9d3e 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -54,6 +54,7 @@ import updateToSchemaVersion75 from './75-noop'; import updateToSchemaVersion76 from './76-optimize-convo-open-2'; import updateToSchemaVersion77 from './77-signal-tokenizer'; import updateToSchemaVersion78 from './78-merge-receipt-jobs'; +import updateToSchemaVersion79 from './79-paging-lightbox'; function updateToSchemaVersion1( currentVersion: number, @@ -1977,6 +1978,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion76, updateToSchemaVersion77, updateToSchemaVersion78, + updateToSchemaVersion79, ]; export function updateSchema(db: Database, logger: LoggerType): void { diff --git a/ts/state/ducks/lightbox.ts b/ts/state/ducks/lightbox.ts index 6751d500b0d2..dbec47fb19d0 100644 --- a/ts/state/ducks/lightbox.ts +++ b/ts/state/ducks/lightbox.ts @@ -18,6 +18,7 @@ import type { StateType as RootStateType } from '../reducer'; import * as log from '../../logging/log'; import { getMessageById } from '../../messages/getMessageById'; +import type { MessageAttributesType } from '../../model-types.d'; import { isGIF } from '../../types/Attachment'; import { isImageTypeSupported, @@ -34,6 +35,7 @@ import { } from './conversations'; import { showStickerPackPreview } from './globalModals'; import { useBoundActions } from '../../hooks/useBoundActions'; +import dataInterface from '../../sql/Client'; // eslint-disable-next-line local-rules/type-alias-readonlydeep export type LightboxStateType = @@ -44,11 +46,14 @@ export type LightboxStateType = isShowingLightbox: true; isViewOnce: boolean; media: ReadonlyArray>; + hasPrevMessage: boolean; + hasNextMessage: boolean; selectedAttachmentPath: string | undefined; }; const CLOSE_LIGHTBOX = 'lightbox/CLOSE'; const SHOW_LIGHTBOX = 'lightbox/SHOW'; +const SET_SELECTED_LIGHTBOX_PATH = 'lightbox/SET_SELECTED_LIGHTBOX_PATH'; type CloseLightboxActionType = ReadonlyDeep<{ type: typeof CLOSE_LIGHTBOX; @@ -60,17 +65,25 @@ type ShowLightboxActionType = { payload: { isViewOnce: boolean; media: ReadonlyArray>; + hasPrevMessage: boolean; + hasNextMessage: boolean; selectedAttachmentPath: string | undefined; }; }; +type SetSelectedLightboxPathActionType = ReadonlyDeep<{ + type: typeof SET_SELECTED_LIGHTBOX_PATH; + payload: string | undefined; +}>; + // eslint-disable-next-line local-rules/type-alias-readonlydeep type LightboxActionType = | CloseLightboxActionType | MessageChangedActionType | MessageDeletedActionType | MessageExpiredActionType - | ShowLightboxActionType; + | ShowLightboxActionType + | SetSelectedLightboxPathActionType; function closeLightbox(): ThunkAction< void, @@ -112,6 +125,8 @@ function showLightboxWithMedia( isViewOnce: false, media, selectedAttachmentPath, + hasPrevMessage: false, + hasNextMessage: false, }, }; } @@ -188,11 +203,21 @@ function showLightboxForViewOnceMedia( isViewOnce: true, media, selectedAttachmentPath: undefined, + hasPrevMessage: false, + hasNextMessage: false, }, }); }; } +function filterValidAttachments( + attributes: MessageAttributesType +): Array { + return (attributes.attachments ?? []).filter( + item => item.thumbnail && !item.pending && !item.error + ); +} + function showLightbox(opts: { attachment: AttachmentType; messageId: string; @@ -232,44 +257,48 @@ function showLightbox(opts: { return; } - const attachments: Array = message.get('attachments') || []; - + const attachments = filterValidAttachments(message.attributes); const loop = isGIF(attachments); const { getAbsoluteAttachmentPath } = window.Signal.Migrations; - const media = attachments - .filter(item => item.thumbnail && !item.pending && !item.error) - .map((item, index) => ({ - objectURL: getAbsoluteAttachmentPath(item.path ?? ''), - path: item.path, - contentType: item.contentType, - loop, - index, - message: { - attachments: message.get('attachments') || [], - id: message.get('id'), - conversationId: - window.ConversationController.lookupOrCreate({ - uuid: message.get('sourceUuid'), - e164: message.get('source'), - reason: 'conversation_view.showLightBox', - })?.id || message.get('conversationId'), - received_at: message.get('received_at'), - received_at_ms: Number(message.get('received_at_ms')), - sent_at: message.get('sent_at'), - }, - attachment: item, - thumbnailObjectUrl: - item.thumbnail?.objectUrl || - getAbsoluteAttachmentPath(item.thumbnail?.path ?? ''), - })); + const authorId = + window.ConversationController.lookupOrCreate({ + uuid: message.get('sourceUuid'), + e164: message.get('source'), + reason: 'conversation_view.showLightBox', + })?.id || message.get('conversationId'); + const receivedAt = message.get('received_at'); + const sentAt = message.get('sent_at'); + + const media = attachments.map((item, index) => ({ + objectURL: getAbsoluteAttachmentPath(item.path ?? ''), + path: item.path, + contentType: item.contentType, + loop, + index, + message: { + attachments: message.get('attachments') || [], + id: messageId, + conversationId: authorId, + received_at: receivedAt, + received_at_ms: Number(message.get('received_at_ms')), + sent_at: sentAt, + }, + attachment: item, + thumbnailObjectUrl: + item.thumbnail?.objectUrl || + getAbsoluteAttachmentPath(item.thumbnail?.path ?? ''), + })); if (!media.length) { log.error( 'showLightbox: unable to load attachment', - attachments.map(x => ({ + sentAt, + message.get('attachments')?.map(x => ({ + thumbnail: !!x.thumbnail, contentType: x.contentType, + pending: x.pending, error: x.error, flags: x.flags, path: x.path, @@ -286,22 +315,172 @@ function showLightbox(opts: { return; } + const { older, newer } = + await dataInterface.getConversationRangeCenteredOnMessage({ + conversationId: message.get('conversationId'), + messageId, + receivedAt, + sentAt, + limit: 1, + storyId: undefined, + includeStoryReplies: false, + + // This is the critical option since we only want messages with visual + // attachments. + requireVisualMediaAttachments: true, + }); + dispatch({ type: SHOW_LIGHTBOX, payload: { isViewOnce: false, media, selectedAttachmentPath: attachment.path, + hasPrevMessage: + older.length > 0 && filterValidAttachments(older[0]).length > 0, + hasNextMessage: + newer.length > 0 && filterValidAttachments(newer[0]).length > 0, }, }); }; } +enum AdjacentMessageDirection { + Previous = 'Previous', + Next = 'Next', +} + +function showLightboxForAdjacentMessage( + direction: AdjacentMessageDirection +): ThunkAction< + void, + RootStateType, + unknown, + ShowLightboxActionType | ShowToastActionType +> { + return async (dispatch, getState) => { + const { lightbox } = getState(); + + if (!lightbox.isShowingLightbox || lightbox.media.length === 0) { + log.warn('showLightboxForAdjacentMessage: empty lightbox'); + return; + } + + const [media] = lightbox.media; + const { + id: messageId, + received_at: receivedAt, + sent_at: sentAt, + } = media.message; + + const message = await getMessageById(messageId); + if (!message) { + log.warn('showLightboxForAdjacentMessage: original message is gone'); + dispatch({ + type: SHOW_TOAST, + payload: { + toastType: ToastType.UnableToLoadAttachment, + }, + }); + return; + } + const conversationId = message.get('conversationId'); + + const options = { + conversationId, + messageId, + receivedAt, + sentAt, + limit: 1, + storyId: undefined, + includeStoryReplies: false, + + // This is the critical option since we only want messages with visual + // attachments. + requireVisualMediaAttachments: true, + }; + + const [adjacent] = + direction === AdjacentMessageDirection.Previous + ? await dataInterface.getOlderMessagesByConversation(options) + : await dataInterface.getNewerMessagesByConversation(options); + + if (!adjacent) { + log.warn( + `showLightboxForAdjacentMessage(${direction}, ${messageId}, ` + + `${sentAt}): no ${direction} message found` + ); + dispatch({ + type: SHOW_TOAST, + payload: { + toastType: ToastType.UnableToLoadAttachment, + }, + }); + return; + } + + const attachments = filterValidAttachments(adjacent); + if (!attachments.length) { + log.warn( + `showLightboxForAdjacentMessage(${direction}, ${messageId}, ` + + `${sentAt}): no valid attachments found` + ); + dispatch({ + type: SHOW_TOAST, + payload: { + toastType: ToastType.UnableToLoadAttachment, + }, + }); + return; + } + + dispatch( + showLightbox({ + attachment: + direction === AdjacentMessageDirection.Previous + ? attachments[attachments.length - 1] + : attachments[0], + messageId: adjacent.id, + }) + ); + }; +} + +function showLightboxForNextMessage(): ThunkAction< + void, + RootStateType, + unknown, + ShowLightboxActionType +> { + return showLightboxForAdjacentMessage(AdjacentMessageDirection.Next); +} + +function showLightboxForPrevMessage(): ThunkAction< + void, + RootStateType, + unknown, + ShowLightboxActionType +> { + return showLightboxForAdjacentMessage(AdjacentMessageDirection.Previous); +} + +function setSelectedLightboxPath( + path: string | undefined +): SetSelectedLightboxPathActionType { + return { + type: SET_SELECTED_LIGHTBOX_PATH, + payload: path, + }; +} + export const actions = { closeLightbox, showLightbox, showLightboxForViewOnceMedia, showLightboxWithMedia, + showLightboxForPrevMessage, + showLightboxForNextMessage, + setSelectedLightboxPath, }; export const useLightboxActions = (): BoundActionCreatorsMapObject< @@ -329,6 +508,17 @@ export function reducer( }; } + if (action.type === SET_SELECTED_LIGHTBOX_PATH) { + if (!state.isShowingLightbox) { + return state; + } + + return { + ...state, + selectedAttachmentPath: action.payload, + }; + } + if ( action.type === MESSAGE_CHANGED || action.type === MESSAGE_DELETED || diff --git a/ts/state/ducks/stories.ts b/ts/state/ducks/stories.ts index 0962775952bd..5256a7644087 100644 --- a/ts/state/ducks/stories.ts +++ b/ts/state/ducks/stories.ts @@ -325,14 +325,12 @@ function loadStoryReplies( ): ThunkAction { return async (dispatch, getState) => { const conversation = getConversationSelector(getState())(conversationId); - const replies = await dataInterface.getOlderMessagesByConversation( + const replies = await dataInterface.getOlderMessagesByConversation({ conversationId, - { - limit: 9000, - storyId: messageId, - includeStoryReplies: !isGroup(conversation), - } - ); + limit: 9000, + storyId: messageId, + includeStoryReplies: !isGroup(conversation), + }); dispatch({ type: LOAD_STORY_REPLIES, diff --git a/ts/state/selectors/lightbox.ts b/ts/state/selectors/lightbox.ts index 64a483161281..189e39e3f57f 100644 --- a/ts/state/selectors/lightbox.ts +++ b/ts/state/selectors/lightbox.ts @@ -40,3 +40,13 @@ export const getMedia = createSelector( (state): ReadonlyArray> => state.isShowingLightbox ? state.media : [] ); + +export const getHasPrevMessage = createSelector( + getLightboxState, + (state): boolean => state.isShowingLightbox && state.hasPrevMessage +); + +export const getHasNextMessage = createSelector( + getLightboxState, + (state): boolean => state.isShowingLightbox && state.hasNextMessage +); diff --git a/ts/state/smart/Lightbox.tsx b/ts/state/smart/Lightbox.tsx index 5fd0d39441b3..f8311381e50e 100644 --- a/ts/state/smart/Lightbox.tsx +++ b/ts/state/smart/Lightbox.tsx @@ -1,7 +1,7 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import React from 'react'; +import React, { useCallback } from 'react'; import { useSelector } from 'react-redux'; import type { ReadonlyDeep } from 'type-fest'; @@ -19,6 +19,8 @@ import { useAudioPlayerActions } from '../ducks/audioPlayer'; import { getIsViewOnce, getMedia, + getHasPrevMessage, + getHasNextMessage, getSelectedIndex, shouldShowLightbox, } from '../selectors/lightbox'; @@ -26,7 +28,12 @@ import { export function SmartLightbox(): JSX.Element | null { const i18n = useSelector(getIntl); const { saveAttachment } = useConversationsActions(); - const { closeLightbox } = useLightboxActions(); + const { + closeLightbox, + showLightboxForNextMessage, + showLightboxForPrevMessage, + setSelectedLightboxPath, + } = useLightboxActions(); const { toggleForwardMessageModal } = useGlobalModalActions(); const { pauseVoiceNotePlayer } = useAudioPlayerActions(); @@ -40,8 +47,49 @@ export function SmartLightbox(): JSX.Element | null { StateType, ReadonlyArray> >(getMedia); + const hasPrevMessage = useSelector(getHasPrevMessage); + const hasNextMessage = useSelector(getHasNextMessage); const selectedIndex = useSelector(getSelectedIndex); + const onPrevAttachment = useCallback(() => { + if (selectedIndex <= 0) { + if (hasPrevMessage) { + showLightboxForPrevMessage(); + } + return; + } + setSelectedLightboxPath(media[selectedIndex - 1]?.attachment.path); + }, [ + showLightboxForPrevMessage, + media, + selectedIndex, + setSelectedLightboxPath, + hasPrevMessage, + ]); + + const onNextAttachment = useCallback(() => { + if (selectedIndex >= media.length - 1) { + if (hasNextMessage) { + showLightboxForNextMessage(); + } + return; + } + setSelectedLightboxPath(media[selectedIndex + 1]?.attachment.path); + }, [ + showLightboxForNextMessage, + media, + selectedIndex, + setSelectedLightboxPath, + hasNextMessage, + ]); + + const onSelectAttachment = useCallback( + (newIndex: number) => { + setSelectedLightboxPath(media[newIndex]?.attachment.path); + }, + [setSelectedLightboxPath, media] + ); + if (!isShowingLightbox) { return null; } @@ -57,6 +105,11 @@ export function SmartLightbox(): JSX.Element | null { selectedIndex={selectedIndex || 0} toggleForwardMessageModal={toggleForwardMessageModal} onMediaPlaybackStart={pauseVoiceNotePlayer} + onPrevAttachment={onPrevAttachment} + onNextAttachment={onNextAttachment} + onSelectAttachment={onSelectAttachment} + hasNextMessage={hasNextMessage} + hasPrevMessage={hasPrevMessage} /> ); } diff --git a/ts/test-electron/sql/timelineFetches_test.ts b/ts/test-electron/sql/timelineFetches_test.ts index 8845d0210c9d..e785362a81e8 100644 --- a/ts/test-electron/sql/timelineFetches_test.ts +++ b/ts/test-electron/sql/timelineFetches_test.ts @@ -92,7 +92,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 5); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: true, limit: 5, storyId: undefined, @@ -149,7 +150,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, storyId, @@ -203,7 +205,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, storyId: undefined, @@ -254,7 +257,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, receivedAt: target, @@ -307,7 +311,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, receivedAt: target, @@ -364,7 +369,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getOlderMessagesByConversation(conversationId, { + const messages = await getOlderMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, messageId: message2.id, @@ -442,7 +448,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 5); - const messages = await getNewerMessagesByConversation(conversationId, { + const messages = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: true, limit: 5, storyId: undefined, @@ -498,7 +505,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getNewerMessagesByConversation(conversationId, { + const messages = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, storyId, @@ -550,7 +558,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getNewerMessagesByConversation(conversationId, { + const messages = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, receivedAt: target, @@ -605,7 +614,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getNewerMessagesByConversation(conversationId, { + const messages = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, storyId: undefined, @@ -658,7 +668,8 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); - const messages = await getNewerMessagesByConversation(conversationId, { + const messages = await getNewerMessagesByConversation({ + conversationId, includeStoryReplies: false, limit: 5, receivedAt: target, @@ -777,12 +788,10 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 8); - const metricsInTimeline = await getMessageMetricsForConversation( + const metricsInTimeline = await getMessageMetricsForConversation({ conversationId, - { - includeStoryReplies: false, - } - ); + includeStoryReplies: false, + }); assert.strictEqual(metricsInTimeline?.oldest?.id, oldest.id, 'oldest'); assert.strictEqual(metricsInTimeline?.newest?.id, newest.id, 'newest'); assert.strictEqual( @@ -792,10 +801,11 @@ describe('sql/timelineFetches', () => { ); assert.strictEqual(metricsInTimeline?.totalUnseen, 2, 'totalUnseen'); - const metricsInStory = await getMessageMetricsForConversation( + const metricsInStory = await getMessageMetricsForConversation({ conversationId, - { storyId, includeStoryReplies: true } - ); + storyId, + includeStoryReplies: true, + }); assert.strictEqual( metricsInStory?.oldest?.id, oldestInStory.id, diff --git a/ts/util/cleanup.ts b/ts/util/cleanup.ts index ac9ed93e8ef6..d18ad7a36fad 100644 --- a/ts/util/cleanup.ts +++ b/ts/util/cleanup.ts @@ -41,15 +41,13 @@ async function cleanupStoryReplies( ): Promise { const { messageId, receivedAt } = pagination || {}; - const replies = await window.Signal.Data.getOlderMessagesByConversation( + const replies = await window.Signal.Data.getOlderMessagesByConversation({ conversationId, - { - includeStoryReplies: false, - messageId, - receivedAt, - storyId, - } - ); + includeStoryReplies: false, + messageId, + receivedAt, + storyId, + }); if (!replies.length) { return;