Skip to content

🐛 Fixed 500 error when deleting posts with threaded comments #29682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mike182uk merged 2 commits into from
Aug 3, 2026
Merged

🐛 Fixed 500 error when deleting posts with threaded comments #29682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95f34db
🐛 Fixed 500 error when deleting posts with threaded comments
mike182uk Jul 30, 2026
1f9a08d
Changed comment cleanup on post deletion to run in the delete transac…
mike182uk Aug 3, 2026
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
11 changes: 10 additions & 1 deletion ghost/core/core/server/models/post.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,16 @@ Post = ghostBookshelf.Model.extend({
destroy: function destroy(unfilteredOptions) {
let options = this.filterOptions(unfilteredOptions, 'destroy', {extraAllowedProperties: ['id']});

const destroyPost = () => {
const destroyPost = async () => {
// The `comments.in_reply_to_id` references form chains between a post's
// comments, which MySQL cannot resolve while cascade-deleting them
// alongside `comments.parent_id`. Clear the references first so the
// `comments.post_id` cascade delete can do its job
await ghostBookshelf.knex('comments')
.where('post_id', options.id)
.update('in_reply_to_id', null)
.transacting(options.transacting);

@sagzy sagzy Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure that there is always a transaction here?

Else we may want to do sth like

 if (!options.transacting) { 
    // create transaction
    // then call destroyPost()
}

@mike182uk mike182uk Aug 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

destroyPost is only called after options.transacting is set the existing if (!options.transacting) block just below (cut off in the diff view) creates the transaction and assigns it before invoking destroyPost. Callers that pass their own transacting (e.g. deleteAllContent) take the direct path

@sagzy sagzy Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sweet, all good then!


return ghostBookshelf.Model.destroy.call(this, options);
};

Expand Down
11 changes: 11 additions & 0 deletions ghost/core/core/server/services/posts/posts-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,17 @@ class PostsService {
});
}

// The `comments.in_reply_to_id` references form chains between a post's
// comments, which MySQL cannot resolve while cascade-deleting them
// alongside `comments.parent_id`. Clear the references first so the
// `comments.post_id` cascade delete can do its job
await this.models.Post.bulkEdit(deleteIds, 'comments', {
data: {in_reply_to_id: null},
column: 'post_id',
transacting: options.transacting,
throwErrors: true
});

// Posts and emails
await this.models.Post.bulkDestroy(deleteEmailIds, 'emails', {transacting: options.transacting, throwErrors: true});
const result = await this.models.Post.bulkDestroy(deleteIds, 'posts', {...options, throwErrors: true});
Expand Down
14 changes: 14 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/posts-bulk.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing

exports[`Posts Bulk API Delete Can delete a post with a threaded comment replying to another reply 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
},
},
}
`;

exports[`Posts Bulk API Delete Can delete all posts 1: [body] 1`] = `
Object {
"bulk": Object {
Expand Down
12 changes: 12 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,18 @@ Object {
}
`;

exports[`Posts API Delete Can destroy a post with a threaded comment replying to another reply 1: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Accept-Version, Origin",
"x-cache-invalidate": "/*",
"x-powered-by": "Express",
}
`;

exports[`Posts API Delete Cannot delete a non-existent posts 1: [body] 1`] = `
Object {
"errors": Array [
Expand Down
59 changes: 59 additions & 0 deletions ghost/core/test/e2e-api/admin/posts-bulk.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,65 @@ describe('Posts Bulk API', function () {
assert.equal(posts.meta.pagination.total, 0, `Expect all matching posts (${amount}) to be deleted`);
});

it('Can delete a post with a threaded comment replying to another reply', async function () {
const {body: {posts: [post]}} = await agent
.post('/posts/')
.body({posts: [{title: 'Post with a threaded comment', status: 'draft'}]})
.expectStatus(201);

const memberId = fixtureManager.get('members', 0).id;
const root = await models.Comment.add({
post_id: post.id,
member_id: memberId,
html: '<p>Root comment</p>',
status: 'published'
});
const reply = await models.Comment.add({
post_id: post.id,
member_id: memberId,
parent_id: root.id,
html: '<p>Reply</p>',
status: 'published'
});
await models.Comment.add({
post_id: post.id,
member_id: memberId,
parent_id: root.id,
in_reply_to_id: reply.id,
html: '<p>Reply to the reply</p>',
status: 'published'
});

// A long back-and-forth conversation, where each reply replies to the
// previous one, chains more levels than MySQL can cascade: InnoDB
// hard-limits nested foreign key cascades to 15 levels and fails the
// delete with error 3008 beyond that, so `in_reply_to_id` cannot use
// ON DELETE CASCADE and must be cleared in the delete transaction
// https://dev.mysql.com/doc/mysql-reslimits-excerpt/8.0/en/ansi-diff-foreign-keys.html
let previous = reply;
for (let i = 0; i < 20; i++) {
previous = await models.Comment.add({
post_id: post.id,
member_id: memberId,
parent_id: root.id,
in_reply_to_id: previous.id,
html: `<p>Reply ${i} in a long conversation</p>`,
status: 'published'
});
}

const filter = `id:['${post.id}']`;
const response = await agent
.delete('/posts/?filter=' + encodeURIComponent(filter))
.expectStatus(200)
.matchBodySnapshot();

assert.equal(response.body.bulk.meta.stats.successful, 1, 'Expect the post with threaded comments to be deleted');

const comments = await models.Base.knex('comments').where('post_id', post.id);
assert.equal(comments.length, 0, 'Expected all comments on the post to be deleted with the post');
});

it('Can delete all posts', async function () {
const filter = 'status:[published,draft,scheduled,sent]';

Expand Down
52 changes: 52 additions & 0 deletions ghost/core/test/e2e-api/admin/posts.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,58 @@ describe('Posts API', function () {
});
});

it('Can destroy a post with a threaded comment replying to another reply', async function () {
const post = fixtureManager.get('posts', 1);

const root = await models.Comment.add({
post_id: post.id,
html: '<p>Root comment</p>',
status: 'published'
});
const reply = await models.Comment.add({
post_id: post.id,
parent_id: root.id,
html: '<p>Reply</p>',
status: 'published'
});
await models.Comment.add({
post_id: post.id,
parent_id: root.id,
in_reply_to_id: reply.id,
html: '<p>Reply to the reply</p>',
status: 'published'
});

// A long back-and-forth conversation, where each reply replies to the
// previous one, chains more levels than MySQL can cascade: InnoDB
// hard-limits nested foreign key cascades to 15 levels and fails the
// delete with error 3008 beyond that, so `in_reply_to_id` cannot use
// ON DELETE CASCADE and must be cleared in the delete transaction
// https://dev.mysql.com/doc/mysql-reslimits-excerpt/8.0/en/ansi-diff-foreign-keys.html
let previous = reply;
for (let i = 0; i < 20; i++) {
previous = await models.Comment.add({
post_id: post.id,
parent_id: root.id,
in_reply_to_id: previous.id,
html: `<p>Reply ${i} in a long conversation</p>`,
status: 'published'
});
}

await agent
.delete(`posts/${post.id}/`)
.expectStatus(204)
.expectEmptyBody()
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
});

const comments = await models.Base.knex('comments').where('post_id', post.id);
assert.equal(comments.length, 0, 'Expected all comments on the post to be deleted with the post');
});

it('Cannot delete a non-existent posts', async function () {
// This error message from the API is not really what I would expect
// Adding this as a guard to demonstrate how future refactoring improves the output
Expand Down
Loading