From 4be697da2a1c2303337fd4e8d7348f659b3431bf Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Fri, 5 Dec 2025 17:49:10 -0600 Subject: [PATCH] Improve handling of group story replies Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com> --- ts/models/conversations.preload.ts | 17 +++ .../1580-expired-group-replies.std.ts | 32 ++++++ ts/sql/migrations/index.node.ts | 2 + ts/test-node/sql/migration_1580_test.node.ts | 108 ++++++++++++++++++ ts/util/cleanup.preload.ts | 15 +-- 5 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 ts/sql/migrations/1580-expired-group-replies.std.ts create mode 100644 ts/test-node/sql/migration_1580_test.node.ts diff --git a/ts/models/conversations.preload.ts b/ts/models/conversations.preload.ts index ba534b0882..0df327e201 100644 --- a/ts/models/conversations.preload.ts +++ b/ts/models/conversations.preload.ts @@ -4119,6 +4119,23 @@ export class ConversationModel { expireTimer = this.get('expireTimer'); } + if (storyId && isGroup(this.attributes)) { + const story = await getMessageById(storyId); + strictAssert(story, 'story being replied to must exist'); + strictAssert( + story.expireTimer != null && story.expireTimer > 0, + 'story missing expireTimer' + ); + strictAssert( + story.expirationStartTimestamp != null && + story.expirationStartTimestamp > 0, + 'story missing expirationStartTimestamp' + ); + + expireTimer = story.expireTimer; + expirationStartTimestamp = story.expirationStartTimestamp; + } + const recipientMaybeConversations = map( this.getRecipients({ isStoryReply: storyId !== undefined, diff --git a/ts/sql/migrations/1580-expired-group-replies.std.ts b/ts/sql/migrations/1580-expired-group-replies.std.ts new file mode 100644 index 0000000000..b916750775 --- /dev/null +++ b/ts/sql/migrations/1580-expired-group-replies.std.ts @@ -0,0 +1,32 @@ +// Copyright 2025 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { LoggerType } from '../../types/Logging.std.js'; +import type { WritableDB } from '../Interface.std.js'; +import { sql } from '../util.std.js'; + +export default function updateToSchemaVersion1580( + db: WritableDB, + logger: LoggerType +): void { + const [query] = sql` + DELETE FROM messages + WHERE id IN ( + SELECT messages.id from messages + INNER JOIN conversations ON messages.conversationId = conversations.id + WHERE + conversations.type = 'group' + AND messages.storyId IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM messages AS messages_exists + WHERE messages.storyId = messages_exists.id AND messages_exists.isErased IS NOT 1 + ) + ) + `; + const result = db.prepare(query).run(); + if (result.changes > 0) { + logger.warn( + `Deleted ${result.changes} group story replies without matching stories` + ); + } +} diff --git a/ts/sql/migrations/index.node.ts b/ts/sql/migrations/index.node.ts index 78de22ac5a..8aadc1a463 100644 --- a/ts/sql/migrations/index.node.ts +++ b/ts/sql/migrations/index.node.ts @@ -134,6 +134,7 @@ import updateToSchemaVersion1550 from './1550-has-link-preview.std.js'; import updateToSchemaVersion1560 from './1560-pinned-messages.std.js'; import updateToSchemaVersion1561 from './1561-cleanup-polls.std.js'; import updateToSchemaVersion1570 from './1570-pinned-messages-updates.std.js'; +import updateToSchemaVersion1580 from './1580-expired-group-replies.std.js'; import { DataWriter } from '../Server.node.js'; @@ -1627,6 +1628,7 @@ export const SCHEMA_VERSIONS: ReadonlyArray = [ // 1561, 1551, and 1541 all refer to the same migration { version: 1561, update: updateToSchemaVersion1561 }, { version: 1570, update: updateToSchemaVersion1570 }, + { version: 1580, update: updateToSchemaVersion1580 }, ]; export class DBVersionFromFutureError extends Error { diff --git a/ts/test-node/sql/migration_1580_test.node.ts b/ts/test-node/sql/migration_1580_test.node.ts new file mode 100644 index 0000000000..63dddab2c7 --- /dev/null +++ b/ts/test-node/sql/migration_1580_test.node.ts @@ -0,0 +1,108 @@ +// Copyright 2025 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only +import { assert } from 'chai'; + +import { type WritableDB } from '../../sql/Interface.std.js'; +import { + createDB, + getTableData, + insertData, + updateToVersion, +} from './helpers.node.js'; + +describe('SQL/updateToSchemaVersion1580', () => { + let db: WritableDB; + + beforeEach(() => { + db = createDB(); + updateToVersion(db, 1570); + }); + afterEach(() => { + db.close(); + }); + + it('deletes any expired group story replies', () => { + insertData(db, 'conversations', [ + { + id: 'groupConvoId', + type: 'group', + }, + { + id: 'directConvoId', + type: 'private', + }, + ]); + + insertData(db, 'messages', [ + { + id: 'story-that-exists', + type: 'story', + conversationId: 'groupConvoId', + timestamp: Date.now(), + }, + { + id: 'doe-story', + type: 'story', + conversationId: 'groupConvoId', + timestamp: Date.now(), + isErased: 1, + }, + { + id: 'group-reply-to-existing-story', + conversationId: 'groupConvoId', + timestamp: Date.now(), + storyId: 'story-that-exists', + }, + { + id: 'group-reply-to-non-existing-story', + conversationId: 'groupConvoId', + timestamp: Date.now(), + storyId: 'story-that-does-not-exist', + }, + { + id: 'group-reply-to-doe-story', + conversationId: 'groupConvoId', + timestamp: Date.now(), + storyId: 'doe-story', + }, + { + id: 'direct-reply-to-existing-story', + conversationId: 'directConvoId', + timestamp: Date.now(), + storyId: 'storyThatExists', + }, + { + id: 'direct-reply-to-non-existing-story', + conversationId: 'directConvoId', + timestamp: Date.now(), + storyId: 'storyThatDoesNotExist', + }, + { + id: 'normal-group-message', + conversationId: 'groupConvoId', + timestamp: Date.now(), + }, + { + id: 'normal-direct-message', + conversationId: 'directConvoId', + timestamp: Date.now(), + }, + ]); + + updateToVersion(db, 1580); + assert.deepStrictEqual( + getTableData(db, 'messages').map(row => row.id), + [ + 'story-that-exists', + 'doe-story', + 'group-reply-to-existing-story', + // 'group-reply-to-non-existing-story', <-- DELETED! + // 'group-reply-to-doe-story', <-- DELETED! + 'direct-reply-to-existing-story', + 'direct-reply-to-non-existing-story', + 'normal-group-message', + 'normal-direct-message', + ] + ); + }); +}); diff --git a/ts/util/cleanup.preload.ts b/ts/util/cleanup.preload.ts index 732c470615..2f36352de3 100644 --- a/ts/util/cleanup.preload.ts +++ b/ts/util/cleanup.preload.ts @@ -180,17 +180,14 @@ async function cleanupStoryReplies( } if (isGroupConversation) { - // Cleanup all group replies - await Promise.all( - replies.map(reply => { - const replyMessageModel = window.MessageCache.register( - new MessageModel(reply) - ); - return eraseMessageContents(replyMessageModel); - }) + // Delete all group replies + await DataWriter.removeMessages( + replies.map(reply => reply.id), + { cleanupMessages } ); } else { - // Refresh the storyReplyContext data for 1:1 conversations + // Clean out the storyReplyContext data for 1:1 conversations; these remain in the + // 1:1 timeline with a "story not found" message await Promise.all( replies.map(async reply => { const model = window.MessageCache.register(new MessageModel(reply));