diff --git a/ts/components/StoryCreator.dom.tsx b/ts/components/StoryCreator.dom.tsx index f7ee64872a..fb300f03bc 100644 --- a/ts/components/StoryCreator.dom.tsx +++ b/ts/components/StoryCreator.dom.tsx @@ -2,7 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only import { useEffect, useState, type JSX } from 'react'; -import lodash from 'lodash'; +import lodash, { omit } from 'lodash'; import { createPortal } from 'react-dom'; import type { AttachmentType } from '../types/Attachment.std.ts'; @@ -178,7 +178,7 @@ export function StoryCreator({ }) ); attachment = { - ...draft, + ...omit(draft, 'screenshotData'), screenshot: { contentType: draft.screenshotContentType, url, diff --git a/ts/jobs/helpers/sendReaction.preload.ts b/ts/jobs/helpers/sendReaction.preload.ts index 343f03e91c..36ab21ba70 100644 --- a/ts/jobs/helpers/sendReaction.preload.ts +++ b/ts/jobs/helpers/sendReaction.preload.ts @@ -36,7 +36,7 @@ import type { ReactionJobData, } from '../conversationJobQueue.preload.ts'; import { sendToGroup } from '../../util/sendToGroup.preload.ts'; -import { hydrateStoryContext } from '../../util/hydrateStoryContext.preload.ts'; +import { getStoryReplyContext } from '../../util/getStoryReplyContext.std.ts'; import { send, sendSyncMessageOnly } from '../../messages/send.preload.ts'; import { itemStorage } from '../../textsecure/Storage.preload.ts'; import { getSendRecipientLists } from './getSendRecipientLists.dom.ts'; @@ -334,8 +334,8 @@ export async function sendReaction( if (!ephemeralMessageForReactionSend.doNotSave) { const reactionMessage = ephemeralMessageForReactionSend; - await hydrateStoryContext(reactionMessage.id, message.attributes, { - shouldSave: false, + reactionMessage.set({ + storyReplyContext: getStoryReplyContext(message.attributes), }); await window.MessageCache.saveMessage(reactionMessage.attributes, { forceSave: true, diff --git a/ts/messageModifiers/Reactions.preload.ts b/ts/messageModifiers/Reactions.preload.ts index d77c1d5104..b7f5b7f9ce 100644 --- a/ts/messageModifiers/Reactions.preload.ts +++ b/ts/messageModifiers/Reactions.preload.ts @@ -36,7 +36,7 @@ import { isSent } from '../messages/MessageSendState.std.ts'; import { strictAssert } from '../util/assert.std.ts'; import { repeat, zipObject } from '../util/iterables.std.ts'; import { getMessageIdForLogging } from '../util/idForLogging.preload.ts'; -import { hydrateStoryContext } from '../util/hydrateStoryContext.preload.ts'; +import { getStoryReplyContext } from '../util/getStoryReplyContext.std.ts'; import { drop } from '../util/drop.std.ts'; import * as reactionUtil from '../reactions/util.std.ts'; import { isNewReactionReplacingPrevious } from '../reactions/util.std.ts'; @@ -403,6 +403,7 @@ export async function handleReaction( ? targetConversation.get('expireTimer') : undefined, storyId: storyMessage.id, + storyReplyContext: getStoryReplyContext(storyMessage), storyReaction: { emoji: reaction.emoji, targetAuthorAci: reaction.targetAuthorAci, @@ -410,9 +411,6 @@ export async function handleReaction( }, }); - await hydrateStoryContext(generatedMessage.id, storyMessage, { - shouldSave: false, - }); // Note: generatedMessage comes with an id, so we have to force this save await window.MessageCache.saveMessage(generatedMessage.attributes, { forceSave: true, @@ -577,8 +575,8 @@ export async function handleReaction( 'Story reactions must provide storyReactionmessage' ); - await hydrateStoryContext(generatedMessage.id, message.attributes, { - shouldSave: false, + generatedMessage.set({ + storyReplyContext: getStoryReplyContext(message.attributes), }); await window.MessageCache.saveMessage(generatedMessage.attributes, { forceSave: true, diff --git a/ts/messages/handleDataMessage.preload.ts b/ts/messages/handleDataMessage.preload.ts index 351d746158..026b77707d 100644 --- a/ts/messages/handleDataMessage.preload.ts +++ b/ts/messages/handleDataMessage.preload.ts @@ -50,7 +50,7 @@ import { findStoryMessage } from '../util/findStoryMessage.preload.ts'; import { getValidLinkPreviews } from '../util/getValidLinkPreviews.node.ts'; import { normalizeServiceId } from '../types/ServiceId.std.ts'; import { BodyRange, trimMessageWhitespace } from '../types/BodyRange.std.ts'; -import { hydrateStoryContext } from '../util/hydrateStoryContext.preload.ts'; +import { getStoryReplyContext } from '../util/getStoryReplyContext.std.ts'; import { isMessageEmpty } from '../util/isMessageEmpty.preload.ts'; import { isValidTapToView } from '../util/isValidTapToView.std.ts'; import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage.preload.ts'; @@ -576,14 +576,11 @@ export async function handleDataMessage( } : undefined, storyId: dataMessage.storyId, + storyReplyContext: storyQuote + ? getStoryReplyContext(storyQuote) + : undefined, }); - if (storyQuote) { - await hydrateStoryContext(message.id, storyQuote, { - shouldSave: true, - }); - } - const isSupported = !isUnsupportedMessage(message.attributes); if (!isSupported) { await eraseMessageContents(message, 'unsupported-message'); diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index e90ad8d115..740bdf74fe 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -114,10 +114,8 @@ export type QuotedMessageType = { messageId?: string; }; -type StoryReplyContextType = { - attachment?: AttachmentType; +export type StoryReplyContextType = { authorAci?: AciString; - messageId: string; }; export type GroupV1Update = { diff --git a/ts/models/conversations.preload.ts b/ts/models/conversations.preload.ts index f993a9f68f..824e308252 100644 --- a/ts/models/conversations.preload.ts +++ b/ts/models/conversations.preload.ts @@ -49,7 +49,7 @@ import { } from '../util/avatarUtils.preload.ts'; import { getDraftPreview } from '../util/getDraftPreview.preload.ts'; import { hasDraft } from '../util/hasDraft.std.ts'; -import { hydrateStoryContext } from '../util/hydrateStoryContext.preload.ts'; +import { getStoryReplyContext } from '../util/getStoryReplyContext.std.ts'; import { normalizeProfileName } from '../util/normalizeProfileName.std.ts'; import type { StickerType, @@ -1592,14 +1592,12 @@ export class ConversationModel { message: MessageAttributesType, { isJustSent }: { isJustSent: boolean } = { isJustSent: false } ): Promise { - await this.#beforeAddSingleMessage(message); + await this.#beforeAddSingleMessage(); this.#doAddSingleMessage(message, { isJustSent }); this.debouncedUpdateLastMessage(); } - async #beforeAddSingleMessage(message: MessageAttributesType): Promise { - await hydrateStoryContext(message.id, undefined, { shouldSave: true }); - + async #beforeAddSingleMessage(): Promise { if (!this.newMessageQueue) { this.newMessageQueue = new PQueue({ concurrency: 1, @@ -2131,14 +2129,6 @@ export class ConversationModel { updated = true; } - const patch = await hydrateStoryContext(message.id, undefined, { - shouldSave: true, - }); - if (patch) { - updated = true; - model.set(patch); - } - if (updated) { upgraded += 1; await window.MessageCache.saveMessage(model.attributes); @@ -4272,8 +4262,9 @@ export class ConversationModel { expireTimer = this.get('expireTimer'); } + const story = storyId ? await getMessageById(storyId) : undefined; + 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, @@ -4289,6 +4280,8 @@ export class ConversationModel { expirationStartTimestamp = story.expirationStartTimestamp; } + const storyReplyContext = story ? getStoryReplyContext(story) : undefined; + const recipientMaybeConversations = map( this.getRecipients({ isStoryReply: storyId !== undefined, @@ -4371,6 +4364,7 @@ export class ConversationModel { }) ), storyId, + storyReplyContext, poll, }); @@ -4417,7 +4411,7 @@ export class ConversationModel { const renderStart = Date.now(); // Perform asynchronous tasks before entering the batching mode - await this.#beforeAddSingleMessage(model.attributes); + await this.#beforeAddSingleMessage(); if (sticker) { await addStickerPackReference({ diff --git a/ts/services/backups/import.preload.ts b/ts/services/backups/import.preload.ts index fc7cee0c67..54dcef495b 100644 --- a/ts/services/backups/import.preload.ts +++ b/ts/services/backups/import.preload.ts @@ -2278,7 +2278,6 @@ export class BackupImportStream extends Writable { reactions: this.#fromReactions(reactions), storyReplyContext: { authorAci: storyAuthorAci, - messageId: '', // stories are never imported }, }; diff --git a/ts/sql/migrations/1760-delete-story-reply-attachment.std.ts b/ts/sql/migrations/1760-delete-story-reply-attachment.std.ts new file mode 100644 index 0000000000..6680a69252 --- /dev/null +++ b/ts/sql/migrations/1760-delete-story-reply-attachment.std.ts @@ -0,0 +1,32 @@ +// Copyright 2026 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Database } from '@signalapp/sqlcipher'; + +import type { LoggerType } from '../../types/Logging.std.ts'; +import { sql } from '../util.std.ts'; + +export default function updateToSchemaVersion1760( + db: Database, + logger: LoggerType +): void { + const [updateQuery, updateParams] = sql` + UPDATE messages + SET json = json_remove(json, '$.storyReplyContext.attachment') + WHERE isStory = 0 + + /* we want to find all messages with a non-null storyId, but using string + comparison (instead of a non-null check) here causes Sqlite to use the + storyId index */ + AND storyId > '0' + + AND json->'$.storyReplyContext.attachment' IS NOT NULL; + `; + + const info = db.prepare(updateQuery).run(updateParams); + + logger.info( + `updateToSchemaVersion1760: removed storyReplyContext.attachment from ` + + `${info.changes} message(s)` + ); +} diff --git a/ts/sql/migrations/index.node.ts b/ts/sql/migrations/index.node.ts index de45826958..3ff6a0d36d 100644 --- a/ts/sql/migrations/index.node.ts +++ b/ts/sql/migrations/index.node.ts @@ -152,6 +152,7 @@ import updateToSchemaVersion1720 from './1720-update-recent-emoji.std.ts'; import updateToSchemaVersion1730 from './1730-protected-attachments-dedupe-token.std.ts'; import updateToSchemaVersion1740 from './1740-cleanup-groups.node.ts'; import updateToSchemaVersion1750 from './1750-fts-url.std.ts'; +import updateToSchemaVersion1760 from './1760-delete-story-reply-attachment.std.ts'; import { DataWriter } from '../Server.node.ts'; import { strictAssert } from '../../util/assert.std.ts'; @@ -1667,6 +1668,7 @@ export const SCHEMA_VERSIONS: ReadonlyArray = [ { version: 1730, update: updateToSchemaVersion1730 }, { version: 1740, update: updateToSchemaVersion1740 }, { version: 1750, update: updateToSchemaVersion1750 }, + { version: 1760, update: updateToSchemaVersion1760 }, ]; class DBVersionFromFutureError extends Error { diff --git a/ts/state/ducks/stories.preload.ts b/ts/state/ducks/stories.preload.ts index 6c1389ac8f..cd484564ff 100644 --- a/ts/state/ducks/stories.preload.ts +++ b/ts/state/ducks/stories.preload.ts @@ -533,10 +533,6 @@ function queueStoryDownload( const message = await getMessageById(storyId); if (message) { - // We want to ensure that we re-hydrate the story reply context with the - // completed attachment download. - message.set({ storyReplyContext: undefined }); - dispatch({ type: QUEUE_STORY_DOWNLOAD, payload: storyId, diff --git a/ts/state/selectors/message.preload.ts b/ts/state/selectors/message.preload.ts index 3043d9bc9d..ed9f1cca8e 100644 --- a/ts/state/selectors/message.preload.ts +++ b/ts/state/selectors/message.preload.ts @@ -117,6 +117,7 @@ import { getCachedConversationMemberColorsSelector, getContactNameColor, getPinnedMessagesMessageIds, + getStoriesState, } from './conversations.dom.ts'; import { getIntl, @@ -225,6 +226,9 @@ export type GetPropsForBubbleOptions = Readonly<{ contactNameColors: Map; defaultConversationColor: DefaultConversationColorType; hasMediaBackups: boolean; + getStoryReplyAttachment: ( + storyMessageId: string + ) => AttachmentType | undefined; }>; export function hasErrors( @@ -651,22 +655,42 @@ const getPollForMessage = ( }; }; +export const getStoryReplyAttachmentSelector = createSelector( + getStoriesState, + ({ stories }) => + (storyMessageId: string): AttachmentType | undefined => { + if (!storyMessageId) { + return undefined; + } + const story = stories.find(item => item.messageId === storyMessageId); + return story?.attachment; + } +); + const getPropsForStoryReplyContext = ( message: Pick< MessageWithUIFieldsType, - 'body' | 'conversationId' | 'storyReaction' | 'storyReplyContext' + | 'body' + | 'conversationId' + | 'storyReaction' + | 'storyId' + | 'storyReplyContext' >, { conversationSelector, ourConversationId, defaultConversationColor, + getStoryReplyAttachment, }: { conversationSelector: GetConversationByIdType; ourConversationId?: string; defaultConversationColor: DefaultConversationColorType; + getStoryReplyAttachment: ( + storyMessageId: string + ) => AttachmentType | undefined; } ): PropsData['storyReplyContext'] => { - const { storyReaction, storyReplyContext } = message; + const { storyReaction, storyId, storyReplyContext } = message; if (!storyReplyContext) { return undefined; } @@ -682,6 +706,9 @@ const getPropsForStoryReplyContext = ( conversation, defaultConversationColor ); + const storyAttachment = storyId + ? getStoryReplyAttachment(storyId) + : undefined; return { authorTitle, @@ -689,14 +716,12 @@ const getPropsForStoryReplyContext = ( customColor, emoji: storyReaction?.emoji, isFromMe, - rawAttachment: storyReplyContext.attachment - ? processQuoteAttachment(storyReplyContext.attachment) + rawAttachment: storyAttachment + ? processQuoteAttachment(storyAttachment) : undefined, - storyId: storyReplyContext.messageId, - text: getStoryReplyText( - window.SignalContext.i18n, - storyReplyContext.attachment - ), + // Only expose the storyId (making the quote clickable) when the story is found. + storyId: storyAttachment ? storyId : undefined, + text: getStoryReplyText(window.SignalContext.i18n, storyAttachment), }; }; @@ -800,6 +825,7 @@ export type GetPropsForMessageOptions = Pick< | 'contactNameColors' | 'defaultConversationColor' | 'hasMediaBackups' + | 'getStoryReplyAttachment' >; function getTextAttachment( @@ -1065,6 +1091,7 @@ export const getMessagePropsSelector = createSelector( getSelectedMessageIds, getDefaultConversationColor, getHasMediaBackups, + getStoryReplyAttachmentSelector, ( conversationSelector, ourConversationId, @@ -1078,7 +1105,8 @@ export const getMessagePropsSelector = createSelector( pinnedMessagesMessageIds, selectedMessageIds, defaultConversationColor, - hasMediaBackups + hasMediaBackups, + getStoryReplyAttachment ) => (message: MessageWithUIFieldsType) => { const contactNameColors = cachedConversationMemberColorsSelector( @@ -1099,6 +1127,7 @@ export const getMessagePropsSelector = createSelector( selectedMessageIds, defaultConversationColor, hasMediaBackups, + getStoryReplyAttachment, }); } ); @@ -2894,6 +2923,7 @@ export const getMessageDetailsSelector = createSelector( getDefaultConversationColor, getHasUnidentifiedDeliveryIndicators, getHasMediaBackups, + getStoryReplyAttachmentSelector, ( accountSelector, cachedConversationMemberColorsSelector, @@ -2909,7 +2939,8 @@ export const getMessageDetailsSelector = createSelector( selectedMessageIds, defaultConversationColor, hasUnidentifiedDeliveryIndicators, - hasMediaBackups + hasMediaBackups, + getStoryReplyAttachment ): ((messageId: string) => SmartMessageDetailPropsType | undefined) => (messageId: string) => { if (!messageLookup || !ourConversationId) { @@ -2963,6 +2994,7 @@ export const getMessageDetailsSelector = createSelector( selectedMessageIds, defaultConversationColor, hasMediaBackups, + getStoryReplyAttachment, }), receivedAt: message.received_at_ms ?? message.received_at ?? 0, }; diff --git a/ts/state/selectors/timeline.preload.ts b/ts/state/selectors/timeline.preload.ts index a3be3d5870..8b061c2610 100644 --- a/ts/state/selectors/timeline.preload.ts +++ b/ts/state/selectors/timeline.preload.ts @@ -31,7 +31,10 @@ import { getHasMediaBackups, } from './items.dom.ts'; import { getActiveCall, getCallSelector } from './calling.std.ts'; -import { getPropsForBubble } from './message.preload.ts'; +import { + getPropsForBubble, + getStoryReplyAttachmentSelector, +} from './message.preload.ts'; import { getCallHistorySelector } from './callHistory.std.ts'; import { useProxySelector } from '../../hooks/useProxySelector.std.ts'; import type { StateSelector } from '../types.std.ts'; @@ -46,6 +49,10 @@ import { missingCaseError } from '../../util/missingCaseError.std.ts'; import { getGroupMemberships } from '../../util/getGroupMemberships.dom.ts'; import type { ContactNameColorType } from '../../types/Colors.std.ts'; +// A no-op lookup for non-story-reply items. Only story replies should take a +// dependency on the stories slice +const noStoryReplyAttachment = () => undefined; + const getTimelineItem = ( state: StateType, messageId: string | undefined, @@ -77,6 +84,9 @@ const getTimelineItem = ( const selectedMessageIds = getSelectedMessageIds(state); const defaultConversationColor = getDefaultConversationColor(state); const hasMediaBackups = getHasMediaBackups(state); + const getStoryReplyAttachment = message.storyReplyContext + ? getStoryReplyAttachmentSelector(state) + : noStoryReplyAttachment; return getPropsForBubble(message, { conversationSelector, @@ -96,6 +106,7 @@ const getTimelineItem = ( selectedMessageIds, defaultConversationColor, hasMediaBackups, + getStoryReplyAttachment, }); }; diff --git a/ts/test-electron/backup/bubble_test.preload.ts b/ts/test-electron/backup/bubble_test.preload.ts index 06f0ebe0c0..b73626b612 100644 --- a/ts/test-electron/backup/bubble_test.preload.ts +++ b/ts/test-electron/backup/bubble_test.preload.ts @@ -1342,10 +1342,7 @@ describe('backup/bubble messages', () => { targetAuthorAci: OUR_ACI, targetTimestamp: 0, // targetTimestamp is not roundtripped }, - storyReplyContext: { - authorAci: OUR_ACI, - messageId: '', - }, + storyReplyContext: { authorAci: OUR_ACI }, }; const outgoingReply: MessageAttributesType = { @@ -1358,10 +1355,7 @@ describe('backup/bubble messages', () => { targetAuthorAci: CONTACT_A, targetTimestamp: 0, // targetTimestamp is not roundtripped }, - storyReplyContext: { - authorAci: CONTACT_A, - messageId: '', - }, + storyReplyContext: { authorAci: CONTACT_A }, sendStateByConversationId: { [contactA.id]: { status: SendStatus.Read, @@ -1391,10 +1385,7 @@ describe('backup/bubble messages', () => { type: 'incoming', unidentifiedDeliveryReceived: true, sourceServiceId: CONTACT_A, - storyReplyContext: { - authorAci: OUR_ACI, - messageId: '', - }, + storyReplyContext: { authorAci: OUR_ACI }, }; const outgoingReply: MessageAttributesType = { @@ -1402,10 +1393,7 @@ describe('backup/bubble messages', () => { id: generateGuid(), type: 'outgoing', sourceServiceId: OUR_ACI, - storyReplyContext: { - authorAci: CONTACT_A, - messageId: '', - }, + storyReplyContext: { authorAci: CONTACT_A }, sendStateByConversationId: { [contactA.id]: { status: SendStatus.Read, @@ -1435,10 +1423,7 @@ describe('backup/bubble messages', () => { timestamp: 3, readStatus: ReadStatus.Read, seenStatus: SeenStatus.Seen, - storyReplyContext: { - authorAci: OUR_ACI, - messageId: '', - }, + storyReplyContext: { authorAci: OUR_ACI }, }, ], [] @@ -1465,10 +1450,7 @@ describe('backup/bubble messages', () => { type: 'incoming', unidentifiedDeliveryReceived: true, sourceServiceId: CONTACT_A, - storyReplyContext: { - authorAci: OUR_ACI, - messageId: '', - }, + storyReplyContext: { authorAci: OUR_ACI }, }; const outgoingReply: MessageAttributesType = { @@ -1476,10 +1458,7 @@ describe('backup/bubble messages', () => { id: generateGuid(), type: 'outgoing', sourceServiceId: OUR_ACI, - storyReplyContext: { - authorAci: CONTACT_A, - messageId: '', - }, + storyReplyContext: { authorAci: CONTACT_A }, sendStateByConversationId: { [CONTACT_A]: { status: SendStatus.Read, diff --git a/ts/test-electron/util/cleanup_message_test.preload.ts b/ts/test-electron/util/cleanup_message_test.preload.ts index b5d9861c50..f52ca81eb4 100644 --- a/ts/test-electron/util/cleanup_message_test.preload.ts +++ b/ts/test-electron/util/cleanup_message_test.preload.ts @@ -14,7 +14,7 @@ import type { PollMessageAttribute } from '../../types/Polls.dom.ts'; import { DataReader, DataWriter } from '../../sql/Client.preload.ts'; import { itemStorage } from '../../textsecure/Storage.preload.ts'; import type { MessageAttributesType } from '../../model-types.d.ts'; -import { IMAGE_BMP, IMAGE_JPEG } from '../../types/MIME.std.ts'; +import { IMAGE_JPEG } from '../../types/MIME.std.ts'; import { SendStatus } from '../../messages/MessageSendState.std.ts'; import { getAbsoluteAttachmentPath } from '../../util/migrations.preload.ts'; import { getAttachmentsPath } from '../../../app/attachments.node.ts'; @@ -68,10 +68,7 @@ describe('cleanupMessage', () => { question: 'poll question', } as PollMessageAttribute, sendStateByConversationId: { aci: { status: SendStatus.Delivered } }, - storyReplyContext: { - attachment: { contentType: IMAGE_BMP, size: 128 }, - messageId: 'messageId', - }, + storyReplyContext: { authorAci: generateAci() }, }; await window.MessageCache.saveMessage(attributes, { forceSave: true }); const message = new MessageModel(attributes); @@ -114,10 +111,7 @@ describe('cleanupMessage', () => { question: 'poll question', } as PollMessageAttribute, sendStateByConversationId: { aci: { status: SendStatus.Delivered } }, - storyReplyContext: { - attachment: { contentType: IMAGE_BMP, size: 128 }, - messageId: 'messageId', - }, + storyReplyContext: { authorAci: generateAci() }, }; await window.MessageCache.saveMessage(attributes, { forceSave: true }); const message = new MessageModel(attributes); @@ -220,11 +214,7 @@ describe('cleanupMessage', () => { conversationId: groupConversationId, timestamp: now - 20, storyId: storyAttributes.id, - storyReplyContext: { - authorAci: storyAuthorAci, - attachment: { contentType: IMAGE_BMP, size: 128 }, - messageId: storyAttributes.id, - }, + storyReplyContext: { authorAci: storyAuthorAci }, }; const reply2: MessageAttributesType = { id: v7(), @@ -234,11 +224,7 @@ describe('cleanupMessage', () => { conversationId: groupConversationId, timestamp: now - 10, storyId: storyAttributes.id, - storyReplyContext: { - authorAci: storyAuthorAci, - attachment: { contentType: IMAGE_BMP, size: 256 }, - messageId: storyAttributes.id, - }, + storyReplyContext: { authorAci: storyAuthorAci }, }; await window.MessageCache.saveMessage(reply1, { forceSave: true }); await window.MessageCache.saveMessage(reply2, { forceSave: true }); @@ -259,7 +245,7 @@ describe('cleanupMessage', () => { assert.isUndefined(await DataReader.getMessageById(reply2.id)); }); - it('cleanupFilesAndReferencesToMessage clears storyReplyContext for 1:1 conversations', async () => { + it('cleanupFilesAndReferencesToMessage preserves 1:1 story replies', async () => { const now = Date.now(); const directConversationId = v7(); const storyAuthorAci = generateAci(); @@ -290,11 +276,7 @@ describe('cleanupMessage', () => { conversationId: directConversationId, timestamp: now - 20, storyId: storyAttributes.id, - storyReplyContext: { - authorAci: storyAuthorAci, - attachment: { contentType: IMAGE_BMP, size: 128 }, - messageId: storyAttributes.id, - }, + storyReplyContext: { authorAci: storyAuthorAci }, }; const reply2: MessageAttributesType = { id: v7(), @@ -304,11 +286,7 @@ describe('cleanupMessage', () => { conversationId: directConversationId, timestamp: now - 10, storyId: storyAttributes.id, - storyReplyContext: { - authorAci: storyAuthorAci, - attachment: { contentType: IMAGE_BMP, size: 256 }, - messageId: storyAttributes.id, - }, + storyReplyContext: { authorAci: storyAuthorAci }, }; await window.MessageCache.saveMessage(reply1, { forceSave: true }); await window.MessageCache.saveMessage(reply2, { forceSave: true }); @@ -326,25 +304,13 @@ describe('cleanupMessage', () => { '1:1 story replies should NOT be deleted' ); - // Verify storyReplyContext was cleared + // Replies are not mutated on story deletion; they render "story unavailable" + // via the live story lookup once the story is gone. The cached author remains. for (const reply of repliesAfter) { - assert.isDefined( - reply.storyReplyContext, - 'storyReplyContext should still exist' - ); - assert.strictEqual( - reply.storyReplyContext?.messageId, - '', - 'messageId should be empty string' - ); - assert.isUndefined( - reply.storyReplyContext?.attachment, - 'attachment should be undefined' - ); assert.strictEqual( reply.storyReplyContext?.authorAci, storyAuthorAci, - 'authorAci should be preserved' + 'storyReplyContext author should be preserved' ); } }); diff --git a/ts/test-node/sql/migration_1760_test.node.ts b/ts/test-node/sql/migration_1760_test.node.ts new file mode 100644 index 0000000000..bb3691d8c2 --- /dev/null +++ b/ts/test-node/sql/migration_1760_test.node.ts @@ -0,0 +1,79 @@ +// Copyright 2026 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import type { WritableDB } from '../../sql/Interface.std.ts'; +import { sql } from '../../sql/util.std.ts'; +import { + createDB, + updateToVersion, + insertData, + getTableData, + explain, +} from './helpers.node.ts'; + +describe('SQL/updateToSchemaVersion1760', () => { + let db: WritableDB; + + beforeEach(() => { + db = createDB(); + }); + + afterEach(() => { + db.close(); + }); + + it('removes the cached attachment but preserves the author', () => { + updateToVersion(db, 1750); + insertData(db, 'messages', [ + { + id: 'story_reply', + json: { + id: 'story_reply', + storyReplyContext: { + attachment: { + contentType: 'video/mp4', + path: 'path', + size: 100, + }, + authorAci: 'author_aci', + }, + }, + storyId: 'story_id', + }, + ]); + + updateToVersion(db, 1760); + + assert.deepStrictEqual( + getTableData(db, 'messages').map(msg => msg.json), + [ + { + id: 'story_reply', + storyReplyContext: { + authorAci: 'author_aci', + }, + }, + ] + ); + }); + + it('should use storyId index', () => { + updateToVersion(db, 1760); + + const details = explain( + db, + sql` + UPDATE messages + SET json = json_remove(json, '$.storyReplyContext.attachment') + WHERE isStory = 0 + AND storyId > '0' + AND json->'$.storyReplyContext.attachment' IS NOT NULL; + ` + ); + + assert.include(details, 'USING INDEX messages_by_storyId'); + assert.notInclude(details, 'SCAN'); + }); +}); diff --git a/ts/util/cleanup.preload.ts b/ts/util/cleanup.preload.ts index 35b87e0017..8185ef0c32 100644 --- a/ts/util/cleanup.preload.ts +++ b/ts/util/cleanup.preload.ts @@ -9,7 +9,7 @@ import type { ConversationAttributesType, MessageAttributesType, } from '../model-types.d.ts'; -import { MessageModel } from '../models/messages.preload.ts'; +import type { MessageModel } from '../models/messages.preload.ts'; import * as Errors from '../types/errors.std.ts'; import { createLogger } from '../logging/log.std.ts'; @@ -39,7 +39,6 @@ import { deleteDraftFile, maybeDeleteAttachmentFile, } from './migrations.preload.ts'; -import { hydrateStoryContext } from './hydrateStoryContext.preload.ts'; import { update as updateExpiringMessagesService } from '../services/expiringMessagesDeletion.preload.ts'; import { tapToViewMessagesDeletionService } from '../services/tapToViewMessagesDeletionService.preload.ts'; import { throttledUpdateBackupMediaDownloadProgress } from './updateBackupMediaDownloadProgress.preload.ts'; @@ -184,9 +183,14 @@ async function cleanupStoryReplies( parentConversation && !isDirectConversation(parentConversation.attributes) ); + // 1:1 story replies stay in the timeline + if (!isGroupConversation) { + return; + } + const replies = await DataReader.getRecentStoryReplies(storyId, pagination); - const logId = `cleanupStoryReplies(${storyId}/isGroup=${isGroupConversation})`; + const logId = `cleanupStoryReplies(${storyId})`; const lastMessage = replies[replies.length - 1]; const lastMessageId = lastMessage?.id; const lastReceivedAt = lastMessage?.received_at; @@ -206,25 +210,11 @@ async function cleanupStoryReplies( return; } - if (isGroupConversation) { - // Delete all group replies - await DataWriter.removeMessagesById( - replies.map(reply => reply.id), - { cleanupMessages } - ); - } else { - // 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)); - await hydrateStoryContext(model.id, story, { - shouldSave: true, - isStoryErased: true, - }); - }) - ); - } + // Delete all group replies + await DataWriter.removeMessagesById( + replies.map(reply => reply.id), + { cleanupMessages } + ); return cleanupStoryReplies(story, { // oxlint-disable-next-line typescript/no-non-null-assertion diff --git a/ts/util/doubleCheckMissingQuoteReference.preload.ts b/ts/util/doubleCheckMissingQuoteReference.preload.ts index e759358ba1..080ca24420 100644 --- a/ts/util/doubleCheckMissingQuoteReference.preload.ts +++ b/ts/util/doubleCheckMissingQuoteReference.preload.ts @@ -1,10 +1,8 @@ // Copyright 2019 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { getMessageById } from '../messages/getMessageById.preload.ts'; import type { MessageModel } from '../models/messages.preload.ts'; -import { hydrateStoryContext } from './hydrateStoryContext.preload.ts'; import { getMessageIdForLogging } from './idForLogging.preload.ts'; import { createLogger } from '../logging/log.std.ts'; @@ -20,24 +18,6 @@ export async function doubleCheckMissingQuoteReference( ): Promise { const logId = getMessageIdForLogging(message.attributes); - const storyId = message.get('storyId'); - if (storyId) { - log.warn(`${logId}: missing story reference`); - - const storyMessage = await getMessageById(storyId); - if (!storyMessage) { - return; - } - - if (message.get('storyReplyContext')) { - message.set({ storyReplyContext: undefined }); - } - await hydrateStoryContext(message.id, storyMessage.attributes, { - shouldSave: true, - }); - return; - } - const quote = message.get('quote'); if (!quote) { log.warn(`${logId}: Missing quote!`); diff --git a/ts/util/getStoryReplyContext.std.ts b/ts/util/getStoryReplyContext.std.ts new file mode 100644 index 0000000000..cc45a7fb53 --- /dev/null +++ b/ts/util/getStoryReplyContext.std.ts @@ -0,0 +1,17 @@ +// Copyright 2026 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { + ReadonlyMessageAttributesType, + StoryReplyContextType, +} from '../model-types.d.ts'; +import { isAciString } from './isAciString.std.ts'; +import { strictAssert } from './assert.std.ts'; + +export function getStoryReplyContext( + storyMessage: Pick +): StoryReplyContextType { + const { sourceServiceId: authorAci } = storyMessage; + strictAssert(isAciString(authorAci), 'Story message author must be an ACI'); + return { authorAci }; +} diff --git a/ts/util/hydrateStoryContext.preload.ts b/ts/util/hydrateStoryContext.preload.ts deleted file mode 100644 index 75bf17c262..0000000000 --- a/ts/util/hydrateStoryContext.preload.ts +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright 2023 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import omit from 'lodash/omit.js'; -import { createLogger } from '../logging/log.std.ts'; -import type { AttachmentType } from '../types/Attachment.std.ts'; -import type { MessageAttributesType } from '../model-types.d.ts'; -import { getAttachmentsForMessage } from '../state/selectors/message.preload.ts'; -import { backupsService } from '../services/backups/index.preload.ts'; -import { isAciString } from './isAciString.std.ts'; -import { isDirectConversation } from './whatTypeOfConversation.dom.ts'; -import { softAssert, strictAssert } from './assert.std.ts'; -import { getMessageSentTimestamp } from './getMessageSentTimestamp.std.ts'; -import { isOlderThan } from './timestamp.std.ts'; -import { DAY } from './durations/index.std.ts'; -import { getMessageById } from '../messages/getMessageById.preload.ts'; -import { MessageModel } from '../models/messages.preload.ts'; - -const log = createLogger('hydrateStoryContext'); - -export async function hydrateStoryContext( - messageId: string, - storyMessageParam?: MessageAttributesType, - { - shouldSave, - isStoryErased, - }: { - shouldSave?: boolean; - isStoryErased?: boolean; - } = {} -): Promise | undefined> { - const message = await getMessageById(messageId); - if (!message) { - log.warn(`Message ${messageId} not found`); - return undefined; - } - - const { storyId, storyReplyContext: context } = message.attributes; - if (!storyId) { - return undefined; - } - - const sentTimestamp = getMessageSentTimestamp(message.attributes, { - includeEdits: false, - log, - }); - const olderThanADay = isOlderThan(sentTimestamp, DAY); - const didNotFindMessage = context && !context.messageId; - const weHaveData = context && context.attachment?.url; - - if ( - !isStoryErased && - ((!olderThanADay && weHaveData) || (olderThanADay && didNotFindMessage)) - ) { - return undefined; - } - - let storyMessage: MessageModel | undefined; - try { - storyMessage = - storyMessageParam === undefined - ? await getMessageById(storyId) - : window.MessageCache.register(new MessageModel(storyMessageParam)); - } catch { - storyMessage = undefined; - } - - if (!storyMessage || isStoryErased) { - const conversation = window.ConversationController.get( - message.attributes.conversationId - ); - softAssert( - conversation && isDirectConversation(conversation.attributes), - 'hydrateStoryContext: Not a type=direct conversation' - ); - const newMessageAttributes: Partial = { - storyReplyContext: { - authorAci: context?.authorAci, - attachment: undefined, - // No messageId = referenced story not found - messageId: '', - }, - }; - message.set(newMessageAttributes); - if (shouldSave) { - await window.MessageCache.saveMessage(message.attributes); - } - - return newMessageAttributes; - } - - const attachments = getAttachmentsForMessage( - { ...storyMessage.attributes }, - { hasMediaBackups: backupsService.hasMediaBackups() } - ); - let attachment: AttachmentType | undefined = attachments?.[0]; - if (attachment && !attachment.url && !attachment.textAttachment) { - attachment = undefined; - } - - const { sourceServiceId: authorAci } = storyMessage.attributes; - strictAssert(isAciString(authorAci), 'Story message from pni'); - const newMessageAttributes: Partial = { - storyReplyContext: { - attachment: attachment ? omit(attachment, 'screenshotData') : undefined, - authorAci, - messageId: storyMessage.id, - }, - }; - message.set(newMessageAttributes); - if (shouldSave) { - await window.MessageCache.saveMessage(message.attributes); - } - - return newMessageAttributes; -}