From c9758112b33d9133f4a14462a4a0759f3db676e6 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Thu, 2 Jun 2022 18:55:17 +0800
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20an=20error=20when=20upda?=
 =?UTF-8?q?ting=20a=20user?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes https://github.com/TryGhost/Team/issues/1655
refs https://github.com/TryGhost/Ghost/commit/4bc14d2c4

- The API should always accept the input it returns. In this case it did not accept the input when it contained an unchanged roles property
- The problem here came from the referenced commit where we can now end up in the situation when the `roleToAssign` is just empty. It was an optimization to prevent a need to do ANY DB operation when none was needed.
---
 core/server/models/user.js       |  2 +-
 test/e2e-api/admin/users.test.js | 34 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/core/server/models/user.js b/core/server/models/user.js
index 19e100bfe6..e686b8e62b 100644
--- a/core/server/models/user.js
+++ b/core/server/models/user.js
@@ -557,7 +557,7 @@ User = ghostBookshelf.Model.extend({
                                 message: tpl(messages.methodDoesNotSupportOwnerRole)
                             })
                         );
-                    } else {
+                    } else if (roleToAssign) {
                         // assign all other roles
                         return user.roles().updatePivot({role_id: roleToAssign.id});
                     }
diff --git a/test/e2e-api/admin/users.test.js b/test/e2e-api/admin/users.test.js
index 215b95d3d2..574debe662 100644
--- a/test/e2e-api/admin/users.test.js
+++ b/test/e2e-api/admin/users.test.js
@@ -185,6 +185,40 @@ describe('User API', function () {
         }
     });
 
+    it('can edit a user fetched from the API', async function () {
+        const userToEditId = testUtils.getExistingData().users[1].id;
+        const res = await request
+            .get(localUtils.API.getApiQuery(`users/${userToEditId}/?include=roles`))
+            .set('Origin', config.get('url'))
+            .expect(200);
+
+        const jsonResponse = res.body;
+        jsonResponse.users[0].name.should.equal('Ghost');
+
+        should.exist(jsonResponse.users[0].roles);
+        jsonResponse.users[0].roles.should.have.length(1);
+        jsonResponse.users[0].roles[0].name.should.equal('Contributor');
+
+        jsonResponse.users[0].name = 'Changed Name';
+
+        const editResponse = await request
+            .put(localUtils.API.getApiQuery(`users/${userToEditId}/?include=roles`))
+            .set('Origin', config.get('url'))
+            .send({
+                users: jsonResponse.users
+            })
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200);
+
+        const editJSONResponse = editResponse.body;
+        editJSONResponse.users[0].name.should.equal('Changed Name');
+
+        should.exist(editJSONResponse.users[0].roles);
+        editJSONResponse.users[0].roles.should.have.length(1);
+        editJSONResponse.users[0].roles[0].name.should.equal('Contributor');
+    });
+
     it('Can edit user with empty roles data and does not change the role', async function () {
         const res = await request.put(localUtils.API.getApiQuery('users/me?include=roles'))
             .set('Origin', config.get('url'))