Merge pull request #17418 from overleaf/msm-expressify-usermembershipctlr

[web] expressify UserMembershipController

GitOrigin-RevId: 54f8d718bffb52609f055490f2a996f6c007f472

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

@@ -1,15 +1,3 @@
-/* eslint-disable
-   max-len,
- */
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const SessionManager = require('../Authentication/SessionManager')
 const UserMembershipHandler = require('./UserMembershipHandler')
 const Errors = require('../Errors/Errors')
@@ -22,39 +10,32 @@ const {
 } = require('./UserMembershipErrors')
 const { SSOConfig } = require('../../models/SSOConfig')
 const CSVParser = require('json2csv').Parser
+const { expressify } = require('@overleaf/promise-utils')
 
 async function manageGroupMembers(req, res, next) {
-  const { entity, entityConfig } = req
+  const { entity: subscription, entityConfig } = req
 
-  const ssoConfig = await SSOConfig.findById(entity.ssoConfig).exec()
-  return entity.fetchV1Data(function (error, entity) {
-    if (error != null) {
-      return next(error)
-    }
-    return UserMembershipHandler.getUsers(
-      entity,
-      entityConfig,
-      function (error, users) {
-        let entityName
-        if (error != null) {
-          return next(error)
-        }
-        const entityPrimaryKey =
-          entity[entityConfig.fields.primaryKey].toString()
-        if (entityConfig.fields.name) {
-          entityName = entity[entityConfig.fields.name]
-        }
+  const entityPrimaryKey =
+    subscription[entityConfig.fields.primaryKey].toString()
 
-        return res.render('user_membership/group-members-react', {
-          name: entityName,
-          groupId: entityPrimaryKey,
-          users,
-          groupSize: entity.membersLimit,
-          managedUsersActive: entity.managedUsersEnabled,
-          groupSSOActive: ssoConfig?.enabled,
-        })
-      }
-    )
+  let entityName
+  if (entityConfig.fields.name) {
+    entityName = subscription[entityConfig.fields.name]
+  }
+
+  const users = await UserMembershipHandler.promises.getUsers(
+    subscription,
+    entityConfig
+  )
+  const ssoConfig = await SSOConfig.findById(subscription.ssoConfig).exec()
+
+  res.render('user_membership/group-members-react', {
+    name: entityName,
+    groupId: entityPrimaryKey,
+    users,
+    groupSize: subscription.membersLimit,
+    managedUsersActive: subscription.managedUsersEnabled,
+    groupSSOActive: ssoConfig?.enabled,
   })
 }
 
@@ -87,38 +68,42 @@ async function managePublisherManagers(req, res, next) {
 
 async function _renderManagersPage(req, res, next, template) {
   const { entity, entityConfig } = req
-  return entity.fetchV1Data(function (error, entity) {
-    if (error != null) {
-      return next(error)
-    }
-    return UserMembershipHandler.getUsers(
-      entity,
-      entityConfig,
-      function (error, users) {
-        let entityName
-        if (error != null) {
-          return next(error)
-        }
-        const entityPrimaryKey =
-          entity[entityConfig.fields.primaryKey].toString()
-        if (entityConfig.fields.name) {
-          entityName = entity[entityConfig.fields.name]
-        }
-        return res.render(template, {
-          name: entityName,
-          users,
-          groupId: entityPrimaryKey,
-        })
+
+  const fetchV1Data = new Promise((resolve, reject) => {
+    entity.fetchV1Data((error, entity) => {
+      if (error) {
+        reject(error)
+      } else {
+        resolve(entity)
       }
-    )
+    })
+  })
+
+  const entityWithV1Data = await fetchV1Data
+
+  const entityPrimaryKey =
+    entityWithV1Data[entityConfig.fields.primaryKey].toString()
+  let entityName
+  if (entityConfig.fields.name) {
+    entityName = entityWithV1Data[entityConfig.fields.name]
+  }
+  const users = await UserMembershipHandler.promises.getUsers(
+    entityWithV1Data,
+    entityConfig
+  )
+
+  res.render(template, {
+    name: entityName,
+    users,
+    groupId: entityPrimaryKey,
   })
 }
 
 module.exports = {
-  manageGroupMembers,
-  manageGroupManagers,
-  manageInstitutionManagers,
-  managePublisherManagers,
+  manageGroupMembers: expressify(manageGroupMembers),
+  manageGroupManagers: expressify(manageGroupManagers),
+  manageInstitutionManagers: expressify(manageInstitutionManagers),
+  managePublisherManagers: expressify(managePublisherManagers),
   add(req, res, next) {
     const { entity, entityConfig } = req
     const email = EmailHelper.parseEmail(req.body.email)
@@ -135,7 +120,7 @@ module.exports = {
       return next(new Errors.NotFoundError('Cannot add users to entity'))
     }
 
-    return UserMembershipHandler.addUser(
+    UserMembershipHandler.addUser(
       entity,
       entityConfig,
       email,
@@ -159,7 +144,7 @@ module.exports = {
         if (error != null) {
           return next(error)
         }
-        return res.json({ user })
+        res.json({ user })
       }
     )
   },
@@ -181,7 +166,7 @@ module.exports = {
       })
     }
 
-    return UserMembershipHandler.removeUser(
+    UserMembershipHandler.removeUser(
       entity,
       entityConfig,
       userId,
@@ -197,7 +182,7 @@ module.exports = {
         if (error != null) {
           return next(error)
         }
-        return res.sendStatus(200)
+        res.sendStatus(200)
       }
     )
   },
@@ -205,7 +190,7 @@ module.exports = {
     const { entity, entityConfig } = req
     const fields = ['email', 'last_logged_in_at', 'last_active_at']
 
-    return UserMembershipHandler.getUsers(
+    UserMembershipHandler.getUsers(
       entity,
       entityConfig,
       function (error, users) {
@@ -218,7 +203,7 @@ module.exports = {
     )
   },
   new(req, res, next) {
-    return res.render('user_membership/new', {
+    res.render('user_membership/new', {
       entityName: req.params.name,
       entityId: req.params.id,
     })
@@ -227,14 +212,14 @@ module.exports = {
     const entityId = req.params.id
     const entityConfig = req.entityConfig
 
-    return UserMembershipHandler.createEntity(
+    UserMembershipHandler.createEntity(
       entityId,
       entityConfig,
       function (error, entity) {
         if (error != null) {
           return next(error)
         }
-        return res.redirect(entityConfig.pathsFor(entityId).index)
+        res.redirect(entityConfig.pathsFor(entityId).index)
       }
     )
   },

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

@@ -1,19 +1,6 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const assertCalledWith = sinon.assert.calledWith
-const assertNotCalled = sinon.assert.notCalled
-const { assert, expect } = require('chai')
+const { expect } = require('chai')
 const modulePath =
   '../../../../app/src/Features/UserMembership/UserMembershipController.js'
 const SandboxedModule = require('sandboxed-module')
@@ -43,7 +30,7 @@ describe('UserMembershipController', function () {
       fetchV1Data: callback => {
         const institution = Object.assign({}, this.institution)
         institution.name = 'Test Institution Name'
-        return callback(null, institution)
+        callback(null, institution)
       },
     }
     this.users = [
@@ -82,6 +69,9 @@ describe('UserMembershipController', function () {
       getUsers: sinon.stub().yields(null, this.users),
       addUser: sinon.stub().yields(null, this.newUser),
       removeUser: sinon.stub().yields(null),
+      promises: {
+        getUsers: sinon.stub().resolves(this.users),
+      },
     }
     this.SplitTestHandler = {
       promises: {
@@ -89,36 +79,33 @@ describe('UserMembershipController', function () {
       },
       getAssignment: sinon.stub().yields(null, { variant: 'default' }),
     }
-    return (this.UserMembershipController = SandboxedModule.require(
-      modulePath,
-      {
-        requires: {
-          './UserMembershipErrors': {
-            UserIsManagerError,
-            UserNotFoundError,
-            UserAlreadyAddedError,
-          },
-          '../Authentication/SessionManager': this.SessionManager,
-          '../SplitTests/SplitTestHandler': this.SplitTestHandler,
-          './UserMembershipHandler': this.UserMembershipHandler,
-          '@overleaf/settings': this.Settings,
-          '../../models/SSOConfig': { SSOConfig: this.SSOConfig },
+    this.UserMembershipController = SandboxedModule.require(modulePath, {
+      requires: {
+        './UserMembershipErrors': {
+          UserIsManagerError,
+          UserNotFoundError,
+          UserAlreadyAddedError,
         },
-      }
-    ))
+        '../Authentication/SessionManager': this.SessionManager,
+        '../SplitTests/SplitTestHandler': this.SplitTestHandler,
+        './UserMembershipHandler': this.UserMembershipHandler,
+        '@overleaf/settings': this.Settings,
+        '../../models/SSOConfig': { SSOConfig: this.SSOConfig },
+      },
+    })
   })
 
   describe('index', function () {
     beforeEach(function () {
       this.req.entity = this.subscription
-      return (this.req.entityConfig = EntityConfigs.group)
+      this.req.entityConfig = EntityConfigs.group
     })
 
     it('get users', async function () {
-      return await this.UserMembershipController.manageGroupMembers(this.req, {
+      await this.UserMembershipController.manageGroupMembers(this.req, {
         render: () => {
           sinon.assert.calledWithMatch(
-            this.UserMembershipHandler.getUsers,
+            this.UserMembershipHandler.promises.getUsers,
             this.subscription,
             { modelName: 'Subscription' }
           )
@@ -128,7 +115,7 @@ describe('UserMembershipController', function () {
 
     it('render group view', async function () {
       this.subscription.managedUsersEnabled = false
-      return await this.UserMembershipController.manageGroupMembers(this.req, {
+      await this.UserMembershipController.manageGroupMembers(this.req, {
         render: (viewPath, viewParams) => {
           expect(viewPath).to.equal('user_membership/group-members-react')
           expect(viewParams.users).to.deep.equal(this.users)
@@ -140,7 +127,7 @@ describe('UserMembershipController', function () {
 
     it('render group view with managed users', async function () {
       this.subscription.managedUsersEnabled = true
-      return await this.UserMembershipController.manageGroupMembers(this.req, {
+      await this.UserMembershipController.manageGroupMembers(this.req, {
         render: (viewPath, viewParams) => {
           expect(viewPath).to.equal('user_membership/group-members-react')
           expect(viewParams.users).to.deep.equal(this.users)
@@ -152,7 +139,7 @@ describe('UserMembershipController', function () {
 
     it('render group managers view', async function () {
       this.req.entityConfig = EntityConfigs.groupManagers
-      return await this.UserMembershipController.manageGroupManagers(this.req, {
+      await this.UserMembershipController.manageGroupManagers(this.req, {
         render: (viewPath, viewParams) => {
           expect(viewPath).to.equal('user_membership/group-managers-react')
           expect(viewParams.groupSize).to.equal(undefined)
@@ -163,18 +150,15 @@ describe('UserMembershipController', function () {
     it('render institution view', async function () {
       this.req.entity = this.institution
       this.req.entityConfig = EntityConfigs.institution
-      return await this.UserMembershipController.manageInstitutionManagers(
-        this.req,
-        {
-          render: (viewPath, viewParams) => {
-            expect(viewPath).to.equal(
-              'user_membership/institution-managers-react'
-            )
-            expect(viewParams.name).to.equal('Test Institution Name')
-            expect(viewParams.groupSize).to.equal(undefined)
-          },
-        }
-      )
+      await this.UserMembershipController.manageInstitutionManagers(this.req, {
+        render: (viewPath, viewParams) => {
+          expect(viewPath).to.equal(
+            'user_membership/institution-managers-react'
+          )
+          expect(viewParams.name).to.equal('Test Institution Name')
+          expect(viewParams.groupSize).to.equal(undefined)
+        },
+      })
     })
   })
 
@@ -182,11 +166,11 @@ describe('UserMembershipController', function () {
     beforeEach(function () {
       this.req.body.email = this.newUser.email
       this.req.entity = this.subscription
-      return (this.req.entityConfig = EntityConfigs.groupManagers)
+      this.req.entityConfig = EntityConfigs.groupManagers
     })
 
     it('add user', function (done) {
-      return this.UserMembershipController.add(this.req, {
+      this.UserMembershipController.add(this.req, {
         json: () => {
           sinon.assert.calledWithMatch(
             this.UserMembershipHandler.addUser,
@@ -194,36 +178,36 @@ describe('UserMembershipController', function () {
             { modelName: 'Subscription' },
             this.newUser.email
           )
-          return done()
+          done()
         },
       })
     })
 
     it('return user object', function (done) {
-      return this.UserMembershipController.add(this.req, {
+      this.UserMembershipController.add(this.req, {
         json: payload => {
           payload.user.should.equal(this.newUser)
-          return done()
+          done()
         },
       })
     })
 
     it('handle readOnly entity', function (done) {
       this.req.entityConfig = EntityConfigs.group
-      return this.UserMembershipController.add(this.req, null, error => {
+      this.UserMembershipController.add(this.req, null, error => {
         expect(error).to.exist
         expect(error).to.be.an.instanceof(Errors.NotFoundError)
-        return done()
+        done()
       })
     })
 
     it('handle user already added', function (done) {
       this.UserMembershipHandler.addUser.yields(new UserAlreadyAddedError())
-      return this.UserMembershipController.add(this.req, {
+      this.UserMembershipController.add(this.req, {
         status: () => ({
           json: payload => {
             expect(payload.error.code).to.equal('user_already_added')
-            return done()
+            done()
           },
         }),
       })
@@ -231,11 +215,11 @@ describe('UserMembershipController', function () {
 
     it('handle user not found', function (done) {
       this.UserMembershipHandler.addUser.yields(new UserNotFoundError())
-      return this.UserMembershipController.add(this.req, {
+      this.UserMembershipController.add(this.req, {
         status: () => ({
           json: payload => {
             expect(payload.error.code).to.equal('user_not_found')
-            return done()
+            done()
           },
         }),
       })
@@ -243,11 +227,11 @@ describe('UserMembershipController', function () {
 
     it('handle invalid email', function (done) {
       this.req.body.email = 'not_valid_email'
-      return this.UserMembershipController.add(this.req, {
+      this.UserMembershipController.add(this.req, {
         status: () => ({
           json: payload => {
             expect(payload.error.code).to.equal('invalid_email')
-            return done()
+            done()
           },
         }),
       })
@@ -258,11 +242,11 @@ describe('UserMembershipController', function () {
     beforeEach(function () {
       this.req.params.userId = this.newUser._id
       this.req.entity = this.subscription
-      return (this.req.entityConfig = EntityConfigs.groupManagers)
+      this.req.entityConfig = EntityConfigs.groupManagers
     })
 
     it('remove user', function (done) {
-      return this.UserMembershipController.remove(this.req, {
+      this.UserMembershipController.remove(this.req, {
         sendStatus: () => {
           sinon.assert.calledWithMatch(
             this.UserMembershipHandler.removeUser,
@@ -270,27 +254,27 @@ describe('UserMembershipController', function () {
             { modelName: 'Subscription' },
             this.newUser._id
           )
-          return done()
+          done()
         },
       })
     })
 
     it('handle readOnly entity', function (done) {
       this.req.entityConfig = EntityConfigs.group
-      return this.UserMembershipController.remove(this.req, null, error => {
+      this.UserMembershipController.remove(this.req, null, error => {
         expect(error).to.exist
         expect(error).to.be.an.instanceof(Errors.NotFoundError)
-        return done()
+        done()
       })
     })
 
     it('prevent self removal', function (done) {
       this.req.params.userId = this.user._id
-      return this.UserMembershipController.remove(this.req, {
+      this.UserMembershipController.remove(this.req, {
         status: () => ({
           json: payload => {
             expect(payload.error.code).to.equal('managers_cannot_remove_self')
-            return done()
+            done()
           },
         }),
       })
@@ -298,11 +282,11 @@ describe('UserMembershipController', function () {
 
     it('prevent admin removal', function (done) {
       this.UserMembershipHandler.removeUser.yields(new UserIsManagerError())
-      return this.UserMembershipController.remove(this.req, {
+      this.UserMembershipController.remove(this.req, {
         status: () => ({
           json: payload => {
             expect(payload.error.code).to.equal('managers_cannot_remove_admin')
-            return done()
+            done()
           },
         }),
       })
@@ -314,11 +298,11 @@ describe('UserMembershipController', function () {
       this.req.entity = this.subscription
       this.req.entityConfig = EntityConfigs.groupManagers
       this.res = new MockResponse()
-      return this.UserMembershipController.exportCsv(this.req, this.res)
+      this.UserMembershipController.exportCsv(this.req, this.res)
     })
 
     it('get users', function () {
-      return sinon.assert.calledWithMatch(
+      sinon.assert.calledWithMatch(
         this.UserMembershipHandler.getUsers,
         this.subscription,
         { modelName: 'Subscription' }
@@ -326,11 +310,11 @@ describe('UserMembershipController', function () {
     })
 
     it('should set the correct content type on the request', function () {
-      return assertCalledWith(this.res.contentType, 'text/csv; charset=utf-8')
+      assertCalledWith(this.res.contentType, 'text/csv; charset=utf-8')
     })
 
     it('should name the exported csv file', function () {
-      return assertCalledWith(
+      assertCalledWith(
         this.res.header,
         'Content-Disposition',
         'attachment; filename="Group.csv"'
@@ -338,7 +322,7 @@ describe('UserMembershipController', function () {
     })
 
     it('should export the correct csv', function () {
-      return assertCalledWith(
+      assertCalledWith(
         this.res.send,
         '"email","last_logged_in_at","last_active_at"\n"mock-email-1@foo.com","2020-08-09T12:43:11.467Z","2021-08-09T12:43:11.467Z"\n"mock-email-2@foo.com","2020-05-20T10:41:11.407Z","2021-05-20T10:41:11.407Z"'
       )
@@ -348,15 +332,15 @@ describe('UserMembershipController', function () {
   describe('new', function () {
     beforeEach(function () {
       this.req.params.name = 'publisher'
-      return (this.req.params.id = 'abc')
+      this.req.params.id = 'abc'
     })
 
     it('renders view', function (done) {
-      return this.UserMembershipController.new(this.req, {
+      this.UserMembershipController.new(this.req, {
         render: (viewPath, data) => {
           expect(data.entityName).to.eq('publisher')
           expect(data.entityId).to.eq('abc')
-          return done()
+          done()
         },
       })
     })
@@ -366,11 +350,11 @@ describe('UserMembershipController', function () {
     beforeEach(function () {
       this.req.params.name = 'institution'
       this.req.entityConfig = EntityConfigs.institution
-      return (this.req.params.id = 123)
+      this.req.params.id = 123
     })
 
     it('creates institution', function (done) {
-      return this.UserMembershipController.create(this.req, {
+      this.UserMembershipController.create(this.req, {
         redirect: path => {
           expect(path).to.eq(EntityConfigs.institution.pathsFor(123).index)
           sinon.assert.calledWithMatch(
@@ -378,7 +362,7 @@ describe('UserMembershipController', function () {
             123,
             { modelName: 'Institution' }
           )
-          return done()
+          done()
         },
       })
     })