From 5e67f6a42456f0885af7a6b1520cc697486e6322 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Mon, 20 Jan 2025 13:25:58 +1100 Subject: [PATCH] fix: screen does not glitch on last message deleted anymore --- .../message-item/ExpirableReadableMessage.tsx | 18 ++--- ts/data/data.ts | 4 +- ts/models/message.ts | 15 ++-- ts/node/sql.ts | 76 ++++++++++--------- ts/session/disappearing_messages/index.ts | 2 +- 5 files changed, 57 insertions(+), 58 deletions(-) diff --git a/ts/components/conversation/message/message-item/ExpirableReadableMessage.tsx b/ts/components/conversation/message/message-item/ExpirableReadableMessage.tsx index 84cdfcba7..5a4f2291c 100644 --- a/ts/components/conversation/message/message-item/ExpirableReadableMessage.tsx +++ b/ts/components/conversation/message/message-item/ExpirableReadableMessage.tsx @@ -1,17 +1,17 @@ -import { useCallback, useState } from 'react'; +import { useCallback } from 'react'; import { useDispatch } from 'react-redux'; import useInterval from 'react-use/lib/useInterval'; import useMount from 'react-use/lib/useMount'; import styled from 'styled-components'; import { useIsDetailMessageView } from '../../../../contexts/isDetailViewContext'; -import { Data } from '../../../../data/data'; import { useMessageExpirationPropsById } from '../../../../hooks/useParamSelector'; import { MessageModelType } from '../../../../models/messageType'; -import { ConvoHub } from '../../../../session/conversations'; -import { PropsForExpiringMessage, messagesExpired } from '../../../../state/ducks/conversations'; +import { messagesExpired, PropsForExpiringMessage } from '../../../../state/ducks/conversations'; import { getIncrement } from '../../../../util/timer'; import { ExpireTimer } from '../../ExpireTimer'; import { ReadableMessage, ReadableMessageProps } from './ReadableMessage'; +import { Data } from '../../../../data/data'; +import { ConvoHub } from '../../../../session/conversations'; const EXPIRATION_CHECK_MINIMUM = 2000; @@ -21,18 +21,10 @@ function useIsExpired( direction: MessageModelType | undefined; } ) { - const { - convoId, - messageId, - expirationDurationMs, - expirationTimestamp, - isExpired: isExpiredProps, - } = props; + const { convoId, messageId, expirationDurationMs, expirationTimestamp, isExpired } = props; const dispatch = useDispatch(); - const [isExpired] = useState(isExpiredProps); - const checkExpired = useCallback(async () => { const now = Date.now(); diff --git a/ts/data/data.ts b/ts/data/data.ts index a7bc155b4..99ce43b28 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -263,7 +263,7 @@ async function removeMessage(id: string): Promise { // it needs to delete all associated on-disk files along with the database delete. if (message) { await channels.removeMessage(id); - await message.cleanup(true); + await message.cleanup(); } } @@ -554,7 +554,7 @@ async function removeAllMessagesInConversation(conversationId: string): Promise< for (let index = 0; index < messages.length; index++) { const message = messages.at(index); // eslint-disable-next-line no-await-in-loop - await message.cleanup(false); // not triggering UI updates, as we remove them from the store just below + await message.cleanup(); } window.log.info( `removeAllMessagesInConversation messages.cleanup() ${conversationId} took ${ diff --git a/ts/models/message.ts b/ts/models/message.ts index 3c9c29aef..4c2a06d1f 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -418,11 +418,16 @@ export class MessageModel extends Backbone.Model { return ''; } - public async cleanup(triggerUIUpdate: boolean) { - const changed = await deleteExternalMessageFiles(this.attributes); - if (changed) { - await this.commit(triggerUIUpdate); - } + /** + * Remove from the DB all the attachments linked to that message. + * Note: does not commit the changes to the DB, on purpose. + * When we cleanup(), we always want to remove the message afterwards. So no commit() are done at all. + * + */ + public async cleanup() { + await deleteExternalMessageFiles(this.attributes); + // Note: we don't commit here, because when we do cleanup, we always + // want to cleanup right before deleting the message itself. } private getPropsForTimerNotification(): PropsForExpirationTimer | null { diff --git a/ts/node/sql.ts b/ts/node/sql.ts index 51c7ca19f..7a6032045 100644 --- a/ts/node/sql.ts +++ b/ts/node/sql.ts @@ -1513,57 +1513,59 @@ function getMessagesByConversation( const floorLoadAllMessagesInConvo = 70; let messages: Array> = []; - let quotes = []; + let quotes: Array = []; if (messageId || firstUnread) { const messageFound = getMessageById(messageId || firstUnread); - if (messageFound && messageFound.conversationId === conversationId) { - const start = Date.now(); - const msgTimestamp = - messageFound.serverTimestamp || messageFound.sent_at || messageFound.received_at; + if (!messageFound || messageFound.conversationId !== conversationId) { + console.info( + `getMessagesByConversation: Could not find messageId ${messageId} in db with conversationId: ${conversationId}. Just fetching the convo as usual. messageFound:`, + messageFound + ); + return { messages, quotes }; + } + const start = Date.now(); + const msgTimestamp = + messageFound.serverTimestamp || messageFound.sent_at || messageFound.received_at; - const commonArgs = { - conversationId, - msgTimestamp, - limit: - numberOfMessagesInConvo < floorLoadAllMessagesInConvo - ? floorLoadAllMessagesInConvo - : absLimit, - }; - - const messagesBefore = assertGlobalInstance() - .prepare( - `SELECT id, conversationId, json + const commonArgs = { + conversationId, + msgTimestamp, + limit: + numberOfMessagesInConvo < floorLoadAllMessagesInConvo + ? floorLoadAllMessagesInConvo + : absLimit, + }; + + const messagesBefore = assertGlobalInstance() + .prepare( + `SELECT id, conversationId, json FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId AND COALESCE(serverTimestamp, sent_at, received_at) <= $msgTimestamp ${orderByClause} LIMIT $limit` - ) - .all(commonArgs); + ) + .all(commonArgs); - const messagesAfter = assertGlobalInstance() - .prepare( - `SELECT id, conversationId, json + const messagesAfter = assertGlobalInstance() + .prepare( + `SELECT id, conversationId, json FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId AND COALESCE(serverTimestamp, sent_at, received_at) > $msgTimestamp ${orderByClauseASC} LIMIT $limit` - ) - .all(commonArgs); + ) + .all(commonArgs); - console.info(`getMessagesByConversation around took ${Date.now() - start}ms `); + console.info(`getMessagesByConversation around took ${Date.now() - start}ms `); - // sorting is made in redux already when rendered, but some things are made outside of redux, so let's make sure the order is right - messages = map([...messagesBefore, ...messagesAfter], row => jsonToObject(row.json)).sort( - (a, b) => { - return ( - (b.serverTimestamp || b.sent_at || b.received_at) - - (a.serverTimestamp || a.sent_at || a.received_at) - ); - } - ); - } - console.info( - `getMessagesByConversation: Could not find messageId ${messageId} in db with conversationId: ${conversationId}. Just fetching the convo as usual.` + // sorting is made in redux already when rendered, but some things are made outside of redux, so let's make sure the order is right + messages = map([...messagesBefore, ...messagesAfter], row => jsonToObject(row.json)).sort( + (a, b) => { + return ( + (b.serverTimestamp || b.sent_at || b.received_at) - + (a.serverTimestamp || a.sent_at || a.received_at) + ); + } ); } else { const limit = diff --git a/ts/session/disappearing_messages/index.ts b/ts/session/disappearing_messages/index.ts index 860916d2a..4a2f69afb 100644 --- a/ts/session/disappearing_messages/index.ts +++ b/ts/session/disappearing_messages/index.ts @@ -46,7 +46,7 @@ export async function destroyMessagesAndUpdateRedux( // TODO make this use getMessagesById and not getMessageById const message = await Data.getMessageById(messageIds[i]); - await message?.cleanup(false); // not triggering UI updates, as we remove them from the store just below + await message?.cleanup(); /* eslint-enable no-await-in-loop */ }