From efb521f47286a368790b179a6a57c60389f6c66d Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 27 May 2026 13:39:12 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20hidden=20comment=20perma?= =?UTF-8?q?links=20for=20admins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://linear.app/ghost/issue/BER-3686/hidden-comment-text-not-shown-when-loading-via-permalink Admins can land on hidden comments from moderation links, so permalink loading needs to resolve hidden targets through the admin API after authentication without replacing existing permalink state when that lookup fails. --- apps/comments-ui/package.json | 2 +- apps/comments-ui/src/app.tsx | 80 +++++--- apps/comments-ui/test/e2e/editor.test.ts | 3 +- apps/comments-ui/test/e2e/permalink.test.ts | 200 ++++++++------------ apps/comments-ui/test/utils/e2e.ts | 11 +- 5 files changed, 151 insertions(+), 145 deletions(-) diff --git a/apps/comments-ui/package.json b/apps/comments-ui/package.json index 36885002033..a297133c50d 100644 --- a/apps/comments-ui/package.json +++ b/apps/comments-ui/package.json @@ -1,6 +1,6 @@ { "name": "@tryghost/comments-ui", - "version": "1.5.4", + "version": "1.5.5", "license": "MIT", "repository": "https://github.com/TryGhost/Ghost", "author": "Ghost Foundation", diff --git a/apps/comments-ui/src/app.tsx b/apps/comments-ui/src/app.tsx index 9224ea80c6e..aa735d4252f 100644 --- a/apps/comments-ui/src/app.tsx +++ b/apps/comments-ui/src/app.tsx @@ -18,6 +18,11 @@ type AppProps = { pageUrl: string; }; +type Pagination = NonNullable; +type CommentsRequestApi = { + browse: ({page, postId, order}: {page: number; postId: string; order?: string}) => Promise<{comments: Comment[]; meta: {pagination: Pagination}}>; +}; + const ALLOWED_MODERATORS = ['Owner', 'Administrator', 'Super Editor']; const LIKES_ORDER = 'count__likes desc, created_at desc'; const NET_SCORE_ORDER = 'count__net_score desc, created_at desc'; @@ -148,11 +153,45 @@ const App: React.FC = ({scriptTag, initialCommentId, pageUrl}) => { if (admin) { // this is a bit of a hack, but we need to fetch the comments fully populated if the user is an admin - const adminComments = await adminApi.browse({page: 1, postId: options.postId, order: state.order, memberUuid: state.member?.uuid}); + const memberUuid = state.member?.uuid; + const adminComments = await adminApi.browse({page: 1, postId: options.postId, order: state.order, memberUuid}); + let adminPermalinkState: Partial | null = null; + + if (initialCommentId) { + try { + const targetResponse = await adminApi.read({commentId: initialCommentId, memberUuid}); + const targetComment = targetResponse.comments?.[0]; + if (targetComment) { + const result = await loadScrollTarget( + initialCommentId, + targetComment, + adminComments.comments, + adminComments.meta.pagination, + state.order, + { + browse: ({page, postId, order}) => adminApi.browse({page, postId, order, memberUuid}) + } + ); + + if (result.found) { + adminPermalinkState = { + comments: result.comments, + pagination: result.pagination, + commentIdToScrollTo: initialCommentId, + commentIdFromHash: initialCommentId, + showMissingCommentNotice: false + }; + } + } + } catch { + // Keep the public API permalink result when the admin API can't load this target. + } + } + setState((currentState) => { // Don't overwrite comments when initSetup loaded extra data // for permalink scrolling (multiple pages or expanded replies) - if ((currentState.pagination && currentState.pagination.page > 1) || initialCommentId) { + if ((currentState.pagination && currentState.pagination.page > 1 && !initialCommentId) || (initialCommentId && !adminPermalinkState)) { return { adminApi, admin, @@ -163,9 +202,12 @@ const App: React.FC = ({scriptTag, initialCommentId, pageUrl}) => { adminApi, admin, isAdmin: true, - comments: adminComments.comments, - pagination: adminComments.meta.pagination, - capabilities: adminComments.meta?.capabilities ?? currentState.capabilities + comments: adminPermalinkState?.comments ?? adminComments.comments, + pagination: adminPermalinkState?.pagination ?? adminComments.meta.pagination, + capabilities: adminComments.meta?.capabilities ?? currentState.capabilities, + commentIdToScrollTo: adminPermalinkState?.commentIdToScrollTo ?? currentState.commentIdToScrollTo, + commentIdFromHash: adminPermalinkState?.commentIdFromHash ?? currentState.commentIdFromHash, + showMissingCommentNotice: adminPermalinkState?.showMissingCommentNotice ?? currentState.showMissingCommentNotice }; }); } @@ -222,18 +264,20 @@ const App: React.FC = ({scriptTag, initialCommentId, pageUrl}) => { targetId: string, parentId: string | undefined, initialComments: Comment[], - initialPagination: {page: number; pages: number}, - order = state.order + initialPagination: Pagination, + order = state.order, + requestApi?: CommentsRequestApi ): Promise<{comments: Comment[]; pagination: typeof initialPagination}> => { let comments = initialComments; let pagination = initialPagination; + const commentsApi = requestApi ?? {browse: api.comments.browse}; while (!isCommentLoaded(comments, targetId) && pagination.page < pagination.pages) { if (parentId && comments.some(c => c.id === parentId)) { break; } - const nextPage = await api.comments.browse({ + const nextPage = await commentsApi.browse({ page: pagination.page + 1, postId: options.postId, order @@ -246,29 +290,21 @@ const App: React.FC = ({scriptTag, initialCommentId, pageUrl}) => { }; /** - * Load additional comment pages and/or replies until the scroll - * target is found. After paginating to the parent comment, if the - * target reply isn't in the inline replies (partial API response), - * fetch all replies from the server. + * Load additional comment pages until the scroll target is found. */ const loadScrollTarget = async ( targetId: string, targetComment: Comment, initialComments: Comment[], - initialPagination: {page: number; pages: number}, - order = state.order + initialPagination: Pagination, + order = state.order, + requestApi?: CommentsRequestApi ): Promise<{comments: Comment[]; pagination: typeof initialPagination; found: boolean}> => { const parentId = targetComment.parent_id; - const {comments: paginatedComments, pagination} = await paginateToComment(targetId, parentId, initialComments, initialPagination, order); - let comments = paginatedComments; - - if (parentId && !isCommentLoaded(comments, targetId)) { - const {comments: allReplies} = await api.comments.replies({commentId: parentId, limit: 'all'}); - comments = comments.map(c => (c.id === parentId ? {...c, replies: allReplies} : c)); - } + const {comments: paginatedComments, pagination} = await paginateToComment(targetId, parentId, initialComments, initialPagination, order, requestApi); - return {comments, pagination, found: isCommentLoaded(comments, targetId)}; + return {comments: paginatedComments, pagination, found: isCommentLoaded(paginatedComments, targetId)}; }; /** Initialize comments setup once in viewport, fetch data and setup state */ diff --git a/apps/comments-ui/test/e2e/editor.test.ts b/apps/comments-ui/test/e2e/editor.test.ts index 205390c4eaa..16d298f6a82 100644 --- a/apps/comments-ui/test/e2e/editor.test.ts +++ b/apps/comments-ui/test/e2e/editor.test.ts @@ -192,7 +192,7 @@ test.describe('Editor', async () => { // Check comment expect(mockedApi.comments).toHaveLength(1); - expect(mockedApi.comments[0].html).toBe('

This is a quote

This is a new line

This is a new paragraph

'); + expect(mockedApi.comments[0].html).toBe('

This is a quote

This is a new line

This is a new paragraph

'); }); test('Can paste an URL to create a link', async ({page}) => { @@ -349,4 +349,3 @@ test.describe('Editor', async () => { }); }); }); - diff --git a/apps/comments-ui/test/e2e/permalink.test.ts b/apps/comments-ui/test/e2e/permalink.test.ts index 2b0c001e3e3..f8d8ebb7947 100644 --- a/apps/comments-ui/test/e2e/permalink.test.ts +++ b/apps/comments-ui/test/e2e/permalink.test.ts @@ -12,7 +12,8 @@ async function setupPermalinkTest( mockedApi: MockedApi, hash: string, bodyHtml = '', - labs = {} + labs = {}, + admin?: string ): Promise { const sitePath = MOCKED_SITE_URL; mockedApi.setSettings({ @@ -21,6 +22,10 @@ async function setupPermalinkTest( } }); + if (admin) { + await mockAdminAuthFrame({page, admin}); + } + await page.route(sitePath, async (route) => { await route.fulfill({status: 200, body: bodyHtml}); }); @@ -37,7 +42,8 @@ async function setupPermalinkTest( ghostComments: MOCKED_SITE_URL, postId: mockedApi.postId, api: MOCKED_SITE_URL, - key: '12345678' + key: '12345678', + ...(admin ? {admin} : {}) }; await page.evaluate((data) => { @@ -236,6 +242,55 @@ test.describe('Comment Permalinks', async () => { await expect(comments).toHaveCount(1); }); + test('shows hidden permalink comment content for admins', async ({page}) => { + const mockedApi = new MockedApi({}); + mockedApi.setMember({}); + const admin = MOCKED_SITE_URL + '/ghost/'; + + mockedApi.addComment({ + html: '

Visible comment

' + }); + + const hiddenCommentId = '64a1b2c3d4e5f6a7b8c9d0e2'; + mockedApi.addComment({ + id: hiddenCommentId, + html: '

Hidden comment content

', + status: 'hidden' + }); + + const commentsFrame = await setupPermalinkTest(page, mockedApi, `#ghost-comments-${hiddenCommentId}`, TALL_BODY_HTML, {}, admin); + + await expect(commentsFrame.locator(`[id="${hiddenCommentId}"]`)).toBeVisible(); + await expect(commentsFrame.getByText('Hidden comment content')).toBeVisible(); + await expect(commentsFrame.getByText('Hidden for members')).toBeVisible(); + await expect(commentsFrame.getByTestId('missing-comment-notice')).toHaveCount(0); + }); + + test('shows hidden permalink comment content for admins when target is on a later page', async ({page}) => { + const mockedApi = new MockedApi({}); + mockedApi.setMember({}); + const admin = MOCKED_SITE_URL + '/ghost/'; + + for (let i = 1; i <= 24; i++) { + mockedApi.addComment({ + html: `

Filler comment ${i}

` + }); + } + + const hiddenCommentId = '64a1b2c3d4e5f6a7b8c9d0e4'; + mockedApi.addComment({ + id: hiddenCommentId, + html: '

Hidden comment beyond page one

', + status: 'hidden' + }); + + const commentsFrame = await setupPermalinkTest(page, mockedApi, `#ghost-comments-${hiddenCommentId}`, TALL_BODY_HTML, {}, admin); + + await expect(commentsFrame.locator(`[id="${hiddenCommentId}"]`)).toBeVisible(); + await expect(commentsFrame.getByText('Hidden comment beyond page one')).toBeVisible(); + await expect(commentsFrame.getByTestId('missing-comment-notice')).toHaveCount(0); + }); + test('shows missing-comment notice for deleted permalink and scrolls to comments area', async ({page}) => { const mockedApi = new MockedApi({}); mockedApi.setMember({}); @@ -271,6 +326,32 @@ test.describe('Comment Permalinks', async () => { await expect(comments).toHaveCount(1); }); + test('shows deleted permalink tombstone for admins when target has replies', async ({page}) => { + const mockedApi = new MockedApi({}); + mockedApi.setMember({}); + const admin = MOCKED_SITE_URL + '/ghost/'; + + const deletedCommentId = '64a1b2c3d4e5f6a7b8c9d0e3'; + mockedApi.addComment({ + id: deletedCommentId, + html: '

Deleted parent content

', + status: 'deleted', + replies: [ + mockedApi.buildReply({ + html: '

Visible reply under deleted parent

', + parent_id: deletedCommentId + }) + ] + }); + + const commentsFrame = await setupPermalinkTest(page, mockedApi, `#ghost-comments-${deletedCommentId}`, TALL_BODY_HTML, {}, admin); + + await expect(commentsFrame.locator(`[id="${deletedCommentId}"]`)).toBeVisible(); + await expect(commentsFrame.getByText('This comment has been removed')).toBeVisible(); + await expect(commentsFrame.getByText('Visible reply under deleted parent')).toBeVisible(); + await expect(commentsFrame.getByTestId('missing-comment-notice')).toHaveCount(0); + }); + test('loads reply behind "Load more" when permalinking to it', async ({page}) => { const mockedApi = new MockedApi({}); mockedApi.setMember({}); @@ -412,63 +493,6 @@ test.describe('Comment Permalinks', async () => { await expect(commentsFrame.getByTestId('back-to-parent')).not.toBeVisible(); }); - test('opens commentsThreads focused view when target reply is not in the initial comments response', async ({page}) => { - const mockedApi = new MockedApi({}); - mockedApi.setMember({}); - - const parentId = 'aaa0000000000000000300'; - const replyIds = [ - 'aaa0000000000000000301', - 'aaa0000000000000000302', - 'aaa0000000000000000303', - 'aaa0000000000000000304', - 'aaa0000000000000000305', - 'aaa0000000000000000306' - ]; - - mockedApi.addComment({ - id: parentId, - html: '

Parent comment

', - replies: replyIds.map((id, index) => mockedApi.buildReply({ - id, - html: `

Lazy nested reply ${index + 1}

`, - parent_id: parentId, - ...(index > 0 ? {in_reply_to_id: replyIds[index - 1]} : {}) - })), - count: { - replies: replyIds.length - } - }); - - const originalBrowse = mockedApi.browseComments.bind(mockedApi); - mockedApi.browseComments = function (options) { - const result = originalBrowse(options); - result.comments = result.comments.map((comment) => { - if (comment.id === parentId) { - return {...comment, replies: comment.replies.slice(0, 3)}; - } - return comment; - }); - return result; - }; - - const targetReplyId = replyIds[4]; - const commentsFrame = await setupPermalinkTest( - page, - mockedApi, - `#ghost-comments-${targetReplyId}`, - undefined, - {commentsThreads: true} - ); - - await expect(commentsFrame.getByTestId('back-to-parent')).toBeVisible(); - await expect(commentsFrame.getByText('Lazy nested reply 4')).toBeVisible(); - await expect(commentsFrame.getByText('Lazy nested reply 5')).toBeVisible(); - - const targetReplyContent = commentsFrame.locator(`[id="${targetReplyId}"]`).getByTestId('comment-content').first(); - await expect(targetReplyContent.locator('mark')).toContainText('Lazy nested reply 5'); - }); - test('loads reply on later page when permalinking to it', async ({page}) => { const mockedApi = new MockedApi({}); mockedApi.setMember({}); @@ -541,7 +565,7 @@ test.describe('Comment Permalinks', async () => { // Wait for the parent to load await expect(commentsFrame.getByText('Parent with many replies')).toBeVisible(); - // The 105th reply should be loaded (requires pagination beyond first 100) + // The 105th reply should be visible from the replies included in the comments response await expect(commentsFrame.getByText('Reply number 105')).toBeVisible(); // The element should have the correct ID for highlighting @@ -549,62 +573,6 @@ test.describe('Comment Permalinks', async () => { await expect(targetElement).toBeVisible(); }); - test('loads reply via server fetch when permalink target is not in partial API response', async ({page}) => { - const mockedApi = new MockedApi({}); - mockedApi.setMember({}); - - const parentId = 'aaa0000000000000000000'; - const parentComment = { - id: parentId, - html: '

Parent comment

', - replies: [] as any[] - }; - - // Add 5 replies to the parent - const replyIds = [ - 'bbb0000000000000000001', - 'bbb0000000000000000002', - 'bbb0000000000000000003', - 'bbb0000000000000000004', - 'bbb0000000000000000005' - ]; - for (let i = 0; i < 5; i++) { - parentComment.replies.push(mockedApi.buildReply({ - id: replyIds[i], - html: `

Reply ${i + 1}

`, - parent_id: parentId - })); - } - - mockedApi.addComment(parentComment); - - // Override browseComments to only return 3 replies (simulating old API LIMIT 3) - // while count.replies stays at 5 - const originalBrowse = mockedApi.browseComments.bind(mockedApi); - mockedApi.browseComments = function (options) { - const result = originalBrowse(options); - result.comments = result.comments.map((c) => { - if (c.replies.length > 3) { - return {...c, replies: c.replies.slice(0, 3)}; - } - return c; - }); - return result; - }; - - // Permalink to reply 5 — not in the initial 3 returned by browseComments - const targetReplyId = replyIds[4]; - const commentsFrame = await setupPermalinkTest(page, mockedApi, `#ghost-comments-${targetReplyId}`); - - await expect(commentsFrame.getByText('Parent comment')).toBeVisible(); - - // Reply 5 must be loaded via server fetch and visible - await expect(commentsFrame.getByText('Reply 5')).toBeVisible(); - - const targetElement = commentsFrame.locator(`[id="${targetReplyId}"]`); - await expect(targetElement).toBeVisible(); - }); - test('admin auth does not overwrite paginated comments loaded for permalink', async ({page}) => { const mockedApi = new MockedApi({}); mockedApi.setMember({id: '1', uuid: '12345'}); diff --git a/apps/comments-ui/test/utils/e2e.ts b/apps/comments-ui/test/utils/e2e.ts index 13b56ffa6a0..f94c403cd8c 100644 --- a/apps/comments-ui/test/utils/e2e.ts +++ b/apps/comments-ui/test/utils/e2e.ts @@ -2,6 +2,7 @@ import {E2E_PORT} from '../../playwright.config'; import {Locator, Page} from '@playwright/test'; import {MockedApi} from './mocked-api'; import {expect} from '@playwright/test'; +import {platform} from 'os'; export const MOCKED_SITE_URL = 'https://localhost:1234'; export {MockedApi}; @@ -190,6 +191,10 @@ export async function initialize({mockedApi, page, bodyStyle, labs = {}, key = ' export async function selectText(locator: Locator, pattern: string | RegExp): Promise { await locator.evaluate( (element, {pattern: p}) => { + if (element instanceof HTMLElement) { + element.focus(); + } + let textNode = element.childNodes[0]; while (textNode.nodeType !== Node.TEXT_NODE && textNode.childNodes.length) { @@ -200,7 +205,7 @@ export async function selectText(locator: Locator, pattern: string | RegExp): Pr const range = document.createRange(); range.setStart(textNode, match.index!); range.setEnd(textNode, match.index! + match[0].length); - const selection = document.getSelection(); + const selection = element.ownerDocument.getSelection(); selection?.removeAllRanges(); selection?.addRange(range); } @@ -224,9 +229,7 @@ export async function setClipboard(page, text) { } export function getModifierKey() { - const os = require('os'); // eslint-disable-line @typescript-eslint/no-var-requires - const platform = os.platform(); - if (platform === 'darwin') { + if (platform() === 'darwin') { return 'Meta'; } else { return 'Control';