Skip to content

Commit

Permalink
Removed bluebird catch predicates from API endpoints
Browse files Browse the repository at this point in the history
refs: #14882

- I found a common pattern where catch predicates were being used to catch non-existent models in destroy methods, and sometimes elsewhere in the API endpoints
- The use of predicates is deprecated, and we're working to remove them from everywhere, so that we can remove bluebird
- In order to still handle these errors correctly, we needed a small change to mw-error-handler so that it can detect EmptyResponse errors from bookshelf, as well as 404s
Note: there is a small change as a result of this - the context on these errors now says "Resource not found" instead of "{ModelName} not found".
- I think this is acceptable for now, as we will be reviewing these errors in more depth later. It's quite easy to make changes, we just have to decide what with proper design input
ErisDS committed Aug 24, 2022

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
1 parent de8f238 commit af94855
Showing 22 changed files with 479 additions and 103 deletions.
17 changes: 10 additions & 7 deletions ghost/core/core/server/api/endpoints/comments-members.js
Original file line number Diff line number Diff line change
@@ -189,21 +189,24 @@ module.exports = {
validation: {},
permissions: true,
query(frame) {
frame.options.require = true;

// TODO: move to likes service
if (frame.options?.context?.member?.id) {
return models.CommentLike.destroy({
...frame.options,
destroyBy: {
member_id: frame.options.context.member.id,
comment_id: frame.options.id
}
},
require: true
}).then(() => null)
.catch(models.CommentLike.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.likeNotFound)
}));
.catch((err) => {
if (err instanceof models.CommentLike.NotFoundError) {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.likeNotFound)
}));
}

throw err;
});
} else {
return Promise.reject(new errors.NotFoundError({
10 changes: 1 addition & 9 deletions ghost/core/core/server/api/endpoints/invites.js
Original file line number Diff line number Diff line change
@@ -76,15 +76,7 @@ module.exports = {
},
permissions: true,
query(frame) {
frame.options.require = true;

return models.Invite.destroy(frame.options)
.then(() => null)
.catch(models.Invite.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.inviteNotFound)
}));
});
return models.Invite.destroy({...frame.options, require: true});
}
},

8 changes: 1 addition & 7 deletions ghost/core/core/server/api/endpoints/labels.js
Original file line number Diff line number Diff line change
@@ -150,13 +150,7 @@ module.exports = {
},
permissions: true,
query(frame) {
return models.Label.destroy(frame.options)
.then(() => null)
.catch(models.Label.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.labelNotFound)
}));
});
return models.Label.destroy({...frame.options, require: true});
}
}
};
16 changes: 3 additions & 13 deletions ghost/core/core/server/api/endpoints/members.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// NOTE: We must not cache references to membersService.api
// as it is a getter and may change during runtime.
const Promise = require('bluebird');
const moment = require('moment-timezone');
const errors = require('@tryghost/errors');
const models = require('../../models');
@@ -253,20 +252,11 @@ module.exports = {
},
permissions: true,
async query(frame) {
frame.options.require = true;
frame.options.cancelStripeSubscriptions = frame.options.cancel;

await Promise.resolve(membersService.api.members.destroy({
return membersService.api.members.destroy({
id: frame.options.id
}, frame.options)).catch(models.Member.NotFoundError, () => {
throw new errors.NotFoundError({
message: tpl(messages.resourceNotFound, {
resource: 'Member'
})
});
}, {
...frame.options, require: true, cancelStripeSubscriptions: frame.options.cancel
});

return null;
}
},

10 changes: 1 addition & 9 deletions ghost/core/core/server/api/endpoints/pages.js
Original file line number Diff line number Diff line change
@@ -186,15 +186,7 @@ module.exports = {
unsafeAttrs: UNSAFE_ATTRS
},
query(frame) {
frame.options.require = true;

return models.Post.destroy(frame.options)
.then(() => null)
.catch(models.Post.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.pageNotFound)
}));
});
return models.Post.destroy({...frame.options, require: true});
}
}
};
10 changes: 1 addition & 9 deletions ghost/core/core/server/api/endpoints/posts.js
Original file line number Diff line number Diff line change
@@ -192,15 +192,7 @@ module.exports = {
unsafeAttrs: unsafeAttrs
},
query(frame) {
frame.options.require = true;

return models.Post.destroy(frame.options)
.then(() => null)
.catch(models.Post.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.postNotFound)
}));
});
return models.Post.destroy({...frame.options, require: true});
}
}
};
10 changes: 1 addition & 9 deletions ghost/core/core/server/api/endpoints/snippets.js
Original file line number Diff line number Diff line change
@@ -101,15 +101,7 @@ module.exports = {
},
permissions: true,
query(frame) {
frame.options.require = true;

return models.Snippet.destroy(frame.options)
.then(() => null)
.catch(models.Snippet.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.snippetNotFound)
}));
});
return models.Snippet.destroy({...frame.options, require: true});
}
}
};
8 changes: 1 addition & 7 deletions ghost/core/core/server/api/endpoints/tags.js
Original file line number Diff line number Diff line change
@@ -147,13 +147,7 @@ module.exports = {
},
permissions: true,
query(frame) {
return models.Tag.destroy(frame.options)
.then(() => null)
.catch(models.Tag.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.tagNotFound)
}));
});
return models.Tag.destroy({...frame.options, require: true});
}
}
};
21 changes: 2 additions & 19 deletions ghost/core/core/server/api/endpoints/webhooks.js
Original file line number Diff line number Diff line change
@@ -78,14 +78,7 @@ module.exports = {
}
},
query({data, options}) {
return models.Webhook.edit(data.webhooks[0], Object.assign(options, {require: true}))
.catch(models.Webhook.NotFoundError, () => {
throw new errors.NotFoundError({
message: tpl(messages.resourceNotFound, {
resource: 'Webhook'
})
});
});
return models.Webhook.edit(data.webhooks[0], {...options, require: true});
}
},

@@ -130,17 +123,7 @@ module.exports = {
}
},
query(frame) {
frame.options.require = true;

return models.Webhook.destroy(frame.options)
.then(() => null)
.catch(models.Webhook.NotFoundError, () => {
return Promise.reject(new errors.NotFoundError({
message: tpl(messages.resourceNotFound, {
resource: 'Webhook'
})
}));
});
return models.Webhook.destroy({...frame.options, require: true});
}
}
};
60 changes: 60 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/labels.test.js.snap
Original file line number Diff line number Diff line change
@@ -184,6 +184,66 @@ Object {
}
`;
exports[`Labels API Cannot destroy non-existent label 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete label.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Labels API Cannot destroy non-existent label 2: [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-length": "258",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Labels API Cannot destroy nonexistent label 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete label.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Labels API Cannot destroy nonexistent label 2: [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-length": "258",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Labels API Errors when adding label with the same name 1: [body] 1`] = `
Object {
"errors": Array [
30 changes: 30 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap
Original file line number Diff line number Diff line change
@@ -3340,6 +3340,36 @@ Object {
}
`;

exports[`Members API Cannot delete a non-existent member 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete member.",
"property": null,
"type": "NotFoundError",
},
],
}
`;

exports[`Members API Cannot delete a non-existent member 2: [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-length": "259",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;

exports[`Members API Cannot edit a non-existing id 1: [body] 1`] = `
Object {
"errors": Array [
90 changes: 90 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/snippets.test.js.snap
Original file line number Diff line number Diff line change
@@ -215,3 +215,93 @@ Object {
"x-powered-by": "Express",
}
`;
exports[`Snippets API Cannot destroy non-existent snippet 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete snippet.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Snippets API Cannot destroy non-existent snippet 2: [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-length": "260",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Snippets API Cannot destroy nonexistent snippet 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete snippet.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Snippets API Cannot destroy nonexistent snippet 2: [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-length": "260",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Snippets API Cannot destroy unknown snippet 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource not found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete snippet.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Snippets API Cannot destroy unknown snippet 2: [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-length": "250",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
120 changes: 120 additions & 0 deletions ghost/core/test/e2e-api/admin/__snapshots__/webhooks.test.js.snap
Original file line number Diff line number Diff line change
@@ -76,6 +76,126 @@ Object {
}
`;
exports[`Webhooks API Cannot delete a non-existent webhook 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete webhook.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Webhooks API Cannot delete a non-existent webhook 2: [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-length": "260",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Webhooks API Cannot delete a nonexistent webhook 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot delete webhook.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Webhooks API Cannot delete a nonexistent webhook 2: [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-length": "260",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Webhooks API Cannot edit a non-existent webhook 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot edit webhook.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Webhooks API Cannot edit a non-existent webhook 2: [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-length": "258",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Webhooks API Cannot edit a nonexistent webhook 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": "Resource could not be found.",
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Resource not found error, cannot edit webhook.",
"property": null,
"type": "NotFoundError",
},
],
}
`;
exports[`Webhooks API Cannot edit a nonexistent webhook 2: [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-length": "258",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Webhooks API Fails nicely when adding a duplicate webhook 1: [body] 1`] = `
Object {
"errors": Array [
12 changes: 12 additions & 0 deletions ghost/core/test/e2e-api/admin/invites.test.js
Original file line number Diff line number Diff line change
@@ -102,4 +102,16 @@ describe('Invites API', function () {

mailService.GhostMailer.prototype.send.called.should.be.false();
});

it('Cannot destroy an non-existent invite', async function () {
await request.del(localUtils.API.getApiQuery(`invites/abcd1234abcd1234abcd1234/`))
.set('Origin', config.get('url'))
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(404)
.expect((res) => {
res.body.errors[0].message.should.eql('Resource not found error, cannot delete invite.');
});

mailService.GhostMailer.prototype.send.called.should.be.false();
});
});
14 changes: 14 additions & 0 deletions ghost/core/test/e2e-api/admin/labels.test.js
Original file line number Diff line number Diff line change
@@ -118,4 +118,18 @@ describe('Labels API', function () {
etag: anyEtag
});
});

it('Cannot destroy non-existent label', async function () {
await agent
.delete('labels/abcd1234abcd1234abcd1234')
.expectStatus(404)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
});
16 changes: 15 additions & 1 deletion ghost/core/test/e2e-api/admin/members.test.js
Original file line number Diff line number Diff line change
@@ -167,7 +167,7 @@ describe('Members API', function () {
beforeEach(function () {
mockManager.mockStripe();
mockManager.mockMail();

// For some reason it is enabled by default?
mockManager.mockLabsDisabled('memberAttribution');
});
@@ -1582,6 +1582,20 @@ describe('Members API', function () {
});
});

it('Cannot delete a non-existent member', async function () {
await agent
.delete('/members/abcd1234abcd1234abcd1234')
.expectStatus(404)
.matchBodySnapshot({
errors: [{
id: anyUuid
}]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});

it('Can delete a member without cancelling Stripe Subscription', async function () {
let subscriptionCanceled = false;
nock('https://api.stripe.com')
14 changes: 14 additions & 0 deletions ghost/core/test/e2e-api/admin/snippets.test.js
Original file line number Diff line number Diff line change
@@ -136,4 +136,18 @@ describe('Snippets API', function () {
etag: anyEtag
});
});

it('Cannot destroy non-existent snippet', async function () {
await agent
.delete('snippets/abcd1234abcd1234abcd1234')
.expectStatus(404)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
});
10 changes: 10 additions & 0 deletions ghost/core/test/e2e-api/admin/tags.test.js
Original file line number Diff line number Diff line change
@@ -160,4 +160,14 @@ describe('Tag API', function () {
should.exist(res.headers['x-cache-invalidate']);
res.body.should.eql({});
});

it('Can destroy a non-existent tag', async function () {
const res = await request
.del(localUtils.API.getApiQuery(`tags/abcd1234abcd1234abcd1234`))
.set('Origin', config.get('url'))
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(404);

res.body.errors[0].message.should.eql('Resource not found error, cannot delete tag.');
});
});
35 changes: 35 additions & 0 deletions ghost/core/test/e2e-api/admin/webhooks.test.js
Original file line number Diff line number Diff line change
@@ -102,6 +102,27 @@ describe('Webhooks API', function () {
});
});

it('Cannot edit a non-existent webhook', async function () {
await agent.put('/webhooks/abcd1234abcd1234abcd1234/')
.body({
webhooks: [{
name: 'Edit Test',
event: 'member.added',
target_url: 'https://example.com/new-member',
integration_id: 'ignore_me'
}]
})
.expectStatus(404)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});

it('Can delete a webhook', async function () {
await agent
.delete(`/webhooks/${createdWebhookId}/`)
@@ -111,4 +132,18 @@ describe('Webhooks API', function () {
etag: anyEtag
});
});

it('Cannot delete a non-existent webhook', async function () {
await agent
.delete('/webhooks/abcd1234abcd1234abcd1234/')
.expectStatus(404)
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
});
Original file line number Diff line number Diff line change
@@ -564,7 +564,7 @@ Object {
}
`;

exports[`Comments API when authenticated Can remove a like 1: [headers] 1`] = `
exports[`Comments API when authenticated Can remove a like (unlike) 1: [headers] 1`] = `
Object {
"access-control-allow-origin": "*",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
@@ -573,7 +573,7 @@ Object {
}
`;

exports[`Comments API when authenticated Can remove a like 2: [body] 1`] = `
exports[`Comments API when authenticated Can remove a like (unlike) 2: [body] 1`] = `
Object {
"comments": Array [
Object {
@@ -619,7 +619,7 @@ Object {
}
`;

exports[`Comments API when authenticated Can remove a like 3: [headers] 1`] = `
exports[`Comments API when authenticated Can remove a like (unlike) 3: [headers] 1`] = `
Object {
"access-control-allow-origin": "*",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
@@ -952,6 +952,36 @@ Object {
}
`;

exports[`Comments API when authenticated Cannot unlike a comment if it has not been liked 1: [body] 1`] = `
Object {
"errors": Array [
Object {
"code": null,
"context": null,
"details": null,
"ghostErrorCode": null,
"help": null,
"id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
"message": "Unable to find like",
"property": null,
"type": "NotFoundError",
},
],
}
`;

exports[`Comments API when authenticated Cannot unlike a comment if it has not been liked 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "*",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "205",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Accept-Encoding",
"x-powered-by": "Express",
}
`;

exports[`Comments API when authenticated Limits returned replies to 3 1: [body] 1`] = `
Object {
"comments": Array [
31 changes: 23 additions & 8 deletions ghost/core/test/e2e-api/members-comments/comments.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const assert = require('assert');
const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
const {anyEtag, anyObjectId, anyLocationFor, anyISODateTime, anyUuid, anyNumber, anyBoolean} = matchers;
const {anyEtag, anyObjectId, anyLocationFor, anyISODateTime, anyErrorId, anyUuid, anyNumber, anyBoolean} = matchers;
const should = require('should');
const models = require('../../../core/server/models');
const moment = require('moment-timezone');
@@ -23,9 +23,9 @@ const commentMatcher = {
};

/**
* @param {Object} [options]
* @param {number} [options.replies]
* @returns
* @param {Object} [options]
* @param {number} [options.replies]
* @returns
*/
function commentMatcherWithReplies(options = {replies: 0}) {
return {
@@ -338,7 +338,7 @@ describe('Comments API', function () {
testReplyId = reply.comments[0].id;
}
}

// Check if we have count.replies = 4, and replies.length == 3
await membersAgent
.get(`/api/comments/${parentId}/`)
@@ -481,8 +481,8 @@ describe('Comments API', function () {
});
});

it('Can remove a like', async function () {
// Create a temporary comment
it('Can remove a like (unlike)', async function () {
// Unlike
await membersAgent
.delete(`/api/comments/${commentId}/like/`)
.expectStatus(204)
@@ -491,7 +491,7 @@ describe('Comments API', function () {
})
.expectEmptyBody();

// Check liked
// Check not liked
await membersAgent
.get(`/api/comments/${commentId}/`)
.expectStatus(200)
@@ -507,6 +507,21 @@ describe('Comments API', function () {
});
});

it('Cannot unlike a comment if it has not been liked', async function () {
// Remove like
await membersAgent
.delete(`/api/comments/${commentId}/like/`)
//.expectStatus(404)
.matchHeaderSnapshot({
etag: anyEtag
})
.matchBodySnapshot({
errors: [{
id: anyErrorId
}]
});
});

it('Can report a comment', async function () {
// Create a temporary comment
await membersAgent
4 changes: 2 additions & 2 deletions ghost/mw-error-handler/lib/mw-error-handler.js
Original file line number Diff line number Diff line change
@@ -62,8 +62,8 @@ module.exports.prepareError = (err, req, res, next) => {
}

if (!errors.utils.isGhostError(err)) {
// We need a special case for 404 errors
if (err.statusCode && err.statusCode === 404) {
// We need a special case for 404 errors & bookshelf empty errors
if ((err.statusCode && err.statusCode === 404) || err.message === 'EmptyResponse') {
err = new errors.NotFoundError({
err: err
});

0 comments on commit af94855

Please sign in to comment.