From 4ef3537a530e22ce104cba892de88c5f7c846d20 Mon Sep 17 00:00:00 2001 From: William Grant Date: Mon, 3 Apr 2023 14:09:05 +0200 Subject: [PATCH] feat: timer notifications expire correctly when disappearing after sending --- .../conversation/TimerNotification.tsx | 11 ++-- ts/models/conversation.ts | 2 + ts/models/message.ts | 46 ++++++++++------- ts/receiver/dataMessage.ts | 8 +-- ts/receiver/queuedJob.ts | 51 ++++++++++++------- .../ExpirationTimerUpdateMessage.ts | 27 +++++----- ts/session/sending/MessageSentHandler.ts | 10 ++-- ts/state/ducks/conversations.ts | 5 +- ts/util/expiringMessages.ts | 25 ++++++--- 9 files changed, 106 insertions(+), 79 deletions(-) diff --git a/ts/components/conversation/TimerNotification.tsx b/ts/components/conversation/TimerNotification.tsx index 94ea5091e..01679e15a 100644 --- a/ts/components/conversation/TimerNotification.tsx +++ b/ts/components/conversation/TimerNotification.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { missingCaseError } from '../../util/missingCaseError'; import { PropsForExpirationTimer } from '../../state/ducks/conversations'; import { NotificationBubble } from './message/message-item/notification-bubble/NotificationBubble'; -import { ReadableMessage } from './message/message-item/ReadableMessage'; +import { ExpirableReadableMessage } from './message/message-item/ExpirableReadableMessage'; export const TimerNotification = (props: PropsForExpirationTimer) => { const { @@ -50,10 +50,15 @@ export const TimerNotification = (props: PropsForExpirationTimer) => { throw new Error('textToRender invalid key used TimerNotification'); } return ( - { iconColor="inherit" notificationText={textToRender} /> - + ); }; diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index fa2822e99..230791984 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -1097,6 +1097,8 @@ export class ConversationModel extends Backbone.Model { source, fromSync, }, + expirationType, + expireTimer, }; let message: MessageModel | undefined; diff --git a/ts/models/message.ts b/ts/models/message.ts index a50622b45..8339b8825 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -280,6 +280,26 @@ export class MessageModel extends Backbone.Model { await deleteExternalMessageFiles(this.attributes); } + public getPropsForExpiringMessage(): PropsForExpiringMessage | null { + const expirationType = this.get('expirationType'); + const expirationLength = this.get('expireTimer') || null; + + const expireTimerStart = this.get('expirationStartTimestamp') || null; + + const expirationTimestamp = + expirationType && expireTimerStart && expirationLength + ? expireTimerStart + expirationLength * DURATION.SECONDS + : null; + + return { + convoId: this.get('conversationId'), + messageId: this.get('id'), + expirationLength, + expirationTimestamp, + isExpired: this.isExpired(), + }; + } + public getPropsForTimerNotification(): PropsForExpirationTimer | null { if (!this.isExpirationTimerUpdate()) { return null; @@ -289,6 +309,12 @@ export class MessageModel extends Backbone.Model { return null; } + // TODO should direction be parts of expiration props? + let direction = this.get('direction'); + if (!direction) { + direction = this.get('type') === 'outgoing' ? 'outgoing' : 'incoming'; + } + const { expirationType, expireTimer, fromSync, source } = timerUpdate; const timespan = ExpirationTimerOptions.getName(expireTimer || 0); const disabled = !expireTimer; @@ -302,29 +328,13 @@ export class MessageModel extends Backbone.Model { receivedAt: this.get('received_at'), isUnread: this.isUnread(), expirationType: expirationType || 'off', + direction, + ...this.getPropsForExpiringMessage(), }; return basicProps; } - public getPropsForExpiringMessage(): PropsForExpiringMessage | null { - const expirationType = this.get('expirationType'); - const expireTimerStart = this.get('expirationStartTimestamp') || null; - const expirationLength = this.get('expireTimer') || null; - const expirationTimestamp = - expirationType && expireTimerStart && expirationLength - ? expireTimerStart + expirationLength * DURATION.SECONDS - : null; - - return { - convoId: this.get('conversationId'), - messageId: this.get('id'), - expirationLength, - expirationTimestamp, - isExpired: this.isExpired(), - }; - } - public getPropsForGroupInvitation(): PropsForGroupInvitation | null { if (!this.isGroupInvitation()) { return null; diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index 2e74dc1fd..b3bb42deb 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -23,7 +23,6 @@ import { toLogFormat } from '../types/attachments/Errors'; import { ConversationTypeEnum } from '../models/conversationAttributes'; import { Reactions } from '../util/reactions'; import { Action, Reaction } from '../types/Reaction'; -import { setExpirationStartTimestamp } from '../util/expiringMessages'; function cleanAttachment(attachment: any) { return { @@ -246,12 +245,7 @@ export async function handleSwarmDataMessage( if (isSyncedMessage) { // TODO handle sync messages separately window.log.info('WIP: Sync Message dropping'); - } else { - if (msgModel.isIncoming() && expireUpdate.expirationType === 'deleteAfterSend') { - msgModel = - setExpirationStartTimestamp(msgModel, 'deleteAfterSend', msgModel.get('sent_at')) || - msgModel; - } + expireUpdate = null; } await handleSwarmMessage( diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index d6d666e32..9842cf74f 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -16,7 +16,10 @@ import { GoogleChrome } from '../util'; import { appendFetchAvatarAndProfileJob } from './userProfileImageUpdates'; import { ConversationTypeEnum } from '../models/conversationAttributes'; import { getUsBlindedInThatServer } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; -import { DisappearingMessageConversationType } from '../util/expiringMessages'; +import { + DisappearingMessageConversationType, + setExpirationStartTimestamp, +} from '../util/expiringMessages'; import { getNowWithNetworkOffset } from '../session/apis/snode_api/SNodeAPI'; function contentTypeSupported(type: string): boolean { @@ -351,26 +354,43 @@ export async function handleMessageJob( try { messageModel.set({ flags: regularDataMessage.flags }); - if (messageModel.isExpirationTimerUpdate()) { - // TODO account for lastDisappearingMessageChangeTimestamp - - // TODO in the future we will remove the dataMessage expireTimer and the expirationTimerUpdate - let expirationType = expireUpdate.expirationType; - let expireTimer = expireUpdate.expireTimer; - - if (!expirationType) { - expirationType = conversation.isPrivate() ? 'deleteAfterRead' : 'deleteAfterSend'; + if (!isEmpty(expireUpdate)) { + messageModel.set({ + expirationType: expireUpdate.expirationType, + expireTimer: expireUpdate.expireTimer, + }); + + if (messageModel.isIncoming() && messageModel.get('expirationType') === 'deleteAfterSend') { + messageModel = + setExpirationStartTimestamp( + messageModel, + 'deleteAfterSend', + messageModel.get('sent_at') + ) || messageModel; } + } + if (messageModel.isExpirationTimerUpdate()) { + // TODO in the future we will remove the dataMessage expireTimer and the expirationTimerUpdate // Backwards compatibility for Disappearing Messages in old clients if (regularDataMessage.expireTimer) { const expirationTimerUpdate = messageModel.get('expirationTimerUpdate'); if (!isEmpty(expirationTimerUpdate)) { - expirationType = expirationTimerUpdate?.expirationType; - expireTimer = expirationTimerUpdate?.expireTimer; + messageModel.set({ + expirationType: expirationTimerUpdate?.expirationType, + expireTimer: expirationTimerUpdate?.expireTimer, + }); } } + // TODO account for lastDisappearingMessageChangeTimestamp + let expirationType = messageModel.get('expirationType'); + const expireTimer = messageModel.get('expireTimer'); + + if (!expirationType) { + expirationType = conversation.isPrivate() ? 'deleteAfterRead' : 'deleteAfterSend'; + } + // TODO compare types and change timestamps const oldTypeValue = conversation.get('expirationType'); const oldTimerValue = conversation.get('expireTimer'); @@ -390,13 +410,6 @@ export async function handleMessageJob( expireTimer ); } else { - // NOTE this is a disappearing message NOT a expiration timer update - if (!isEmpty(expireUpdate)) { - messageModel.set({ - expirationType: expireUpdate.expirationType, - expireTimer: expireUpdate.expireTimer, - }); - } // this does not commit to db nor UI unless we need to approve a convo await handleRegularMessage( conversation, diff --git a/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts b/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts index 1cbfd0656..bc3cb6d4d 100644 --- a/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts +++ b/ts/session/messages/outgoing/controlMessage/ExpirationTimerUpdateMessage.ts @@ -1,9 +1,9 @@ -import { DataMessage } from '..'; import { SignalService } from '../../../../protobuf'; import { DisappearingMessageType } from '../../../../util/expiringMessages'; import { PubKey } from '../../../types'; import { StringUtils } from '../../../utils'; import { MessageParams } from '../Message'; +import { VisibleMessage } from '../visibleMessage/VisibleMessage'; interface ExpirationTimerUpdateMessageParams extends MessageParams { groupId?: string | PubKey; @@ -16,22 +16,23 @@ interface ExpirationTimerUpdateMessageParams extends MessageParams { // Note the old disappearing messages used a data message for the expiration time. // The new ones use properties on the Content Message // We will remove support for the old one 2 weeks after the release -export class ExpirationTimerUpdateMessage extends DataMessage { +export class ExpirationTimerUpdateMessage extends VisibleMessage { public readonly groupId?: PubKey; - public readonly syncTarget?: string; - public readonly expirationType: DisappearingMessageType | null; - public readonly expireTimer: number | null; public readonly lastDisappearingMessageChangeTimestamp: number | null; constructor(params: ExpirationTimerUpdateMessageParams) { - super({ timestamp: params.timestamp, identifier: params.identifier }); - this.expirationType = params.expirationType; - this.expireTimer = params.expireTimer; + super({ + timestamp: params.timestamp, + identifier: params.identifier, + expirationType: params.expirationType, + expireTimer: params.expireTimer || undefined, + syncTarget: params.syncTarget ? PubKey.cast(params.syncTarget).key : undefined, + }); + this.lastDisappearingMessageChangeTimestamp = params.lastDisappearingMessageChangeTimestamp; - const { groupId, syncTarget } = params; + const { groupId } = params; this.groupId = groupId ? PubKey.cast(groupId) : undefined; - this.syncTarget = syncTarget ? PubKey.cast(syncTarget).key : undefined; } public contentProto(): SignalService.Content { @@ -47,7 +48,7 @@ export class ExpirationTimerUpdateMessage extends DataMessage { } public dataProto(): SignalService.DataMessage { - const data = new SignalService.DataMessage(); + const data = super.dataProto(); data.flags = SignalService.DataMessage.Flags.EXPIRATION_TIMER_UPDATE; @@ -64,10 +65,6 @@ export class ExpirationTimerUpdateMessage extends DataMessage { data.group = groupMessage; } - if (this.syncTarget) { - data.syncTarget = this.syncTarget; - } - // TODO remove 2 weeks after the release if (this.expireTimer) { data.expireTimer = this.expireTimer; diff --git a/ts/session/sending/MessageSentHandler.ts b/ts/session/sending/MessageSentHandler.ts index 2b30bf706..e12330067 100644 --- a/ts/session/sending/MessageSentHandler.ts +++ b/ts/session/sending/MessageSentHandler.ts @@ -136,9 +136,10 @@ async function handleMessageSentSuccess( if (!shouldMarkMessageAsSynced) { const expirationType = fetchedMessage.get('expirationType'); - if (expirationType) { + if (expirationType === 'deleteAfterSend') { fetchedMessage = - setExpirationStartTimestamp(fetchedMessage, expirationType) || fetchedMessage; + setExpirationStartTimestamp(fetchedMessage, expirationType, effectiveTimestamp) || + fetchedMessage; } } @@ -169,11 +170,6 @@ async function handleMessageSentFailure( } } - const expirationType = fetchedMessage.get('expirationType'); - if (expirationType) { - fetchedMessage = setExpirationStartTimestamp(fetchedMessage, expirationType) || fetchedMessage; - } - // always mark the message as sent. // the fact that we have errors on the sent is based on the saveErrors() fetchedMessage.set({ diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index fc2ab20bb..826342494 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -83,7 +83,7 @@ export type PropsForExpiringMessage = { isExpired?: boolean; }; -export type PropsForExpirationTimer = { +export interface PropsForExpirationTimer extends PropsForExpiringMessage { expirationType: DisappearingMessageConversationType; timespan: string; disabled: boolean; @@ -96,7 +96,8 @@ export type PropsForExpirationTimer = { messageId: string; isUnread: boolean; receivedAt: number | undefined; -}; + direction: MessageModelType; +} export type PropsForGroupUpdateGeneral = { type: 'general'; diff --git a/ts/util/expiringMessages.ts b/ts/util/expiringMessages.ts index 3d7c9d37c..4406f164d 100644 --- a/ts/util/expiringMessages.ts +++ b/ts/util/expiringMessages.ts @@ -203,33 +203,42 @@ export function setExpirationStartTimestamp( timestamp?: number ): MessageModel | null { if (message.get('expirationStartTimestamp') > 0) { - window.log.info(`WIP: Expiration Timer already set. Ignoring.`); + window.log.info(`WIP: Expiration Timer already set. Ignoring new value.`); return null; } let expirationStartTimestamp = getNowWithNetworkOffset(); if (timestamp) { - expirationStartTimestamp = Math.min(getNowWithNetworkOffset(), timestamp); + window.log.info( + `WIP: We compare 2 timestamps for a delete after ${ + mode === 'deleteAfterRead' ? 'read' : 'send' + } message: \expirationStartTimestamp `, + new Date(expirationStartTimestamp).toLocaleTimeString(), + '\ntimestamp ', + new Date(timestamp).toLocaleTimeString() + ); + expirationStartTimestamp = Math.min(expirationStartTimestamp, timestamp); } message.set('expirationStartTimestamp', expirationStartTimestamp); if (mode === 'deleteAfterRead') { window.log.info( - `WIP: setExpirationStartTimestamp we set the start timestamp for a delete after read message`, + `WIP: We set the start timestamp for a delete after read message to ${new Date( + expirationStartTimestamp + ).toLocaleTimeString()}`, message ); } else if (mode === 'deleteAfterSend') { window.log.info( - `WIP: setExpirationStartTimestamp we set the start timestamp for a delete after send message`, + `WIP: We set the start timestamp for a delete after send message to ${new Date( + expirationStartTimestamp + ).toLocaleTimeString()}`, message ); } else { - console.log( - `WIP: setExpirationStartTimestamp Invalid disappearing message mode set. Ignoring.`, - message - ); + console.log(`WIP: Invalid disappearing message mode set. Ignoring.`, message); return null; }