Skip to content

🐛 Fixed missing hidden comment bodies for Admins when loading comment permalinks#28195

Open
kevinansfield wants to merge 1 commit into
mainfrom
fix-comments-hidden-permalink
Open

🐛 Fixed missing hidden comment bodies for Admins when loading comment permalinks#28195
kevinansfield wants to merge 1 commit into
mainfrom
fix-comments-hidden-permalink

Conversation

@kevinansfield
Copy link
Copy Markdown
Member

@kevinansfield kevinansfield commented May 27, 2026

ref https://linear.app/ghost/issue/BER-3686/hidden-comment-text-not-shown-when-loading-via-permalink

Summary

  • resolve comment permalink targets through the admin API after admin auth succeeds
  • paginate admin comment pages so hidden targets on later pages are shown to moderators
  • preserve the existing public permalink state when admin lookup fails
  • remove obsolete partial-reply permalink fallback coverage now that comment API browse returns all replies

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: eaa886ac-640d-411f-90a2-2747a25a594a

📥 Commits

Reviewing files that changed from the base of the PR and between b1ebb32 and efb521f.

📒 Files selected for processing (5)
  • apps/comments-ui/package.json
  • apps/comments-ui/src/app.tsx
  • apps/comments-ui/test/e2e/editor.test.ts
  • apps/comments-ui/test/e2e/permalink.test.ts
  • apps/comments-ui/test/utils/e2e.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/comments-ui/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/comments-ui/src/app.tsx

Walkthrough

This PR extends comment pagination to support an optional pluggable request API, enabling admin-specific comment browsing during permalink resolution. The implementation adds Pagination and CommentsRequestApi types, refactors paginateToComment and loadScrollTarget to accept an optional requestApi parameter, and updates initAdminAuth to leverage this mechanism for resolving admin permalinks to hidden comments. The admin initialization now captures memberUuid once, browses admin comments, and when an initialCommentId is present, resolves scroll pagination through the admin API. E2E tests are updated to support admin authorization context and new test cases verify that admins can access hidden comments via permalink across pagination boundaries.

Possibly related PRs

  • TryGhost/Ghost#28052: Removes obsolete server-has-more fallback in reply rendering, complementing this PR's removal of reply-expansion fetching from loadScrollTarget.

Suggested reviewers

  • rob-ghost
  • jonatansberg
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: enabling admins to see hidden comment bodies when loading permalinks.
Description check ✅ Passed The description is directly related to the changeset, clearly outlining the four key changes: admin API resolution, pagination, state preservation, and removal of obsolete fallback logic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-comments-hidden-permalink

Comment @coderabbitai help to get the list of available commands and usage tips.

@kevinansfield kevinansfield changed the title 🐛 Fixed hidden comment permalinks for admins 🐛 Fixed missing hidden comment bodies for Admins when loading comment permalinks May 27, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/comments-ui/src/app.tsx (1)

186-188: 💤 Low value

Consider logging a warning for debugging purposes.

The empty catch block is intentional to preserve public API results on failure, but silent failures can make debugging harder. A warning would help diagnose issues without changing the fallback behavior.

🔧 Optional: Add debug logging
-                        } catch {
-                            // Keep the public API permalink result when the admin API can't load this target.
+                        } catch (e) {
+                            // Keep the public API permalink result when the admin API can't load this target.
+                            // eslint-disable-next-line no-console
+                            console.warn('[Comments] Admin permalink resolution failed, using public API result:', e);
                         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/comments-ui/src/app.tsx` around lines 186 - 188, The empty catch
swallowing errors makes silent failures hard to debug; in the catch block that
preserves the public API permalink result (the try/catch around loading the
admin API in apps/comments-ui/src/app.tsx), log a warning including the caught
error and a short contextual message (e.g., use console.warn or the app's
logger.warn) so behavior/fallback remains unchanged but failures are visible for
debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/comments-ui/src/app.tsx`:
- Around line 186-188: The empty catch swallowing errors makes silent failures
hard to debug; in the catch block that preserves the public API permalink result
(the try/catch around loading the admin API in apps/comments-ui/src/app.tsx),
log a warning including the caught error and a short contextual message (e.g.,
use console.warn or the app's logger.warn) so behavior/fallback remains
unchanged but failures are visible for debugging.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: af622cb7-b43a-4e5d-b648-a38f32a3d77f

📥 Commits

Reviewing files that changed from the base of the PR and between 7b7db30 and b1ebb32.

📒 Files selected for processing (2)
  • apps/comments-ui/src/app.tsx
  • apps/comments-ui/test/e2e/permalink.test.ts

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1ebb32765

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const adminComments = await adminApi.browse({page: 1, postId: options.postId, order: state.order, memberUuid});
let adminPermalinkState: Partial<EditableAppContext> | null = null;

if (initialCommentId) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render auth frame for empty hidden-comment permalinks

This admin permalink recovery only runs after AuthFrame calls initAdminAuth, but the frame is still rendered only when state.comments.length > 0. For a permalink to a hidden comment on a post with no public comments, the member API returns an empty list and showMissingCommentNotice, so the auth frame is never mounted and this new adminApi.read path never executes; an authenticated admin still sees the missing-comment notice instead of the hidden comment.

Useful? React with 👍 / 👎.

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.
@kevinansfield kevinansfield force-pushed the fix-comments-hidden-permalink branch from 75040b2 to efb521f Compare May 27, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant