diff --git a/services/web/app/src/Features/Contacts/ContactController.js b/services/web/app/src/Features/Contacts/ContactController.js index ee5b6eda2c..9c600593ed 100644 --- a/services/web/app/src/Features/Contacts/ContactController.js +++ b/services/web/app/src/Features/Contacts/ContactController.js @@ -1,92 +1,60 @@ -/* eslint-disable - max-len, - no-unused-vars, -*/ -// 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 - */ -let ContactsController const SessionManager = require('../Authentication/SessionManager') const ContactManager = require('./ContactManager') const UserGetter = require('../User/UserGetter') -const logger = require('@overleaf/logger') const Modules = require('../../infrastructure/Modules') +const { expressify } = require('../../util/promises') -module.exports = ContactsController = { - getContacts(req, res, next) { - const userId = SessionManager.getLoggedInUserId(req.session) - return ContactManager.getContactIds( - userId, - { limit: 50 }, - function (error, contactIds) { - if (error != null) { - return next(error) - } - return UserGetter.getUsers( - contactIds, - { - email: 1, - first_name: 1, - last_name: 1, - holdingAccount: 1, - }, - function (error, contacts) { - if (error != null) { - return next(error) - } - - // UserGetter.getUsers may not preserve order so put them back in order - const positions = {} - for (let i = 0; i < contactIds.length; i++) { - const contactId = contactIds[i] - positions[contactId] = i - } - contacts.sort( - (a, b) => - positions[a._id != null ? a._id.toString() : undefined] - - positions[b._id != null ? b._id.toString() : undefined] - ) - - // Don't count holding accounts to discourage users from repeating mistakes (mistyped or wrong emails, etc) - contacts = contacts.filter(c => !c.holdingAccount) - - contacts = contacts.map(ContactsController._formatContact) - - return Modules.hooks.fire( - 'getContacts', - userId, - contacts, - function (error, additionalContacts) { - if (error != null) { - return next(error) - } - contacts = contacts.concat( - ...Array.from(additionalContacts || []) - ) - return res.json({ - contacts, - }) - } - ) - } - ) - } - ) - }, - - _formatContact(contact) { - return { - id: contact._id != null ? contact._id.toString() : undefined, - email: contact.email || '', - first_name: contact.first_name || '', - last_name: contact.last_name || '', - type: 'user', - } - }, +function _formatContact(contact) { + return { + id: contact._id?.toString(), + email: contact.email || '', + first_name: contact.first_name || '', + last_name: contact.last_name || '', + type: 'user', + } +} + +async function getContacts(req, res) { + const userId = SessionManager.getLoggedInUserId(req.session) + + const contactIds = await ContactManager.promises.getContactIds(userId, { + limit: 50, + }) + + let contacts = await UserGetter.promises.getUsers(contactIds, { + email: 1, + first_name: 1, + last_name: 1, + holdingAccount: 1, + }) + + // UserGetter.getUsers may not preserve order so put them back in order + const positions = {} + for (let i = 0; i < contactIds.length; i++) { + const contactId = contactIds[i] + positions[contactId] = i + } + contacts.sort( + (a, b) => positions[a._id?.toString()] - positions[b._id?.toString()] + ) + + // Don't count holding accounts to discourage users from repeating mistakes (mistyped or wrong emails, etc) + contacts = contacts.filter(c => !c.holdingAccount) + + contacts = contacts.map(_formatContact) + + const additionalContacts = await Modules.promises.hooks.fire( + 'getContacts', + userId, + contacts + ) + + contacts = contacts.concat(...(additionalContacts || [])) + return res.json({ + contacts, + }) +} + +module.exports = { + getContacts: expressify(getContacts), } diff --git a/services/web/app/src/Features/Contacts/ContactManager.js b/services/web/app/src/Features/Contacts/ContactManager.js index ae0f872fe4..788bcf9434 100644 --- a/services/web/app/src/Features/Contacts/ContactManager.js +++ b/services/web/app/src/Features/Contacts/ContactManager.js @@ -1,90 +1,64 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - 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 - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let ContactManager +const { callbackify } = require('util') const OError = require('@overleaf/o-error') -const request = require('request') +const fetch = require('node-fetch') const settings = require('@overleaf/settings') -module.exports = ContactManager = { - getContactIds(userId, options, callback) { - if (options == null) { - options = { limits: 50 } - } - if (callback == null) { - callback = function () {} - } - const url = `${settings.apis.contacts.url}/user/${userId}/contacts` - return request.get( - { - url, - qs: options, - json: true, - jar: false, - }, - function (error, res, data) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - return callback( - null, - (data != null ? data.contact_ids : undefined) || [] - ) - } else { - error = new OError( - `contacts api responded with non-success code: ${res.statusCode}`, - { user_id: userId } - ) - return callback(error) - } - } - ) - }, +async function getContactIds(userId, options) { + options = options ?? { limit: 50 } - addContact(userId, contactId, callback) { - if (callback == null) { - callback = function () {} - } - const url = `${settings.apis.contacts.url}/user/${userId}/contacts` - return request.post( + const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`) + + for (const [key, val] of Object.entries(options)) { + url.searchParams.set(key, val) + } + + const response = await fetch(url.toString(), { + method: 'GET', + headers: { Accept: 'application/json' }, + }) + + const body = await response.json() + + if (!response.ok) { + throw new OError( + `contacts api responded with non-success code: ${response.statusCode}`, + { user_id: userId } + ) + } + + return body?.contact_ids || [] +} + +async function addContact(userId, contactId) { + const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`) + const response = await fetch(url.toString(), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + }, + body: JSON.stringify({ contact_id: contactId }), + }) + + const body = await response.json() + if (!response.ok) { + throw new OError( + `contacts api responded with non-success code: ${response.statusCode}`, { - url, - json: { - contact_id: contactId, - }, - jar: false, - }, - function (error, res, data) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - return callback( - null, - (data != null ? data.contact_ids : undefined) || [] - ) - } else { - error = new OError( - `contacts api responded with non-success code: ${res.statusCode}`, - { - user_id: userId, - contact_id: contactId, - } - ) - return callback(error) - } + user_id: userId, + contact_id: contactId, } ) + } + + return body?.contact_ids || [] +} + +module.exports = { + getContactIds: callbackify(getContactIds), + addContact: callbackify(addContact), + promises: { + getContactIds, + addContact, }, } diff --git a/services/web/test/unit/src/Contact/ContactControllerTests.js b/services/web/test/unit/src/Contact/ContactControllerTests.js index 668703c337..7cb8becae8 100644 --- a/services/web/test/unit/src/Contact/ContactControllerTests.js +++ b/services/web/test/unit/src/Contact/ContactControllerTests.js @@ -1,18 +1,5 @@ -/* eslint-disable - max-len, - no-dupe-keys, - 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/Contacts/ContactController.js' const SandboxedModule = require('sandboxed-module') const MockResponse = require('../helpers/MockResponse') @@ -22,14 +9,15 @@ describe('ContactController', function () { this.SessionManager = { getLoggedInUserId: sinon.stub() } this.ContactController = SandboxedModule.require(modulePath, { requires: { - '../User/UserGetter': (this.UserGetter = {}), - './ContactManager': (this.ContactManager = {}), + '../User/UserGetter': (this.UserGetter = { promises: {} }), + './ContactManager': (this.ContactManager = { promises: {} }), '../Authentication/SessionManager': (this.SessionManager = {}), - '../../infrastructure/Modules': (this.Modules = { hooks: {} }), + '../../infrastructure/Modules': (this.Modules = { + promises: { hooks: {} }, + }), }, }) - this.next = sinon.stub() this.req = {} this.res = new MockResponse() }) @@ -63,63 +51,77 @@ describe('ContactController', function () { }, ] this.SessionManager.getLoggedInUserId = sinon.stub().returns(this.user_id) - this.ContactManager.getContactIds = sinon + this.ContactManager.promises.getContactIds = sinon .stub() - .callsArgWith(2, null, this.contact_ids) - this.UserGetter.getUsers = sinon - .stub() - .callsArgWith(2, null, this.contacts) - this.Modules.hooks.fire = sinon.stub().callsArg(3) - - return this.ContactController.getContacts(this.req, this.res, this.next) + .resolves(this.contact_ids) + this.UserGetter.promises.getUsers = sinon.stub().resolves(this.contacts) + this.Modules.promises.hooks.fire = sinon.stub() }) - it('should look up the logged in user id', function () { - return this.SessionManager.getLoggedInUserId + it('should look up the logged in user id', async function () { + this.ContactController.getContacts(this.req, this.res) + this.SessionManager.getLoggedInUserId .calledWith(this.req.session) .should.equal(true) }) - it('should get the users contact ids', function () { - return this.ContactManager.getContactIds - .calledWith(this.user_id, { limit: 50 }) - .should.equal(true) + it('should get the users contact ids', async function () { + this.res.callback = () => { + expect( + this.ContactManager.promises.getContactIds + ).to.have.been.calledWith(this.user_id, { limit: 50 }) + } + this.ContactController.getContacts(this.req, this.res) }) - it('should populate the users contacts ids', function () { - return this.UserGetter.getUsers - .calledWith(this.contact_ids, { - email: 1, - first_name: 1, - last_name: 1, - holdingAccount: 1, - }) - .should.equal(true) + it('should populate the users contacts ids', function (done) { + this.res.callback = () => { + expect(this.UserGetter.promises.getUsers).to.have.been.calledWith( + this.contact_ids, + { + email: 1, + first_name: 1, + last_name: 1, + holdingAccount: 1, + } + ) + done() + } + this.ContactController.getContacts(this.req, this.res, done) }) - it('should fire the getContact module hook', function () { - return this.Modules.hooks.fire - .calledWith('getContacts', this.user_id) - .should.equal(true) + it('should fire the getContact module hook', function (done) { + this.res.callback = () => { + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'getContacts', + this.user_id + ) + done() + } + this.ContactController.getContacts(this.req, this.res, done) }) - it('should return a formatted list of contacts in contact list order, without holding accounts', function () { - return this.res.json.args[0][0].contacts.should.deep.equal([ - { - id: 'contact-1', - email: 'joe@example.com', - first_name: 'Joe', - last_name: 'Example', - type: 'user', - }, - { - id: 'contact-3', - email: 'jim@example.com', - first_name: 'Jim', - last_name: 'Example', - type: 'user', - }, - ]) + it('should return a formatted list of contacts in contact list order, without holding accounts', function (done) { + this.res.callback = () => { + this.res.json.args[0][0].contacts.should.deep.equal([ + { + id: 'contact-1', + email: 'joe@example.com', + first_name: 'Joe', + last_name: 'Example', + type: 'user', + }, + { + id: 'contact-3', + email: 'jim@example.com', + first_name: 'Jim', + last_name: 'Example', + type: 'user', + }, + ]) + done() + } + this.ContactController.getContacts(this.req, this.res, done) }) }) }) diff --git a/services/web/test/unit/src/Contact/ContactManagerTests.js b/services/web/test/unit/src/Contact/ContactManagerTests.js index 96dfae7839..31dddabf53 100644 --- a/services/web/test/unit/src/Contact/ContactManagerTests.js +++ b/services/web/test/unit/src/Contact/ContactManagerTests.js @@ -1,158 +1,114 @@ -/* eslint-disable - max-len, - no-return-assign, -*/ -// 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 { expect } = require('chai') const sinon = require('sinon') const modulePath = '../../../../app/src/Features/Contacts/ContactManager' const SandboxedModule = require('sandboxed-module') describe('ContactManager', function () { beforeEach(function () { + this.user_id = 'user-id-123' + this.contact_id = 'contact-id-123' + this.contact_ids = ['mock', 'contact_ids'] + this.qs = new URLSearchParams({ limit: 42 }) + this.fetch = sinon.stub() this.ContactManager = SandboxedModule.require(modulePath, { requires: { - request: (this.request = sinon.stub()), + 'node-fetch': this.fetch, '@overleaf/settings': (this.settings = { apis: { contacts: { - url: 'contacts.sharelatex.com', + url: 'http://contacts.sharelatex.com', }, }, }), }, }) - - this.user_id = 'user-id-123' - this.contact_id = 'contact-id-123' - return (this.callback = sinon.stub()) }) describe('getContacts', function () { describe('with a successful response code', function () { - beforeEach(function () { - this.request.get = sinon - .stub() - .callsArgWith( - 1, - null, - { statusCode: 204 }, - { contact_ids: (this.contact_ids = ['mock', 'contact_ids']) } - ) - return this.ContactManager.getContactIds( + beforeEach(async function () { + this.response = { + ok: true, + json: sinon.stub().resolves({ contact_ids: this.contact_ids }), + } + this.fetch.resolves(this.response) + + this.result = await this.ContactManager.promises.getContactIds( this.user_id, - { limit: 42 }, - this.callback + { limit: 42 } ) }) it('should get the contacts from the contacts api', function () { - return this.request.get - .calledWith({ - url: `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts`, - qs: { limit: 42 }, - json: true, - jar: false, + this.fetch.should.have.been.calledWithMatch( + `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts?${this.qs}`, + sinon.match({ + method: 'GET', + headers: { Accept: 'application/json' }, }) - .should.equal(true) + ) }) - it('should call the callback with the contatcs', function () { - return this.callback - .calledWith(null, this.contact_ids) - .should.equal(true) + it('should return the contacts', function () { + this.result.should.equal(this.contact_ids) }) }) describe('with a failed response code', function () { - beforeEach(function () { - this.request.get = sinon - .stub() - .callsArgWith(1, null, { statusCode: 500 }, null) - return this.ContactManager.getContactIds( - this.user_id, - { limit: 42 }, - this.callback - ) + beforeEach(async function () { + this.response = { + ok: false, + statusCode: 500, + json: sinon.stub().resolves({ contact_ids: this.contact_ids }), + } + this.fetch.resolves(this.response) }) - it('should call the callback with an error', function () { - return this.callback - .calledWith( - sinon.match - .instanceOf(Error) - .and( - sinon.match.has( - 'message', - 'contacts api responded with non-success code: 500' - ) - ) - ) - .should.equal(true) + it('should reject the promise', async function () { + await expect( + this.ContactManager.promises.getContactIds(this.user_id, { + limit: 42, + }) + ).to.be.rejectedWith( + 'contacts api responded with non-success code: 500' + ) }) }) }) describe('addContact', function () { describe('with a successful response code', function () { - beforeEach(function () { - this.request.post = sinon - .stub() - .callsArgWith(1, null, { statusCode: 200 }, null) - return this.ContactManager.addContact( + beforeEach(async function () { + this.response = { + ok: true, + json: sinon.stub().resolves({ contact_ids: this.contact_ids }), + } + this.fetch.resolves(this.response) + + this.result = await this.ContactManager.promises.addContact( this.user_id, - this.contact_id, - this.callback + this.contact_id ) }) it('should add the contacts for the user in the contacts api', function () { - return this.request.post - .calledWith({ - url: `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts`, - json: { - contact_id: this.contact_id, + this.fetch.should.have.been.calledWithMatch( + `${this.settings.apis.contacts.url}/user/${this.user_id}/contacts`, + sinon.match({ + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', }, - jar: false, + body: JSON.stringify({ + contact_id: this.contact_id, + }), }) - .should.equal(true) - }) - - it('should call the callback', function () { - return this.callback.called.should.equal(true) - }) - }) - - describe('with a failed response code', function () { - beforeEach(function () { - this.request.post = sinon - .stub() - .callsArgWith(1, null, { statusCode: 500 }, null) - return this.ContactManager.addContact( - this.user_id, - this.contact_id, - this.callback ) }) - it('should call the callback with an error', function () { - return this.callback - .calledWith( - sinon.match - .instanceOf(Error) - .and( - sinon.match.has( - 'message', - 'contacts api responded with non-success code: 500' - ) - ) - ) - .should.equal(true) + it('should call the callback', function () { + this.result.should.equal(this.contact_ids) }) }) })