From 4973b9b2045e62d2f219bf66160619ccbff85cbd Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Wed, 1 Oct 2025 16:59:29 -0700 Subject: [PATCH] Fix missing all chat folder on startup without new manifest --- ts/background.ts | 11 ++ ts/services/storage.ts | 40 +++--- ts/sql/Interface.ts | 1 + ts/sql/Server.ts | 2 + ts/sql/server/chatFolders.ts | 13 ++ ts/state/ducks/chatFolders.ts | 17 ++- ts/test-mock/storage/chat_folder_test.ts | 166 +++++++++++++---------- ts/test-mock/storage/fixtures.ts | 23 ++++ ts/types/ChatFolder.ts | 11 ++ 9 files changed, 192 insertions(+), 92 deletions(-) diff --git a/ts/background.ts b/ts/background.ts index 7be0f5128e..65bfdb78cb 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -1014,6 +1014,17 @@ export async function startApp(): Promise { ) { await window.storage.put('needProfileMovedModal', true); } + + if (window.isBeforeVersion(lastVersion, 'v7.75.0-beta.1')) { + const hasAllChatsChatFolder = await DataReader.hasAllChatsChatFolder(); + if (!hasAllChatsChatFolder) { + log.info('Creating "all chats" chat folder'); + await DataWriter.createAllChatsChatFolder(); + StorageService.storageServiceUploadJobAfterEnabled({ + reason: 'createAllChatsChatFolder', + }); + } + } } setAppLoadingScreenMessage( diff --git a/ts/services/storage.ts b/ts/services/storage.ts index a92c1f57f8..cf67ab0540 100644 --- a/ts/services/storage.ts +++ b/ts/services/storage.ts @@ -88,11 +88,7 @@ import { isDone as isRegistrationDone } from '../util/registration.js'; import { callLinkRefreshJobQueue } from '../jobs/callLinkRefreshJobQueue.js'; import { isMockEnvironment } from '../environment.js'; import { validateConversation } from '../util/validateConversation.js'; -import { - ChatFolderType, - toCurrentChatFolders, - type ChatFolder, -} from '../types/ChatFolder.js'; +import { hasAllChatsChatFolder, type ChatFolder } from '../types/ChatFolder.js'; const { debounce, isNumber, chunk } = lodash; @@ -1663,20 +1659,9 @@ async function processManifest( }); }); - const chatFoldersHasAllChatsFolder = chatFolders.some(chatFolder => { - return ( - chatFolder.folderType === ChatFolderType.ALL && - chatFolder.deletedAtTimestampMs === 0 - ); - }); - - if (!chatFoldersHasAllChatsFolder) { + if (!hasAllChatsChatFolder(chatFolders)) { log.info(`process(${version}): creating all chats chat folder`); - await DataWriter.createAllChatsChatFolder(); - const currentChatFolders = await DataReader.getCurrentChatFolders(); - window.reduxActions.chatFolders.replaceAllChatFolderRecords( - toCurrentChatFolders(currentChatFolders) - ); + window.reduxActions.chatFolders.createAllChatsChatFolder(); } } @@ -2235,6 +2220,7 @@ async function upload({ } let storageServiceEnabled = false; +let storageServiceNeedsUploadAfterEnabled = false; export function enableStorageService(): void { if (storageServiceEnabled) { @@ -2243,6 +2229,12 @@ export function enableStorageService(): void { storageServiceEnabled = true; log.info('enableStorageService'); + + if (storageServiceNeedsUploadAfterEnabled) { + storageServiceUploadJob({ + reason: 'storageServiceNeedsUploadAfterEnabled', + }); + } } export function disableStorageService(reason: string): void { @@ -2351,6 +2343,18 @@ export async function reprocessUnknownFields(): Promise { ); } +export function storageServiceUploadJobAfterEnabled({ + reason, +}: { + reason: string; +}): void { + if (storageServiceEnabled) { + return storageServiceUploadJob({ reason }); + } + log.info(`storageServiceNeedsUploadAfterEnabled: ${reason}`); + storageServiceNeedsUploadAfterEnabled = true; +} + export const storageServiceUploadJob = debounce( ({ reason }: { reason: string }) => { if (!storageServiceEnabled) { diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 8e65e435e6..32acb4a8cc 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -923,6 +923,7 @@ type ReadableInterface = { getAllChatFolders: () => ReadonlyArray; getCurrentChatFolders: () => ReadonlyArray; getChatFolder: (id: ChatFolderId) => ChatFolder | null; + hasAllChatsChatFolder: () => boolean; getOldestDeletedChatFolder: () => ChatFolder | null; getMessagesNeedingUpgrade: ( diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 43fa91a3ce..489f620064 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -240,6 +240,7 @@ import { getCurrentChatFolders, getChatFolder, createChatFolder, + hasAllChatsChatFolder, createAllChatsChatFolder, updateChatFolder, markChatFolderDeleted, @@ -459,6 +460,7 @@ export const DataReader: ServerReadableInterface = { getAllChatFolders, getCurrentChatFolders, getChatFolder, + hasAllChatsChatFolder, getOldestDeletedChatFolder, callLinkExists, diff --git a/ts/sql/server/chatFolders.ts b/ts/sql/server/chatFolders.ts index b98609a53e..c939384fcf 100644 --- a/ts/sql/server/chatFolders.ts +++ b/ts/sql/server/chatFolders.ts @@ -146,6 +146,19 @@ export function createChatFolder(db: WritableDB, chatFolder: ChatFolder): void { })(); } +export function hasAllChatsChatFolder(db: ReadableDB): boolean { + const [query, params] = sql` + SELECT EXISTS ( + SELECT 1 FROM chatFolders + WHERE folderType IS ${ChatFolderType.ALL} + AND deletedAtTimestampMs IS 0 + LIMIT 1 + ) + `; + const result = db.prepare(query, { pluck: true }).get(params); + return result === 1; +} + export function createAllChatsChatFolder(db: WritableDB): ChatFolder { return db.transaction(() => { const allChatsChatFolder: ChatFolder = { diff --git a/ts/state/ducks/chatFolders.ts b/ts/state/ducks/chatFolders.ts index 4363c6d6cb..ed1152f78e 100644 --- a/ts/state/ducks/chatFolders.ts +++ b/ts/state/ducks/chatFolders.ts @@ -17,7 +17,7 @@ import { type CurrentChatFolders, } from '../../types/ChatFolder.js'; import { getCurrentChatFolders } from '../selectors/chatFolders.js'; -import { DataWriter } from '../../sql/Client.js'; +import { DataReader, DataWriter } from '../../sql/Client.js'; import { storageServiceUploadJob } from '../../services/storage.js'; import { parseStrict } from '../../util/schemas.js'; import { chatFolderCleanupService } from '../../services/expiring/chatFolderCleanupService.js'; @@ -142,6 +142,20 @@ function createChatFolder( }; } +function createAllChatsChatFolder(): ThunkAction< + void, + RootStateType, + unknown, + ChatFolderRecordReplaceAll +> { + return async dispatch => { + await DataWriter.createAllChatsChatFolder(); + storageServiceUploadJob({ reason: 'createAllChatsChatFolder' }); + const chatFolders = await DataReader.getCurrentChatFolders(); + dispatch(replaceAllChatFolderRecords(toCurrentChatFolders(chatFolders))); + }; +} + function updateChatFolder( chatFolderId: ChatFolderId, chatFolderParams: ChatFolderParams @@ -210,6 +224,7 @@ export const actions = { replaceChatFolderRecord, removeChatFolderRecord, createChatFolder, + createAllChatsChatFolder, updateChatFolder, deleteChatFolder, updateChatFoldersPositions, diff --git a/ts/test-mock/storage/chat_folder_test.ts b/ts/test-mock/storage/chat_folder_test.ts index f2cbe90a89..796c1644b2 100644 --- a/ts/test-mock/storage/chat_folder_test.ts +++ b/ts/test-mock/storage/chat_folder_test.ts @@ -3,10 +3,11 @@ import Long from 'long'; import { v4 as generateUuid } from 'uuid'; import { Proto, StorageState } from '@signalapp/mock-server'; +import type { Page } from 'playwright/test'; import { expect } from 'playwright/test'; import * as durations from '../../util/durations/index.js'; import type { App } from './fixtures.js'; -import { Bootstrap, debug } from './fixtures.js'; +import { Bootstrap, debug, getChatFolderRecordPredicate } from './fixtures.js'; import { uuidToBytes } from '../../util/uuidToBytes.js'; import { CHAT_FOLDER_DELETED_POSITION } from '../../types/ChatFolder.js'; @@ -48,10 +49,7 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { await bootstrap.teardown(); }); - it('should update from storage service', async () => { - const { phone } = bootstrap; - const window = await app.getWindow(); - + async function openChatFolderSettings(window: Page) { const openSettingsBtn = window.locator( '[data-testid="NavTabsItem--Settings"]' ); @@ -60,50 +58,42 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { '.Preferences__control:has-text("Add a chat folder")' ); - const ALL_CHATS_ID = generateUuid(); + await openSettingsBtn.click(); + await openChatsSettingsBtn.click(); + await openChatFoldersSettingsBtn.click(); + } + + it('should update from storage service', async () => { + const { phone } = bootstrap; + const window = await app.getWindow(); + + const ALL_CHATS_PREDICATE = getChatFolderRecordPredicate('ALL', '', false); + const ALL_GROUPS_ID = generateUuid(); const ALL_GROUPS_NAME = 'All Groups'; const ALL_GROUPS_NAME_UPDATED = 'The Groups'; - const allChatsListItem = window.getByTestId(`ChatFolder--${ALL_CHATS_ID}`); + const allChatsListItem = window + .getByTestId('ChatFoldersList') + .locator('.Preferences__ChatFolders__ChatSelection__Item') + .getByText('All chats'); + const allGroupsListItem = window.getByTestId( `ChatFolder--${ALL_GROUPS_ID}` ); - await openSettingsBtn.click(); - await openChatsSettingsBtn.click(); - await openChatFoldersSettingsBtn.click(); - - debug('adding ALL chat folder via storage service'); { let state = await phone.expectStorageState('initial state'); - - state = state.addRecord({ - type: IdentifierType.CHAT_FOLDER, - record: { - chatFolder: { - id: uuidToBytes(ALL_CHATS_ID), - folderType: Proto.ChatFolderRecord.FolderType.ALL, - name: null, - position: 0, - showOnlyUnread: false, - showMutedChats: false, - includeAllIndividualChats: true, - includeAllGroupChats: true, - includedRecipients: [], - excludedRecipients: [], - deletedAtTimestampMs: Long.fromNumber(0), - }, - }, - }); - - await phone.setStorageState(state); - await phone.sendFetchStorage({ timestamp: bootstrap.getTimestamp() }); - await app.waitForManifestVersion(state.version); - - await expect(allChatsListItem).toBeVisible(); + // wait for initial creation of story distribution list and "all chats" chat folder + state = await phone.waitForStorageState({ after: state }); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(true); } + await openChatFolderSettings(window); + + debug('expect all chats folder to be created'); + await expect(allChatsListItem).toBeVisible(); + debug('adding "All Groups" chat folder via storage service'); { let state = await phone.expectStorageState('adding all groups'); @@ -140,9 +130,7 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { let state = await phone.expectStorageState('updating all groups'); state = state.updateRecord( - item => { - return item.record.chatFolder?.name === ALL_GROUPS_NAME; - }, + getChatFolderRecordPredicate('CUSTOM', ALL_GROUPS_NAME, false), item => { return { ...item, @@ -168,9 +156,7 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { let state = await phone.expectStorageState('removing all groups'); state = state.updateRecord( - item => { - return item.record.chatFolder?.name === ALL_GROUPS_NAME_UPDATED; - }, + getChatFolderRecordPredicate('CUSTOM', ALL_GROUPS_NAME_UPDATED, false), item => { return { ...item, @@ -196,13 +182,12 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { const { phone } = bootstrap; const window = await app.getWindow(); - const openSettingsBtn = window.locator( - '[data-testid="NavTabsItem--Settings"]' - ); - const openChatsSettingsBtn = window.locator('.Preferences__button--chats'); - const openChatFoldersSettingsBtn = window.locator( - '.Preferences__control:has-text("Add a chat folder")' - ); + const ALL_CHATS_PREDICATE = getChatFolderRecordPredicate('ALL', '', false); + + const allChatsListItem = window + .getByTestId('ChatFoldersList') + .locator('.Preferences__ChatFolders__ChatSelection__Item') + .getByText('All chats'); const groupPresetBtn = window .getByTestId('ChatFolderPreset--GroupChats') @@ -228,27 +213,26 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { .locator('button:has-text("Delete")'); let state = await phone.expectStorageState('initial state'); - // wait for initial creation of story distribution list + // wait for initial creation of story distribution list and "all chats" chat folder state = await phone.waitForStorageState({ after: state }); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(true); + + await openChatFolderSettings(window); + + debug('expect all chats folder to be created'); + await expect(allChatsListItem).toBeVisible(); debug('creating group'); { - await openSettingsBtn.click(); - await openChatsSettingsBtn.click(); - await openChatFoldersSettingsBtn.click(); - await groupPresetBtn.click(); await expect(groupsFolderBtn).toBeVisible(); debug('waiting for storage sync'); state = await phone.waitForStorageState({ after: state }); - const found = state.hasRecord(item => { - return ( - item.type === IdentifierType.CHAT_FOLDER && - item.record.chatFolder?.name === 'Groups' - ); - }); + const found = state.hasRecord( + getChatFolderRecordPredicate('CUSTOM', 'Groups', false) + ); expect(found).toBe(true); } @@ -262,12 +246,9 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { debug('waiting for storage sync'); state = await phone.waitForStorageState({ after: state }); - const found = state.hasRecord(item => { - return ( - item.type === IdentifierType.CHAT_FOLDER && - item.record.chatFolder?.name === 'My Groups' - ); - }); + const found = state.hasRecord( + getChatFolderRecordPredicate('CUSTOM', 'My Groups', false) + ); expect(found).toBe(true); } @@ -281,12 +262,9 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { debug('waiting for storage sync'); state = await phone.waitForStorageState({ after: state }); - const found = state.findRecord(item => { - return ( - item.type === IdentifierType.CHAT_FOLDER && - item.record.chatFolder?.name === 'My Groups' - ); - }); + const found = state.findRecord( + getChatFolderRecordPredicate('CUSTOM', 'My Groups', true) + ); await expect(groupsFolderBtn).not.toBeAttached(); await expect(groupPresetBtn).toBeVisible(); @@ -296,4 +274,46 @@ describe('storage service/chat folders', function (this: Mocha.Suite) { ).toBeGreaterThan(0); } }); + + it('should recover from all chats folder being deleted', async () => { + const { phone } = bootstrap; + const window = await app.getWindow(); + + const ALL_CHATS_PREDICATE = getChatFolderRecordPredicate('ALL', '', false); + + let state = await phone.expectStorageState('initial state'); + expect(state.version).toBe(1); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(false); + + // wait for initial creation of story distribution list and "all chats" chat folder + state = await phone.waitForStorageState({ after: state }); + expect(state.version).toBe(2); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(true); + + await openChatFolderSettings(window); + + // update record + state = state.updateRecord(ALL_CHATS_PREDICATE, item => { + return { + ...item, + chatFolder: { + ...item.chatFolder, + position: CHAT_FOLDER_DELETED_POSITION, + deletedAtTimestampMs: Long.fromNumber(Date.now()), + }, + }; + }); + state = await phone.setStorageState(state); + expect(state.version).toBe(3); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(false); + + // sync from phone to app + await phone.sendFetchStorage({ timestamp: bootstrap.getTimestamp() }); + await app.waitForManifestVersion(state.version); + + // wait for app to insert a new "All chats" chat folder + state = await phone.waitForStorageState({ after: state }); + expect(state.version).toBe(4); + expect(state.hasRecord(ALL_CHATS_PREDICATE)).toBe(true); + }); }); diff --git a/ts/test-mock/storage/fixtures.ts b/ts/test-mock/storage/fixtures.ts index d0f428022c..c1723a627f 100644 --- a/ts/test-mock/storage/fixtures.ts +++ b/ts/test-mock/storage/fixtures.ts @@ -239,3 +239,26 @@ export function getCallLinkRecordPredicate( return roomId === recordRoomId; }; } + +export function getChatFolderRecordPredicate( + folderType: keyof typeof Proto.ChatFolderRecord.FolderType, + name: string, + deleted: boolean +): (record: StorageStateRecord) => boolean { + return ({ type, record }) => { + const { chatFolder } = record; + if (type !== IdentifierType.CHAT_FOLDER || chatFolder == null) { + return false; + } + + const deletedAtTimestampMs = + chatFolder.deletedAtTimestampMs?.toNumber() ?? 0; + const isDeleted = deletedAtTimestampMs > 0; + + return ( + chatFolder.folderType === Proto.ChatFolderRecord.FolderType[folderType] && + chatFolder.name === name && + isDeleted === deleted + ); + }; +} diff --git a/ts/types/ChatFolder.ts b/ts/types/ChatFolder.ts index 83e30c0995..2a197ccd23 100644 --- a/ts/types/ChatFolder.ts +++ b/ts/types/ChatFolder.ts @@ -271,3 +271,14 @@ export function lookupCurrentChatFolder( strictAssert(chatFolder != null, 'Missing chat folder'); return chatFolder; } + +export function hasAllChatsChatFolder( + chatFolders: ReadonlyArray +): boolean { + return chatFolders.some(chatFolder => { + return ( + chatFolder.folderType === ChatFolderType.ALL && + chatFolder.deletedAtTimestampMs === 0 + ); + }); +}