Merge pull request #6693 from overleaf/jpa-error-is-manager

[web] refactor error handling for rejected manager removal

GitOrigin-RevId: 48be3bf254de74c2799d1368aee329fd9038dfa6

services/web/app/src/Features/UserMembership/UserMembershipController.js

@@ -15,6 +15,7 @@ const UserMembershipHandler = require('./UserMembershipHandler')
 const Errors = require('../Errors/Errors')
 const EmailHelper = require('../Helpers/EmailHelper')
 const { csvAttachment } = require('../../infrastructure/Response')
+const { UserIsManagerError } = require('./UserMembershipErrors')
 const CSVParser = require('json2csv').Parser
 
 module.exports = {
@@ -119,7 +120,7 @@ module.exports = {
       entityConfig,
       userId,
       function (error, user) {
-        if (error != null ? error.isAdmin : undefined) {
+        if (error && error instanceof UserIsManagerError) {
           return res.status(400).json({
             error: {
               code: 'managers_cannot_remove_admin',

services/web/app/src/Features/UserMembership/UserMembershipErrors.js

@@ -0,0 +1,7 @@
+const OError = require('@overleaf/o-error')
+
+class UserIsManagerError extends OError {}
+
+module.exports = {
+  UserIsManagerError,
+}

services/web/app/src/Features/UserMembership/UserMembershipHandler.js

@@ -25,6 +25,7 @@ const UserMembershipViewModel = require('./UserMembershipViewModel')
 const UserGetter = require('../User/UserGetter')
 const logger = require('@overleaf/logger')
 const UserMembershipEntityConfigs = require('./UserMembershipEntityConfigs')
+const { UserIsManagerError } = require('./UserMembershipErrors')
 
 const UserMembershipHandler = {
   getEntityWithoutAuthorizationCheck(entityId, entityConfig, callback) {
@@ -81,8 +82,7 @@ const UserMembershipHandler = {
     }
     const attribute = entityConfig.fields.write
     if (entity.admin_id != null ? entity.admin_id.equals(userId) : undefined) {
-      const error = { isAdmin: true }
-      return callback(error)
+      return callback(new UserIsManagerError())
     }
     return removeUserFromEntity(entity, attribute, userId, callback)
   },

services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js

@@ -21,6 +21,9 @@ const MockRequest = require('../helpers/MockRequest')
 const MockResponse = require('../helpers/MockResponse')
 const EntityConfigs = require('../../../../app/src/Features/UserMembership/UserMembershipEntityConfigs')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
+const {
+  UserIsManagerError,
+} = require('../../../../app/src/Features/UserMembership/UserMembershipErrors')
 
 describe('UserMembershipController', function () {
   beforeEach(function () {
@@ -71,6 +74,7 @@ describe('UserMembershipController', function () {
       modulePath,
       {
         requires: {
+          './UserMembershipErrors': { UserIsManagerError },
           '../Authentication/SessionManager': this.SessionManager,
           './UserMembershipHandler': this.UserMembershipHandler,
         },
@@ -263,7 +267,7 @@ describe('UserMembershipController', function () {
     })
 
     it('prevent admin removal', function (done) {
-      this.UserMembershipHandler.removeUser.yields({ isAdmin: true })
+      this.UserMembershipHandler.removeUser.yields(new UserIsManagerError())
       return this.UserMembershipController.remove(this.req, {
         status: () => ({
           json: payload => {

services/web/test/unit/src/UserMembership/UserMembershipHandlerTests.js

@@ -21,6 +21,9 @@ const modulePath =
 const SandboxedModule = require('sandboxed-module')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
 const EntityConfigs = require('../../../../app/src/Features/UserMembership/UserMembershipEntityConfigs')
+const {
+  UserIsManagerError,
+} = require('../../../../app/src/Features/UserMembership/UserMembershipErrors')
 
 describe('UserMembershipHandler', function () {
   beforeEach(function () {
@@ -68,6 +71,7 @@ describe('UserMembershipHandler', function () {
     return (this.UserMembershipHandler = SandboxedModule.require(modulePath, {
       requires: {
         mongodb: { ObjectId },
+        './UserMembershipErrors': { UserIsManagerError },
         './UserMembershipViewModel': this.UserMembershipViewModel,
         '../User/UserGetter': this.UserGetter,
         '../../models/Institution': {
@@ -265,7 +269,7 @@ describe('UserMembershipHandler', function () {
           this.newUser._id,
           (error, user) => {
             expect(error).to.exist
-            expect(error.isAdmin).to.equal(true)
+            expect(error).to.be.instanceof(UserIsManagerError)
             return done()
           }
         )