Fixed updating a non-existent member internal error (#14658)

refs https://github.com/TryGhost/Team/issues/1580 - When you try to edit a member that doesn't exist, a 500 error is thrown. We should throw a 404 error instead - This is fixed by TryGhost/Members#395
TryGhost · May 4, 2022 · 473ac3b · 473ac3b
1 parent ad1ebe6
commit 473ac3b
Show file tree

Hide file tree

Showing 4 changed files with 193 additions and 107 deletions.
diff --git a/package.json b/package.json
@@ -67,12 +67,12 @@
     "@tryghost/custom-theme-settings-service": "0.3.2",
     "@tryghost/database-info": "0.3.3",
     "@tryghost/debug": "0.1.16",
-    "@tryghost/domain-events": "0.1.11",
+    "@tryghost/domain-events": "0.1.12",
     "@tryghost/email-analytics-provider-mailgun": "1.0.8",
     "@tryghost/email-analytics-service": "1.0.6",
     "@tryghost/email-content-generator": "0.1.0",
     "@tryghost/errors": "1.2.12",
-    "@tryghost/express-dynamic-redirects": "0.2.9",
+    "@tryghost/express-dynamic-redirects": "0.2.11",
     "@tryghost/helpers": "1.1.64",
     "@tryghost/image-transform": "1.0.30",
     "@tryghost/job-manager": "0.8.22",
@@ -83,14 +83,14 @@
     "@tryghost/kg-mobiledoc-html-renderer": "5.3.5",
     "@tryghost/limit-service": "1.1.0",
     "@tryghost/logging": "2.1.8",
-    "@tryghost/magic-link": "1.0.22",
-    "@tryghost/member-events": "0.4.2",
-    "@tryghost/members-api": "6.3.0",
-    "@tryghost/members-events-service": "0.4.0",
-    "@tryghost/members-importer": "0.5.10",
-    "@tryghost/members-offers": "0.11.3",
-    "@tryghost/members-ssr": "1.0.24",
-    "@tryghost/members-stripe-service": "0.10.2",
+    "@tryghost/magic-link": "1.0.24",
+    "@tryghost/member-events": "0.4.4",
+    "@tryghost/members-api": "6.3.1",
+    "@tryghost/members-events-service": "0.4.1",
+    "@tryghost/members-importer": "0.5.12",
+    "@tryghost/members-offers": "0.11.4",
+    "@tryghost/members-ssr": "1.0.26",
+    "@tryghost/members-stripe-service": "0.10.3",
     "@tryghost/metrics": "1.0.11",
     "@tryghost/minifier": "0.1.13",
     "@tryghost/mw-api-version-mismatch": "0.1.1",
@@ -112,7 +112,7 @@
     "@tryghost/update-check-service": "0.3.2",
     "@tryghost/url-utils": "2.1.0",
     "@tryghost/validator": "0.1.24",
-    "@tryghost/verification-trigger": "0.2.2",
+    "@tryghost/verification-trigger": "0.2.3",
     "@tryghost/version": "0.1.14",
     "@tryghost/version-notifications-data-service": "0.1.0",
     "@tryghost/vhost-middleware": "1.0.24",

diff --git a/test/e2e-api/admin/__snapshots__/members.test.js.snap b/test/e2e-api/admin/__snapshots__/members.test.js.snap
@@ -2824,6 +2824,64 @@ Object {
 }
 `;
 
+exports[`Members API Cannot edit a non-existing id 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": "Resource could not be found.",
+      "details": 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 member.",
+      "property": null,
+      "type": "NotFoundError",
+    },
+  ],
+}
+`;
+
+exports[`Members API Cannot edit a non-existing id 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": "235",
+  "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 with newsletters 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": Any<String>,
+      "details": 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 member.",
+      "property": null,
+      "type": "NotFoundError",
+    },
+  ],
+}
+`;
+
+exports[`Members API Cannot edit a non-existing id with newsletters 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": "253",
+  "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 Errors when fetching stats with unknown days param value 1: [body] 1`] = `
 Object {
   "errors": Array [

diff --git a/test/e2e-api/admin/members.test.js b/test/e2e-api/admin/members.test.js
@@ -1,5 +1,6 @@
 const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
 const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyISODate, anyString, anyArray, anyLocationFor, anyErrorId} = matchers;
+const ObjectId = require('bson-objectid');
 
 const assert = require('assert');
 const nock = require('nock');
@@ -83,9 +84,7 @@ describe('Members API without Stripe', function () {
         agent = await agentProvider.getAdminAPIAgent();
         await fixtureManager.init();
         await agent.loginAsOwner();
-    });
 
-    before(async function () {
         await agent
             .delete('/settings/stripe/connect/')
             .expectStatus(204);
@@ -1140,6 +1139,49 @@ describe('Members API', function () {
         });
     });
 
+    // Internally a different error is thrown for newsletters/products changes
+    it('Cannot edit a non-existing id with newsletters', async function () {
+        const memberChanged = {
+            name: 'changed',
+            email: 'just-a-member@test.com',
+            newsletters: []
+        };
+
+        await agent
+            .put(`/members/${ObjectId().toHexString()}/`)
+            .body({members: [memberChanged]})
+            .expectStatus(404)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyUuid,
+                    context: anyString
+                }]
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag
+            });
+    });
+
+    it('Cannot edit a non-existing id', async function () {
+        const memberChanged = {
+            name: 'changed',
+            email: 'just-a-member@test.com'
+        };
+
+        await agent
+            .put(`/members/${ObjectId().toHexString()}/`)
+            .body({members: [memberChanged]})
+            .expectStatus(404)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyUuid
+                }]
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag
+            });
+    });
+
     it('Can subscribe to a newsletter', async function () {
         const clock = sinon.useFakeTimers(Date.now());
         const memberToChange = {