From 728cda401495f87ceba05bbceb4744288313d0ed Mon Sep 17 00:00:00 2001 From: Jamie <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:49:51 -0700 Subject: [PATCH] Fix expiring call messages missing timer icon --- ts/models/conversations.preload.ts | 1 - ts/sql/Interface.std.ts | 22 +++- ts/sql/Server.node.ts | 124 ++++++++++++++----- ts/state/ducks/callHistory.preload.ts | 37 ++---- ts/test-node/sql/migration_1100_test.node.ts | 6 +- ts/util/callDisposition.preload.ts | 114 +++++++++++++---- ts/util/onCallLogEventSync.preload.ts | 51 +++----- 7 files changed, 226 insertions(+), 129 deletions(-) diff --git a/ts/models/conversations.preload.ts b/ts/models/conversations.preload.ts index 12e19c3be7..ea8fe36d4b 100644 --- a/ts/models/conversations.preload.ts +++ b/ts/models/conversations.preload.ts @@ -5110,7 +5110,6 @@ export class ConversationModel { ): Promise { await markConversationRead(this.attributes, readMessage, options); this.throttledUpdateUnread(); - window.reduxActions.callHistory.updateCallHistoryUnreadCount([]); } async #updateUnread(): Promise { diff --git a/ts/sql/Interface.std.ts b/ts/sql/Interface.std.ts index 44b8dc3996..fbacbff648 100644 --- a/ts/sql/Interface.std.ts +++ b/ts/sql/Interface.std.ts @@ -543,6 +543,15 @@ export type GetUnreadByConversationAndMarkReadResultType = Array< > >; +export type GetUnreadCallMessagesAndMarkReadResult = Pick< + MessageType, + | 'id' + | 'conversationId' + | 'readStatus' + | 'seenStatus' + | 'expirationStartTimestamp' +>; + export type GetConversationRangeCenteredOnMessageResultType = Readonly<{ older: Array; @@ -1284,17 +1293,20 @@ type WritableInterface = { _removeAllCallHistory: () => void; markCallHistoryDeleted: (callId: string) => void; cleanupCallHistoryMessages: () => void; - markCallHistoryRead: (callId: string) => void; - markAllCallHistoryRead: ( + getUnreadCallMessagesAndMarkRead: ( target: CallLogEventTarget, readAt: number, activeCallIds: Set - ) => number; - markAllCallHistoryReadInConversation: ( + ) => ReadonlyArray; + getUnreadCallMessageAndMarkRead: ( + callId: string, + readAt: number + ) => GetUnreadCallMessagesAndMarkReadResult | null; + getUnreadCallMessagesInConversationAndMarkRead: ( target: CallLogEventTarget, readAt: number, activeCallIds: Set - ) => number; + ) => ReadonlyArray; saveCallHistory: (callHistory: CallHistoryDetails) => void; markCallHistoryMissed: (callIds: ReadonlyArray) => void; getRecentStaleRingsAndMarkOlderMissed: () => ReadonlyArray; diff --git a/ts/sql/Server.node.ts b/ts/sql/Server.node.ts index 5b1029bed3..83ddd31dce 100644 --- a/ts/sql/Server.node.ts +++ b/ts/sql/Server.node.ts @@ -202,6 +202,7 @@ import type { MaybeStaleCallHistory, ExistingAttachmentData, ExistingAttachmentUploadData, + GetUnreadCallMessagesAndMarkReadResult, } from './Interface.std.ts'; import { AttachmentDownloadSource, @@ -672,9 +673,9 @@ export const DataWriter: ServerWritableInterface = { _removeAllCallHistory, markCallHistoryDeleted, cleanupCallHistoryMessages, - markCallHistoryRead, - markAllCallHistoryRead, - markAllCallHistoryReadInConversation, + getUnreadCallMessageAndMarkRead, + getUnreadCallMessagesAndMarkRead, + getUnreadCallMessagesInConversationAndMarkRead, saveCallHistory, markCallHistoryMissed, insertCallLink, @@ -4877,22 +4878,6 @@ function getCallHistoryUnreadCount(db: ReadableDB): number { return row ?? 0; } -function markCallHistoryRead(db: WritableDB, callId: string): void { - const jsonPatch = JSON.stringify({ - seenStatus: SeenStatus.Seen, - }); - - const [query, params] = sql` - UPDATE messages - SET - seenStatus = ${SEEN_STATUS_SEEN}, - json = json_patch(json, ${jsonPatch}) - WHERE type IS 'call-history' - AND callId IS ${callId} - `; - db.prepare(query).run(params); -} - function getCallHistoryForCallLogEventTarget( db: ReadableDB, target: CallLogEventTarget @@ -5032,18 +5017,59 @@ function getMessageReceivedAtForCall( return receivedAt; } -export function markAllCallHistoryRead( +function getUnreadCallMessageAndMarkRead( + db: WritableDB, + callId: string, + readAt: number +): GetUnreadCallMessagesAndMarkReadResult | null { + const jsonPatch = JSON.stringify({ + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + }); + + const [query, params] = sql` + UPDATE messages + SET + readStatus = ${READ_STATUS_READ}, + seenStatus = ${SEEN_STATUS_SEEN}, + json = json_patch(json, ${jsonPatch}), + expirationStartTimestamp = CASE + WHEN messages.hasExpireTimer IS 1 + AND messages.expirationStartTimestamp IS NULL + THEN + ${readAt} + ELSE + expirationStartTimestamp + END + WHERE messages.type IS 'call-history' + AND messages.callId IS ${callId} + RETURNING + messages.id, + messages.conversationId, + messages.readStatus, + messages.seenStatus, + messages.expirationStartTimestamp + `; + + const result = db + .prepare(query) + .get(params); + + return result ?? null; +} + +export function getUnreadCallMessagesAndMarkRead( db: WritableDB, target: CallLogEventTarget, readAt: number, activeCallIds: Set, inConversation = false -): number { +): ReadonlyArray { return db.transaction(() => { const callHistory = getCallHistoryForCallLogEventTarget(db, target); if (callHistory == null) { logger.warn('markAllCallHistoryRead: Target call not found'); - return 0; + return []; } const { callId } = callHistory; @@ -5069,7 +5095,7 @@ export function markAllCallHistoryRead( const conversationId = getConversationIdForCallHistory(db, callHistory); if (conversationId == null) { logger.warn('markAllCallHistoryRead: Conversation not found for call'); - return 0; + return []; } logger.info( @@ -5084,7 +5110,7 @@ export function markAllCallHistoryRead( if (receivedAt == null) { logger.warn('markAllCallHistoryRead: Message not found for call'); - return 0; + return []; } const jsonPatch = JSON.stringify({ @@ -5096,6 +5122,15 @@ export function markAllCallHistoryRead( `markAllCallHistoryRead: Marking calls before ${receivedAt} read` ); + const returning = sqlFragment` + RETURNING + messages.id, + messages.conversationId, + messages.readStatus, + messages.seenStatus, + messages.expirationStartTimestamp + `; + const [updateQuery, updateParams] = sql` UPDATE messages SET @@ -5105,10 +5140,13 @@ export function markAllCallHistoryRead( WHERE messages.type IS 'call-history' AND ${predicate} AND messages.seenStatus IS ${SEEN_STATUS_UNSEEN} - AND messages.received_at <= ${receivedAt}; + AND messages.received_at <= ${receivedAt} + ${returning} `; - const result = db.prepare(updateQuery).run(updateParams); + const updateResult = db + .prepare(updateQuery) + .all(updateParams); const [updateExpirationQuery, updateExpirationParams] = sql` UPDATE messages @@ -5121,20 +5159,44 @@ export function markAllCallHistoryRead( AND hasExpireTimer IS 1 AND expirationStartTimestamp IS NULL AND messages.callId NOT IN (${sqlJoin(Array.from(activeCallIds))}) + ${returning} `; - db.prepare(updateExpirationQuery).run(updateExpirationParams); + const updateExpirationResult = db + .prepare(updateExpirationQuery) + .all(updateExpirationParams); - return result.changes; + const seen = new Set(); + const merged: Array = []; + + for (const item of updateExpirationResult) { + seen.add(item.id); + merged.push(item); + } + + for (const item of updateResult) { + if (seen.has(item.id)) { + continue; // prefer data from the second update + } + merged.push(item); + } + + return merged; })(); } -function markAllCallHistoryReadInConversation( +function getUnreadCallMessagesInConversationAndMarkRead( db: WritableDB, target: CallLogEventTarget, readAt: number, activeCallIds: Set -): number { - return markAllCallHistoryRead(db, target, readAt, activeCallIds, true); +): ReadonlyArray { + return getUnreadCallMessagesAndMarkRead( + db, + target, + readAt, + activeCallIds, + true + ); } function getCallHistoryGroupData( diff --git a/ts/state/ducks/callHistory.preload.ts b/ts/state/ducks/callHistory.preload.ts index 0cded255bd..d9db764040 100644 --- a/ts/state/ducks/callHistory.preload.ts +++ b/ts/state/ducks/callHistory.preload.ts @@ -8,12 +8,13 @@ import type { StateType as RootStateType } from '../reducer.preload.ts'; import { clearCallHistoryDataAndSync, markAllCallHistoryReadAndSync, + markCallHistoryReadWithoutSync, } from '../../util/callDisposition.preload.ts'; import type { BoundActionCreatorsMapObject } from '../../hooks/useBoundActions.std.ts'; import { useBoundActions } from '../../hooks/useBoundActions.std.ts'; import type { ToastActionType } from './toast.preload.ts'; import { showToast } from './toast.preload.ts'; -import { DataReader, DataWriter } from '../../sql/Client.preload.ts'; +import { DataReader } from '../../sql/Client.preload.ts'; import { ToastType } from '../../types/Toast.dom.tsx'; import { ClearCallHistoryResult, @@ -135,37 +136,27 @@ function updateCallHistoryUnreadCount( } function markCallHistoryRead( - conversationId: string, callId: string ): ThunkAction { - return async dispatch => { - try { - await DataWriter.markCallHistoryRead(callId); - } catch (error) { - log.error( - 'markCallHistoryRead: Error marking call history read', - Errors.toLogFormat(error) - ); - } finally { - dispatch(updateCallHistoryUnreadCount([conversationId])); - } + return async () => { + await markCallHistoryReadWithoutSync({ + mode: 'only-target-call', + target: { callId }, + readAt: Date.now(), + }); }; } export function markCallHistoryReadInConversation( callId: string ): ThunkAction { - return async (dispatch, getState) => { + return async (_dispatch, getState) => { const callHistorySelector = getCallHistorySelector(getState()); const callHistory = callHistorySelector(callId); if (callHistory == null) { return; } - try { - await markAllCallHistoryReadAndSync(callHistory, true); - } finally { - dispatch(updateCallHistoryUnreadCount([callHistory.peerId])); - } + await markAllCallHistoryReadAndSync(callHistory, Date.now(), true); }; } @@ -175,14 +166,10 @@ function markCallsTabViewed(): ThunkAction< unknown, CallHistoryUpdateUnread > { - return async (dispatch, getState) => { + return async (_dispatch, getState) => { const latestCall = getCallHistoryLatestCall(getState()); - if (latestCall != null) { - const conversationIds = - await DataReader.getCallHistoryUnreadCallConversationIds(); - await markAllCallHistoryReadAndSync(latestCall, false); - dispatch(updateCallHistoryUnreadCount(conversationIds)); + await markAllCallHistoryReadAndSync(latestCall, Date.now(), false); } }; } diff --git a/ts/test-node/sql/migration_1100_test.node.ts b/ts/test-node/sql/migration_1100_test.node.ts index 19b212e448..8a87fb3b69 100644 --- a/ts/test-node/sql/migration_1100_test.node.ts +++ b/ts/test-node/sql/migration_1100_test.node.ts @@ -4,7 +4,7 @@ import { assert } from 'chai'; import lodash from 'lodash'; import type { WritableDB } from '../../sql/Interface.std.ts'; -import { markAllCallHistoryRead } from '../../sql/Server.node.ts'; +import { getUnreadCallMessagesAndMarkRead } from '../../sql/Server.node.ts'; import { SeenStatus } from '../../MessageSeenStatus.std.ts'; import { CallMode, @@ -92,7 +92,7 @@ describe('SQL/updateToSchemaVersion1100', () => { const readAt = target.timestamp + 1; const start = performance.now(); - const changes = markAllCallHistoryRead( + const changes = getUnreadCallMessagesAndMarkRead( db, target, readAt, @@ -100,7 +100,7 @@ describe('SQL/updateToSchemaVersion1100', () => { true ); const end = performance.now(); - assert.equal(changes, Math.ceil(COUNT / CONVERSATIONS)); + assert.equal(changes.length, Math.ceil(COUNT / CONVERSATIONS)); assert.isBelow(end - start, 50); }); }); diff --git a/ts/util/callDisposition.preload.ts b/ts/util/callDisposition.preload.ts index cfe0dd0dcd..02a8da8237 100644 --- a/ts/util/callDisposition.preload.ts +++ b/ts/util/callDisposition.preload.ts @@ -55,6 +55,7 @@ import type { CallEventDetails, CallHistoryDetails, CallLogEventDetails, + CallLogEventTarget, CallStatus, GroupCallMeta, } from '../types/CallDisposition.std.ts'; @@ -72,6 +73,7 @@ import { itemStorage } from '../textsecure/Storage.preload.ts'; import { update as updateExpiringMessagesService } from '../services/expiringMessagesDeletion.preload.ts'; import type { DurationInSeconds } from './durations/duration-in-seconds.std.ts'; import { isFeaturedEnabledNoRedux } from './isFeatureEnabled.dom.ts'; +import type { GetUnreadCallMessagesAndMarkReadResult } from '../sql/Interface.std.ts'; const { isEqual } = lodash; @@ -1297,6 +1299,11 @@ async function saveCallHistory({ message.set({ id }); log.info('saveCallHistory: Saved call history message:', message.id); + if (prevMessage != null) { + // Remove the previous message so it's forced to update in the cache + window.MessageCache.unregister(prevMessage.id); + } + const model = window.MessageCache.register(message); if (prevMessage == null) { @@ -1511,38 +1518,91 @@ export async function clearCallHistoryDataAndSync( return ClearCallHistoryResult.Success; } +export type MarkCallHistoryReadParams = + | { + mode: 'only-target-call'; + target: { callId: string; timestamp?: never }; + readAt: number; + } + | { + mode: 'all-calls-in-conversation' | 'all-calls'; + target: CallLogEventTarget; + readAt: number; + }; + +export async function markCallHistoryReadWithoutSync( + params: MarkCallHistoryReadParams +): Promise { + log.info( + `markAllCallHistoryReadWithoutSync: Marking call history read before (${params.target.callId}, ${params.target.timestamp})` + ); + const activeCallIds = calling.getActiveCallIds(); + let updatedMessages: ReadonlyArray; + if (params.mode === 'only-target-call') { + const updatedMessage = await DataWriter.getUnreadCallMessageAndMarkRead( + params.target.callId, + params.readAt + ); + updatedMessages = updatedMessage != null ? [updatedMessage] : []; + } else if (params.mode === 'all-calls-in-conversation') { + updatedMessages = + await DataWriter.getUnreadCallMessagesInConversationAndMarkRead( + params.target, + params.readAt, + activeCallIds + ); + } else if (params.mode === 'all-calls') { + updatedMessages = await DataWriter.getUnreadCallMessagesAndMarkRead( + params.target, + params.readAt, + activeCallIds + ); + } else { + throw missingCaseError(params.mode); + } + + const count = updatedMessages.length; + + log.info( + `markAllCallHistoryReadWithoutSync: Marked ${count} call history messages read` + ); + + const conversationIds = new Set(); + + for (const updatedMessage of updatedMessages) { + conversationIds.add(updatedMessage.conversationId); + + const model = window.MessageCache.getById(updatedMessage.id); + if (model == null) { + continue; + } + model.set({ + readStatus: updatedMessage.readStatus, + seenStatus: updatedMessage.seenStatus, + expirationStartTimestamp: updatedMessage.expirationStartTimestamp, + }); + } + + if (count > 0) { + updateExpiringMessagesService(); + } + + window.reduxActions.callHistory.updateCallHistoryUnreadCount( + Array.from(conversationIds) + ); +} + export async function markAllCallHistoryReadAndSync( latestCall: CallHistoryDetails, + readAt: number, inConversation: boolean ): Promise { try { - log.info( - `markAllCallHistoryReadAndSync: Marking call history read before (${latestCall.callId}, ${latestCall.timestamp})` - ); - const readAt = Date.now(); - const activeCallIds = calling.getActiveCallIds(); - let count: number; - if (inConversation) { - count = await DataWriter.markAllCallHistoryReadInConversation( - latestCall, - readAt, - activeCallIds - ); - } else { - count = await DataWriter.markAllCallHistoryRead( - latestCall, - readAt, - activeCallIds - ); - } - - log.info( - `markAllCallHistoryReadAndSync: Marked ${count} call history messages read` - ); - - if (count > 0) { - updateExpiringMessagesService(); - } + await markCallHistoryReadWithoutSync({ + mode: inConversation ? 'all-calls-in-conversation' : 'all-calls', + target: latestCall, + readAt, + }); const ourAci = itemStorage.user.getCheckedAci(); diff --git a/ts/util/onCallLogEventSync.preload.ts b/ts/util/onCallLogEventSync.preload.ts index d02e887816..7dfbb27955 100644 --- a/ts/util/onCallLogEventSync.preload.ts +++ b/ts/util/onCallLogEventSync.preload.ts @@ -7,9 +7,10 @@ import { DataReader, DataWriter } from '../sql/Client.preload.ts'; import type { CallLogEventTarget } from '../types/CallDisposition.std.ts'; import { CallLogEvent } from '../types/CallDisposition.std.ts'; import { missingCaseError } from './missingCaseError.std.ts'; -import { updateDeletedMessages } from './callDisposition.preload.ts'; -import { update as updateExpiringMessagesService } from '../services/expiringMessagesDeletion.preload.ts'; -import { calling } from '../services/calling.preload.ts'; +import { + markCallHistoryReadWithoutSync, + updateDeletedMessages, +} from './callDisposition.preload.ts'; const log = createLogger('onCallLogEventSync'); @@ -55,43 +56,19 @@ export async function onCallLogEventSync( confirm(); } else if (type === CallLogEvent.MarkedAsRead) { log.info('Marking call history read'); - - let unreadConversationIds: ReadonlyArray = []; - try { - unreadConversationIds = - await DataReader.getCallHistoryUnreadCallConversationIds(); - const count = await DataWriter.markAllCallHistoryRead( - target, - Math.min(Date.now(), eventTimestamp), - calling.getActiveCallIds() - ); - log.info(`Marked ${count} call history messages read`); - if (count !== 0) { - updateExpiringMessagesService(); - } - } finally { - window.reduxActions.callHistory.updateCallHistoryUnreadCount( - unreadConversationIds - ); - } + await markCallHistoryReadWithoutSync({ + mode: 'all-calls', + target, + readAt: Math.min(Date.now(), eventTimestamp), + }); confirm(); } else if (type === CallLogEvent.MarkedAsReadInConversation) { log.info('Marking call history read in conversation'); - try { - const count = await DataWriter.markAllCallHistoryReadInConversation( - target, - Math.min(Date.now(), eventTimestamp), - calling.getActiveCallIds() - ); - log.info(`Marked ${count} call history messages read`); - if (count !== 0) { - updateExpiringMessagesService(); - } - } finally { - window.reduxActions.callHistory.updateCallHistoryUnreadCount( - peerIdAsConversationId != null ? [peerIdAsConversationId] : [] - ); - } + await markCallHistoryReadWithoutSync({ + mode: 'all-calls-in-conversation', + target, + readAt: Math.min(Date.now(), eventTimestamp), + }); confirm(); } else if (type === CallLogEvent.UNIMPLEMENTED_ClearInConversation) { log.warn('CallLogEvent.CLEAR_IN_CONVERSATION not supported');