From 4cd210c29cf1226ac3b3637c34e8a697aab00d93 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 22 Aug 2022 17:35:37 +0100 Subject: [PATCH] Added post deletion tests using new e2e framework - copied over and rewrote the deletion test from the legacy file - added a new test that checks that we get a 404 when attempting to delete an unknown post - this is a guard to protect and futureproof the API whilst we do refactoring to improve 404 handling from bookshelf - in turn this is aimed at helping to get rid of a bunch of catch predicates from the API --- .../admin/__snapshots__/posts.test.js.snap | 72 +++++++++++++++++++ .../test/e2e-api/admin/posts-legacy.test.js | 51 ++++++------- ghost/core/test/e2e-api/admin/posts.test.js | 45 ++++++++++++ 3 files changed, 137 insertions(+), 31 deletions(-) create mode 100644 ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap create mode 100644 ghost/core/test/e2e-api/admin/posts.test.js diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap new file mode 100644 index 0000000000..cf0b7c9c06 --- /dev/null +++ b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap @@ -0,0 +1,72 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Posts API Cannot delete a non-existent posts 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Post 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 post.", + "property": null, + "type": "NotFoundError", + }, + ], +} +`; + +exports[`Posts API Cannot delete a non-existent posts 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": "244", + "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[`Posts API Delete Can destroy a post 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", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Origin", + "x-cache-invalidate": "/*", + "x-powered-by": "Express", +} +`; + +exports[`Posts API Delete Cannot delete a non-existent posts 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": "Post 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 post.", + "property": null, + "type": "NotFoundError", + }, + ], +} +`; + +exports[`Posts API Delete Cannot delete a non-existent posts 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": "244", + "content-type": "application/json; charset=utf-8", + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; diff --git a/ghost/core/test/e2e-api/admin/posts-legacy.test.js b/ghost/core/test/e2e-api/admin/posts-legacy.test.js index 6f73c9ab64..23f28404ca 100644 --- a/ghost/core/test/e2e-api/admin/posts-legacy.test.js +++ b/ghost/core/test/e2e-api/admin/posts-legacy.test.js @@ -734,7 +734,7 @@ describe('Posts API', function () { should(finalPost.body.posts[0].newsletter).eql(null); should(finalPost.body.posts[0].email_segment).eql('all'); should.not.exist(finalPost.body.posts[0].newsletter_id); - + // Check status is set to published finalPost.body.posts[0].status.should.eql('published'); @@ -1095,7 +1095,7 @@ describe('Posts API', function () { id, status: 'all' }, testUtils.context.internal); - + should(model.get('status')).eql('sent'); should(model.get('newsletter_id')).eql(newsletterId); should(model.get('email_recipient_filter')).eql('status:free'); @@ -1245,7 +1245,7 @@ describe('Posts API', function () { .expect(200); const scheduledPost = scheduledRes.body.posts[0]; - + scheduledPost.newsletter.id.should.eql(newsletterId); scheduledPost.email_segment.should.eql('status:free'); should.not.exist(scheduledPost.newsletter_id); @@ -1276,7 +1276,7 @@ describe('Posts API', function () { .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); - + const publishedPost = publishedRes.body.posts[0]; model = await models.Post.findOne({ @@ -1336,7 +1336,7 @@ describe('Posts API', function () { .expect(200); const scheduledPost = scheduledRes.body.posts[0]; - + should(scheduledPost.newsletter).eql(null); scheduledPost.email_segment.should.eql('all'); // should be igored should.not.exist(scheduledPost.newsletter_id); @@ -1367,7 +1367,7 @@ describe('Posts API', function () { .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); - + const publishedPost = publishedRes.body.posts[0]; model = await models.Post.findOne({ @@ -1487,7 +1487,7 @@ describe('Posts API', function () { }); it('Can\'t change the newsletter once it has been sent', async function () { - // Note: this test only works if there are members subscribed to the initial newsletter + // Note: this test only works if there are members subscribed to the initial newsletter // (so it won't get reset when changing the post status to draft again) let model; @@ -1636,7 +1636,7 @@ describe('Posts API', function () { }); it('Can change the newsletter if it has not been sent', async function () { - // Note: this test only works if there are NO members subscribed to the initial newsletter + // Note: this test only works if there are NO members subscribed to the initial newsletter // (so it will get reset when changing the post status to draft again) let model; @@ -1789,17 +1789,6 @@ describe('Posts API', function () { should(model.get('email_recipient_filter')).eql('status:-free'); }); - it('Can destroy a post', async function () { - const res = await request - .del(localUtils.API.getApiQuery('posts/' + testUtils.DataGenerator.Content.posts[0].id + '/')) - .set('Origin', config.get('url')) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(204); - - res.body.should.be.empty(); - res.headers['x-cache-invalidate'].should.eql('/*'); - }); - it('Cannot get post via pages endpoint', async function () { await request.get(localUtils.API.getApiQuery(`pages/${testUtils.DataGenerator.Content.posts[3].id}/`)) .set('Origin', config.get('url')) @@ -1902,7 +1891,7 @@ describe('Posts API', function () { it('Can publish an email_only post', async function () { const newsletterId = testUtils.DataGenerator.Content.newsletters[1].id; const newsletterSlug = testUtils.DataGenerator.Content.newsletters[1].slug; - + const post = { title: 'My post', status: 'draft', @@ -1915,22 +1904,22 @@ describe('Posts API', function () { created_by: ObjectId().toHexString(), updated_by: ObjectId().toHexString() }; - + const res = await request.post(localUtils.API.getApiQuery('posts')) .set('Origin', config.get('url')) .send({posts: [post]}) .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) .expect(201); - + const id = res.body.posts[0].id; - + const updatedPost = res.body.posts[0]; - + updatedPost.status = 'published'; //updatedPost.published_at = moment().add(2, 'days').toDate(); updatedPost.email_only = true; - + const publishedRes = await request .put(localUtils.API.getApiQuery('posts/' + id + '/?newsletter=' + newsletterSlug)) .set('Origin', config.get('url')) @@ -1938,28 +1927,28 @@ describe('Posts API', function () { .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) .expect(200); - + const publishedPost = publishedRes.body.posts[0]; - + publishedPost.newsletter.id.should.eql(newsletterId); publishedPost.email_segment.should.eql('all'); publishedPost.status.should.eql('sent'); should.not.exist(publishedPost.newsletter_id); - + let model = await models.Post.findOne({ id, status: 'all' }, testUtils.context.internal); - + should(model.get('status')).eql('sent'); should(model.get('newsletter_id')).eql(newsletterId); should(model.get('email_recipient_filter')).eql('all'); - + // We should have an email const email = await models.Email.findOne({ post_id: id }, testUtils.context.internal); - + should(email.get('newsletter_id')).eql(newsletterId); should(email.get('recipient_filter')).eql('all'); should(email.get('status')).eql('pending'); diff --git a/ghost/core/test/e2e-api/admin/posts.test.js b/ghost/core/test/e2e-api/admin/posts.test.js new file mode 100644 index 0000000000..7dcc0e025c --- /dev/null +++ b/ghost/core/test/e2e-api/admin/posts.test.js @@ -0,0 +1,45 @@ +const assert = require('assert'); +const {agentProvider, fixtureManager, mockManager, matchers} = require('../../utils/e2e-framework'); +const {anyEtag, anyErrorId} = matchers; + +describe('Posts API', function () { + let agent; + + before(async function () { + agent = await agentProvider.getAdminAPIAgent(); + await fixtureManager.init('posts'); + await agent.loginAsOwner(); + }); + + afterEach(function () { + mockManager.restore(); + }); + + describe('Delete', function () { + it('Can destroy a post', async function () { + await agent + .delete(`posts/${fixtureManager.get('posts', 0).id}/`) + .expectStatus(204) + .expectEmptyBody() + .matchHeaderSnapshot({ + etag: anyEtag + }); + }); + + 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 + await agent + .delete('/posts/abcd1234abcd1234abcd1234/') + .expectStatus(404) + .matchHeaderSnapshot({ + etag: anyEtag + }) + .matchBodySnapshot({ + errors: [{ + id: anyErrorId + }] + }); + }); + }); +});