From 997bffc9b1088853976574bb43ff03782028ae0b Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Wed, 6 Apr 2022 14:33:18 +0200 Subject: [PATCH] Merge pull request #7166 from overleaf/ab-decaf-user-info-controller [web] Decaf cleanup UserInfoController GitOrigin-RevId: c1bc531ed2923f05652090d792b7f1e7b4c1275c --- .../src/Features/User/UserInfoController.js | 154 +++++++++--------- .../app/src/Features/User/UserInfoManager.js | 23 ++- .../unit/src/User/UserInfoControllerTests.js | 104 ++---------- 3 files changed, 106 insertions(+), 175 deletions(-) diff --git a/services/web/app/src/Features/User/UserInfoController.js b/services/web/app/src/Features/User/UserInfoController.js index 8de15065c7..43f0c923e8 100644 --- a/services/web/app/src/Features/User/UserInfoController.js +++ b/services/web/app/src/Features/User/UserInfoController.js @@ -1,82 +1,86 @@ -let UserController const UserGetter = require('./UserGetter') const SessionManager = require('../Authentication/SessionManager') const { ObjectId } = require('mongodb') -module.exports = UserController = { - getLoggedInUsersPersonalInfo(req, res, next) { - const userId = SessionManager.getLoggedInUserId(req.session) - if (!userId) { - return next(new Error('User is not logged in')) - } - UserGetter.getUser( - userId, - { - first_name: true, - last_name: true, - role: true, - institution: true, - email: true, - signUpDate: true, - }, - function (error, user) { - if (error) { - return next(error) - } - UserController.sendFormattedPersonalInfo(user, res, next) +function getLoggedInUsersPersonalInfo(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + if (!userId) { + return next(new Error('User is not logged in')) + } + UserGetter.getUser( + userId, + { + first_name: true, + last_name: true, + role: true, + institution: true, + email: true, + signUpDate: true, + }, + function (error, user) { + if (error) { + return next(error) } - ) - }, - - getPersonalInfo(req, res, next) { - let query - const userId = req.params.user_id - - if (/^\d+$/.test(userId)) { - query = { 'overleaf.id': parseInt(userId, 10) } - } else if (/^[a-f0-9]{24}$/.test(userId)) { - query = { _id: ObjectId(userId) } - } else { - return res.sendStatus(400) + sendFormattedPersonalInfo(user, res, next) } - - UserGetter.getUser( - query, - { _id: true, first_name: true, last_name: true, email: true }, - function (error, user) { - if (error) { - return next(error) - } - if (!user) { - return res.sendStatus(404) - } - UserController.sendFormattedPersonalInfo(user, res, next) - } - ) - }, - - sendFormattedPersonalInfo(user, res, next) { - const info = UserController.formatPersonalInfo(user) - res.json(info) - }, - - formatPersonalInfo(user, callback) { - if (!user) { - return {} - } - const formattedUser = { id: user._id.toString() } - for (const key of [ - 'first_name', - 'last_name', - 'email', - 'signUpDate', - 'role', - 'institution', - ]) { - if (user[key]) { - formattedUser[key] = user[key] - } - } - return formattedUser - }, + ) +} + +function getPersonalInfo(req, res, next) { + let query + const userId = req.params.user_id + + if (/^\d+$/.test(userId)) { + query = { 'overleaf.id': parseInt(userId, 10) } + } else if (/^[a-f0-9]{24}$/.test(userId)) { + query = { _id: ObjectId(userId) } + } else { + return res.sendStatus(400) + } + + UserGetter.getUser( + query, + { _id: true, first_name: true, last_name: true, email: true }, + function (error, user) { + if (error) { + return next(error) + } + if (!user) { + return res.sendStatus(404) + } + sendFormattedPersonalInfo(user, res, next) + } + ) +} + +function sendFormattedPersonalInfo(user, res, next) { + const info = formatPersonalInfo(user) + res.json(info) +} + +function formatPersonalInfo(user) { + if (!user) { + return {} + } + const formattedUser = { id: user._id.toString() } + for (const key of [ + 'first_name', + 'last_name', + 'email', + 'signUpDate', + 'role', + 'institution', + ]) { + if (user[key]) { + formattedUser[key] = user[key] + } + } + return formattedUser +} + +module.exports = { + getLoggedInUsersPersonalInfo, + getPersonalInfo, + sendFormattedPersonalInfo, + formatPersonalInfo, } diff --git a/services/web/app/src/Features/User/UserInfoManager.js b/services/web/app/src/Features/User/UserInfoManager.js index d1a44e22c7..f5d23d7cd8 100644 --- a/services/web/app/src/Features/User/UserInfoManager.js +++ b/services/web/app/src/Features/User/UserInfoManager.js @@ -1,13 +1,18 @@ const UserGetter = require('./UserGetter') +const { callbackify } = require('../../util/promises') -const UserInfoManager = { - getPersonalInfo(userId, callback) { - UserGetter.getUser( - userId, - { _id: true, first_name: true, last_name: true, email: true }, - callback - ) - }, +async function getPersonalInfo(userId) { + return UserGetter.promises.getUser(userId, { + _id: true, + first_name: true, + last_name: true, + email: true, + }) } -module.exports = UserInfoManager +module.exports = { + getPersonalInfo: callbackify(getPersonalInfo), + promises: { + getPersonalInfo, + }, +} diff --git a/services/web/test/unit/src/User/UserInfoControllerTests.js b/services/web/test/unit/src/User/UserInfoControllerTests.js index 0012d643ba..6f48fb8731 100644 --- a/services/web/test/unit/src/User/UserInfoControllerTests.js +++ b/services/web/test/unit/src/User/UserInfoControllerTests.js @@ -1,20 +1,7 @@ -/* 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 { assert, expect } = require('chai') +const { expect } = require('chai') const modulePath = '../../../../app/src/Features/User/UserInfoController.js' const SandboxedModule = require('sandboxed-module') -const events = require('events') const MockResponse = require('../helpers/MockResponse') const MockRequest = require('../helpers/MockRequest') const { ObjectId } = require('mongodb') @@ -39,31 +26,7 @@ describe('UserInfoController', function () { this.req = new MockRequest() this.res = new MockResponse() - return (this.next = sinon.stub()) - }) - - describe('getLoggedInUsersPersonalInfo', function () { - beforeEach(function () { - this.user = { _id: ObjectId() } - this.req.user = this.user - this.req.session.user = this.user - this.UserInfoController.sendFormattedPersonalInfo = sinon.stub() - this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user) - this.SessionManager.getLoggedInUserId = sinon - .stub() - .returns(this.user._id) - return this.UserInfoController.getLoggedInUsersPersonalInfo( - this.req, - this.res, - this.next - ) - }) - - it('should call sendFormattedPersonalInfo', function () { - return this.UserInfoController.sendFormattedPersonalInfo - .calledWith(this.user, this.res, this.next) - .should.equal(true) - }) + this.next = sinon.stub() }) describe('getPersonalInfo', function () { @@ -74,27 +37,17 @@ describe('UserInfoController', function () { this.req.params = { user_id: this.user_id } this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user) this.UserInfoController.sendFormattedPersonalInfo = sinon.stub() - return this.UserInfoController.getPersonalInfo( - this.req, - this.res, - this.next - ) + this.UserInfoController.getPersonalInfo(this.req, this.res, this.next) }) it('should look up the user in the database', function () { - return this.UserGetter.getUser + this.UserGetter.getUser .calledWith( { _id: ObjectId(this.user_id) }, { _id: true, first_name: true, last_name: true, email: true } ) .should.equal(true) }) - - it('should send the formatted details back to the client', function () { - return this.UserInfoController.sendFormattedPersonalInfo - .calledWith(this.user, this.res, this.next) - .should.equal(true) - }) }) describe('when the user exists with overleaf id', function () { @@ -108,28 +61,17 @@ describe('UserInfoController', function () { } this.req.params = { user_id: this.user_id.toString() } this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user) - this.UserInfoController.sendFormattedPersonalInfo = sinon.stub() - return this.UserInfoController.getPersonalInfo( - this.req, - this.res, - this.next - ) + this.UserInfoController.getPersonalInfo(this.req, this.res, this.next) }) it('should look up the user in the database', function () { - return this.UserGetter.getUser + this.UserGetter.getUser .calledWith( { 'overleaf.id': this.user_id }, { _id: true, first_name: true, last_name: true, email: true } ) .should.equal(true) }) - - it('should send the formatted details back to the client', function () { - return this.UserInfoController.sendFormattedPersonalInfo - .calledWith(this.user, this.res, this.next) - .should.equal(true) - }) }) describe('when the user does not exist', function () { @@ -137,15 +79,11 @@ describe('UserInfoController', function () { this.user_id = ObjectId().toString() this.req.params = { user_id: this.user_id } this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) - return this.UserInfoController.getPersonalInfo( - this.req, - this.res, - this.next - ) + this.UserInfoController.getPersonalInfo(this.req, this.res, this.next) }) it('should return 404 to the client', function () { - return this.res.statusCode.should.equal(404) + this.res.statusCode.should.equal(404) }) }) @@ -154,15 +92,11 @@ describe('UserInfoController', function () { this.user_id = 'invalid' this.req.params = { user_id: this.user_id } this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) - return this.UserInfoController.getPersonalInfo( - this.req, - this.res, - this.next - ) + this.UserInfoController.getPersonalInfo(this.req, this.res, this.next) }) it('should return 400 to the client', function () { - return this.res.statusCode.should.equal(400) + this.res.statusCode.should.equal(400) }) }) }) @@ -181,23 +115,11 @@ describe('UserInfoController', function () { last_name: this.user.last_name, email: this.user.email, } - this.UserInfoController.formatPersonalInfo = sinon - .stub() - .returns(this.formattedInfo) - return this.UserInfoController.sendFormattedPersonalInfo( - this.user, - this.res - ) - }) - - it('should format the user details for the response', function () { - return this.UserInfoController.formatPersonalInfo - .calledWith(this.user) - .should.equal(true) + this.UserInfoController.sendFormattedPersonalInfo(this.user, this.res) }) it('should send the formatted details back to the client', function () { - return this.res.body.should.equal(JSON.stringify(this.formattedInfo)) + this.res.body.should.equal(JSON.stringify(this.formattedInfo)) }) }) @@ -213,7 +135,7 @@ describe('UserInfoController', function () { role: 'student', institution: 'sheffield', } - return expect( + expect( this.UserInfoController.formatPersonalInfo(this.user) ).to.deep.equal({ id: this.user._id.toString(),