Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/comments-ui/src/app-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export type Comment = {
member: Member | null,
edited_at: string,
created_at: string,
html: string
html: string | null
}

export type OpenCommentForm = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,25 +48,6 @@ const commentMapper = (model, frame) => {

const isPublicRequest = utils.isMembersAPI(frame);

// Deleted comments are tombstones - just enough info to show "Reply to deleted comment"
if (jsonModel.status === 'deleted') {
const tombstone = {
id: jsonModel.id,
parent_id: jsonModel.parent_id || null,
status: 'deleted'
};
// Include post relation if loaded
if (jsonModel.post) {
url.forPost(jsonModel.post.id, jsonModel.post, frame);
tombstone.post = _.pick(jsonModel.post, postFields);
}
// Include replies if present (for tree structure), recursively mapped
if (jsonModel.replies) {
tombstone.replies = jsonModel.replies.map(reply => commentMapper(reply, frame));
}
return tombstone;
}

if (jsonModel.inReplyTo && (jsonModel.inReplyTo.status === 'published' || (!isPublicRequest && jsonModel.inReplyTo.status === 'hidden'))) {
jsonModel.in_reply_to_snippet = htmlToPlaintext.commentSnippet(jsonModel.inReplyTo.html);
} else if (jsonModel.inReplyTo && jsonModel.inReplyTo.status !== 'published') {
Expand Down Expand Up @@ -115,6 +96,11 @@ const commentMapper = (model, frame) => {
}
}

// Deleted comments should never expose their content
if (jsonModel.status === 'deleted') {
response.html = null;
}

return response;
};

Expand Down
28 changes: 17 additions & 11 deletions ghost/core/core/server/models/comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,24 @@ const Comment = ghostBookshelf.Model.extend({

// Called by our filtered-collection bookshelf plugin
applyCustomQuery(options) {
const excludedCommentStatuses = options.isAdmin ? ['deleted'] : ['hidden', 'deleted'];
const excludedStatuses = options.isAdmin ? ['deleted'] : ['hidden', 'deleted'];

this.query((qb) => {
qb.where(function () {
this.whereNotIn('comments.status', excludedCommentStatuses)
.orWhereExists(function () {
this.select(1)
.from('comments as replies')
.whereRaw('replies.parent_id = comments.id')
.whereNotIn('replies.status', excludedCommentStatuses);
});
});
if (options.browseAll) {
// Browse All: simply exclude statuses, no thread structure preservation
qb.whereNotIn('comments.status', excludedStatuses);
Comment on lines +69 to +71
Copy link
Member

Choose a reason for hiding this comment

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

Non blocker: It would be great to find a more long-term solution to this behaviour switch that will scale/be less likely to bite us as we build out more features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I have a few ideas but I didn't want to bloat this PR with trying to solve it now.

It will boil down to removing the flags and having something more composable which each individual use case can build upon, as theres:

  1. What status to include
  2. If we should be including threaded replies or not
  3. Should we be including roots or leaves

Depends on which API you're using and we're getting by with flags but they're a major 🚩

} else {
// Default: preserve thread structure by including deleted parents with replies
qb.where(function () {
this.whereNotIn('comments.status', excludedStatuses)
.orWhereExists(function () {
this.select(1)
.from('comments as replies')
.whereRaw('replies.parent_id = comments.id')
.whereNotIn('replies.status', excludedStatuses);
});
});
}
});
},

Expand Down Expand Up @@ -317,9 +323,9 @@ const Comment = ghostBookshelf.Model.extend({
*/
permittedOptions: function permittedOptions(methodName) {
let options = ghostBookshelf.Model.permittedOptions.call(this, methodName);
// The comment model additionally supports having a parentId and isAdmin option
options.push('parentId');
options.push('isAdmin');
options.push('browseAll');

return options;
}
Expand Down
7 changes: 3 additions & 4 deletions ghost/core/core/server/services/comments/CommentsService.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,15 @@ class CommentsService {
order,
page,
limit,
// If includeNested is false, only return top-level comments
parentId: includeNested ? undefined : null,
// Admin context: see hidden comments with full content, and all statuses including deleted
isAdmin: true
isAdmin: true,
browseAll: true
});
}

async getAdminComments(options) {
this.checkEnabled();
const page = await this.models.Comment.findPage({...options, parentId: null});
const page = await this.models.Comment.findPage({...options, parentId: null, isAdmin: true});

return page;
}
Expand Down
Loading