🐛 Fixed duplicate replies when replying after opening a comment permalink

jonatansberg · jonatansberg · commit d3d7c1982753 · 2026-06-11T08:44:44.000+02:00
ref https://linear.app/ghost/issue/BER-3706 - Replies are returned ordered by created_at, but the client paginated them with an `id:>` cursor; when created_at order diverges from id order (e.g. imported or backdated comments) the cursor re-returned already-loaded replies, which were appended without de-duplication and rendered twice — most visibly after a permalink bulk-loads every reply and the member then clicks Reply. - Switched the bulk reply load to page-based pagination (limit=100&page=N) over the server's stable order, so there is no client-side cursor to diverge and no reliance on limit=all (which Ghost 6 clamps to 100). loadMoreReplies rebuilds the replies array from the fetched set and de-dupes by id, while preserving any just-posted reply the cache has not returned yet. - Removed the unreachable admin reply branch; the only caller always loads replies via the members API.
diff --git a/apps/comments-ui/src/actions.ts b/apps/comments-ui/src/actions.ts
@@ -69,48 +69,34 @@ async function setOrder({state, data: {order}, options, api, dispatchAction}: {s
     }
 }
 
-async function loadMoreReplies({state, api, data: {comment, limit}, isReply}: {state: EditableAppContext, api: GhostApi, data: {comment: Comment, limit?: number | 'all'}, isReply: boolean}): Promise<Partial<EditableAppContext>> {
-    const fetchReplies = async (afterReplyId: string | undefined, requestLimit: number) => {
-        if (state.admin && state.adminApi && !isReply) { // we don't want the admin api to load reply data for replying to a reply, so we pass isReply: true
-            return await state.adminApi.replies({commentId: comment.id, afterReplyId, limit: requestLimit, memberUuid: state.member?.uuid});
-        } else {
-            return await api.comments.replies({commentId: comment.id, afterReplyId, limit: requestLimit});
-        }
-    };
-
-    let afterReplyId: string | undefined = comment.replies && comment.replies.length > 0
-        ? comment.replies[comment.replies.length - 1]?.id
-        : undefined;
-
-    let allComments: Comment[] = [];
-
-    if (limit === 'all') {
-        let hasMore = true;
-
-        while (hasMore) {
-            const data = await fetchReplies(afterReplyId, 100);
-            allComments.push(...data.comments);
-            hasMore = !!data.meta?.pagination?.next;
-
-            if (data.comments && data.comments.length > 0) {
-                afterReplyId = data.comments[data.comments.length - 1]?.id;
-            } else {
-                // If no comments returned, stop pagination to prevent infinite loop
-                hasMore = false;
-            }
+async function loadMoreReplies({state, api, data: {comment, limit}}: {state: EditableAppContext, api: GhostApi, data: {comment: Comment, limit?: number | 'all'}}): Promise<Partial<EditableAppContext>> {
+    const data = await api.comments.replies({commentId: comment.id, limit});
+
+    // The freshly fetched set is authoritative for ordering and completeness, so we
+    // rebuild the replies array from it rather than appending with a cursor (that
+    // re-fetched already-loaded replies and duplicated them — BER-3706). But a reply
+    // the member just posted may not be in the (cached) response yet — addReply keeps
+    // it locally — so preserve any already-loaded replies the server didn't return.
+    // De-dupe by id so nothing can render twice.
+    const fetchedReplies: Comment[] = data.comments;
+    const fetchedReplyIds = new Set<string>(fetchedReplies.map((reply: Comment) => reply.id));
+    const pendingReplies = comment.replies.filter(reply => !fetchedReplyIds.has(reply.id));
+
+    const seenReplyIds = new Set<string>();
+    const replies: Comment[] = [...fetchedReplies, ...pendingReplies].filter((reply) => {
+        if (seenReplyIds.has(reply.id)) {
+            return false;
         }
-    } else {
-        const data = await fetchReplies(afterReplyId, limit as number || 100);
-        allComments = data.comments;
-    }
+        seenReplyIds.add(reply.id);
+        return true;
+    });
 
-    // Note: we store the comments from new to old, and show them in reverse order
     return {
         comments: state.comments.map((c) => {
             if (c.id === comment.id) {
                 return {
                     ...comment,
-                    replies: [...comment.replies, ...allComments]
+                    replies
                 };
             }
             return c;
@@ -710,7 +696,7 @@ async function openCommentForm({data: newForm, api, state}: {data: OpenCommentFo
 
         if (comment) {
             try {
-                const newCommentsState = await loadMoreReplies({state, api, data: {comment, limit: 'all'}, isReply: true});
+                const newCommentsState = await loadMoreReplies({state, api, data: {comment, limit: 'all'}});
                 otherStateChanges = {...otherStateChanges, ...newCommentsState};
             } catch (e) {
                 // If loading replies fails, continue anyway - the form should still open
diff --git a/apps/comments-ui/src/utils/api.ts b/apps/comments-ui/src/utils/api.ts
@@ -169,19 +169,22 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}: {site
 
                 return response;
             },
-            async replies({commentId, afterReplyId, limit}: {commentId: string; afterReplyId?: string; limit?: number | 'all'}) {
+            async replies({commentId, afterReplyId, limit, page}: {commentId: string; afterReplyId?: string; limit?: number | 'all'; page?: number}) {
                 if (limit === 'all') {
+                    // Walk the pages instead of seeking with an `id:>` cursor: replies
+                    // are ordered by (created_at ASC, id ASC), and an id cursor breaks
+                    // whenever created_at order diverges from id order (e.g. imported or
+                    // backdated comments), re-fetching already-loaded replies (BER-3706).
+                    // Page-based pagination over that stable order is dialect-agnostic.
                     const all: Comment[] = [];
-                    let cursor: string | undefined = afterReplyId;
+                    let currentPage = 1;
                     let hasMore = true;
 
                     while (hasMore) {
-                        const data = await this.replies({commentId, afterReplyId: cursor, limit: 100});
+                        const data = await this.replies({commentId, limit: 100, page: currentPage});
                         all.push(...data.comments);
                         hasMore = !!data.meta?.pagination?.next && data.comments.length > 0;
-                        if (data.comments.length > 0) {
-                            cursor = data.comments[data.comments.length - 1]?.id;
-                        }
+                        currentPage += 1;
                     }
 
                     return {comments: all, meta: {pagination: {next: false}}};
@@ -190,6 +193,10 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}: {site
                 const params = new URLSearchParams();
                 params.set('limit', (limit ?? 5).toString());
 
+                if (page) {
+                    params.set('page', page.toString());
+                }
+
                 if (afterReplyId) {
                     params.set('filter', `id:>'${afterReplyId}'`);
                 }
diff --git a/apps/comments-ui/test/unit/actions.test.js b/apps/comments-ui/test/unit/actions.test.js
@@ -81,6 +81,78 @@ describe('Actions', function () {
         });
     });
 
+    describe('loadMoreReplies', function () {
+        it('rebuilds the replies from the freshly loaded full set', async function () {
+            // Only a partial set of replies is loaded inline...
+            const comment = makeComment({
+                id: 'comment-1',
+                replies: [
+                    makeComment({id: 'reply-a', created_at: '2026-06-04T09:00:07.000Z', parent_id: 'comment-1'})
+                ]
+            });
+            const state = {comments: [comment]};
+            // ...and the server returns the full set (ordered created_at ASC, id ASC).
+            const fullReplies = [
+                makeComment({id: 'reply-a', created_at: '2026-06-04T09:00:07.000Z', parent_id: 'comment-1'}),
+                makeComment({id: 'reply-b', created_at: '2026-06-04T09:00:30.000Z', parent_id: 'comment-1'})
+            ];
+            const api = {
+                comments: {
+                    replies: vi.fn(() => Promise.resolve({comments: fullReplies, meta: {pagination: {next: null}}}))
+                }
+            };
+
+            const newState = await Actions.loadMoreReplies({state, api, data: {comment, limit: 'all'}});
+
+            expect(api.comments.replies).toHaveBeenCalledWith({commentId: 'comment-1', limit: 'all'});
+            expect(newState.comments[0].replies.map(reply => reply.id)).toEqual(['reply-a', 'reply-b']);
+        });
+
+        it('de-duplicates replies so none render twice (BER-3706)', async function () {
+            const comment = makeComment({
+                id: 'comment-1',
+                replies: [
+                    makeComment({id: 'reply-a', created_at: '2026-06-04T09:00:07.000Z', parent_id: 'comment-1'})
+                ]
+            });
+            const state = {comments: [comment]};
+            // Simulate a response that overlaps the same reply across pages.
+            const replyA = makeComment({id: 'reply-a', created_at: '2026-06-04T09:00:07.000Z', parent_id: 'comment-1'});
+            const replyB = makeComment({id: 'reply-b', created_at: '2026-06-04T09:00:30.000Z', parent_id: 'comment-1'});
+            const api = {
+                comments: {
+                    replies: vi.fn(() => Promise.resolve({comments: [replyA, replyB, replyB], meta: {pagination: {next: null}}}))
+                }
+            };
+
+            const newState = await Actions.loadMoreReplies({state, api, data: {comment, limit: 'all'}});
+
+            expect(newState.comments[0].replies.map(reply => reply.id)).toEqual(['reply-a', 'reply-b']);
+        });
+
+        it('preserves a just-posted reply the server refetch has not caught up to yet', async function () {
+            // addReply keeps a locally-posted reply when a cached GET omits it; opening
+            // another reply form must not drop it when reloading replies.
+            const serverReply = makeComment({id: 'reply-a', created_at: '2026-06-04T09:00:07.000Z', parent_id: 'comment-1'});
+            const pendingReply = makeComment({id: 'reply-b', created_at: '2026-06-04T09:00:30.000Z', parent_id: 'comment-1'});
+            const comment = makeComment({
+                id: 'comment-1',
+                replies: [serverReply, pendingReply]
+            });
+            const state = {comments: [comment]};
+            // Stale response: missing the just-posted reply-b.
+            const api = {
+                comments: {
+                    replies: vi.fn(() => Promise.resolve({comments: [serverReply], meta: {pagination: {next: null}}}))
+                }
+            };
+
+            const newState = await Actions.loadMoreReplies({state, api, data: {comment, limit: 'all'}});
+
+            expect(newState.comments[0].replies.map(reply => reply.id)).toEqual(['reply-a', 'reply-b']);
+        });
+    });
+
     describe('addReply', function () {
         it('adds the created reply when the replies refetch is stale', async function () {
             const newReply = makeComment({
diff --git a/apps/comments-ui/test/utils/mocked-api.ts b/apps/comments-ui/test/utils/mocked-api.ts
@@ -257,7 +257,7 @@ export class MockedApi {
         };
     }
 
-    browseReplies({commentId, filter, limit = 5}: {commentId: string, filter?: string, limit?: number}) {
+    browseReplies({commentId, filter, limit = 5, page = 1}: {commentId: string, filter?: string, limit?: number, page?: number}) {
         const comment = this.comments.find(c => c.id === commentId);
         if (!comment) {
             return {
@@ -288,18 +288,19 @@ export class MockedApi {
             });
         }
 
-        const limitedReplies = filteredReplies.slice(0, limit);
-        const hasMore = filteredReplies.length > limit;
+        const startIndex = (page - 1) * limit;
+        const limitedReplies = filteredReplies.slice(startIndex, startIndex + limit);
+        const hasMore = filteredReplies.length > startIndex + limit;
 
         return {
             comments: limitedReplies,
             meta: {
                 pagination: {
-                    page: 1,
+                    page,
                     pages: Math.ceil(filteredReplies.length / limit),
                     total: filteredReplies.length,
                     limit,
-                    next: hasMore ? 2 : null,
+                    next: hasMore ? page + 1 : null,
                     prev: null
                 }
             }
@@ -528,13 +529,15 @@ export class MockedApi {
             const url = new URL(route.request().url());
 
             const limit = parseInt(url.searchParams.get('limit') ?? '5');
+            const page = parseInt(url.searchParams.get('page') ?? '1');
             const commentId = url.pathname.split('/').reverse()[2];
             const filter = url.searchParams.get('filter') ?? '';
 
             await route.fulfill({
                 status: 200,
                 body: JSON.stringify(this.browseReplies({
                     limit,
+                    page,
                     filter,
                     commentId
                 }))
