diff --git a/CONTEXT.md b/CONTEXT.md index 30d903b1115..25a741e22f8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -104,6 +104,23 @@ Independent boolean markers on a Message, orthogonal to its Status — a Message | **Room History** | Older Messages of a Room fetched on demand from the server (distinct from **Server History**) | Message history | | **Jump to Message** | Re-position the Room view onto a target Message that may be far from the Live Tail or not yet synced — fetches a surrounding Chunk | Scroll to message | +## Timestamp Trust Boundary + +Not every `_updatedAt` is worth the same. A Message's `_updatedAt` read out of a **server response** is server truth, and is the only legitimate source for the sync cursor. The same field read off of a **WatermelonDB row** is device-tainted: offline sends, Temp and Error sends, push-inserted rows, and `normalizeMessage`'s `_updatedAt || new Date()` fallback all stamp the device clock. + +Therefore the **Last Open** must be taken from the raw payload _before_ `normalizeMessage` / `buildMessage` runs — never from a database row, and never from `Date.now()`. + +| Term | Definition | Aliases to avoid | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------- | ---------------------- | +| **Last Open** | The fetch cursor for a Subscription (`lastOpen` column): the newest server `_updatedAt` actually received for that Room | last open, last update | +| **Last Seen** | The Subscription's read receipt (`ls`): the newest Message the user has read, which anchors the Unread Separator | last read | +| **Server Timestamp** | An `_updatedAt` taken from a server response — the only value the server can meaningfully compare a cursor against | Timestamp (ambiguous) | +| **Device Timestamp** | An `_updatedAt` present on a local row but written by the device clock; unusable as a cursor because the server never saw it | Timestamp (ambiguous) | + +A **Last Open** below a change's **Server Timestamp** only costs a re-fetch; one above it makes the server stay silent, and the change is never delivered. When in doubt, the lower cursor is the safe one. + +A **Last Open** and a **Last Seen** are not interchangeable — conflating them (one column serving as both fetch cursor and unread anchor) is what produced permanently invisible Messages. + ## Message Action & Position State Two distinct kinds of transient per-Room state drive how the Room view renders Messages. Keep them apart. @@ -243,6 +260,7 @@ A **Message Action** is the active mode on a Message in the Room view. The three - **"Forward"** in omnichannel context means **Transfer** (reassigning a room to another agent/department). The codebase uses both `forwardRoom` and "transfer" — prefer **Transfer** as the domain term. - **"History"** is overloaded: **Server History** is the recent-Servers reconnection list; **Room History** is older Messages fetched on demand. The action `roomHistoryRequest` and saga `ROOM.HISTORY_REQUEST` refer to **Room History**. - **"Window"** is used metaphorically in the Subscriptions dialogue ("a Subscription is the user's window into it"); a **Message Window** is the concrete observed Message range in the Room view. Disambiguate when both could be meant. +- **"`lastOpen`"** names a database column, not a concept: it stores the **Last Open**, a server-clock fetch cursor. It has never meant "when the user last opened the room". The Unread Separator anchor is **Last Seen** (`ls`). Do not read `lastOpen` as a read receipt or write a device clock into it. - **"Load more"** is directional: older Messages are an **Older Loader** (`MORE`/`PREVIOUS_CHUNK`), newer Messages are a **Newer Loader** (`NEXT_CHUNK`). Avoid bare "load more". - **"System message" vs "Info message"** — **System Message** is the umbrella (any `t`-bearing server Message); **Info Message** is the narrower set of room-event System Messages. The typed events `e2e`, `discussion-created`, `jitsi_call_started`, and `videoconf` are System Messages but NOT Info Messages — each gets its own rendering branch. - **"Thread reply"** is overloaded. The glossary's **Thread Message** is the data concept (any Message with `tmid`); the code's `isThreadReply` is a _rendering position_ — the first Thread Message in a run shown in the parent Room, which gets the "in reply to" header. Do not use "thread reply" for the data concept. diff --git a/app/containers/MessageActions/index.tsx b/app/containers/MessageActions/index.tsx index ef5c79075ff..d611d9d91b2 100644 --- a/app/containers/MessageActions/index.tsx +++ b/app/containers/MessageActions/index.tsx @@ -249,7 +249,7 @@ const MessageActions = memo( await db.write(async () => { try { - await subRecord.update(sub => (sub.lastOpen = ts as Date)); // TODO: reevaluate IMessage + await subRecord.update(sub => (sub.ls = ts as Date)); } catch { // do nothing } diff --git a/app/lib/methods/loadMessagesForRoom.test.ts b/app/lib/methods/loadMessagesForRoom.test.ts index 2b8cd7e7882..5442776fe3a 100644 --- a/app/lib/methods/loadMessagesForRoom.test.ts +++ b/app/lib/methods/loadMessagesForRoom.test.ts @@ -5,6 +5,7 @@ import { getMessageById } from '../database/services/Message'; import { getSubscriptionByRoomId } from '../database/services/Subscription'; import updateMessages from './updateMessages'; import { store } from '../store/auxStore'; +import { updateLastOpen } from './updateLastOpen'; jest.mock('../services/sdk', () => ({ __esModule: true, @@ -31,6 +32,10 @@ jest.mock('../store/auxStore', () => ({ })); jest.mock('./updateMessages', () => jest.fn()); +jest.mock('./updateLastOpen', () => ({ + ...jest.requireActual('./updateLastOpen'), + updateLastOpen: jest.fn() +})); const mockedSdkGet = sdk.get as jest.MockedFunction; const mockedGetMessageById = getMessageById as jest.MockedFunction; @@ -232,4 +237,76 @@ describe('loadMessagesForRoom', () => { expect(mockedDispatch).not.toHaveBeenCalledWith(expect.objectContaining({ type: ROOM.HISTORY_UI_LOADER_PUSH })); expect(mockedDispatch).not.toHaveBeenCalledWith(expect.objectContaining({ type: ROOM.HISTORY_UI_LOADER_POP })); }); + + describe('last open', () => { + const mockedUpdateLastOpen = updateLastOpen as jest.MockedFunction; + + const buildStampedBatch = (prefix: string, hour: number, length: number) => + Array.from( + { length }, + (_, index) => + ({ + _id: `${prefix}-${index + 1}`, + rid: 'ROOM_ID', + ts: new Date(Date.UTC(2024, 0, 1, hour, 0, length - index)).toISOString(), + _updatedAt: new Date(Date.UTC(2024, 0, 1, hour, 0, length - index)).toISOString(), + t: 'uj' + } as any) + ); + + it('writes the Last Open from every fetched batch on the initial tail load', async () => { + const firstBatch = buildStampedBatch('first', 11, 50); + const secondBatch = buildStampedBatch('second', 10, 50); + + mockedSdkGet + .mockResolvedValueOnce({ success: true, messages: firstBatch } as any) + .mockResolvedValueOnce({ success: true, messages: secondBatch } as any); + + await loadMessagesForRoom({ rid: 'ROOM_ID', t: 'c' }); + + expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); + // Every batch's server stamps contribute to the cursor, not just the newest page. + expect(mockedUpdateLastOpen).toHaveBeenCalledWith( + 'ROOM_ID', + expect.arrayContaining([ + ...firstBatch.map(message => ({ _updatedAt: message._updatedAt })), + ...secondBatch.map(message => ({ _updatedAt: message._updatedAt })) + ]) + ); + }); + + it('uses the highest _updatedAt even when it arrives in an older batch', async () => { + const firstBatch = buildStampedBatch('first', 11, 50); + const secondBatch = buildStampedBatch('second', 10, 50).map(message => ({ + ...message, + _updatedAt: new Date(Date.UTC(2024, 0, 1, 12, 0, 0)).toISOString() + })); + + mockedSdkGet + .mockResolvedValueOnce({ success: true, messages: firstBatch } as any) + .mockResolvedValueOnce({ success: true, messages: secondBatch } as any); + + await loadMessagesForRoom({ rid: 'ROOM_ID', t: 'c' }); + + const received = mockedUpdateLastOpen.mock.calls[0][1]; + const timestamps = received.map(m => new Date(m._updatedAt as string | Date).getTime()).filter(t => !Number.isNaN(t)); + expect(new Date(Math.max(...timestamps))).toEqual(new Date(Date.UTC(2024, 0, 1, 12, 0, 0))); + }); + + it('does not write when loading an older page (latest)', async () => { + mockedSdkGet.mockResolvedValueOnce({ success: true, messages: buildStampedBatch('older', 9, 10) } as any); + + await loadMessagesForRoom({ rid: 'ROOM_ID', t: 'c', latest: new Date(Date.UTC(2024, 0, 1, 10, 0, 0)) }); + + expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); + }); + + it('does not write when filling a gap (loaderItem)', async () => { + mockedSdkGet.mockResolvedValueOnce({ success: true, messages: buildStampedBatch('gap', 9, 10) } as any); + + await loadMessagesForRoom({ rid: 'ROOM_ID', t: 'c', loaderItem: { id: 'tapped-load-more' } as any }); + + expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); + }); + }); }); diff --git a/app/lib/methods/loadMessagesForRoom.ts b/app/lib/methods/loadMessagesForRoom.ts index ce3d213c978..edb7152d914 100644 --- a/app/lib/methods/loadMessagesForRoom.ts +++ b/app/lib/methods/loadMessagesForRoom.ts @@ -10,6 +10,7 @@ import sdk from '../services/sdk'; import { store } from '../store/auxStore'; import updateMessages from './updateMessages'; import { generateLoadMoreId } from './helpers/generateLoadMoreId'; +import { snapshotServerTimestamps, type TServerTimestamps, updateLastOpen } from './updateLastOpen'; const COUNT = 50; const MAX_BATCHES = 10; @@ -32,15 +33,16 @@ async function load(args: { t: RoomTypes; loaderItem?: TMessageModel; onUiLoaderPushed?: (loaderId: string) => void; -}): Promise<{ messages: IMessage[]; lastBatchWasFull: boolean }> { +}): Promise<{ messages: IMessage[]; serverTimestamps: TServerTimestamps; lastBatchWasFull: boolean }> { const roomId = args.rid; const hideSystemMessages = await resolveHideSystemMessages(roomId); const apiType = roomTypeToApiType(args.t); if (!apiType) { - return { messages: [], lastBatchWasFull: false }; + return { messages: [], serverTimestamps: [], lastBatchWasFull: false }; } const allMessages: IMessage[] = []; + const serverTimestamps: TServerTimestamps = []; let visibleMainMessagesCount = 0; let batchesFetched = 0; let lastBatchWasFull = false; @@ -73,6 +75,7 @@ async function load(args: { } const batch = data.messages as IMessage[]; + serverTimestamps.push(...snapshotServerTimestamps(batch)); allMessages.push(...batch); lastBatchWasFull = batch.length === COUNT; @@ -108,7 +111,7 @@ async function load(args: { const startTimestamp = args.latest ? new Date(args.latest).toISOString() : undefined; await fetchBatch(startTimestamp); - return { messages: allMessages, lastBatchWasFull }; + return { messages: allMessages, serverTimestamps, lastBatchWasFull }; } export async function loadMessagesForRoom(args: { @@ -119,7 +122,7 @@ export async function loadMessagesForRoom(args: { }): Promise { let uiLoaderId: string | null = null; try { - const { messages, lastBatchWasFull } = await load({ + const { messages, serverTimestamps, lastBatchWasFull } = await load({ ...args, onUiLoaderPushed: id => { uiLoaderId = id; @@ -139,6 +142,10 @@ export async function loadMessagesForRoom(args: { } await updateMessages({ rid: args.rid, update: messages, loaderItem: args.loaderItem }); } + + if (!args.latest && !args.loaderItem) { + await updateLastOpen(args.rid, serverTimestamps); + } } catch (e) { log(e); throw e; diff --git a/app/lib/methods/loadMissedMessages.test.ts b/app/lib/methods/loadMissedMessages.test.ts new file mode 100644 index 00000000000..8a9b9e9a9f9 --- /dev/null +++ b/app/lib/methods/loadMissedMessages.test.ts @@ -0,0 +1,188 @@ +import { loadMissedMessages } from './loadMissedMessages'; +import sdk from '../services/sdk'; +import updateMessages from './updateMessages'; +import { getSubscriptionByRoomId } from '../database/services/Subscription'; +import { updateLastOpen } from './updateLastOpen'; +import { store } from '../store/auxStore'; + +jest.mock('../services/sdk', () => ({ + __esModule: true, + default: { + get: jest.fn() + } +})); + +jest.mock('../database/services/Subscription', () => ({ + getSubscriptionByRoomId: jest.fn() +})); + +jest.mock('../store/auxStore', () => ({ + store: { + getState: jest.fn(() => ({ server: { version: '7.4.0' } })), + dispatch: jest.fn() + } +})); + +jest.mock('./updateMessages', () => jest.fn()); +jest.mock('./updateLastOpen', () => ({ + ...jest.requireActual('./updateLastOpen'), + updateLastOpen: jest.fn() +})); +jest.mock('./helpers/log', () => ({ __esModule: true, default: jest.fn() })); + +const mockedSdkGet = sdk.get as jest.MockedFunction; +const mockedUpdateMessages = updateMessages as jest.MockedFunction; +const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; +const mockedUpdateLastOpen = updateLastOpen as jest.MockedFunction; + +const RID = 'ROOM_ID'; + +describe('loadMissedMessages', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedUpdateMessages.mockResolvedValue(0); + mockedGetSubscriptionByRoomId.mockResolvedValue(null as never); + (store.getState as jest.Mock).mockReturnValue({ server: { version: '7.4.0' } }); + }); + + it('routes a deleted-only recursion payload to remove, not update', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); + const deletedMessage = { _id: 'deleted-1', rid: RID, _updatedAt: new Date(Date.UTC(2024, 0, 1, 12, 0, 0)) }; + mockedSdkGet.mockResolvedValue({ + result: { updated: [], deleted: [deletedMessage], cursor: { next: null } } + } as never); + + await loadMissedMessages({ rid: RID, deletedNext: 1704110400000 }); + + expect(mockedSdkGet).toHaveBeenCalledTimes(1); + expect(mockedSdkGet).toHaveBeenCalledWith('chat.syncMessages', expect.objectContaining({ roomId: RID, type: 'DELETED' })); + expect(mockedUpdateMessages).toHaveBeenCalledWith( + expect.objectContaining({ + rid: RID, + update: [], + remove: [deletedMessage] + }) + ); + }); + + it('fetches nothing when the subscription has no cursor', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); + + await loadMissedMessages({ rid: RID }); + + expect(mockedSdkGet).not.toHaveBeenCalled(); + }); + + describe('last open', () => { + const CURSOR = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); + const flush = () => new Promise(resolve => setImmediate(resolve)); + + const message = (id: string, updatedAt: string) => ({ _id: id, rid: RID, _updatedAt: updatedAt }); + + beforeEach(() => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + }); + + it('writes the Last Open from the updated payload once the cursor has drained', async () => { + mockedSdkGet.mockResolvedValue({ + result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [], cursor: { next: null } } + } as never); + + await loadMissedMessages({ rid: RID }); + + expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); + expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [{ _updatedAt: '2024-01-01T11:30:00.000Z' }]); + }); + + it('does not write mid-pagination, only after the final page of a paginated run', async () => { + const PAGE_2 = Date.UTC(2024, 0, 1, 11, 30, 0); + mockedSdkGet.mockImplementation(((_endpoint: string, params: { type?: string; next?: number }) => { + if (params.type === 'DELETED') { + return Promise.resolve({ result: { deleted: [], cursor: { next: null } } }); + } + if (params.next === PAGE_2) { + return Promise.resolve({ + result: { updated: [message('b', '2024-01-01T11:45:00.000Z')], deleted: [], cursor: { next: null } } + }); + } + return Promise.resolve({ + result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [], cursor: { next: PAGE_2 } } + }); + }) as never); + + await loadMissedMessages({ rid: RID }); + + // First page still has a next cursor, so nothing may be persisted yet. + expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); + + await flush(); + + expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); + // Every page walked contributes its stamps, not only the last one. + expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [ + { _updatedAt: '2024-01-01T11:30:00.000Z' }, + { _updatedAt: '2024-01-01T11:45:00.000Z' } + ]); + }); + + it('keeps the highest _updatedAt when it arrives on an earlier page', async () => { + const PAGE_2 = Date.UTC(2024, 0, 1, 11, 30, 0); + mockedSdkGet.mockImplementation(((_endpoint: string, params: { type?: string; next?: number }) => { + if (params.type === 'DELETED') { + return Promise.resolve({ result: { deleted: [], cursor: { next: null } } }); + } + if (params.next === PAGE_2) { + return Promise.resolve({ + result: { updated: [message('b', '2024-01-01T11:10:00.000Z')], deleted: [], cursor: { next: null } } + }); + } + return Promise.resolve({ + result: { updated: [message('a', '2024-01-01T11:59:00.000Z')], deleted: [], cursor: { next: PAGE_2 } } + }); + }) as never); + + await loadMissedMessages({ rid: RID }); + await flush(); + + const received = mockedUpdateLastOpen.mock.calls[0][1]; + const timestamps = received.map(m => new Date(m._updatedAt as string | Date).getTime()).filter(t => !Number.isNaN(t)); + expect(new Date(Math.max(...timestamps))).toEqual(new Date('2024-01-01T11:59:00.000Z')); + }); + + it('does not write again on a deleted-only continuation page', async () => { + mockedSdkGet.mockResolvedValue({ + result: { updated: [], deleted: [message('gone', '2024-01-01T11:30:00.000Z')], cursor: { next: null } } + } as never); + + await loadMissedMessages({ rid: RID, deletedNext: Date.UTC(2024, 0, 1, 11, 30, 0) }); + + expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); + }); + + it('writes nothing derived from deleted rows when the payload is deleted-only', async () => { + mockedSdkGet.mockResolvedValue({ + result: { updated: [], deleted: [message('gone', '2024-01-01T11:30:00.000Z')], cursor: { next: null } } + } as never); + + await loadMissedMessages({ rid: RID }); + + expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, []); + }); + + it('writes once on the legacy unpaginated server branch', async () => { + (store.getState as jest.Mock).mockReturnValue({ server: { version: '7.0.0' } }); + mockedSdkGet.mockResolvedValue({ + result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [] } + } as never); + + await loadMissedMessages({ rid: RID }); + + expect(mockedSdkGet).toHaveBeenCalledWith( + 'chat.syncMessages', + expect.objectContaining({ lastUpdate: CURSOR.toISOString() }) + ); + expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); + expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [{ _updatedAt: '2024-01-01T11:30:00.000Z' }]); + }); + }); +}); diff --git a/app/lib/methods/loadMissedMessages.ts b/app/lib/methods/loadMissedMessages.ts index ba2e6c23458..be285d1eabf 100644 --- a/app/lib/methods/loadMissedMessages.ts +++ b/app/lib/methods/loadMissedMessages.ts @@ -4,6 +4,8 @@ import updateMessages from './updateMessages'; import sdk from '../services/sdk'; import { store } from '../store/auxStore'; import { getSubscriptionByRoomId } from '../database/services/Subscription'; +import log from './helpers/log'; +import { snapshotServerTimestamps, type TServerTimestamps, updateLastOpen } from './updateLastOpen'; const count = 50; @@ -15,24 +17,25 @@ const syncMessages = async ({ roomId, next, type }: { roomId: string; next: numb const getSyncMessagesFromCursor = async ( roomId: string, - lastOpen?: number, + cursor?: number, updatedNext?: number | null, deletedNext?: number | null ) => { - const promises = []; + let updatedPromise; + let deletedPromise; - if (lastOpen && !updatedNext && !deletedNext) { - promises.push(syncMessages({ roomId, next: lastOpen, type: 'UPDATED' })); - promises.push(syncMessages({ roomId, next: lastOpen, type: 'DELETED' })); + if (cursor && !updatedNext && !deletedNext) { + updatedPromise = syncMessages({ roomId, next: cursor, type: 'UPDATED' }); + deletedPromise = syncMessages({ roomId, next: cursor, type: 'DELETED' }); } if (updatedNext) { - promises.push(syncMessages({ roomId, next: updatedNext, type: 'UPDATED' })); + updatedPromise = syncMessages({ roomId, next: updatedNext, type: 'UPDATED' }); } if (deletedNext) { - promises.push(syncMessages({ roomId, next: deletedNext, type: 'DELETED' })); + deletedPromise = syncMessages({ roomId, next: deletedNext, type: 'DELETED' }); } - const [updatedMessages, deletedMessages] = await Promise.all(promises); + const [updatedMessages, deletedMessages] = await Promise.all([updatedPromise, deletedPromise]); return { deleted: deletedMessages?.deleted ?? [], deletedNext: deletedMessages?.cursor.next, @@ -41,60 +44,43 @@ const getSyncMessagesFromCursor = async ( }; }; -const getLastUpdate = async (rid: string) => { - const sub = await getSubscriptionByRoomId(rid); - if (!sub) { - return null; - } - return sub.lastOpen; -}; - async function load({ rid: roomId, - lastOpen, updatedNext, deletedNext }: { rid: string; - lastOpen?: Date; updatedNext?: number | null; deletedNext?: number | null; }) { + const sub = await getSubscriptionByRoomId(roomId); + if (!sub) { + return; + } + const cursor = sub.lastOpen; + const { version: serverVersion } = store.getState().server; if (compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.1.0')) { - let lastOpenTimestamp; - if (lastOpen) { - lastOpenTimestamp = new Date(lastOpen).getTime(); - } else { - const lastUpdate = await getLastUpdate(roomId); - lastOpenTimestamp = lastUpdate?.getTime(); - } - const result = await getSyncMessagesFromCursor(roomId, lastOpenTimestamp, updatedNext, deletedNext); + const result = await getSyncMessagesFromCursor(roomId, cursor?.getTime(), updatedNext, deletedNext); return result; } - let lastOpenISOString; - if (lastOpen) { - lastOpenISOString = new Date(lastOpen).toISOString(); - } else { - const lastUpdate = await getLastUpdate(roomId); - lastOpenISOString = lastUpdate?.toISOString(); - } // RC 0.60.0 // @ts-ignore // this method dont have type - const { result } = await sdk.get('chat.syncMessages', { roomId, lastUpdate: lastOpenISOString }); + const { result } = await sdk.get('chat.syncMessages', { roomId, lastUpdate: cursor?.toISOString() }); return result; } export async function loadMissedMessages(args: { rid: string; - lastOpen?: Date; updatedNext?: number | null; deletedNext?: number | null; + serverTimestamps?: TServerTimestamps; }): Promise { + // A DELETED-only continuation fetches no UPDATED page, so it must not write the cursor again. + const fetchedUpdatedPage = !!args.updatedNext || !args.deletedNext; const data = await load({ rid: args.rid, - lastOpen: args.lastOpen, updatedNext: args.updatedNext, deletedNext: args.deletedNext }); @@ -105,16 +91,27 @@ export async function loadMissedMessages(args: { deleted, deletedNext }: { updated: ILastMessage[]; deleted: ILastMessage[]; updatedNext: number | null; deletedNext: number | null } = data; + + const serverTimestamps = [...(args.serverTimestamps ?? []), ...snapshotServerTimestamps(updated)]; + // @ts-ignore // TODO: remove loaderItem obligatoriness await updateMessages({ rid: args.rid, update: updated, remove: deleted }); if (deletedNext || updatedNext) { loadMissedMessages({ rid: args.rid, - lastOpen: args.lastOpen, updatedNext, - deletedNext - }); + deletedNext, + serverTimestamps + }).catch(log); + } + + // Only once the UPDATED cursor has drained, from the stamps of every page walked: the + // pages descend from the newest, so the last one alone would lower the cursor. Advancing + // mid-pagination would skip pages not yet fetched; `deleted` is never a source, its rows + // carry no new history. + if (fetchedUpdatedPage && !updatedNext) { + await updateLastOpen(args.rid, serverTimestamps); } } } diff --git a/app/lib/methods/readMessages.ts b/app/lib/methods/readMessages.ts index 790cafbe505..b593e9341ae 100644 --- a/app/lib/methods/readMessages.ts +++ b/app/lib/methods/readMessages.ts @@ -5,7 +5,7 @@ import sdk from '../services/sdk'; import { hasE2EEWarning } from '../encryption/utils'; import { store } from '../store/auxStore'; -export async function readMessages(rid: string, ls: Date, updateLastOpen = false): Promise { +export async function readMessages(rid: string): Promise { try { const db = database.active; let subscription; @@ -44,10 +44,6 @@ export async function readMessages(rid: string, ls: Date, updateLastOpen = false s.unread = 0; s.userMentions = 0; s.groupMentions = 0; - s.ls = ls; - if (updateLastOpen) { - s.lastOpen = ls; - } }); } catch (e) { // Do nothing diff --git a/app/lib/methods/roomTypeToApiType.ts b/app/lib/methods/roomTypeToApiType.ts index dc081fdd519..b3fd9754c75 100644 --- a/app/lib/methods/roomTypeToApiType.ts +++ b/app/lib/methods/roomTypeToApiType.ts @@ -24,3 +24,5 @@ export const types: { [K in RoomTypes]: ApiTypes } = { }; export const roomTypeToApiType = (t: T) => types[t]; + +export const isRoomType = (t: unknown): t is RoomTypes => typeof t === 'string' && t in types; diff --git a/app/lib/methods/subscriptions/room.resumeSync.test.ts b/app/lib/methods/subscriptions/room.resumeSync.test.ts new file mode 100644 index 00000000000..6d5004403f5 --- /dev/null +++ b/app/lib/methods/subscriptions/room.resumeSync.test.ts @@ -0,0 +1,101 @@ +import RoomSubscription from './room'; +import sdk from '../../services/sdk'; +import updateMessages from '../updateMessages'; +import { getSubscriptionByRoomId } from '../../database/services/Subscription'; + +jest.mock('../../services/sdk', () => ({ + __esModule: true, + default: { + get: jest.fn() + } +})); + +jest.mock('../../database', () => ({ + __esModule: true, + default: { active: { get: jest.fn(), write: jest.fn() } } +})); + +jest.mock('../../database/services/Subscription', () => ({ + getSubscriptionByRoomId: jest.fn() +})); + +jest.mock('../../database/services/Message', () => ({ + getMessageById: jest.fn(() => Promise.resolve(null)) +})); + +jest.mock('../../store/auxStore', () => ({ + store: { + getState: jest.fn(() => ({ server: { version: '7.4.0' }, settings: {}, login: { user: {} }, room: {} })), + dispatch: jest.fn() + } +})); + +jest.mock('../updateMessages', () => jest.fn()); +jest.mock('../readMessages', () => ({ readMessages: jest.fn() })); +jest.mock('../../encryption', () => ({ Encryption: { decryptMessage: jest.fn(m => m) } })); + +const mockedSdkGet = sdk.get as jest.MockedFunction; +const mockedUpdateMessages = updateMessages as jest.MockedFunction; +const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; + +const RID = 'ROOM_ID'; + +const missedMessage = { + _id: 'missed-1', + rid: RID, + msg: 'sent while the app was backgrounded', + ts: new Date(Date.UTC(2024, 0, 1, 12, 0, 0)).toISOString(), + u: { _id: 'user2', username: 'user2' } +}; + +const syncMessagesResponse = ( + updated: unknown[] +): { result: { updated: unknown[]; deleted: unknown[]; cursor: { next: number | null } } } => ({ + result: { updated, deleted: [], cursor: { next: null } } +}); + +describe('RoomSubscription resume sync', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedUpdateMessages.mockResolvedValue(0); + mockedSdkGet.mockResolvedValue(syncMessagesResponse([missedMessage]) as never); + }); + + it('fetches and persists messages missed while backgrounded when the room has a sync cursor', async () => { + const persistedCursor = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: persistedCursor } as never); + + await new RoomSubscription(RID).handleConnection(); + + expect(mockedSdkGet).toHaveBeenCalledWith( + 'chat.syncMessages', + expect.objectContaining({ roomId: RID, type: 'UPDATED', next: persistedCursor.getTime() }) + ); + expect(mockedUpdateMessages).toHaveBeenCalledWith( + expect.objectContaining({ + rid: RID, + update: expect.arrayContaining([expect.objectContaining({ _id: 'missed-1' })]) + }) + ); + }); + + it('fetches nothing for a room without a sync cursor (null lastOpen): RoomView owns the initial load', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'c' } as never); + + await new RoomSubscription(RID).handleConnection(); + + expect(mockedSdkGet).not.toHaveBeenCalled(); + }); + + it('writes nothing to the subscription when the room is closed', async () => { + const subscriptionUpdate = jest.fn(); + mockedGetSubscriptionByRoomId.mockResolvedValue({ + lastOpen: new Date(Date.UTC(2024, 0, 1, 11, 0, 0)), + update: subscriptionUpdate + } as never); + + await new RoomSubscription(RID).unsubscribe(); + + expect(subscriptionUpdate).not.toHaveBeenCalled(); + }); +}); diff --git a/app/lib/methods/subscriptions/room.ts b/app/lib/methods/subscriptions/room.ts index 1fdf573e7d7..acfe1c74029 100644 --- a/app/lib/methods/subscriptions/room.ts +++ b/app/lib/methods/subscriptions/room.ts @@ -27,7 +27,6 @@ import { type IDDPMessage } from '../../../definitions/IDDPMessage'; import sdk from '../../services/sdk'; import { readMessages } from '../readMessages'; import { loadMissedMessages } from '../loadMissedMessages'; -import { updateLastOpen } from '../updateLastOpen'; import markMessagesRead from '../helpers/markMessagesRead'; export default class RoomSubscription { @@ -64,7 +63,6 @@ export default class RoomSubscription { unsubscribe = async () => { console.log(`[RCRN] Unsubscribing from room ${this.rid}`); - updateLastOpen(this.rid); this.isAlive = false; reduxStore.dispatch(unsubscribeRoom(this.rid)); if (this.promises) { @@ -239,7 +237,7 @@ export default class RoomSubscription { }); read = debounce(() => { - readMessages(this.rid, new Date()); + readMessages(this.rid); }, 300); updateMessage = async (message: IMessage): Promise => { diff --git a/app/lib/methods/subscriptions/roomCloseCursor.test.ts b/app/lib/methods/subscriptions/roomCloseCursor.test.ts new file mode 100644 index 00000000000..62e3892c8fa --- /dev/null +++ b/app/lib/methods/subscriptions/roomCloseCursor.test.ts @@ -0,0 +1,127 @@ +import RoomSubscription from './room'; +import sdk from '../../services/sdk'; +import updateMessages from '../updateMessages'; +import { getSubscriptionByRoomId } from '../../database/services/Subscription'; + +jest.mock('../../services/sdk', () => ({ + __esModule: true, + default: { + get: jest.fn() + } +})); + +jest.mock('../../database', () => ({ + __esModule: true, + default: { active: { get: jest.fn(), write: jest.fn((cb: () => Promise) => cb()) } } +})); + +jest.mock('../../database/services/Subscription', () => ({ + getSubscriptionByRoomId: jest.fn() +})); + +jest.mock('../../database/services/Message', () => ({ + getMessageById: jest.fn(() => Promise.resolve(null)) +})); + +jest.mock('../../store/auxStore', () => ({ + store: { + getState: jest.fn(() => ({ server: { version: '7.4.0' }, settings: {}, login: { user: {} }, room: {} })), + dispatch: jest.fn() + } +})); + +jest.mock('../updateMessages', () => jest.fn()); +jest.mock('../readMessages', () => ({ readMessages: jest.fn() })); +jest.mock('../loadMessagesForRoom', () => ({ loadMessagesForRoom: jest.fn() })); +jest.mock('../../encryption', () => ({ Encryption: { decryptMessage: jest.fn(m => m) } })); + +const mockedSdkGet = sdk.get as jest.MockedFunction; +const mockedUpdateMessages = updateMessages as jest.MockedFunction; +const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; + +const RID = 'ROOM_ID'; + +/** Server clock. The cursor the client legitimately reached by actually fetching. */ +const FETCHED_UP_TO = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); +/** Server clock. Message written while the client was offline — never delivered to the device. */ +const MISSED_SERVER_UPDATED_AT = new Date(Date.UTC(2024, 0, 1, 11, 30, 0)); +/** Device clock while the room is closed: ahead of the newest message the client actually holds. */ +const DEVICE_NOW = new Date(Date.UTC(2024, 0, 1, 12, 0, 0)); + +const missedMessage = { + _id: 'missed-1', + rid: RID, + msg: 'sent while the device was offline', + ts: MISSED_SERVER_UPDATED_AT.toISOString(), + _updatedAt: MISSED_SERVER_UPDATED_AT.toISOString(), + u: { _id: 'user2', username: 'user2' } +}; + +/** Stands in for the persisted subscription row; `lastOpen` is the sync cursor. */ +const makeSubscription = (lastOpen: Date | null) => { + const subscription = { + t: 'c', + lastOpen, + update: (updater: (s: { lastOpen: Date | null }) => void) => { + updater(subscription); + return Promise.resolve(); + } + }; + return subscription; +}; + +/** Server behaviour of chat.syncMessages: only returns messages at or after the requested cursor. */ +const respondFromServer = () => + mockedSdkGet.mockImplementation(((_endpoint: string, params: { next?: number; type?: string }) => + Promise.resolve({ + result: { + updated: + params.type === 'UPDATED' && typeof params.next === 'number' && MISSED_SERVER_UPDATED_AT.getTime() >= params.next + ? [missedMessage] + : [], + deleted: [], + cursor: { next: null } + } + })) as never); + +describe('closing a room while offline must not advance the sync cursor', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'] }); + jest.setSystemTime(DEVICE_NOW); + mockedUpdateMessages.mockResolvedValue(0); + respondFromServer(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('leaves the cursor at what was actually fetched when the room is closed without fetching', async () => { + const subscription = makeSubscription(FETCHED_UP_TO); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await new RoomSubscription(RID).unsubscribe(); + await Promise.resolve(); + + expect(mockedSdkGet).not.toHaveBeenCalled(); + expect(subscription.lastOpen).toEqual(FETCHED_UP_TO); + }); + + it('still delivers a message written while offline after the room was closed and reopened', async () => { + const subscription = makeSubscription(FETCHED_UP_TO); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await new RoomSubscription(RID).unsubscribe(); + await Promise.resolve(); + + await new RoomSubscription(RID).handleConnection(); + + expect(mockedUpdateMessages).toHaveBeenCalledWith( + expect.objectContaining({ + rid: RID, + update: expect.arrayContaining([expect.objectContaining({ _id: 'missed-1' })]) + }) + ); + }); +}); diff --git a/app/lib/methods/updateLastOpen.test.ts b/app/lib/methods/updateLastOpen.test.ts new file mode 100644 index 00000000000..f05d7870c95 --- /dev/null +++ b/app/lib/methods/updateLastOpen.test.ts @@ -0,0 +1,105 @@ +import { updateLastOpen } from './updateLastOpen'; +import { getSubscriptionByRoomId } from '../database/services/Subscription'; + +jest.mock('../database', () => ({ + __esModule: true, + default: { active: { write: jest.fn((cb: () => Promise) => cb()) } } +})); + +jest.mock('../database/services/Subscription', () => ({ + getSubscriptionByRoomId: jest.fn() +})); + +jest.mock('./helpers/log', () => ({ __esModule: true, default: jest.fn() })); + +const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; + +const RID = 'ROOM_ID'; + +const makeSubscription = (lastOpen: Date | null) => { + const subscription = { + lastOpen, + update: (updater: (s: { lastOpen: Date | null }) => void) => { + updater(subscription); + return Promise.resolve(); + } + }; + return subscription; +}; + +describe('updateLastOpen', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('persists the newest server _updatedAt in the payload', async () => { + const subscription = makeSubscription(null); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [ + { _updatedAt: '2024-01-01T10:00:00.000Z' }, + { _updatedAt: '2024-01-01T12:00:00.000Z' }, + { _updatedAt: '2024-01-01T11:00:00.000Z' } + ]); + + expect(subscription.lastOpen).toEqual(new Date('2024-01-01T12:00:00.000Z')); + }); + + it('ignores entries with no _updatedAt', async () => { + const subscription = makeSubscription(null); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [{ _updatedAt: '2024-01-01T10:00:00.000Z' }, {}, {}]); + + expect(subscription.lastOpen).toEqual(new Date('2024-01-01T10:00:00.000Z')); + }); + + it('ignores malformed _updatedAt values', async () => { + const subscription = makeSubscription(null); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [{ _updatedAt: 'not-a-date' }, { _updatedAt: '2024-01-01T12:00:00.000Z' }, { _updatedAt: '' }]); + + expect(subscription.lastOpen).toEqual(new Date('2024-01-01T12:00:00.000Z')); + }); + + it('does not write when every _updatedAt is null', async () => { + const subscription = makeSubscription(null); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [{ _updatedAt: null }, { _updatedAt: null }]); + + expect(subscription.lastOpen).toBeNull(); + }); + + it('does not write when every _updatedAt is invalid', async () => { + const subscription = makeSubscription(null); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [{ _updatedAt: 'not-a-date' }, { _updatedAt: '' }, {}]); + + expect(subscription.lastOpen).toBeNull(); + }); + + it('is a no-op on an empty payload', async () => { + await updateLastOpen(RID, []); + + expect(mockedGetSubscriptionByRoomId).not.toHaveBeenCalled(); + }); + + it('overwrites a cursor already poisoned into the future, so the room self-heals', async () => { + const poisonedFutureCursor = new Date('2099-01-01T00:00:00.000Z'); + const subscription = makeSubscription(poisonedFutureCursor); + mockedGetSubscriptionByRoomId.mockResolvedValue(subscription as never); + + await updateLastOpen(RID, [{ _updatedAt: '2024-01-01T12:00:00.000Z' }]); + + expect(subscription.lastOpen).toEqual(new Date('2024-01-01T12:00:00.000Z')); + }); + + it('is a silent no-op when the subscription row does not exist', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue(null as never); + + await expect(updateLastOpen(RID, [{ _updatedAt: '2024-01-01T12:00:00.000Z' }])).resolves.toBeUndefined(); + }); +}); diff --git a/app/lib/methods/updateLastOpen.ts b/app/lib/methods/updateLastOpen.ts index e264648afce..4fc7c385d40 100644 --- a/app/lib/methods/updateLastOpen.ts +++ b/app/lib/methods/updateLastOpen.ts @@ -3,13 +3,30 @@ import { getSubscriptionByRoomId } from '../database/services/Subscription'; import log from './helpers/log'; import { type TSubscriptionModel } from '../../definitions'; -export async function updateLastOpen(rid: string, lastOpen = new Date()): Promise { +export type TServerTimestamps = { _updatedAt?: string | Date | null }[]; + +export const snapshotServerTimestamps = (payload: TServerTimestamps): TServerTimestamps => + payload.map(message => ({ _updatedAt: message._updatedAt })); + +export async function updateLastOpen(rid: string, payload: TServerTimestamps): Promise { try { - const db = database.active; + const timestamps = payload + .map(message => message._updatedAt) + .filter((updatedAt): updatedAt is string | Date => updatedAt != null) + .map(updatedAt => new Date(updatedAt).getTime()) + .filter(t => !Number.isNaN(t)); + if (!timestamps.length) { + return; + } + + const lastOpen = new Date(Math.max(...timestamps)); + const subscription = await getSubscriptionByRoomId(rid); if (!subscription) { return; } + + const db = database.active; await db.write(async () => { await subscription.update((s: TSubscriptionModel) => { s.lastOpen = lastOpen; diff --git a/app/sagas/encryption.js b/app/sagas/encryption.js index 9731adfdd27..f89be7cce42 100644 --- a/app/sagas/encryption.js +++ b/app/sagas/encryption.js @@ -113,7 +113,7 @@ const handleEncryptionDecodeKey = function* handleEncryptionDecodeKey({ password // If subscribed to a room, read it const subscribedRoom = yield select(state => state.room.subscribedRoom); if (subscribedRoom) { - yield readMessages(subscribedRoom, new Date()); + yield readMessages(subscribedRoom); } } catch (e) { log(e); diff --git a/app/views/RoomView/constants.ts b/app/views/RoomView/constants.ts index 448cf161420..60dafc11c0c 100644 --- a/app/views/RoomView/constants.ts +++ b/app/views/RoomView/constants.ts @@ -2,7 +2,7 @@ import { type TRoomUpdate, type TStateAttrsUpdate } from './definitions'; export const stateAttrsUpdate = [ 'joined', - 'lastOpen', + 'lastSeen', 'canAutoTranslate', 'loading', 'readOnly', diff --git a/app/views/RoomView/definitions.ts b/app/views/RoomView/definitions.ts index 812e4f3ce03..281ff7266b9 100644 --- a/app/views/RoomView/definitions.ts +++ b/app/views/RoomView/definitions.ts @@ -49,7 +49,7 @@ export interface IRoomViewState { [K in TRoomUpdate]?: any; }; member: any; - lastOpen: Date | null; + lastSeen: Date | null; canAutoTranslate: boolean; loading: boolean; readOnly: boolean; diff --git a/app/views/RoomView/index.test.tsx b/app/views/RoomView/index.test.tsx new file mode 100644 index 00000000000..8715887d297 --- /dev/null +++ b/app/views/RoomView/index.test.tsx @@ -0,0 +1,180 @@ +import { act, render } from '@testing-library/react-native'; +import { Provider } from 'react-redux'; +import { BehaviorSubject, Subject } from 'rxjs'; + +import { RoomView } from './index'; +import { type IRoomViewProps } from './definitions'; +import RoomServices from './services'; +import { readMessages } from '../../lib/methods/readMessages'; +import { mockedStore } from '../../reducers/mockedStore'; +import { initStore } from '../../lib/store/auxStore'; +import { setUser } from '../../actions/login'; + +jest.mock('./List', () => 'List'); +jest.mock('./LoadMore', () => 'LoadMore'); +jest.mock('./UploadProgress', () => 'UploadProgress'); +jest.mock('./JoinCode', () => 'JoinCode'); +jest.mock('./Banner', () => 'Banner'); +jest.mock('../../containers/MessageComposer', () => ({ + MessageComposerContainer: 'MessageComposerContainer', + ComposerAttachments: 'ComposerAttachments' +})); +jest.mock('../../containers/MessageActions', () => 'MessageActions'); +jest.mock('../../containers/MessageErrorActions', () => 'MessageErrorActions'); +jest.mock('../../containers/message', () => 'Message'); +jest.mock('../../lib/methods/subscriptions/room', () => + jest.fn().mockImplementation(() => ({ + subscribe: jest.fn(), + unsubscribe: jest.fn() + })) +); + +jest.mock('../../lib/services/restApi', () => ({ + getRoutingConfig: jest.fn().mockResolvedValue({ returnQueue: false }), + getUserInfo: jest.fn().mockResolvedValue({ success: false }), + editMessage: jest.fn(), + setReaction: jest.fn(), + joinRoom: jest.fn(), + toggleFollowMessage: jest.fn() +})); +jest.mock('../../lib/encryption/utils', () => ({ + isE2EEDisabledEncryptedRoom: () => false, + isMissingRoomE2EEKey: () => false +})); + +jest.mock('../../lib/hooks/useNewMediaCall', () => ({ + useNewMediaCall: () => ({ openNewMediaCall: jest.fn(), hasMediaCallPermission: false, isInActiveCall: false }) +})); +jest.mock('../../lib/services/voip/isInActiveVoipCall', () => ({ + isInActiveVoipCall: () => false, + useIsInActiveVoipCall: () => false +})); + +jest.mock('./services', () => ({ + __esModule: true, + default: { getMessages: jest.fn().mockResolvedValue(undefined) } +})); +jest.mock('../../lib/methods/readMessages', () => ({ readMessages: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../../lib/methods/loadThreadMessages', () => ({ loadThreadMessages: jest.fn().mockResolvedValue(undefined) })); +jest.mock('../../lib/methods/helpers/isReadOnly', () => ({ isReadOnly: jest.fn().mockResolvedValue(false) })); +jest.mock('../../lib/methods/AudioManager', () => ({ + __esModule: true, + default: { pauseAudio: jest.fn(), unloadRoomAudios: jest.fn().mockResolvedValue(undefined) } +})); + +const mockedGetMessages = RoomServices.getMessages as jest.Mock; +const mockedReadMessages = readMessages as jest.Mock; + +const mockSubscriptionsQuery = { observe: jest.fn(), observeWithColumns: jest.fn(() => new Subject()) }; +const mockSubscriptionsCollection = { + query: jest.fn(() => mockSubscriptionsQuery), + find: jest.fn() +}; + +jest.mock('../../lib/database', () => ({ + __esModule: true, + default: { + get active() { + return { get: jest.fn(() => mockSubscriptionsCollection) }; + } + } +})); + +const buildProps = (params: Record): IRoomViewProps => + ({ + route: { params }, + navigation: { setOptions: jest.fn(), addListener: jest.fn(() => jest.fn()), navigate: jest.fn() }, + dispatch: jest.fn(), + user: { id: 'u1', username: 'user', token: 't' }, + isAuthenticated: true, + insets: { left: 0, right: 0, top: 0, bottom: 0 }, + theme: 'light', + width: 400, + height: 800, + isMasterDetail: false, + baseUrl: 'https://open.rocket.chat', + serverVersion: '7.0.0', + Message_GroupingPeriod: 300, + Message_Read_Receipt_Enabled: false, + Hide_System_Messages: [], + encryptionEnabled: false, + inAppFeedback: {}, + permissions: {}, + showActionSheet: jest.fn(), + hideActionSheet: jest.fn(), + fontScale: 1 + } as unknown as IRoomViewProps); + +// A WatermelonDB subscription row observes itself; only the fields RoomView reads matter here. +const buildRow = (overrides: Record = {}) => { + const row = { + id: 'sub-1', + rid: 'rid-1', + t: 'c', + name: 'room', + encrypted: false, + alert: true, + unread: 1, + userMentions: 0, + ls: new Date('2026-01-01T00:00:00.000Z'), + observe: () => new BehaviorSubject(row), + ...overrides + }; + return row; +}; + +const renderRoomView = (params: Record) => + render( + + + + ); + +beforeAll(() => { + initStore(mockedStore); + mockedStore.dispatch(setUser({ id: 'u1', username: 'user' })); +}); + +describe('RoomView init cursor predicate', () => { + // The awaited microtask is what lets queued promises settle inside act. + const flush = () => + act(async () => { + await Promise.resolve(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + mockedGetMessages.mockResolvedValue(undefined); + mockedReadMessages.mockResolvedValue(undefined); + mockSubscriptionsCollection.find.mockRejectedValue(new Error('not found')); + mockSubscriptionsQuery.observe.mockReturnValue(new Subject()); + }); + + it('loads messages for a route-param room that lacks a subscription row', async () => { + renderRoomView({ rid: 'rid-1', t: 'c' }); + await flush(); + + expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1', t: 'c' }); + expect(mockedReadMessages).not.toHaveBeenCalled(); + }); + + it('routes a cursor-less subscribed room to the room-history loader directly', async () => { + renderRoomView({ rid: 'rid-1', t: 'c', room: buildRow() }); + await flush(); + + expect(mockedGetMessages).toHaveBeenCalledTimes(1); + expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1', t: 'c' }); + }); + + it('routes a subscribed room with a cursor to the missed-messages loader', async () => { + renderRoomView({ + rid: 'rid-1', + t: 'c', + room: buildRow({ lastOpen: new Date('2026-01-01T00:00:00.000Z') }) + }); + await flush(); + + expect(mockedGetMessages).toHaveBeenCalledTimes(1); + expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1' }); + }); +}); diff --git a/app/views/RoomView/index.tsx b/app/views/RoomView/index.tsx index edffea7126a..df2bcfb29d3 100644 --- a/app/views/RoomView/index.tsx +++ b/app/views/RoomView/index.tsx @@ -118,7 +118,7 @@ import { isInviteSubscription } from '../../lib/methods/isInviteSubscription'; const EMPTY_HIDE_SYSTEM_MESSAGES: string[] = []; -class RoomView extends Component { +export class RoomView extends Component { private rid?: string; private t?: string; private tmid?: string; @@ -177,7 +177,7 @@ class RoomView extends Component { room, roomUpdate: {}, member: {}, - lastOpen: null, + lastSeen: null, canAutoTranslate: false, loading: true, readOnly: false, @@ -678,21 +678,19 @@ class RoomView extends Component { this.consumeJumpParam(messageId); } } else { - const newLastOpen = new Date(); await RoomServices.getMessages({ rid: room.rid, - t: room.t as RoomType, - ...('lastOpen' in room && room.lastOpen ? { lastOpen: room.lastOpen } : {}) + ...('lastOpen' in room && room.lastOpen ? {} : { t: room.t as RoomType }) }); // if room is joined if (joined && 'id' in room) { if (room.alert || room.unread || room.userMentions) { - this.setLastOpen(room.ls); + this.setLastSeen(room.ls); } else { - this.setLastOpen(null); + this.setLastSeen(null); } - readMessages(room.rid, newLastOpen, true).catch(e => console.log(e)); + readMessages(room.rid).catch(e => console.log(e)); } } @@ -1121,14 +1119,14 @@ class RoomView extends Component { const { user } = this.props; sendMessage(rid, message, this.tmid, user, tshow).then(() => { if (this.mounted) { - this.setLastOpen(null); + this.setLastSeen(null); } Review.pushPositiveEvent(); }); this.resetAction(); }; - setLastOpen = (lastOpen: Date | null) => this.setState({ lastOpen }); + setLastSeen = (lastSeen: Date | null) => this.setState({ lastSeen }); onJoin = () => { this.internalSetState({ @@ -1404,19 +1402,19 @@ class RoomView extends Component { }; renderItem = (item: TAnyMessageModel, previousItem: TAnyMessageModel, highlightedMessage?: string) => { - const { room, lastOpen } = this.state; + const { room, lastSeen } = this.state; const { inAppFeedback } = this.props; let dateSeparator = null; let showUnreadSeparator = false; if (!previousItem) { dateSeparator = item.ts; - showUnreadSeparator = lastOpen ? dayjs(item.ts).isAfter(lastOpen) : false; + showUnreadSeparator = lastSeen ? dayjs(item.ts).isAfter(lastSeen) : false; } else { showUnreadSeparator = - (lastOpen && - (dayjs(item.ts).isSame(lastOpen) || dayjs(item.ts).isAfter(lastOpen)) && - dayjs(previousItem.ts).isBefore(lastOpen)) ?? + (lastSeen && + (dayjs(item.ts).isSame(lastSeen) || dayjs(item.ts).isAfter(lastSeen)) && + dayjs(previousItem.ts).isBefore(lastSeen)) ?? false; if (!dayjs(item.ts).isSame(previousItem.ts, 'day')) { dateSeparator = item.ts; diff --git a/app/views/RoomView/services/getMessages.ts b/app/views/RoomView/services/getMessages.ts index c92f00f07ea..7d4165120d6 100644 --- a/app/views/RoomView/services/getMessages.ts +++ b/app/views/RoomView/services/getMessages.ts @@ -2,23 +2,16 @@ import { loadMessagesForRoom } from '../../../lib/methods/loadMessagesForRoom'; import { loadMissedMessages } from '../../../lib/methods/loadMissedMessages'; import { type RoomTypes } from '../../../lib/methods/roomTypeToApiType'; -interface IBaseParams { +interface IGetMessagesParams { rid: string; + t?: RoomTypes; } -interface ILoadMessagesForRoomParams extends IBaseParams { - t: RoomTypes; -} - -interface ILoadMissedMessagesParams extends IBaseParams { - lastOpen: Date; -} - -const getMessages = (params: ILoadMissedMessagesParams | ILoadMessagesForRoomParams): Promise => { - if ('lastOpen' in params) { - return loadMissedMessages(params); +const getMessages = ({ rid, t }: IGetMessagesParams): Promise => { + if (!t) { + return loadMissedMessages({ rid }); } - return loadMessagesForRoom(params); + return loadMessagesForRoom({ rid, t }); }; export default getMessages;