From 105508c50f2610585b4550e7b059a3f535b86f7d Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Wed, 11 May 2022 19:45:20 -0700 Subject: [PATCH] Update unread count when creating important local notifications --- ts/groups.ts | 51 +++- ts/models/conversations.ts | 6 + ts/sql/migrations/58-update-unread.ts | 132 ++++++++++ ts/sql/migrations/index.ts | 2 + ts/test-node/sql_migrations_test.ts | 336 ++++++++++++++++++++++++++ 5 files changed, 521 insertions(+), 6 deletions(-) create mode 100644 ts/sql/migrations/58-update-unread.ts diff --git a/ts/groups.ts b/ts/groups.ts index bf39ab929192..3d5b3d7fb8f3 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -83,6 +83,8 @@ import { conversationJobQueue, conversationQueueJobEnum, } from './jobs/conversationJobQueue'; +import { ReadStatus } from './messages/MessageReadStatus'; +import { SeenStatus } from './MessageSeenStatus'; type AccessRequiredEnum = Proto.AccessControl.AccessRequired; @@ -272,7 +274,10 @@ type UploadedAvatarType = { key: string; }; -type BasicMessageType = Pick; +type BasicMessageType = Pick< + MessageAttributesType, + 'id' | 'schemaVersion' | 'readStatus' | 'seenStatus' +>; type GroupV2ChangeMessageType = { type: 'group-v2-change'; @@ -1845,9 +1850,11 @@ export async function createGroupV2({ type: 'group-v2-change', sourceUuid: ourUuid, conversationId: conversation.id, + readStatus: ReadStatus.Read, received_at: window.Signal.Util.incrementMessageCounter(), received_at_ms: timestamp, timestamp, + seenStatus: SeenStatus.Seen, sent_at: timestamp, groupV2Change: { from: ourUuid, @@ -2289,6 +2296,8 @@ export async function initiateMigrationToGroupV2( type: 'group-v1-migration', invitedGV2Members: pendingMembersV2, droppedGV2MemberIds, + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, }); await updateGroup({ @@ -2634,7 +2643,11 @@ export async function respondToGroupV2Migration({ ), }, groupChangeMessages: [ - getBasicMigrationBubble(), + { + ...getBasicMigrationBubble(), + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + }, { ...generateBasicMessage(), type: 'group-v2-change', @@ -2646,6 +2659,8 @@ export async function respondToGroupV2Migration({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Unseen, }, ], members: [], @@ -2663,7 +2678,13 @@ export async function respondToGroupV2Migration({ sentAt, updates: { newAttributes: attributes, - groupChangeMessages: [getBasicMigrationBubble()], + groupChangeMessages: [ + { + ...getBasicMigrationBubble(), + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + }, + ], members: [], }, }); @@ -2694,9 +2715,11 @@ export async function respondToGroupV2Migration({ // Generate notifications into the timeline const groupChangeMessages: Array = []; - groupChangeMessages.push( - buildMigrationBubble(previousGroupV1MembersIds, newAttributes) - ); + groupChangeMessages.push({ + ...buildMigrationBubble(previousGroupV1MembersIds, newAttributes), + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + }); const areWeInvited = (newAttributes.pendingMembersV2 || []).some( item => item.uuid === ourUuid @@ -2717,6 +2740,8 @@ export async function respondToGroupV2Migration({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Unseen, }); } @@ -3190,6 +3215,7 @@ async function appendChangeMessages( // We updated the message, but didn't add new ones - refresh left pane if (!newMessages && mergedMessages.length > 0) { await conversation.updateLastMessage(); + conversation.updateUnread(); } } @@ -3519,6 +3545,8 @@ async function generateLeftGroupChanges( }, ], }, + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Unseen, }; return { @@ -4299,6 +4327,7 @@ function extractDiffs({ let timerNotification: GroupChangeMessageType | undefined; const firstUpdate = !isNumber(old.revision); + const isFromUs = ourUuid === sourceUuid; // Here we hardcode initial messages if this is our first time processing data this // group. Ideally we can collapse it down to just one of: 'you were added', @@ -4317,6 +4346,8 @@ function extractDiffs({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, }; } else if (firstUpdate && dropInitialJoinMessage) { // None of the rest of the messages should be added if dropInitialJoinMessage = true @@ -4333,6 +4364,8 @@ function extractDiffs({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, }; } else if (firstUpdate && areWeInGroup) { message = { @@ -4347,6 +4380,8 @@ function extractDiffs({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, }; } else if (firstUpdate) { message = { @@ -4360,6 +4395,8 @@ function extractDiffs({ }, ], }, + readStatus: ReadStatus.Read, + seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, }; } else if (details.length > 0) { message = { @@ -4370,6 +4407,8 @@ function extractDiffs({ from: sourceUuid, details, }, + readStatus: ReadStatus.Read, + seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, }; } diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 21642373bc70..b93be743600a 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -2909,6 +2909,7 @@ export class ConversationModel extends window.Backbone ); this.trigger('newmessage', model); + this.updateUnread(); } async addDeliveryIssue({ @@ -2954,6 +2955,7 @@ export class ConversationModel extends window.Backbone this.trigger('newmessage', model); await this.notify(model); + this.updateUnread(); } async addKeyChange(keyChangedId: UUID): Promise { @@ -3050,6 +3052,7 @@ export class ConversationModel extends window.Backbone ); this.trigger('newmessage', model); + this.updateUnread(); const uuid = this.getUuid(); if (isDirectConversation(this.attributes) && uuid) { @@ -3115,6 +3118,7 @@ export class ConversationModel extends window.Backbone ); this.trigger('newmessage', model); + this.updateUnread(); } /** @@ -4546,7 +4550,9 @@ export class ConversationModel extends window.Backbone model.set({ id }); const message = window.MessageController.register(id, model); + this.addSingleMessage(message); + this.updateUnread(); return message; } diff --git a/ts/sql/migrations/58-update-unread.ts b/ts/sql/migrations/58-update-unread.ts new file mode 100644 index 000000000000..ae1e6d724217 --- /dev/null +++ b/ts/sql/migrations/58-update-unread.ts @@ -0,0 +1,132 @@ +// Copyright 2021-2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Database } from 'better-sqlite3'; +import { ReadStatus } from '../../messages/MessageReadStatus'; +import { SeenStatus } from '../../MessageSeenStatus'; + +import type { LoggerType } from '../../types/Logging'; + +export default function updateToSchemaVersion58( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 58) { + return; + } + + db.transaction(() => { + db.exec( + ` + --- Promote unread status in JSON to SQL column + + UPDATE messages + SET + readStatus = ${ReadStatus.Unread}, + seenStatus = ${SeenStatus.Unseen} + WHERE + json_extract(json, '$.unread') IS true OR + json_extract(json, '$.unread') IS 1; + + --- Clean up all old messages that still have a null read status + --- Note: we don't need to update seenStatus, because that was defaulted to zero + + UPDATE messages + SET + readStatus = ${ReadStatus.Read} + WHERE + readStatus IS NULL; + + --- Re-run unseen/unread queries from migration 56 + + UPDATE messages + SET + seenStatus = ${SeenStatus.Unseen} + WHERE + readStatus = ${ReadStatus.Unread} AND + ( + type IS NULL + OR + type IN ( + 'call-history', + 'change-number-notification', + 'chat-session-refreshed', + 'delivery-issue', + 'group', + 'incoming', + 'keychange', + 'timer-notification', + 'verified-change' + ) + ); + + UPDATE messages + SET + readStatus = ${ReadStatus.Read} + WHERE + readStatus = ${ReadStatus.Unread} AND + type IS NOT NULL AND + type NOT IN ( + 'call-history', + 'change-number-notification', + 'chat-session-refreshed', + 'delivery-issue', + 'group', + 'incoming', + 'keychange', + 'timer-notification', + 'verified-change' + ); + + --- (new) Ensure these message types are not unread, just unseen + + UPDATE messages + SET + readStatus = ${ReadStatus.Read} + WHERE + readStatus = ${ReadStatus.Unread} AND + ( + type IN ( + 'change-number-notification', + 'keychange' + ) + ); + + --- (new) Ensure that these message types are neither unseen nor unread + + UPDATE messages + SET + readStatus = ${ReadStatus.Read}, + seenStatus = ${SeenStatus.Seen} + WHERE + type IN ( + 'group-v1-migration', + 'message-history-unsynced', + 'outgoing', + 'profile-change', + 'universal-timer-notification' + ); + + --- Make sure JSON reflects SQL columns + + UPDATE messages + SET + json = json_patch( + json, + json_object( + 'readStatus', readStatus, + 'seenStatus', seenStatus + ) + ) + WHERE + readStatus IS NOT NULL OR + seenStatus IS NOT 0; + ` + ); + + db.pragma('user_version = 58'); + })(); + + logger.info('updateToSchemaVersion58: success!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index 2e3ab1bb4ee9..2e0ba9cc4ed4 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -33,6 +33,7 @@ import updateToSchemaVersion54 from './54-unprocessed-received-at-counter'; import updateToSchemaVersion55 from './55-report-message-aci'; import updateToSchemaVersion56 from './56-add-unseen-to-message'; import updateToSchemaVersion57 from './57-rm-message-history-unsynced'; +import updateToSchemaVersion58 from './58-update-unread'; function updateToSchemaVersion1( currentVersion: number, @@ -1929,6 +1930,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion55, updateToSchemaVersion56, updateToSchemaVersion57, + updateToSchemaVersion58, ]; export function updateSchema(db: Database, logger: LoggerType): void { diff --git a/ts/test-node/sql_migrations_test.ts b/ts/test-node/sql_migrations_test.ts index 20169e9a0ab5..5eba117d784a 100644 --- a/ts/test-node/sql_migrations_test.ts +++ b/ts/test-node/sql_migrations_test.ts @@ -2026,4 +2026,340 @@ describe('SQL migrations test', () => { assert.notInclude(second, 'SCAN', 'second'); }); }); + + describe('updateToSchemaVersion58', () => { + it('updates readStatus/seenStatus for messages with unread: true/1 in JSON', () => { + const MESSAGE_ID_1 = generateGuid(); + const MESSAGE_ID_2 = generateGuid(); + const MESSAGE_ID_3 = generateGuid(); + const MESSAGE_ID_4 = generateGuid(); + const CONVERSATION_ID = generateGuid(); + + updateToVersion(57); + + // prettier-ignore + db.exec( + ` + INSERT INTO messages + (id, conversationId, type, json) + VALUES + ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify( + { unread: true } + )}'), + ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify( + { unread: 1 } + )}'), + ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify( + { unread: undefined } + )}'), + ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify( + { unread: 0 } + )}'); + ` + ); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 4, + 'starting total' + ); + + updateToVersion(58); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 4, + 'ending total' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};` + ) + .pluck() + .get(), + 2, + 'ending unread count' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE seenStatus = ${SeenStatus.Unseen};` + ) + .pluck() + .get(), + 2, + 'ending unread count' + ); + + assert.strictEqual( + db + .prepare( + `SELECT readStatus FROM messages WHERE id = '${MESSAGE_ID_2}' LIMIT 1;` + ) + .pluck() + .get(), + ReadStatus.Unread, + 'checking read status for message2' + ); + }); + + it('updates unseenStatus for previously-unread messages', () => { + const MESSAGE_ID_1 = generateGuid(); + const MESSAGE_ID_2 = generateGuid(); + const MESSAGE_ID_3 = generateGuid(); + const MESSAGE_ID_4 = generateGuid(); + const MESSAGE_ID_5 = generateGuid(); + const MESSAGE_ID_6 = generateGuid(); + const MESSAGE_ID_7 = generateGuid(); + const MESSAGE_ID_8 = generateGuid(); + const MESSAGE_ID_9 = generateGuid(); + const MESSAGE_ID_10 = generateGuid(); + const MESSAGE_ID_11 = generateGuid(); + const CONVERSATION_ID = generateGuid(); + + updateToVersion(55); + + db.exec( + ` + INSERT INTO messages + (id, conversationId, type, readStatus) + VALUES + ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'call-history', ${ReadStatus.Unread}), + ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'change-number-notification', ${ReadStatus.Unread}), + ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'chat-session-refreshed', ${ReadStatus.Unread}), + ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'delivery-issue', ${ReadStatus.Unread}), + ('${MESSAGE_ID_5}', '${CONVERSATION_ID}', 'group', ${ReadStatus.Unread}), + ('${MESSAGE_ID_6}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}), + ('${MESSAGE_ID_7}', '${CONVERSATION_ID}', 'keychange', ${ReadStatus.Unread}), + ('${MESSAGE_ID_8}', '${CONVERSATION_ID}', 'timer-notification', ${ReadStatus.Unread}), + ('${MESSAGE_ID_9}', '${CONVERSATION_ID}', 'verified-change', ${ReadStatus.Unread}), + ('${MESSAGE_ID_10}', '${CONVERSATION_ID}', NULL, ${ReadStatus.Unread}), + ('${MESSAGE_ID_11}', '${CONVERSATION_ID}', 'other', ${ReadStatus.Unread}); + ` + ); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 11, + 'starting total' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};` + ) + .pluck() + .get(), + 11, + 'starting unread count' + ); + + updateToVersion(56); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 11, + 'ending total' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};` + ) + .pluck() + .get(), + 10, + 'ending unread count' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE seenStatus = ${SeenStatus.Unseen};` + ) + .pluck() + .get(), + 10, + 'ending unseen count' + ); + + assert.strictEqual( + db + .prepare( + "SELECT readStatus FROM messages WHERE type = 'other' LIMIT 1;" + ) + .pluck() + .get(), + ReadStatus.Read, + "checking read status for 'other' message" + ); + }); + + it('Sets readStatus=Read for keychange and change-number-notification messages', () => { + const MESSAGE_ID_1 = generateGuid(); + const MESSAGE_ID_2 = generateGuid(); + const MESSAGE_ID_3 = generateGuid(); + const CONVERSATION_ID = generateGuid(); + + updateToVersion(57); + + db.exec( + ` + INSERT INTO messages + (id, conversationId, type, readStatus) + VALUES + ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}), + ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'change-number-notification', ${ReadStatus.Unread}), + ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'keychange', ${ReadStatus.Unread}); + ` + ); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 3, + 'starting total' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};` + ) + .pluck() + .get(), + 3, + 'starting unread count' + ); + + updateToVersion(58); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 3, + 'ending total' + ); + assert.strictEqual( + db + .prepare( + `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};` + ) + .pluck() + .get(), + 1, + 'ending unread count' + ); + + assert.strictEqual( + db + .prepare( + "SELECT readStatus FROM messages WHERE type = 'keychange' LIMIT 1;" + ) + .pluck() + .get(), + ReadStatus.Read, + "checking read status for 'keychange' message" + ); + assert.strictEqual( + db + .prepare( + "SELECT seenStatus FROM messages WHERE type = 'keychange' LIMIT 1;" + ) + .pluck() + .get(), + SeenStatus.Unseen, + "checking seen status for 'keychange' message" + ); + }); + + it('updates readStatus/seenStatus for messages with unread: true/1 in JSON', () => { + const MESSAGE_ID_1 = generateGuid(); + const MESSAGE_ID_2 = generateGuid(); + const MESSAGE_ID_3 = generateGuid(); + const MESSAGE_ID_4 = generateGuid(); + const CONVERSATION_ID = generateGuid(); + + updateToVersion(57); + + // prettier-ignore + db.exec( + ` + INSERT INTO messages + (id, conversationId, type, readStatus, seenStatus, json) + VALUES + ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}, NULL, '${JSON.stringify( + { body: 'message1' } + )}'), + ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Read}, NULL, '${JSON.stringify( + { body: 'message2' } + )}'), + ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'incoming', NULL, ${SeenStatus.Unseen}, '${JSON.stringify( + { body: 'message3' } + )}'), + ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'incoming', NULL, ${SeenStatus.Seen}, '${JSON.stringify( + { body: 'message4' } + )}'); + ` + ); + + assert.strictEqual( + db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(), + 4, + 'starting total' + ); + + updateToVersion(58); + + assert.strictEqual( + db + .prepare( + `SELECT json FROM messages WHERE id = '${MESSAGE_ID_1}' LIMIT 1;` + ) + .pluck() + .get(), + JSON.stringify({ + body: 'message1', + readStatus: ReadStatus.Unread, + seenStatus: SeenStatus.Unseen, + }), + 'checking JSON for message1' + ); + assert.strictEqual( + db + .prepare( + `SELECT json FROM messages WHERE id = '${MESSAGE_ID_2}' LIMIT 1;` + ) + .pluck() + .get(), + JSON.stringify({ body: 'message2', readStatus: ReadStatus.Read }), + 'checking JSON for message2' + ); + assert.strictEqual( + db + .prepare( + `SELECT json FROM messages WHERE id = '${MESSAGE_ID_3}' LIMIT 1;` + ) + .pluck() + .get(), + JSON.stringify({ + body: 'message3', + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Unseen, + }), + 'checking JSON for message3' + ); + assert.strictEqual( + db + .prepare( + `SELECT json FROM messages WHERE id = '${MESSAGE_ID_4}' LIMIT 1;` + ) + .pluck() + .get(), + JSON.stringify({ + body: 'message4', + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + }), + 'checking JSON for message4' + ); + }); + }); });