diff --git a/packages/app/ui/components/Channel/PinnedPostBanner.tsx b/packages/app/ui/components/Channel/PinnedPostBanner.tsx index 61a9b80268..85a6439961 100644 --- a/packages/app/ui/components/Channel/PinnedPostBanner.tsx +++ b/packages/app/ui/components/Channel/PinnedPostBanner.tsx @@ -41,7 +41,12 @@ export function PinnedPostBanner({ } }, [pinnedPostId]); - if (!pinnedPostId || !postQuery.data || isDismissed) { + if ( + !pinnedPostId || + !postQuery.data || + postQuery.data.isDeleted || + isDismissed + ) { return null; } diff --git a/packages/shared/src/db/queries.test.ts b/packages/shared/src/db/queries.test.ts index 4bd281b35a..a9eacaad71 100644 --- a/packages/shared/src/db/queries.test.ts +++ b/packages/shared/src/db/queries.test.ts @@ -3,7 +3,7 @@ import { v0PeersToClientProfiles } from '@tloncorp/api'; import { toClientGroupsV7 } from '@tloncorp/api'; import type * as ub from '@tloncorp/api/urbit/groups'; import * as $ from 'drizzle-orm'; -import { afterEach, describe, expect, test } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; import * as schema from '../db/schema'; import { syncContacts, syncInitData } from '../store/sync'; @@ -1216,7 +1216,7 @@ describe('getPendingPosts', () => { expect(ids).not.toContain(failedCleared.id); }); - test('includes deleted markPostSent / needs_verification rows so the remount path still has a tombstone source', async () => { + test('includes deleted markPostSent / needs_verification rows while the delete is not settled', async () => { await queries.insertChannels([{ id: channelId, type: 'chat' }]); // `markPostSent` catch-up: server already acknowledged, sequenced // `addPost` hasn't arrived yet, and the user deleted in the gap. @@ -1225,6 +1225,7 @@ describe('getPendingPosts', () => { deliveryStatus: 'sent', sequenceNum: 0, isDeleted: true, + deleteStatus: null, }); // `needs_verification` may also have reached the server. const verifyCleared = await seedPost({ @@ -1232,6 +1233,7 @@ describe('getPendingPosts', () => { deliveryStatus: 'needs_verification', sequenceNum: 0, isDeleted: true, + deleteStatus: 'pending', }); const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id); expect(ids).toEqual( @@ -1246,12 +1248,14 @@ describe('getPendingPosts', () => { deliveryStatus: 'enqueued', sequenceNum: 0, isDeleted: true, + deleteStatus: null, }); const pendingCleared = await seedPost({ id: 'pending-cleared', deliveryStatus: 'pending', sequenceNum: 0, isDeleted: true, + deleteStatus: 'pending', }); const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id); expect(ids).toEqual( @@ -1259,6 +1263,21 @@ describe('getPendingPosts', () => { ); }); + test('excludes settled deleted rows without deleting them from storage', async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + const settled = await seedPost({ + id: 'settled-delete', + deliveryStatus: 'sent', + sequenceNum: 0, + isDeleted: true, + deleteStatus: 'sent', + }); + + const ids = (await queries.getPendingPosts(channelId)).map((p) => p.id); + expect(ids).not.toContain(settled.id); + expect(await queries.getPost({ postId: settled.id })).toBeTruthy(); + }); + test('excludes confirmed rows (null deliveryStatus) regardless of isDeleted', async () => { await queries.insertChannels([{ id: channelId, type: 'chat' }]); const confirmed = await seedPost({ @@ -1292,6 +1311,222 @@ describe('getPendingPosts', () => { }); }); +describe('deleteUnsequencedAcknowledgedPost', () => { + const channelId = 'conditional-delete-channel'; + + async function seedPost(id: string, overrides: Partial = {}) { + const post = { + id, + type: 'chat', + channelId, + authorId: '~zod', + sentAt: Date.now(), + receivedAt: Date.now(), + sequenceNum: 0, + deliveryStatus: 'sent', + content: JSON.stringify([{ inline: ['seed'] }]), + syncedAt: Date.now(), + ...overrides, + } as Post; + await queries.insertChannelPosts({ posts: [post] }); + return post; + } + + test('deletes and returns an acknowledged row while it is still unsequenced', async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + const post = await seedPost('still-unsequenced'); + + const deleted = await queries.deleteUnsequencedAcknowledgedPost(post.id); + + expect(deleted?.id).toBe(post.id); + expect(await queries.getPost({ postId: post.id })).toBeNull(); + }); + + test('preserves the row when a sequenced echo wins the race', async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + const post = await seedPost('sequenced-before-delete'); + // This update represents the addPost echo landing after the caller chose + // the hard-delete path but before SQLite executes the guarded DELETE. + await queries.updatePost({ id: post.id, sequenceNum: 42 }); + + const deleted = await queries.deleteUnsequencedAcknowledgedPost(post.id); + + expect(deleted).toBeNull(); + expect(await queries.getPost({ postId: post.id })).toMatchObject({ + id: post.id, + sequenceNum: 42, + }); + }); +}); + +describe('deleteSettledUnsequencedDeletedPost', () => { + const channelId = 'settled-delete-on-send-channel'; + + async function seedPost(id: string, overrides: Partial = {}) { + const post = { + id, + type: 'chat', + channelId, + authorId: '~zod', + sentAt: Date.now(), + receivedAt: Date.now(), + sequenceNum: 0, + deliveryStatus: 'sent', + isDeleted: true, + deleteStatus: 'sent', + content: JSON.stringify([{ inline: ['seed'] }]), + syncedAt: Date.now(), + ...overrides, + } as Post; + await queries.insertChannelPosts({ posts: [post] }); + return post; + } + + beforeEach(async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + }); + + test('removes a settled-delete row once its send resolves to sent', async () => { + const post = await seedPost('settled-delete-delivered'); + + const deleted = await queries.deleteSettledUnsequencedDeletedPost(post.id); + + expect(deleted?.id).toBe(post.id); + expect(await queries.getPost({ postId: post.id })).toBeNull(); + }); + + test('leaves a normally delivered post untouched', async () => { + // Same acknowledged/unsequenced shape but NOT deleted — must never be + // removed just because its send resolved. + const post = await seedPost('live-delivered', { + isDeleted: false, + deleteStatus: null, + }); + + const deleted = await queries.deleteSettledUnsequencedDeletedPost(post.id); + + expect(deleted).toBeNull(); + expect(await queries.getPost({ postId: post.id })).toMatchObject({ + id: post.id, + }); + }); + + test('waits while the send is still in flight', async () => { + const post = await seedPost('settled-delete-in-flight', { + deliveryStatus: 'pending', + }); + + const deleted = await queries.deleteSettledUnsequencedDeletedPost(post.id); + + expect(deleted).toBeNull(); + expect(await queries.getPost({ postId: post.id })).toMatchObject({ + id: post.id, + }); + }); + + test('preserves a sequenced echo that wins the race', async () => { + const post = await seedPost('settled-delete-sequenced', { + sequenceNum: 42, + }); + + const deleted = await queries.deleteSettledUnsequencedDeletedPost(post.id); + + expect(deleted).toBeNull(); + expect(await queries.getPost({ postId: post.id })).toMatchObject({ + id: post.id, + sequenceNum: 42, + }); + }); +}); + +describe('getSettledDeletedGhostChannelIds', () => { + async function seedPost( + channelId: string, + id: string, + overrides: Partial = {} + ) { + await queries.insertChannelPosts({ + posts: [ + { + id, + type: 'chat', + channelId, + authorId: '~zod', + sentAt: Date.now(), + receivedAt: Date.now(), + sequenceNum: 0, + deliveryStatus: 'sent', + content: JSON.stringify([{ inline: ['seed'] }]), + syncedAt: Date.now(), + ...overrides, + } as Post, + ], + }); + } + + const GHOST_RECEIVED_AT = 5_000; + + test('returns only channels with an acknowledged, unsequenced settled-delete ghost', async () => { + const ghostChannel = 'ghost-evidence-channel'; + const inFlightChannel = 'in-flight-delete-channel'; + const ordinaryDeleteChannel = 'ordinary-delete-channel'; + await queries.insertChannels( + [ghostChannel, inFlightChannel, ordinaryDeleteChannel].map((id) => ({ + id, + type: 'chat' as const, + })) + ); + await seedPost(ghostChannel, 'settled-ghost', { + receivedAt: GHOST_RECEIVED_AT, + isDeleted: true, + deleteStatus: 'sent', + }); + await seedPost(inFlightChannel, 'in-flight-delete', { + receivedAt: GHOST_RECEIVED_AT, + deliveryStatus: 'pending', + isDeleted: true, + deleteStatus: 'sent', + }); + await seedPost(ordinaryDeleteChannel, 'ordinary-delete', { + receivedAt: GHOST_RECEIVED_AT, + sequenceNum: 42, + deliveryStatus: null, + isDeleted: true, + deleteStatus: 'sent', + }); + + const ids = await queries.getSettledDeletedGhostChannelIds( + [ghostChannel, inFlightChannel, ordinaryDeleteChannel].map( + (channelId) => ({ channelId, lastPostAt: GHOST_RECEIVED_AT }) + ) + ); + + expect(ids).toEqual([ghostChannel]); + }); + + test('excludes a ghost row whose receivedAt does not match the stale lastPostAt', async () => { + // The stale `lastPostId: null` shape came from an ordinary delete of a + // different, newer head (nulled while the local cache was partial); the + // channel only incidentally also holds an older settled-delete ghost. + // Recomputing here would blank/rewind the real preview, so match the + // specific ghost row before repairing. + const channel = 'mismatched-ghost-channel'; + await queries.insertChannels([{ id: channel, type: 'chat' }]); + await seedPost(channel, 'older-ghost', { + receivedAt: GHOST_RECEIVED_AT, + isDeleted: true, + deleteStatus: 'sent', + }); + + const staleLastPostAt = GHOST_RECEIVED_AT + 1_000; + const ids = await queries.getSettledDeletedGhostChannelIds([ + { channelId: channel, lastPostAt: staleLastPostAt }, + ]); + + expect(ids).toEqual([]); + }); +}); + // TLON-5606: `undoOptimisticReplyBump` must undo a single optimistic reply // add without clobbering server-sourced reply summary state. Two regimes: // complete local cache → full recompute; partial local cache → decrement @@ -1572,6 +1807,32 @@ describe('recomputeChannelLastPost', () => { expect(channel!.lastPostAt).toBe(1000); }); + test('repairs the stale preview shape left by a persisted settled-delete ghost', async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + await seedTopLevel('previous-post', 1000); + await seedTopLevel('persisted-ghost', 2000, { + sequenceNum: 0, + deliveryStatus: 'sent', + isDeleted: true, + deleteStatus: 'sent', + }); + // The legacy delete path nulled only the id, leaving the old timestamp. + await queries.updateChannel({ + id: channelId, + lastPostId: null, + lastPostAt: 2000, + }); + + await queries.recomputeChannelLastPost({ channelId }); + + const channel = await queries.getChannel({ id: channelId }); + expect(channel!.lastPostId).toBe('previous-post'); + expect(channel!.lastPostAt).toBe(1000); + // Read-layer repair is non-destructive; the settled row stays inert in + // storage and remains available for any later sync reconciliation. + expect(await queries.getPost({ postId: 'persisted-ghost' })).toBeTruthy(); + }); + test('also repairs parent group lastPostId / lastPostAt when the channel belongs to a group', async () => { const groupId = '~zod/group-with-preview'; await queries.insertGroups({ @@ -1889,7 +2150,7 @@ describe('getDeliveryPendingPosts', () => { return base; } - test('includes enqueued and pending rows regardless of isDeleted', async () => { + test('includes enqueued and pending rows while their delete is not settled', async () => { await queries.insertChannels([{ id: channelId, type: 'chat' }]); const enqueuedLive = await seedPost({ id: 'enq-live', @@ -1900,6 +2161,7 @@ describe('getDeliveryPendingPosts', () => { id: 'enq-cleared', deliveryStatus: 'enqueued', isDeleted: true, + deleteStatus: null, }); const pendingLive = await seedPost({ id: 'pend-live', @@ -1910,6 +2172,7 @@ describe('getDeliveryPendingPosts', () => { id: 'pend-cleared', deliveryStatus: 'pending', isDeleted: true, + deleteStatus: 'pending', }); const ids = (await queries.getDeliveryPendingPosts(channelId)).map( @@ -1982,6 +2245,37 @@ describe('getDeliveryPendingPosts', () => { expect(ids).toContain(catchUp.id); }); + test('keeps settled deletes polling while the original send is in flight, then excludes sent ghosts', async () => { + await queries.insertChannels([{ id: channelId, type: 'chat' }]); + const enqueued = await seedPost({ + id: 'settled-enqueued', + deliveryStatus: 'enqueued', + isDeleted: true, + deleteStatus: 'sent', + }); + const pending = await seedPost({ + id: 'settled-pending', + deliveryStatus: 'pending', + isDeleted: true, + deleteStatus: 'sent', + }); + const sent = await seedPost({ + id: 'settled-sent', + deliveryStatus: 'sent', + sequenceNum: 0, + isDeleted: true, + deleteStatus: 'sent', + }); + + const ids = (await queries.getDeliveryPendingPosts(channelId)).map( + (p) => p.id + ); + expect(ids).toContain(enqueued.id); + expect(ids).toContain(pending.id); + expect(ids).not.toContain(sent.id); + expect(await queries.getPost({ postId: sent.id })).toBeTruthy(); + }); + test('excludes sequenced sent rows (the sequenced addPost has already reconciled the row)', async () => { await queries.insertChannels([{ id: channelId, type: 'chat' }]); const reconciled = await seedPost({ diff --git a/packages/shared/src/db/queries.ts b/packages/shared/src/db/queries.ts index 902e4f8579..c5539d1801 100644 --- a/packages/shared/src/db/queries.ts +++ b/packages/shared/src/db/queries.ts @@ -4179,6 +4179,64 @@ export const deletePost = createWriteQuery( ['posts'] ); +/** + * Delete an acknowledged optimistic top-level post only while it is still + * unsequenced. Keeping the state guard in the DELETE predicate closes the + * race where a sequenced addPost echo updates the row between a read and a + * later unconditional delete. + */ +export const deleteUnsequencedAcknowledgedPost = createWriteQuery( + 'deleteUnsequencedAcknowledgedPost', + async (postId: string, ctx: QueryCtx) => { + const deleted = await ctx.db + .delete($posts) + .where( + and( + eq($posts.id, postId), + eq($posts.sequenceNum, 0), + isNull($posts.parentId), + inArray($posts.deliveryStatus, ['sent', 'needs_verification']) + ) + ) + .returning(); + return deleted[0] ?? null; + }, + ['posts'] +); + +/** + * Finish a settled delete once the original send finally resolves to `sent`. + * + * Covers the ordering where a user deletes an optimistic post while its send + * is still `enqueued`/`pending`: the delete can be acknowledged before + * delivery flips the row to `sent`, so `deleteUnsequencedAcknowledgedPost` + * matches nothing at delete time (delivery isn't acknowledged yet) and the + * settled-delete row lingers. When `markPostSent` later flips delivery to + * `sent`, this removes the row. The `isDeleted` / `deleteStatus` / delivery + * guard is enforced in the DELETE predicate so it can never touch a normally + * delivered post, and returns the row only when it actually removed it. + */ +export const deleteSettledUnsequencedDeletedPost = createWriteQuery( + 'deleteSettledUnsequencedDeletedPost', + async (postId: string, ctx: QueryCtx) => { + const deleted = await ctx.db + .delete($posts) + .where( + and( + eq($posts.id, postId), + eq($posts.isDeleted, true), + eq($posts.deleteStatus, 'sent'), + eq($posts.sequenceNum, 0), + isNull($posts.parentId), + eq($posts.deliveryStatus, 'sent') + ) + ) + .returning(); + return deleted[0] ?? null; + }, + ['posts'] +); + export const markPostAsDeleted = createWriteQuery( 'markPostAsDeleted', async (postId: string, ctx: QueryCtx) => { @@ -4598,22 +4656,25 @@ export const getPendingPosts = createReadQuery( eq($posts.channelId, channelId), isNotNull($posts.deliveryStatus), not(eq($posts.type, 'reply')), - // Exclude only truly local-only deleted sends. Deleted rows whose - // `deliveryStatus` is `'sent'` or `'needs_verification'` may still - // be server-backed but not yet sequenced; keep them as a DB-backed - // tombstone source so the channel remount path still renders them - // before the sequenced `addPost` arrives. Enqueued / pending rows - // are still in-flight and also remain visible — they either reach - // `'sent'` (stay as tombstone) or `'failed'` (vanish on next tick). + // Exclude deleted sends once they are either known to be local-only + // failures or their server delete has settled. The latter also makes + // old ghost rows inert without a startup cleanup task. Rows whose + // delete is still in flight remain a tombstone source until the + // outcome is known. // - // SQL NULL handling: `isDeleted` is a nullable boolean. A bare - // `not(and(eq(isDeleted, true), ...))` would evaluate to NULL for - // rows where `isDeleted IS NULL`, which SQL treats as filter-out. - // Enumerate the keep condition explicitly instead. + // SQL NULL handling: `isDeleted` and `deleteStatus` are nullable. A + // negated conjunction would evaluate to NULL for unset values, which + // SQL treats as filter-out, so enumerate the keep cases explicitly. or( isNull($posts.isDeleted), eq($posts.isDeleted, false), - not(eq($posts.deliveryStatus, 'failed')) + and( + not(eq($posts.deliveryStatus, 'failed')), + or( + isNull($posts.deleteStatus), + not(eq($posts.deleteStatus, 'sent')) + ) + ) ) ), }); @@ -4621,14 +4682,61 @@ export const getPendingPosts = createReadQuery( ['posts'] ); +/** + * Return only channels whose persisted TLON-5911 ghost row is the same post + * that left the stale preview behind. This lets preview repair distinguish a + * settled acknowledged delete from ordinary `lastPostId: null` states that may + * have only a partial local post cache. + * + * A candidate is `{ channelId, lastPostAt }`: `deletePost` nulls + * `channels.lastPostId` but leaves `lastPostAt` at the deleted head's + * `receivedAt`, so the ghost row that produced the stale shape has a matching + * `receivedAt`. Matching on it avoids recomputing (and thereby blanking or + * rewinding) a head that an unrelated ordinary delete nulled while the local + * post cache was only partial, even when the channel happens to also hold an + * older settled-delete ghost. + */ +export const getSettledDeletedGhostChannelIds = createReadQuery( + 'getSettledDeletedGhostChannelIds', + async ( + candidates: { channelId: string; lastPostAt: number }[], + ctx: QueryCtx + ) => { + if (candidates.length === 0) return []; + const rows = await ctx.db + .selectDistinct({ channelId: $posts.channelId }) + .from($posts) + .where( + and( + or( + ...candidates.map((candidate) => + and( + eq($posts.channelId, candidate.channelId), + eq($posts.receivedAt, candidate.lastPostAt) + ) + ) + ), + eq($posts.isDeleted, true), + eq($posts.deleteStatus, 'sent'), + eq($posts.sequenceNum, 0), + isNull($posts.parentId), + inArray($posts.deliveryStatus, ['sent', 'needs_verification']) + ) + ); + return rows.map((row) => row.channelId); + }, + ['posts'] +); + /** * Rows that are still in-flight from the sender's perspective, used by * `syncChannelWithBackoff` to decide whether delivery polling should - * continue. Unlike `getPendingPosts` (UI merge input), this does NOT - * exclude `isDeleted` rows — a user can delete a post mid-flight, and the - * server still needs to acknowledge the original send before anything is - * truly settled. `failed` and `needs_verification` are handled by dedicated - * retry / verification flows, so they are not treated as "still delivering". + * continue. Deleted `enqueued` / `pending` rows remain eligible even after + * their delete settles: the original send lifecycle is still in flight and + * must reconcile independently. Once an unsequenced send is acknowledged as + * `sent`, a settled delete makes it inert and it drops out. `failed` and + * `needs_verification` are handled by dedicated retry / verification flows, + * so they are not treated as "still delivering". * * Server-acknowledged but unsequenced rows (`deliveryStatus: 'sent'` while * still `sequenceNum === 0`) are also included: `markPostSent` sets the @@ -4648,7 +4756,18 @@ export const getDeliveryPendingPosts = createReadQuery( or( eq($posts.deliveryStatus, 'enqueued'), eq($posts.deliveryStatus, 'pending'), - and(eq($posts.deliveryStatus, 'sent'), eq($posts.sequenceNum, 0)) + and( + eq($posts.deliveryStatus, 'sent'), + eq($posts.sequenceNum, 0), + // Keep nullable values explicit to avoid filtering ordinary rows + // via SQL's three-valued logic. + or( + isNull($posts.isDeleted), + eq($posts.isDeleted, false), + isNull($posts.deleteStatus), + not(eq($posts.deleteStatus, 'sent')) + ) + ) ) ), }); diff --git a/packages/shared/src/store/dbHooks.ts b/packages/shared/src/store/dbHooks.ts index bc1a04beb7..7a185071d9 100644 --- a/packages/shared/src/store/dbHooks.ts +++ b/packages/shared/src/store/dbHooks.ts @@ -36,13 +36,55 @@ export const useAllChannels = ({ enabled }: { enabled?: boolean }) => { export const useCurrentChats = ( queryConfig?: CustomQueryConfig ): UseQueryResult => { - return useQuery({ + const query = useQuery({ queryFn: async () => { return db.getChats(); }, queryKey: ['currentChats', useKeyFromQueryDeps(db.getChats)], ...queryConfig, }); + + useEffect(() => { + if (!query.data) return; + + // Older settled-delete ghosts can leave `lastPostId` nulled while the old + // `lastPostAt` survives. Treat this metadata shape only as a candidate: + // ordinary deletes can produce it while the local post cache is partial. + // The DB check below gates recomputation on the actual persisted ghost row + // whose `receivedAt` matches the stale `lastPostAt`. + const staleChannels = new Map(); + const chats = [ + ...query.data.pinned, + ...query.data.pending, + ...query.data.unpinned, + ]; + for (const chat of chats) { + const channels = + chat.type === 'channel' ? [chat.channel] : chat.group.channels ?? []; + for (const channel of channels) { + if (channel.lastPostId === null && channel.lastPostAt != null) { + staleChannels.set(channel.id, channel.lastPostAt); + } + } + } + if (staleChannels.size === 0) return; + + void (async () => { + const ghostChannelIds = await db.getSettledDeletedGhostChannelIds( + [...staleChannels].map(([channelId, lastPostAt]) => ({ + channelId, + lastPostAt, + })) + ); + for (const channelId of ghostChannelIds) { + await db.recomputeChannelLastPost({ channelId }); + } + })().catch((error) => { + console.error('Failed to repair stale channel previews', error); + }); + }, [query.data]); + + return query; }; // Probe %notes once to detect whether the notes desk is installed on the diff --git a/packages/shared/src/store/mergePendingPosts.test.ts b/packages/shared/src/store/mergePendingPosts.test.ts index 9c736ca1ea..a4bd308dc2 100644 --- a/packages/shared/src/store/mergePendingPosts.test.ts +++ b/packages/shared/src/store/mergePendingPosts.test.ts @@ -631,6 +631,25 @@ describe('mergePendingPosts markPostSent catch-up window', () => { expect(merged[0].isDeleted).toBe(true); }); + test('drops stale live snapshots once the delete path marks them removed', () => { + const sentCatchUp = { + ...makePost(10), + id: 'marked-sent-removed', + sequenceNum: 0, + deliveryStatus: 'sent' as const, + }; + const merged = mergePendingPosts({ + newPosts: [sentCatchUp], + pendingPosts: [], + existingPosts: [], + deletedPosts: { [sentCatchUp.id]: 'removed' }, + hasNewest: true, + filterDeleted: false, + }); + + expect(merged.map((p) => p.id)).not.toContain(sentCatchUp.id); + }); + test('still drops a failed, locally-cleared optimistic row (TLON-5606 regression guard)', () => { const failed = { ...makePost(10), diff --git a/packages/shared/src/store/postActions/finishSettledDelete.ts b/packages/shared/src/store/postActions/finishSettledDelete.ts new file mode 100644 index 0000000000..4b3c9c592e --- /dev/null +++ b/packages/shared/src/store/postActions/finishSettledDelete.ts @@ -0,0 +1,28 @@ +import * as db from '../../db'; +import { QueryCtx } from '../../db/query'; +import { removeFromChannelPosts } from '../useChannelPosts/subscriptions'; + +/** + * Finish a delete that settled while its original send was still in flight, + * once delivery finally resolves to `sent`. + * + * Deleting an optimistic post while its send is `enqueued`/`pending` can have + * the delete acknowledged before delivery resolves, so the delete-time guarded + * hard-delete matches nothing. Delivery later reaches `sent` through two paths + * — `markPostSent` subscription events and `verifyPostDelivery` after a send + * timeout — and both must run this cleanup, or the settled-delete row lingers + * in a mounted channel's live snapshot as a tombstone until remount. + * + * `deleteSettledUnsequencedDeletedPost` guards on the settled-delete shape in + * its DELETE predicate, so this can never remove a normally delivered post. + */ +export async function finishSettledDeleteOnDelivery( + postId: string, + ctx?: QueryCtx +) { + const removed = await db.deleteSettledUnsequencedDeletedPost(postId, ctx); + if (removed) { + removeFromChannelPosts(removed); + await db.recomputeChannelLastPost({ channelId: removed.channelId }, ctx); + } +} diff --git a/packages/shared/src/store/postActions/postActions.test.ts b/packages/shared/src/store/postActions/postActions.test.ts index 1a8e00a706..f097aff770 100644 --- a/packages/shared/src/store/postActions/postActions.test.ts +++ b/packages/shared/src/store/postActions/postActions.test.ts @@ -13,6 +13,8 @@ import { getClient, setupDatabaseTestSuite } from '../../test/helpers'; import { updateSession } from '../session'; import { setUploadState } from '../storage'; import * as sync from '../sync'; +import { subscribeToDeletedPosts } from '../useChannelPosts/subscriptions'; +import type { DeletedPostState } from '../useChannelPosts/subscriptions'; import { mergePendingPosts } from '../useMergePendingPosts'; import { deleteFailedPost, @@ -742,11 +744,14 @@ describe('clearing a failed optimistic post', () => { // Pending merge layer is clean. const pending = await db.getPendingPosts(TEST_CHANNEL); expect(pending.map((p) => p.id)).not.toContain(post.id); + // A mounted channel can still hold the pre-delete snapshot in its + // session-local `newPosts` array. The hard-delete event marks that input + // removed so it disappears immediately without a remount. const merged = mergePendingPosts({ - newPosts: [], + newPosts: [post], pendingPosts: pending, existingPosts: [], - deletedPosts: {}, + deletedPosts: { [post.id]: 'removed' }, hasNewest: true, }); expect(merged.map((p) => p.id)).not.toContain(post.id); @@ -1443,6 +1448,119 @@ describe('clearing a failed optimistic post', () => { }); }); +// TLON-5911: deleting a server-acknowledged-but-unsequenced send +// (`deliveryStatus: 'sent'`, `sequenceNum: 0`). Once the server confirms the +// delete, the sequenced `addPost` that would normally replace the cached row +// is never coming, so the row must be hard-deleted — otherwise it survives as +// an `isDeleted` tombstone that `mergePendingPosts` sorts unconfirmed-first, +// i.e. pinned to the bottom of the chat scroller indefinitely. +describe('deleting a sent-but-unsequenced post', () => { + beforeEach(async () => { + await db.insertChannels([ + db.buildChannel({ id: TEST_CHANNEL, type: 'chat' }), + ]); + vi.mocked(poke).mockResolvedValue(0); + updateSession({ startTime: Date.now(), channelStatus: 'active' }); + }); + + afterEach(() => { + vi.mocked(poke).mockClear(); + updateSession(null); + }); + + async function seedSentUnsequencedPost(): Promise { + const channel = (await db.getChannel({ id: TEST_CHANNEL }))!; + const post = db.buildPost({ + authorId: '~zod', + author: null, + channel, + sequenceNum: 0, + content: [{ inline: [friendlyUniqueString()] }], + deliveryStatus: 'sent', + }); + await db.insertChannelPosts({ posts: [post] }); + return post; + } + + test('deletePost() sends the server delete, then hard-deletes the unsequenced row', async () => { + const channel = (await db.getChannel({ id: TEST_CHANNEL }))!; + const previous = db.buildPost({ + authorId: '~zod', + author: null, + channel, + sequenceNum: 5, + content: [{ inline: ['previous previewable post'] }], + deliveryStatus: 'sent', + }); + await db.insertChannelPosts({ posts: [previous] }); + + const post = await seedSentUnsequencedPost(); + expect((await db.getPendingPosts(TEST_CHANNEL)).map((p) => p.id)).toContain( + post.id + ); + + let liveDeleteState: DeletedPostState | undefined; + const unsubscribe = subscribeToDeletedPosts((postId, state) => { + if (postId === post.id) liveDeleteState = state; + }); + await deletePost({ post }); + unsubscribe(); + + // Unlike the failed-optimistic short-circuit, the row may exist on the + // server, so the delete round trip must happen. + expect(poke).toHaveBeenCalled(); + + // Once the server confirms, the row is gone — no ghost tombstone left in + // the DB or the pending merge layer. + expect(await fetchPost(post.id)).toBeUndefined(); + const pending = await db.getPendingPosts(TEST_CHANNEL); + expect(pending.map((p) => p.id)).not.toContain(post.id); + expect(liveDeleteState).toBe('removed'); + const merged = mergePendingPosts({ + newPosts: [post], + pendingPosts: pending, + existingPosts: [], + deletedPosts: { [post.id]: liveDeleteState! }, + hasNewest: true, + }); + expect(merged.map((p) => p.id)).not.toContain(post.id); + + // Channel preview repoints to the newest remaining previewable post. + const chan = await db.getChannel({ id: TEST_CHANNEL }); + expect(chan!.lastPostId).toBe(previous.id); + }); + + test('deletePost() keeps the normal tombstone when the sequenced echo lands during the delete round trip', async () => { + const post = await seedSentUnsequencedPost(); + vi.mocked(poke).mockImplementationOnce(async () => { + // The sequenced addPost catches up mid-flight: the row is no longer + // unsequenced by the time the delete is acknowledged. + await db.updatePost({ id: post.id, sequenceNum: 42 }); + return 0; + }); + + await deletePost({ post }); + + const rowAfter = await fetchPost(post.id); + expect(rowAfter).toBeTruthy(); + expect(rowAfter!.isDeleted).toBe(true); + expect(rowAfter!.sequenceNum).toBe(42); + }); + + test('deletePost() failure rolls back and does NOT hard-delete the row', async () => { + const post = await seedSentUnsequencedPost(); + vi.mocked(poke).mockRejectedValueOnce(new Error('nack')); + + await deletePost({ post }); + + const rowAfter = await fetchPost(post.id); + expect(rowAfter).toBeTruthy(); + expect(rowAfter!.isDeleted).toBeFalsy(); + expect(rowAfter!.deliveryStatus).toBe('sent'); + expect(rowAfter!.deleteStatus).toBe('failed'); + }); +}); + // TLON-6133: deleting a post that is pinned/arranged also removes it from // the channel order. describe('deleting a pinned post', () => { @@ -1581,4 +1699,53 @@ describe('deleting a pinned post', () => { const channelAfter = await db.getChannel({ id: GROUP_CHANNEL }); expect(channelAfter!.order).toEqual([post.id]); }); + + // A pinned post can also be an acknowledged-but-unsequenced send. When the + // delete poke's ack is lost but the server confirms the delete, the + // verification branches must run the same guarded hard-delete as the success + // path — otherwise the `sequenceNum: 0` ghost (and its live overlay) is left + // behind exactly like TLON-5911, just reached via the lost-ack path. + async function seedPinnedUnsequencedPost(): Promise { + const channel = (await db.getChannel({ id: GROUP_CHANNEL }))!; + const post = db.buildPost({ + authorId: '~zod', + author: null, + channel, + sequenceNum: 0, + content: [{ inline: [friendlyUniqueString()] }], + deliveryStatus: 'sent', + }); + await db.insertChannelPosts({ posts: [post] }); + await db.updateChannel({ id: GROUP_CHANNEL, order: [post.id] }); + return post; + } + + test('lost ack via isDeleted hard-deletes an unsequenced pinned post', async () => { + const post = await seedPinnedUnsequencedPost(); + vi.mocked(poke).mockRejectedValue(new Error('ack lost')); + vi.spyOn(api, 'getPostWithReplies').mockResolvedValue({ + ...post, + isDeleted: true, + }); + + await deletePost({ post }); + + expect(await fetchPost(post.id)).toBeUndefined(); + const channelAfter = await db.getChannel({ id: GROUP_CHANNEL }); + expect(channelAfter!.order).toEqual([]); + }); + + test('lost ack via 404 hard-deletes an unsequenced pinned post', async () => { + const post = await seedPinnedUnsequencedPost(); + vi.mocked(poke).mockRejectedValue(new Error('ack lost')); + vi.spyOn(api, 'getPostWithReplies').mockRejectedValue( + new api.BadResponseError(404, '') + ); + + await deletePost({ post }); + + expect(await fetchPost(post.id)).toBeUndefined(); + const channelAfter = await db.getChannel({ id: GROUP_CHANNEL }); + expect(channelAfter!.order).toEqual([]); + }); }); diff --git a/packages/shared/src/store/postActions/postActions.ts b/packages/shared/src/store/postActions/postActions.ts index 1f597c4169..9c4b3889f8 100644 --- a/packages/shared/src/store/postActions/postActions.ts +++ b/packages/shared/src/store/postActions/postActions.ts @@ -18,6 +18,7 @@ import * as sync from '../sync'; import { clearChannelPostsQueries, deleteFromChannelPosts, + removeFromChannelPosts, rollbackDeletedChannelPost, } from '../useChannelPosts'; import { logger } from './logger'; @@ -717,6 +718,11 @@ function isUnsentOptimisticRow(post: db.Post): boolean { async function clearUnsentPost(post: db.Post) { deleteFromChannelPosts(post); await db.deletePost(post.id); + await finishHardDeletedPost(post); +} + +async function finishHardDeletedPost(post: db.Post) { + removeFromChannelPosts(post); if (post.parentId) { // Optimistic reply creation bumps the parent's replyCount / replyTime / // replyContactIds via `addReplyToPost`. Undo that bump, but do not @@ -782,6 +788,21 @@ export async function deletePost({ post }: { post: db.Post }) { ? channel.order : null; + // Mark the delete as server-confirmed and, for an acknowledged optimistic + // top-level row, remove it only if it is still unsequenced. The database + // predicate checks sequence/status atomically with the DELETE, so an addPost + // echo that lands at the last moment preserves the confirmed tombstone. + // Runs on both the normal success path and the lost-ack verification + // branches — a confirmed delete must never leave a `sequenceNum: 0` ghost + // (and its live `isDeleted` overlay) behind, however it was confirmed. + const settleConfirmedDelete = async () => { + await db.updatePost({ id: post.id, deleteStatus: 'sent' }); + const deletedPost = await db.deleteUnsequencedAcknowledgedPost(post.id); + if (deletedPost) { + await finishHardDeletedPost(deletedPost); + } + }; + // optimistic update deleteFromChannelPosts(post); await db.markPostAsDeleted(post.id); @@ -808,7 +829,8 @@ export async function deletePost({ post }: { post: db.Post }) { }) : api.deletePost(post.channelId, post.id, post.authorId) ); - await db.updatePost({ id: post.id, deleteStatus: 'sent' }); + + await settleConfirmedDelete(); } catch (e) { // A rejected poke may only mean its acknowledgement was lost. Before // restoring a pinned post from a stale snapshot, ask the ship whether @@ -822,7 +844,7 @@ export async function deletePost({ post }: { post: db.Post }) { authorId: post.authorId, }); if (serverPost.isDeleted) { - await db.updatePost({ id: post.id, deleteStatus: 'sent' }); + await settleConfirmedDelete(); return; } } catch (verifyError) { @@ -830,7 +852,7 @@ export async function deletePost({ post }: { post: db.Post }) { verifyError instanceof api.BadResponseError && verifyError.status === 404 ) { - await db.updatePost({ id: post.id, deleteStatus: 'sent' }); + await settleConfirmedDelete(); return; } } diff --git a/packages/shared/src/store/postActions/verifyPostDelivery.test.ts b/packages/shared/src/store/postActions/verifyPostDelivery.test.ts new file mode 100644 index 0000000000..d2ee4ec649 --- /dev/null +++ b/packages/shared/src/store/postActions/verifyPostDelivery.test.ts @@ -0,0 +1,104 @@ +import * as api from '@tloncorp/api'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +import * as db from '../../db'; +import { setupDatabaseTestSuite } from '../../test/helpers'; +import { + DeletedPostState, + subscribeToDeletedPosts, +} from '../useChannelPosts/subscriptions'; +import { verifyPostDelivery } from './verifyPostDelivery'; + +const CHANNEL = 'chat/~zod/verify'; + +setupDatabaseTestSuite(); + +// A user can delete an in-flight send while it is still `pending`, so the +// delete-time hard-delete guard (which matches acknowledged delivery states) +// finds nothing. If that send later times out to `needs_verification` and +// verification confirms it landed, delivery resolves to `sent` HERE rather +// than through `markPostSent`. The settled-delete cleanup must run on this +// path too, or the row lingers as a live tombstone until remount. +describe('verifyPostDelivery settled-delete cleanup', () => { + beforeEach(async () => { + await db.insertChannels([db.buildChannel({ id: CHANNEL, type: 'chat' })]); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + async function seedNeedsVerification( + id: string, + overrides: Partial = {} + ): Promise { + const post = { + id, + type: 'chat', + channelId: CHANNEL, + authorId: '~zod', + sentAt: 1000, + receivedAt: 1000, + sequenceNum: 0, + deliveryStatus: 'needs_verification', + content: JSON.stringify([{ inline: ['delivered while sending'] }]), + syncedAt: Date.now(), + ...overrides, + } as db.Post; + await db.insertChannelPosts({ posts: [post] }); + return post; + } + + test('hard-deletes a settled delete once verification confirms delivery', async () => { + const post = await seedNeedsVerification('settled-delete-verified', { + isDeleted: true, + deleteStatus: 'sent', + }); + vi.spyOn(api, 'getChannelPosts').mockResolvedValue({ + posts: [{ authorId: post.authorId, sentAt: post.sentAt } as db.Post], + totalPosts: 1, + numStubs: 0, + numDeletes: 0, + newestSequenceNum: null, + older: null, + newer: null, + }); + + const removed: string[] = []; + const unsubscribe = subscribeToDeletedPosts( + (id: string, state: DeletedPostState) => { + if (state === 'removed') removed.push(id); + } + ); + + const verified = await verifyPostDelivery(post); + unsubscribe(); + + expect(verified).toBe(true); + expect(await db.getPost({ postId: post.id })).toBeNull(); + expect(removed).toContain(post.id); + }); + + test('leaves a live delivered post untouched', async () => { + const post = await seedNeedsVerification('live-verified'); + vi.spyOn(api, 'getChannelPosts').mockResolvedValue({ + posts: [{ authorId: post.authorId, sentAt: post.sentAt } as db.Post], + totalPosts: 1, + numStubs: 0, + numDeletes: 0, + newestSequenceNum: null, + older: null, + newer: null, + }); + + const verified = await verifyPostDelivery(post); + + expect(verified).toBe(true); + // Not a settled delete, so the cleanup guard leaves it in place; only the + // delivery status advanced. + expect(await db.getPost({ postId: post.id })).toMatchObject({ + id: post.id, + deliveryStatus: 'sent', + }); + }); +}); diff --git a/packages/shared/src/store/postActions/verifyPostDelivery.ts b/packages/shared/src/store/postActions/verifyPostDelivery.ts index fe792465e4..ea200486e6 100644 --- a/packages/shared/src/store/postActions/verifyPostDelivery.ts +++ b/packages/shared/src/store/postActions/verifyPostDelivery.ts @@ -1,6 +1,7 @@ import * as api from '@tloncorp/api'; import * as db from '../../db'; +import { finishSettledDeleteOnDelivery } from './finishSettledDelete'; import { logger } from './logger'; /** @@ -46,6 +47,10 @@ export async function verifyPostDelivery(post: db.Post): Promise { }); await db.updatePost({ id: post.id, deliveryStatus: 'sent' }); + // If the user deleted this send while it was in flight and the delete + // settled first, delivery only just resolved to `sent` here — finish the + // hard-delete so the settled-delete row doesn't linger as a tombstone. + await finishSettledDeleteOnDelivery(post.id); return true; } else { logger.crumb('post verified as not delivered', { postId: post.id }); diff --git a/packages/shared/src/store/sync/sync.test.ts b/packages/shared/src/store/sync/sync.test.ts index 28c2811db6..b8af6b14cd 100644 --- a/packages/shared/src/store/sync/sync.test.ts +++ b/packages/shared/src/store/sync/sync.test.ts @@ -26,6 +26,7 @@ import { expect, test, vi } from 'vitest'; import rawChannelPostWithRepliesData from '../../../../api/src/__tests__/fixtures/channelPostWithReplies.json'; import rawChannelPostsData from '../../../../api/src/__tests__/fixtures/channelPosts.json'; import * as db from '../../db'; +import { batchEffects } from '../../db/query'; import rawNewestPostData from '../../test/channelNewestPost.json'; import rawAfterNewestPostData from '../../test/channelPostsAfterNewest.json'; import rawContactsData from '../../test/contacts.json'; @@ -39,8 +40,10 @@ import { setupDatabaseTestSuite, } from '../../test/helpers'; import rawGroupsInit2 from '../../test/init.json'; +import { subscribeToDeletedPosts } from '../useChannelPosts/subscriptions'; import { ensureDmInviteChannel, + handleChannelsUpdate, syncChannelWithBackoff, syncDms, syncGroups, @@ -188,6 +191,9 @@ test('syncChannelWithBackoff keeps polling when a deleted row is still in flight deliveryStatus: 'pending', // User deleted the optimistic post mid-flight. isDeleted: true, + // The delete request can settle before the independent original send + // lifecycle does. Delivery polling must continue in this shape. + deleteStatus: 'sent', syncedAt: Date.now(), } as unknown as db.Post, ], @@ -195,9 +201,9 @@ test('syncChannelWithBackoff keeps polling when a deleted row is still in flight // Delivery polling query keeps the row visible — still in flight. expect((await db.getDeliveryPendingPosts(channelId)).length).toBe(1); - // UI query also surfaces it as a tombstone source so remount renders a - // "Message deleted" row instead of a gap while the send reconciles. - expect((await db.getPendingPosts(channelId)).length).toBe(1); + // The UI pending layer hides the settled deletion, independently of the + // original send lifecycle that delivery polling still needs to reconcile. + expect((await db.getPendingPosts(channelId)).length).toBe(0); }); // TLON-5606 regression guard: deleted rows with the final local-only shape @@ -278,6 +284,50 @@ test('syncChannelWithBackoff keeps polling across the markPostSent catch-up wind ).not.toContain(catchUpId); }); +// A user can delete an optimistic post while its send is still in flight; the +// delete settles first, so the guarded hard-delete can't run yet. When the +// send finally resolves, `markPostSent` must finish the hard-delete — both +// removing the DB row and broadcasting the `'removed'` overlay — so a mounted +// channel's live snapshot doesn't keep rendering the settled-delete tombstone. +test('markPostSent finishes a delete that settled while the send was in flight', async () => { + const channelId = 'settled-delete-on-send-sync-channel'; + await db.insertChannels([{ id: channelId, type: 'chat' }]); + + const postId = 'settled-delete-inflight'; + await db.insertChannelPosts({ + posts: [ + { + id: postId, + type: 'chat', + channelId, + authorId: '~zod', + sentAt: Date.now(), + receivedAt: Date.now(), + sequenceNum: 0, + content: JSON.stringify([{ inline: ['deleted while sending'] }]), + // Delete already settled, but the send is still in flight. + deliveryStatus: 'pending', + isDeleted: true, + deleteStatus: 'sent', + syncedAt: Date.now(), + } as unknown as db.Post, + ], + }); + + const removedIds: string[] = []; + const unsubscribe = subscribeToDeletedPosts((id, state) => { + if (state === 'removed') removedIds.push(id); + }); + + await batchEffects('test markPostSent', (ctx) => + handleChannelsUpdate({ type: 'markPostSent', cacheId: postId }, ctx) + ); + unsubscribe(); + + expect(await db.getPost({ postId })).toBeNull(); + expect(removedIds).toContain(postId); +}); + test('syncs contacts', async () => { setScryOutputs([contactsData, contactBookData, suggestionsData]); await syncContacts(); diff --git a/packages/shared/src/store/sync/sync.ts b/packages/shared/src/store/sync/sync.ts index 9845aa7b00..5fc45706fb 100644 --- a/packages/shared/src/store/sync/sync.ts +++ b/packages/shared/src/store/sync/sync.ts @@ -31,6 +31,7 @@ import { partitionDiscoveryMatches, } from '../lanyardActions'; import { useLureState } from '../lure'; +import { finishSettledDeleteOnDelivery } from '../postActions/finishSettledDelete'; import { verifyPostDelivery } from '../postActions/verifyPostDelivery'; import { clearPresenceState, handlePresenceEvent } from '../presence'; import { getSession, setSession, updateSession } from '../session'; @@ -1630,6 +1631,12 @@ export const handleChannelsUpdate = async ( } case 'markPostSent': await db.updatePost({ id: update.cacheId, deliveryStatus: 'sent' }, ctx); + // If this row's delete already settled while the send was still in + // flight, the guarded hard-delete couldn't run at delete time (delivery + // wasn't acknowledged yet). Now that delivery has resolved, finish it so + // a mounted channel's live snapshot doesn't keep rendering the + // settled-delete row as a tombstone until remount. + await finishSettledDeleteOnDelivery(update.cacheId, ctx); break; case 'initialPostsOnChannelJoin': await db.insertChannelPosts( diff --git a/packages/shared/src/store/useChannelPosts/index.ts b/packages/shared/src/store/useChannelPosts/index.ts index 462f48e03c..aa9689ceac 100644 --- a/packages/shared/src/store/useChannelPosts/index.ts +++ b/packages/shared/src/store/useChannelPosts/index.ts @@ -2,6 +2,7 @@ export * from './useChannelPosts'; export { addToChannelPosts, deleteFromChannelPosts, + removeFromChannelPosts, rollbackDeletedChannelPost, } from './subscriptions'; export { clearChannelPostsQueries } from './queries'; diff --git a/packages/shared/src/store/useChannelPosts/subscriptions.ts b/packages/shared/src/store/useChannelPosts/subscriptions.ts index bb7dc1440c..153e2f63ff 100644 --- a/packages/shared/src/store/useChannelPosts/subscriptions.ts +++ b/packages/shared/src/store/useChannelPosts/subscriptions.ts @@ -25,27 +25,33 @@ export const useNewPostListener = (listener: SubscriptionPostListener) => { }, [listener]); }; -type DeletedPostListener = (postId: string, isDeleted: boolean) => void; +export type DeletedPostState = boolean | 'removed'; + +type DeletedPostListener = (postId: string, state: DeletedPostState) => void; const deletedPostListeners: DeletedPostListener[] = []; +export const subscribeToDeletedPosts = (listener: DeletedPostListener) => { + deletedPostListeners.push(listener); + return () => { + const index = deletedPostListeners.indexOf(listener); + if (index !== -1) { + deletedPostListeners.splice(index, 1); + } + }; +}; + const useDeletedPostListener = (listener: DeletedPostListener) => { - useEffect(() => { - deletedPostListeners.push(listener); - return () => { - const index = deletedPostListeners.indexOf(listener); - if (index !== -1) { - deletedPostListeners.splice(index, 1); - } - }; - }, [listener]); + useEffect(() => subscribeToDeletedPosts(listener), [listener]); }; export const useDeletedPosts = (channelId: string) => { - const [deletedPosts, setDeletedPosts] = useState>({}); + const [deletedPosts, setDeletedPosts] = useState< + Record + >({}); const handleDeletedPost = useCallback( - (postId: string, isDeleted: boolean) => { - setDeletedPosts((value) => ({ ...value, [postId]: isDeleted })); + (postId: string, state: DeletedPostState) => { + setDeletedPosts((value) => ({ ...value, [postId]: state })); }, [] ); @@ -67,6 +73,16 @@ export const deleteFromChannelPosts = (post: db.Post) => { deletedPostListeners.forEach((listener) => listener(post.id, true)); }; +/** + * Remove a hard-deleted post from the live merge inputs as well as marking it + * deleted. This is distinct from the optimistic delete signal above: the + * server outcome is settled, so a stale snapshot in `newPosts` must disappear + * instead of rendering as a tombstone until remount. + */ +export const removeFromChannelPosts = (post: db.Post) => { + deletedPostListeners.forEach((listener) => listener(post.id, 'removed')); +}; + export const rollbackDeletedChannelPost = (post: db.Post) => { deletedPostListeners.forEach((listener) => listener(post.id, false)); }; diff --git a/packages/shared/src/store/useMergePendingPosts.ts b/packages/shared/src/store/useMergePendingPosts.ts index 77fd779e34..271e97fac4 100644 --- a/packages/shared/src/store/useMergePendingPosts.ts +++ b/packages/shared/src/store/useMergePendingPosts.ts @@ -1,4 +1,5 @@ import * as db from '../db'; +import type { DeletedPostState } from './useChannelPosts/subscriptions'; /** * Pending posts aren't assigned sequence numbers, so we need to weave them into the existing posts @@ -7,7 +8,7 @@ import * as db from '../db'; * @param newPosts An array of posts that have come in since we started querying * @param pendingPosts An array of all pending posts for the channel, sorted newest first. * @param existingPosts A contiguous sequence of confirmed posts, sorted newest first. - * @param deletedPosts A map of post IDs to a boolean indicating if the post has been deleted. + * @param deletedPosts A map of post IDs to their live deletion state. * @param hasNewest A boolean indicating if existingPosts represents the newest available messages. * @param filterDeleted A boolean indicating if deleted posts should be filtered out. * @returns A single array of posts, sorted newest first, with relevant pending posts woven in. @@ -23,7 +24,7 @@ export const mergePendingPosts = ({ newPosts: db.Post[]; pendingPosts: db.Post[]; existingPosts: db.Post[]; - deletedPosts: Record; + deletedPosts: Record; hasNewest: boolean; filterDeleted?: boolean; }): db.Post[] => { @@ -35,10 +36,14 @@ export const mergePendingPosts = ({ return keys; }; const deletedPostKeys = new Set(); + const removedPostKeys = new Set(); [...newPosts, ...pendingPosts, ...existingPosts].forEach((post) => { if (post.isDeleted || deletedPosts[post.id]) { postMergeKeys(post).forEach((key) => deletedPostKeys.add(key)); } + if (deletedPosts[post.id] === 'removed') { + postMergeKeys(post).forEach((key) => removedPostKeys.add(key)); + } }); const hasDeletedOverlay = (post: db.Post) => { return ( @@ -47,6 +52,12 @@ export const mergePendingPosts = ({ postMergeKeys(post).some((key) => deletedPostKeys.has(key)) ); }; + const hasRemovedOverlay = (post: db.Post) => { + return ( + deletedPosts[post.id] === 'removed' || + postMergeKeys(post).some((key) => removedPostKeys.has(key)) + ); + }; // Drop **truly local-only** rows the user has locally cleared. The send // either failed outright or was never acknowledged by the server, so the @@ -80,7 +91,9 @@ export const mergePendingPosts = ({ }); const sentAtMap = new Map(); [...newPosts, ...pendingPosts] - .filter((post) => !isLocallyClearedOptimistic(post)) + .filter( + (post) => !hasRemovedOverlay(post) && !isLocallyClearedOptimistic(post) + ) .forEach((post) => { if (!sentAtMap.has(post.sentAt)) { sentAtMap.set(post.sentAt, post);