🐛 Fixed duplicate replies when replying after opening a comment permalink (#28504)

ref https://linear.app/ghost/issue/BER-3706

Renoved the problematic reload since the comments API already returns every reply inline, so reloading them when a reply form opens was redundant and duplicated them.

Author: jonatansberg

apps/comments-ui/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@tryghost/comments-ui",
-  "version": "1.5.8",
+  "version": "1.5.9",
   "license": "MIT",
   "repository": "https://github.com/TryGhost/Ghost",
   "author": "Ghost Foundation",

apps/comments-ui/src/actions.ts

@@ -69,55 +69,6 @@ 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;
-            }
-        }
-    } else {
-        const data = await fetchReplies(afterReplyId, limit as number || 100);
-        allComments = data.comments;
-    }
-
-    // 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]
-                };
-            }
-            return c;
-        })
-    };
-}
-
 async function addComment({state, api, data: comment}: {state: EditableAppContext, api: GhostApi, data: AddComment}) {
     const data = await api.comments.add({comment});
     const newComment = data.comments[0];
@@ -699,27 +650,7 @@ function closePopup() {
     };
 }
 
-async function openCommentForm({data: newForm, api, state}: {data: OpenCommentForm, api: GhostApi, state: EditableAppContext}) {
-    let otherStateChanges = {};
-
-    // When opening a reply form, load all replies for the parent comment so the
-    // reply appears in the correct position after posting
-    const topLevelCommentId = newForm.parent_id || newForm.id;
-    if (newForm.type === 'reply' && !state.openCommentForms.some(f => f.id === topLevelCommentId || f.parent_id === topLevelCommentId)) {
-        const comment = state.comments.find(c => c.id === topLevelCommentId);
-
-        if (comment) {
-            try {
-                const newCommentsState = await loadMoreReplies({state, api, data: {comment, limit: 'all'}, isReply: true});
-                otherStateChanges = {...otherStateChanges, ...newCommentsState};
-            } catch (e) {
-                // If loading replies fails, continue anyway - the form should still open
-                // and replies will be loaded when the user submits
-                console.error('[Comments] Failed to load replies before opening form:', e); // eslint-disable-line no-console
-            }
-        }
-    }
-
+function openCommentForm({data: newForm, state}: {data: OpenCommentForm, state: EditableAppContext}) {
     // We want to keep the number of displayed forms to a minimum so when opening a
     // new form, we close any existing forms that are empty or have had no changes
     const openFormsAfterAutoclose = state.openCommentForms.filter(form => form.hasUnsavedChanges);
@@ -729,9 +660,9 @@ async function openCommentForm({data: newForm, api, state}: {data: OpenCommentFo
     const openFormIndexForId = openFormsAfterAutoclose.findIndex(form => form.id === newForm.id);
     if (openFormIndexForId > -1) {
         openFormsAfterAutoclose[openFormIndexForId] = newForm;
-        return {openCommentForms: openFormsAfterAutoclose, ...otherStateChanges};
+        return {openCommentForms: openFormsAfterAutoclose};
     } else {
-        return {openCommentForms: [...openFormsAfterAutoclose, newForm], ...otherStateChanges};
+        return {openCommentForms: [...openFormsAfterAutoclose, newForm]};
     }
 }
 
@@ -809,7 +740,6 @@ export const Actions = {
     reportComment,
     addReply,
     loadMoreComments,
-    loadMoreReplies,
     openCommentForm,
     updateMember,
     setOrder,

apps/comments-ui/src/utils/api.ts

@@ -169,19 +169,19 @@ 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') {
+                    // Paginate by page, not an `id:>` cursor: replies are ordered by
+                    // created_at, so an id cursor can re-fetch or skip replies (BER-3706).
                     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 +190,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}'`);
                 }

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
                 }))