From 2377e25c3ada77f3f4bced893a50d19ab1ff8e88 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Sat, 28 Sep 2024 02:37:52 +1000 Subject: [PATCH] MediaGallery: Only update if changed message is within time range --- ts/state/ducks/mediaGallery.ts | 109 +++++++++++++++++++++++---------- 1 file changed, 78 insertions(+), 31 deletions(-) diff --git a/ts/state/ducks/mediaGallery.ts b/ts/state/ducks/mediaGallery.ts index dd3553e260b8..a30612e4d0c5 100644 --- a/ts/state/ducks/mediaGallery.ts +++ b/ts/state/ducks/mediaGallery.ts @@ -1,6 +1,7 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +import { orderBy } from 'lodash'; import type { ThunkAction } from 'redux-thunk'; import type { ReadonlyDeep } from 'type-fest'; @@ -35,6 +36,7 @@ import type { MessageAttributesType } from '../../model-types'; type MediaItemMessage = ReadonlyDeep<{ attachments: Array; + // Note that this reflects the sender, and not the parent conversation conversationId: string; id: string; receivedAt: number; @@ -107,6 +109,19 @@ type MediaGalleryActionType = ReadonlyDeep< | SetLoadingActionType >; +function _sortMedia(media: ReadonlyArray): ReadonlyArray { + return orderBy(media, [ + 'message.receivedAt', + 'message.sentAt', + 'message.index', + ]); +} +function _sortDocuments( + documents: ReadonlyArray +): ReadonlyArray { + return orderBy(documents, ['message.receivedAt', 'message.sentAt']); +} + function _getMediaItemMessage( message: ReadonlyDeep ): MediaItemMessage { @@ -128,10 +143,10 @@ function _getMediaItemMessage( function _cleanVisualAttachments( rawMedia: ReadonlyDeep> ): ReadonlyArray { - let index = 0; - return rawMedia .flatMap(message => { + let index = 0; + return (message.attachments || []).map( (attachment: AttachmentType): MediaType | undefined => { if ( @@ -276,8 +291,8 @@ function loadMoreMedia( return; } - const firstMedia = previousMedia[0]; - if (!firstMedia) { + const oldestLoadedMedia = previousMedia[0]; + if (!oldestLoadedMedia) { log.warn('loadMoreMedia: no previous media; calling initialLoad()'); initialLoad(conversationId)(dispatch, getState, {}); return; @@ -288,7 +303,7 @@ function loadMoreMedia( payload: { loading: true }, }); - const { sentAt, receivedAt, id: messageId } = firstMedia.message; + const { sentAt, receivedAt, id: messageId } = oldestLoadedMedia.message; const rawMedia = await DataReader.getOlderMessagesByConversation({ conversationId, @@ -336,8 +351,8 @@ function loadMoreDocuments( return; } - const firstDocument = previousDocuments[0]; - if (!firstDocument) { + const oldestLoadedDocument = previousDocuments[0]; + if (!oldestLoadedDocument) { log.warn( 'loadMoreDocuments: no previous documents; calling initialLoad()' ); @@ -350,7 +365,7 @@ function loadMoreDocuments( payload: { loading: true }, }); - const { sentAt, receivedAt, id: messageId } = firstDocument.message; + const { sentAt, receivedAt, id: messageId } = oldestLoadedDocument.message; const rawDocuments = await DataReader.getOlderMessagesByConversation({ conversationId, @@ -415,7 +430,9 @@ export function reducer( return { ...state, loading: false, - ...payload, + conversationId: payload.conversationId, + media: _sortMedia(payload.media), + documents: _sortDocuments(payload.documents), haveOldestMedia: payload.media.length === 0, haveOldestDocument: payload.documents.length === 0, }; @@ -431,7 +448,7 @@ export function reducer( ...state, loading: false, haveOldestMedia: media.length === 0, - media: media.concat(state.media), + media: _sortMedia(media.concat(state.media)), }; } @@ -445,12 +462,13 @@ export function reducer( ...state, loading: false, haveOldestDocument: documents.length === 0, - documents: documents.concat(state.documents), + documents: _sortDocuments(documents.concat(state.documents)), }; } - // We don't capture the initial message add, but we do capture the moment when its - // attachments have been downloaded + // A time-ordered subset of all conversation media is loaded at once. + // When a message changes, check that the changed message falls within this time range, + // and if so insert it into the loaded media. if (action.type === MESSAGE_CHANGED) { const { payload } = action; const { conversationId, data: message } = payload; @@ -459,37 +477,66 @@ export function reducer( return state; } - if (!message.attachments || message.attachments.length === 0) { - return state; - } - const mediaWithout = state.media.filter( item => item.message.id !== message.id ); const documentsWithout = state.documents.filter( item => item.message.id !== message.id ); + const mediaDifference = state.media.length - mediaWithout.length; + const documentDifference = state.documents.length - documentsWithout.length; - if (message.deletedForEveryone) { - return { - ...state, - media: mediaWithout, - documents: documentsWithout, - }; + if (message.deletedForEveryone || message.isErased) { + if (mediaDifference > 0 || documentDifference > 0) { + return { + ...state, + media: mediaWithout, + documents: documentsWithout, + }; + } + return state; } - // Check whether we have new downloaded media, or an attachment has been deleted - const mediaCount = state.media.length - mediaWithout.length; - const documentCount = state.documents.length - mediaWithout.length; + const oldestLoadedMedia = state.media[0]; + const oldestLoadedDocument = state.documents[0]; - const media = _cleanVisualAttachments([message]); - const documents = _cleanFileAttachments([message]); + const newMedia = _cleanVisualAttachments([message]); + const newDocuments = _cleanFileAttachments([message]); - if (mediaCount !== media.length || documentCount !== documents.length) { + let { documents, haveOldestDocument, haveOldestMedia, media } = state; + + const inMediaTimeRange = + !oldestLoadedMedia || + (message.received_at >= oldestLoadedMedia.message.receivedAt && + message.sent_at >= oldestLoadedMedia.message.sentAt); + if (mediaDifference !== media.length && inMediaTimeRange) { + media = _sortMedia(mediaWithout.concat(newMedia)); + } else if (!inMediaTimeRange) { + haveOldestMedia = false; + } + + const inDocumentTimeRange = + !oldestLoadedDocument || + (message.received_at >= oldestLoadedDocument.message.receivedAt && + message.sent_at >= oldestLoadedDocument.message.sentAt); + if (documentDifference !== documents.length && inDocumentTimeRange) { + documents = _sortDocuments(documentsWithout.concat(newDocuments)); + } else if (!inDocumentTimeRange) { + haveOldestDocument = false; + } + + if ( + state.haveOldestDocument !== haveOldestDocument || + state.haveOldestMedia !== haveOldestMedia || + state.documents !== documents || + state.media !== media + ) { return { ...state, - media: mediaWithout.concat(media), - documents: documentsWithout.concat(documents), + documents, + haveOldestDocument, + haveOldestMedia, + media, }; }