From 67c706a7ef1a43c6a6267c427b0774d80a7a0db3 Mon Sep 17 00:00:00 2001 From: Josh Perez <60019601+josh-signal@users.noreply.github.com> Date: Thu, 29 Sep 2022 20:57:11 -0400 Subject: [PATCH] Ensure deleting conversation deletes story replies --- ts/models/conversations.ts | 22 ++- ts/sql/Client.ts | 35 ++-- ts/sql/Interface.ts | 24 +-- ts/sql/Server.ts | 161 ++++++++++-------- ts/state/ducks/stories.ts | 6 +- .../sql/conversationSummary_test.ts | 10 +- ts/test-electron/sql/markRead_test.ts | 78 ++++++--- ts/test-electron/sql/timelineFetches_test.ts | 37 ++-- ts/test-node/sql_migrations_test.ts | 7 +- ts/util/markConversationRead.ts | 2 +- 10 files changed, 219 insertions(+), 163 deletions(-) diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 36f64c323..e8ddc38bf 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -1457,11 +1457,9 @@ export class ConversationModel extends window.Backbone } } - const metrics = await getMessageMetricsForConversation( - conversationId, - undefined, - isGroup(this.attributes) - ); + const metrics = await getMessageMetricsForConversation(conversationId, { + includeStoryReplies: !isGroup(this.attributes), + }); // If this is a message request that has not yet been accepted, we always show the // oldest messages, to ensure that the ConversationHero is shown. We don't want to @@ -1480,7 +1478,7 @@ export class ConversationModel extends window.Backbone } const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, storyId: undefined, }); @@ -1533,7 +1531,7 @@ export class ConversationModel extends window.Backbone const receivedAt = message.received_at; const sentAt = message.sent_at; const models = await getOlderMessagesByConversation(conversationId, { - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, messageId: oldestMessageId, receivedAt, @@ -1588,7 +1586,7 @@ export class ConversationModel extends window.Backbone const receivedAt = message.received_at; const sentAt = message.sent_at; const models = await getNewerMessagesByConversation(conversationId, { - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, receivedAt, sentAt, @@ -1646,7 +1644,7 @@ export class ConversationModel extends window.Backbone const { older, newer, metrics } = await getConversationRangeCenteredOnMessage({ conversationId, - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), limit: MESSAGE_LOAD_CHUNK_SIZE, messageId, receivedAt, @@ -2067,7 +2065,7 @@ export class ConversationModel extends window.Backbone messages = await window.Signal.Data.getOlderMessagesByConversation( this.get('id'), { - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), limit: 100, messageId: first ? first.id : undefined, receivedAt: first ? first.received_at : undefined, @@ -4134,7 +4132,7 @@ export class ConversationModel extends window.Backbone const ourUuid = window.textsecure.storage.user.getCheckedUuid().toString(); const stats = await window.Signal.Data.getConversationMessageStats({ conversationId, - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), ourUuid, }); @@ -4621,7 +4619,7 @@ export class ConversationModel extends window.Backbone this.id, { storyId: undefined, - isGroup: isGroup(this.attributes), + includeStoryReplies: !isGroup(this.attributes), } ); diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index fbc1672c0..e3dbd389a 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -1259,7 +1259,7 @@ async function getTotalUnreadForConversation( conversationId: string, options: { storyId: UUIDStringType | undefined; - isGroup: boolean; + includeStoryReplies: boolean; } ): Promise { return channels.getTotalUnreadForConversation(conversationId, options); @@ -1267,7 +1267,7 @@ async function getTotalUnreadForConversation( async function getUnreadByConversationAndMarkRead(options: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; newestUnreadAt: number; now?: number; readAt?: number; @@ -1320,14 +1320,14 @@ function handleMessageJSON( async function getOlderMessagesByConversation( conversationId: string, { - isGroup, + includeStoryReplies, limit = 100, messageId, receivedAt = Number.MAX_VALUE, sentAt = Number.MAX_VALUE, storyId, }: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId?: string; receivedAt?: number; @@ -1338,7 +1338,7 @@ async function getOlderMessagesByConversation( const messages = await channels.getOlderMessagesByConversation( conversationId, { - isGroup, + includeStoryReplies, limit, receivedAt, sentAt, @@ -1362,13 +1362,13 @@ async function getOlderStories(options: { async function getNewerMessagesByConversation( conversationId: string, { - isGroup, + includeStoryReplies, limit = 100, receivedAt = 0, sentAt = 0, storyId, }: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; receivedAt?: number; sentAt?: number; @@ -1378,7 +1378,7 @@ async function getNewerMessagesByConversation( const messages = await channels.getNewerMessagesByConversation( conversationId, { - isGroup, + includeStoryReplies, limit, receivedAt, sentAt, @@ -1390,17 +1390,17 @@ async function getNewerMessagesByConversation( } async function getConversationMessageStats({ conversationId, - isGroup, + includeStoryReplies, ourUuid, }: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; ourUuid: UUIDStringType; }): Promise { const { preview, activity, hasUserInitiatedMessages } = await channels.getConversationMessageStats({ conversationId, - isGroup, + includeStoryReplies, ourUuid, }); @@ -1419,20 +1419,21 @@ async function getLastConversationMessage({ } async function getMessageMetricsForConversation( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + options: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): Promise { const result = await channels.getMessageMetricsForConversation( conversationId, - storyId, - isGroup + options ); return result; } async function getConversationRangeCenteredOnMessage(options: { conversationId: string; - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId: string; receivedAt: number; @@ -1479,7 +1480,7 @@ async function removeAllMessagesInConversation( // time so we don't use too much memory. messages = await getOlderMessagesByConversation(conversationId, { limit: chunkSize, - isGroup: true, + includeStoryReplies: true, storyId: undefined, }); diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 88644663b..965926e4d 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -458,12 +458,12 @@ export type DataInterface = { conversationId: string, options: { storyId: UUIDStringType | undefined; - isGroup: boolean; + includeStoryReplies: boolean; } ) => Promise; getUnreadByConversationAndMarkRead: (options: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; newestUnreadAt: number; now?: number; readAt?: number; @@ -517,13 +517,15 @@ export type DataInterface = { // getNewerMessagesByConversation is JSON on server, full message on Client getMessageMetricsForConversation: ( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + options: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ) => Promise; // getConversationRangeCenteredOnMessage is JSON on server, full message on client getConversationMessageStats: (options: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; ourUuid: UUIDStringType; }) => Promise; getLastConversationMessage(options: { @@ -706,7 +708,7 @@ export type ServerInterface = DataInterface & { getOlderMessagesByConversation: ( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId?: string; receivedAt?: number; @@ -717,7 +719,7 @@ export type ServerInterface = DataInterface & { getNewerMessagesByConversation: ( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; receivedAt?: number; sentAt?: number; @@ -726,7 +728,7 @@ export type ServerInterface = DataInterface & { ) => Promise>; getConversationRangeCenteredOnMessage: (options: { conversationId: string; - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId: string; receivedAt: number; @@ -804,7 +806,7 @@ export type ClientInterface = DataInterface & { getOlderMessagesByConversation: ( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId?: string; receivedAt?: number; @@ -815,7 +817,7 @@ export type ClientInterface = DataInterface & { getNewerMessagesByConversation: ( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; receivedAt?: number; sentAt?: number; @@ -824,7 +826,7 @@ export type ClientInterface = DataInterface & { ) => Promise>; getConversationRangeCenteredOnMessage: (options: { conversationId: string; - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId: string; receivedAt: number; diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index b67e33890..6dd75c347 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -1757,6 +1757,8 @@ async function getMessageCount(conversationId?: string): Promise { return getMessageCountSync(conversationId); } +// Note: we really only use this in 1:1 conversations, where story replies are always +// shown, so this has no need to be story-aware. function hasUserInitiatedMessages(conversationId: string): boolean { const db = getInstance(); @@ -2151,27 +2153,30 @@ async function getMessageBySender({ export function _storyIdPredicate( storyId: string | undefined, - isGroup?: boolean + includeStoryReplies: boolean ): string { - if (!isGroup && storyId === undefined) { - // We could use 'TRUE' here, but it is better to require `$storyId` - // parameter + // This is unintuitive, but 'including story replies' means that we need replies to + // lots of different stories. So, we remove the storyId check with a clause that will + // always be true. We don't just return TRUE because we want to use our passed-in + // $storyId parameter. + if (includeStoryReplies && storyId === undefined) { return '$storyId IS NULL'; } + // In constrast to: replies to a specific story return 'storyId IS $storyId'; } async function getUnreadByConversationAndMarkRead({ conversationId, - isGroup, + includeStoryReplies, newestUnreadAt, storyId, readAt, now = Date.now(), }: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; newestUnreadAt: number; storyId?: UUIDStringType; readAt?: number; @@ -2189,7 +2194,7 @@ async function getUnreadByConversationAndMarkRead({ json = json_patch(json, $jsonPatch) WHERE conversationId = $conversationId AND - (${_storyIdPredicate(storyId, isGroup)}) AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND isStory IS 0 AND type IS 'incoming' AND ( @@ -2215,7 +2220,7 @@ async function getUnreadByConversationAndMarkRead({ conversationId = $conversationId AND seenStatus = ${SeenStatus.Unseen} AND isStory = 0 AND - (${_storyIdPredicate(storyId, isGroup)}) AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND received_at <= $newestUnreadAt ORDER BY received_at DESC, sent_at DESC; ` @@ -2237,7 +2242,7 @@ async function getUnreadByConversationAndMarkRead({ conversationId = $conversationId AND seenStatus = ${SeenStatus.Unseen} AND isStory = 0 AND - (${_storyIdPredicate(storyId, isGroup)}) AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND received_at <= $newestUnreadAt; ` ).run({ @@ -2439,7 +2444,7 @@ async function _removeAllReactions(): Promise { async function getOlderMessagesByConversation( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId?: string; receivedAt?: number; @@ -2452,14 +2457,14 @@ async function getOlderMessagesByConversation( function getOlderMessagesByConversationSync( conversationId: string, { - isGroup, + includeStoryReplies, limit = 100, messageId, receivedAt = Number.MAX_VALUE, sentAt = Number.MAX_VALUE, storyId, }: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId?: string; receivedAt?: number; @@ -2476,7 +2481,7 @@ function getOlderMessagesByConversationSync( conversationId = $conversationId AND ($messageId IS NULL OR id IS NOT $messageId) AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND ( (received_at = $received_at AND sent_at < $sent_at) OR received_at < $received_at @@ -2540,7 +2545,7 @@ async function getOlderStories({ async function getNewerMessagesByConversation( conversationId: string, options: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; receivedAt?: number; sentAt?: number; @@ -2552,13 +2557,13 @@ async function getNewerMessagesByConversation( function getNewerMessagesByConversationSync( conversationId: string, { - isGroup, + includeStoryReplies, limit = 100, receivedAt = 0, sentAt = 0, storyId, }: { - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; receivedAt?: number; sentAt?: number; @@ -2572,7 +2577,7 @@ function getNewerMessagesByConversationSync( SELECT json FROM messages WHERE conversationId = $conversationId AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) AND + (${_storyIdPredicate(storyId, includeStoryReplies)}) AND ( (received_at = $received_at AND sent_at > $sent_at) OR received_at > $received_at @@ -2593,8 +2598,13 @@ function getNewerMessagesByConversationSync( } function getOldestMessageForConversation( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + { + storyId, + includeStoryReplies, + }: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): MessageMetricsType | undefined { const db = getInstance(); const row = db @@ -2603,7 +2613,7 @@ function getOldestMessageForConversation( SELECT * FROM messages WHERE conversationId = $conversationId AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) + (${_storyIdPredicate(storyId, includeStoryReplies)}) ORDER BY received_at ASC, sent_at ASC LIMIT 1; ` @@ -2621,8 +2631,13 @@ function getOldestMessageForConversation( } function getNewestMessageForConversation( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + { + storyId, + includeStoryReplies, + }: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): MessageMetricsType | undefined { const db = getInstance(); const row = db @@ -2631,7 +2646,7 @@ function getNewestMessageForConversation( SELECT * FROM messages WHERE conversationId = $conversationId AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) + (${_storyIdPredicate(storyId, includeStoryReplies)}) ORDER BY received_at DESC, sent_at DESC LIMIT 1; ` @@ -2650,11 +2665,11 @@ function getNewestMessageForConversation( function getLastConversationActivity({ conversationId, - isGroup, + includeStoryReplies, ourUuid, }: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; ourUuid: UUIDStringType; }): MessageType | undefined { const db = getInstance(); @@ -2666,7 +2681,7 @@ function getLastConversationActivity({ conversationId = $conversationId AND shouldAffectActivity IS 1 AND isTimerChangeFromSync IS 0 AND - ${isGroup ? 'storyId IS NULL AND' : ''} + ${includeStoryReplies ? '' : 'storyId IS NULL AND'} isGroupLeaveEventFromOther IS 0 ORDER BY received_at DESC, sent_at DESC LIMIT 1; @@ -2684,10 +2699,10 @@ function getLastConversationActivity({ } function getLastConversationPreview({ conversationId, - isGroup, + includeStoryReplies, }: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; }): MessageType | undefined { const db = getInstance(); const row = prepare( @@ -2698,7 +2713,7 @@ function getLastConversationPreview({ conversationId = $conversationId AND shouldAffectPreview IS 1 AND isGroupLeaveEventFromOther IS 0 AND - ${isGroup ? 'storyId IS NULL AND' : ''} + ${includeStoryReplies ? '' : 'storyId IS NULL AND'} ( expiresAt IS NULL OR @@ -2721,11 +2736,11 @@ function getLastConversationPreview({ async function getConversationMessageStats({ conversationId, - isGroup, + includeStoryReplies, ourUuid, }: { conversationId: string; - isGroup?: boolean; + includeStoryReplies: boolean; ourUuid: UUIDStringType; }): Promise { const db = getInstance(); @@ -2734,10 +2749,13 @@ async function getConversationMessageStats({ return { activity: getLastConversationActivity({ conversationId, - isGroup, + includeStoryReplies, ourUuid, }), - preview: getLastConversationPreview({ conversationId, isGroup }), + preview: getLastConversationPreview({ + conversationId, + includeStoryReplies, + }), hasUserInitiatedMessages: hasUserInitiatedMessages(conversationId), }; })(); @@ -2771,8 +2789,13 @@ async function getLastConversationMessage({ function getOldestUnseenMessageForConversation( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + { + storyId, + includeStoryReplies, + }: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): MessageMetricsType | undefined { const db = getInstance(); const row = db @@ -2782,7 +2805,7 @@ function getOldestUnseenMessageForConversation( conversationId = $conversationId AND seenStatus = ${SeenStatus.Unseen} AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) + (${_storyIdPredicate(storyId, includeStoryReplies)}) ORDER BY received_at ASC, sent_at ASC LIMIT 1; ` @@ -2803,7 +2826,7 @@ async function getTotalUnreadForConversation( conversationId: string, options: { storyId: UUIDStringType | undefined; - isGroup: boolean; + includeStoryReplies: boolean; } ): Promise { return getTotalUnreadForConversationSync(conversationId, options); @@ -2812,10 +2835,10 @@ function getTotalUnreadForConversationSync( conversationId: string, { storyId, - isGroup, + includeStoryReplies, }: { storyId: UUIDStringType | undefined; - isGroup: boolean; + includeStoryReplies: boolean; } ): number { const db = getInstance(); @@ -2828,7 +2851,7 @@ function getTotalUnreadForConversationSync( conversationId = $conversationId AND readStatus = ${ReadStatus.Unread} AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) + (${_storyIdPredicate(storyId, includeStoryReplies)}) ` ) .get({ @@ -2844,8 +2867,13 @@ function getTotalUnreadForConversationSync( } function getTotalUnseenForConversationSync( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + { + storyId, + includeStoryReplies, + }: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): number { const db = getInstance(); const row = db @@ -2857,7 +2885,7 @@ function getTotalUnseenForConversationSync( conversationId = $conversationId AND seenStatus = ${SeenStatus.Unseen} AND isStory IS 0 AND - (${_storyIdPredicate(storyId, isGroup)}) + (${_storyIdPredicate(storyId, includeStoryReplies)}) ` ) .get({ @@ -2874,35 +2902,29 @@ function getTotalUnseenForConversationSync( async function getMessageMetricsForConversation( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + options: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): Promise { - return getMessageMetricsForConversationSync(conversationId, storyId, isGroup); + return getMessageMetricsForConversationSync(conversationId, options); } function getMessageMetricsForConversationSync( conversationId: string, - storyId?: UUIDStringType, - isGroup?: boolean + options: { + storyId?: UUIDStringType; + includeStoryReplies: boolean; + } ): ConversationMetricsType { - const oldest = getOldestMessageForConversation( - conversationId, - storyId, - isGroup - ); - const newest = getNewestMessageForConversation( - conversationId, - storyId, - isGroup - ); + const oldest = getOldestMessageForConversation(conversationId, options); + const newest = getNewestMessageForConversation(conversationId, options); const oldestUnseen = getOldestUnseenMessageForConversation( conversationId, - storyId, - isGroup + options ); const totalUnseen = getTotalUnseenForConversationSync( conversationId, - storyId, - isGroup + options ); return { @@ -2917,7 +2939,7 @@ function getMessageMetricsForConversationSync( async function getConversationRangeCenteredOnMessage({ conversationId, - isGroup, + includeStoryReplies, limit, messageId, receivedAt, @@ -2925,7 +2947,7 @@ async function getConversationRangeCenteredOnMessage({ storyId, }: { conversationId: string; - isGroup: boolean; + includeStoryReplies: boolean; limit?: number; messageId: string; receivedAt: number; @@ -2939,7 +2961,7 @@ async function getConversationRangeCenteredOnMessage({ return db.transaction(() => { return { older: getOlderMessagesByConversationSync(conversationId, { - isGroup, + includeStoryReplies, limit, messageId, receivedAt, @@ -2947,17 +2969,16 @@ async function getConversationRangeCenteredOnMessage({ storyId, }), newer: getNewerMessagesByConversationSync(conversationId, { - isGroup, + includeStoryReplies, limit, receivedAt, sentAt, storyId, }), - metrics: getMessageMetricsForConversationSync( - conversationId, + metrics: getMessageMetricsForConversationSync(conversationId, { storyId, - isGroup - ), + includeStoryReplies, + }), }; })(); } diff --git a/ts/state/ducks/stories.ts b/ts/state/ducks/stories.ts index 630bfac8c..a4988a5e4 100644 --- a/ts/state/ducks/stories.ts +++ b/ts/state/ducks/stories.ts @@ -392,7 +392,11 @@ function loadStoryReplies( const conversation = getConversationSelector(getState())(conversationId); const replies = await dataInterface.getOlderMessagesByConversation( conversationId, - { limit: 9000, storyId: messageId, isGroup: isGroup(conversation) } + { + limit: 9000, + storyId: messageId, + includeStoryReplies: !isGroup(conversation), + } ); dispatch({ diff --git a/ts/test-electron/sql/conversationSummary_test.ts b/ts/test-electron/sql/conversationSummary_test.ts index 784ed7660..5e43915e2 100644 --- a/ts/test-electron/sql/conversationSummary_test.ts +++ b/ts/test-electron/sql/conversationSummary_test.ts @@ -70,6 +70,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.activity?.body, message2.body, 'activity'); @@ -122,7 +123,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, - isGroup: true, + includeStoryReplies: false, ourUuid, }); @@ -215,6 +216,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.preview?.body, message1.body); @@ -322,6 +324,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.activity?.body, message1.body); @@ -370,6 +373,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.activity?.body, message1.body); @@ -419,6 +423,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.activity?.body, message1.body); @@ -461,6 +466,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.preview?.body, message1.body); @@ -505,6 +511,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.preview?.body, message1.body); @@ -564,6 +571,7 @@ describe('sql/conversationSummary', () => { const messages = await getConversationMessageStats({ conversationId, ourUuid, + includeStoryReplies: false, }); assert.strictEqual(messages.activity?.body, message1.body, 'activity'); diff --git a/ts/test-electron/sql/markRead_test.ts b/ts/test-electron/sql/markRead_test.ts index 0b4f782bb..405e2400a 100644 --- a/ts/test-electron/sql/markRead_test.ts +++ b/ts/test-electron/sql/markRead_test.ts @@ -41,7 +41,7 @@ describe('sql/markRead', () => { const conversationId = getUuid(); const ourUuid = getUuid(); - const message1: MessageAttributesType = { + const oldest: MessageAttributesType = { id: getUuid(), body: 'message 1', type: 'incoming', @@ -51,7 +51,7 @@ describe('sql/markRead', () => { timestamp: start + 1, readStatus: ReadStatus.Read, }; - const message2: MessageAttributesType = { + const oldestUnread: MessageAttributesType = { id: getUuid(), body: 'message 2', type: 'incoming', @@ -61,7 +61,7 @@ describe('sql/markRead', () => { timestamp: start + 2, readStatus: ReadStatus.Unread, }; - const message3: MessageAttributesType = { + const unreadInAnotherConvo: MessageAttributesType = { id: getUuid(), body: 'message 3', type: 'incoming', @@ -71,7 +71,7 @@ describe('sql/markRead', () => { timestamp: start + 3, readStatus: ReadStatus.Unread, }; - const message4: MessageAttributesType = { + const unread: MessageAttributesType = { id: getUuid(), body: 'message 4', type: 'incoming', @@ -81,7 +81,7 @@ describe('sql/markRead', () => { timestamp: start + 4, readStatus: ReadStatus.Unread, }; - const message5: MessageAttributesType = { + const unreadStory: MessageAttributesType = { id: getUuid(), body: 'message 5', type: 'story', @@ -92,7 +92,7 @@ describe('sql/markRead', () => { readStatus: ReadStatus.Unread, storyId: getUuid(), }; - const message6: MessageAttributesType = { + const unreadStoryReply: MessageAttributesType = { id: getUuid(), body: 'message 6', type: 'incoming', @@ -103,7 +103,7 @@ describe('sql/markRead', () => { readStatus: ReadStatus.Unread, storyId: getUuid(), }; - const message7: MessageAttributesType = { + const newestUnread: MessageAttributesType = { id: getUuid(), body: 'message 7', type: 'incoming', @@ -115,7 +115,15 @@ describe('sql/markRead', () => { }; await saveMessages( - [message1, message2, message3, message4, message5, message6, message7], + [ + oldest, + oldestUnread, + unreadInAnotherConvo, + unread, + unreadStory, + unreadStoryReply, + newestUnread, + ], { forceSave: true, ourUuid, @@ -126,56 +134,68 @@ describe('sql/markRead', () => { assert.strictEqual( await getTotalUnreadForConversation(conversationId, { storyId: undefined, - isGroup: false, + includeStoryReplies: false, }), - 4, - 'unread count' + 3, + 'no stories/unread count - before' ); const markedRead = await getUnreadByConversationAndMarkRead({ conversationId, - newestUnreadAt: message4.received_at, + newestUnreadAt: unreadStoryReply.received_at, readAt, + includeStoryReplies: false, }); - assert.lengthOf(markedRead, 2, 'two messages marked read'); + assert.lengthOf(markedRead, 2, 'no stories/two messages marked read'); assert.strictEqual( await getTotalUnreadForConversation(conversationId, { storyId: undefined, - isGroup: false, + includeStoryReplies: false, }), - 2, - 'unread count' + 1, + 'no stories/unread count - after' ); // Sorted in descending order assert.strictEqual( markedRead[0].id, - message4.id, - 'first should be message4' + unread.id, + 'no stories/first should be "unread" message' ); assert.strictEqual( markedRead[1].id, - message2.id, - 'second should be message2' + oldestUnread.id, + 'no stories/second should be oldestUnread' ); const markedRead2 = await getUnreadByConversationAndMarkRead({ conversationId, - newestUnreadAt: message7.received_at, + newestUnreadAt: newestUnread.received_at, readAt, + includeStoryReplies: true, }); - assert.lengthOf(markedRead2, 2, 'two messages marked read'); - assert.strictEqual(markedRead2[0].id, message7.id, 'should be message7'); + assert.lengthOf(markedRead2, 2, 'with stories/two messages marked read'); + + assert.strictEqual( + markedRead2[0].id, + newestUnread.id, + 'with stories/should be newestUnread' + ); + assert.strictEqual( + markedRead2[1].id, + unreadStoryReply.id, + 'with stories/should be unreadStoryReply' + ); assert.strictEqual( await getTotalUnreadForConversation(conversationId, { storyId: undefined, - isGroup: false, + includeStoryReplies: true, }), 0, - 'unread count' + 'with stories/unread count' ); }); @@ -281,6 +301,7 @@ describe('sql/markRead', () => { newestUnreadAt: message7.received_at, readAt, storyId, + includeStoryReplies: false, }); assert.lengthOf(markedRead, 3, 'three messages marked read'); @@ -377,7 +398,7 @@ describe('sql/markRead', () => { assert.strictEqual( await getTotalUnreadForConversation(conversationId, { storyId: undefined, - isGroup: false, + includeStoryReplies: true, }), 2, 'unread count' @@ -388,6 +409,7 @@ describe('sql/markRead', () => { conversationId, newestUnreadAt: message4.received_at, readAt, + includeStoryReplies: false, now, }); @@ -400,7 +422,7 @@ describe('sql/markRead', () => { assert.strictEqual( await getTotalUnreadForConversation(conversationId, { storyId: undefined, - isGroup: false, + includeStoryReplies: true, }), 1, 'unread count' @@ -798,7 +820,7 @@ describe('sql/markRead', () => { const markedRead = await getUnreadByConversationAndMarkRead({ conversationId, - isGroup: true, + includeStoryReplies: false, newestUnreadAt: message4.received_at, readAt, }); diff --git a/ts/test-electron/sql/timelineFetches_test.ts b/ts/test-electron/sql/timelineFetches_test.ts index 6f04bb817..8845d0210 100644 --- a/ts/test-electron/sql/timelineFetches_test.ts +++ b/ts/test-electron/sql/timelineFetches_test.ts @@ -93,7 +93,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 5); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: false, + includeStoryReplies: true, limit: 5, storyId: undefined, }); @@ -150,7 +150,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, storyId, }); @@ -204,7 +204,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, storyId: undefined, }); @@ -255,7 +255,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, receivedAt: target, sentAt: target, @@ -308,7 +308,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, receivedAt: target, sentAt: target, @@ -365,7 +365,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getOlderMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, messageId: message2.id, receivedAt: target, @@ -443,7 +443,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 5); const messages = await getNewerMessagesByConversation(conversationId, { - isGroup: false, + includeStoryReplies: true, limit: 5, storyId: undefined, }); @@ -499,7 +499,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getNewerMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, storyId, }); @@ -551,7 +551,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getNewerMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, receivedAt: target, sentAt: target, @@ -606,7 +606,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getNewerMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, storyId: undefined, receivedAt: target, @@ -659,7 +659,7 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 3); const messages = await getNewerMessagesByConversation(conversationId, { - isGroup: true, + includeStoryReplies: false, limit: 5, receivedAt: target, sentAt: target, @@ -778,24 +778,23 @@ describe('sql/timelineFetches', () => { assert.lengthOf(await _getAllMessages(), 8); const metricsInTimeline = await getMessageMetricsForConversation( - conversationId - ); - assert.strictEqual( - metricsInTimeline?.oldest?.id, - oldestInStory.id, - 'oldest' + conversationId, + { + includeStoryReplies: false, + } ); + assert.strictEqual(metricsInTimeline?.oldest?.id, oldest.id, 'oldest'); assert.strictEqual(metricsInTimeline?.newest?.id, newest.id, 'newest'); assert.strictEqual( metricsInTimeline?.oldestUnseen?.id, oldestUnseen.id, 'oldestUnseen' ); - assert.strictEqual(metricsInTimeline?.totalUnseen, 3, 'totalUnseen'); + assert.strictEqual(metricsInTimeline?.totalUnseen, 2, 'totalUnseen'); const metricsInStory = await getMessageMetricsForConversation( conversationId, - storyId + { storyId, includeStoryReplies: true } ); assert.strictEqual( metricsInStory?.oldest?.id, diff --git a/ts/test-node/sql_migrations_test.ts b/ts/test-node/sql_migrations_test.ts index 104e27a14..737f062a8 100644 --- a/ts/test-node/sql_migrations_test.ts +++ b/ts/test-node/sql_migrations_test.ts @@ -1643,11 +1643,12 @@ describe('SQL migrations test', () => { function insertPredicate( query: string, - storyId: string | undefined + storyId: string | undefined, + includeStoryReplies: boolean ): string { return query.replaceAll( ':story_id_predicate:', - _storyIdPredicate(storyId) + _storyIdPredicate(storyId, includeStoryReplies) ); } @@ -1657,7 +1658,7 @@ describe('SQL migrations test', () => { for (const storyId of ['123', undefined]) { for (const { query, index } of queries) { const details = db - .prepare(insertPredicate(query, storyId)) + .prepare(insertPredicate(query, storyId, true)) .all({ storyId }) .map(({ detail }) => detail) .join('\n'); diff --git a/ts/util/markConversationRead.ts b/ts/util/markConversationRead.ts index 59cca37ff..f41c8a847 100644 --- a/ts/util/markConversationRead.ts +++ b/ts/util/markConversationRead.ts @@ -33,7 +33,7 @@ export async function markConversationRead( conversationId, newestUnreadAt, readAt: options.readAt, - isGroup: isGroup(conversationAttrs), + includeStoryReplies: !isGroup(conversationAttrs), }), window.Signal.Data.getUnreadReactionsAndMarkRead({ conversationId,