From 4bcab345803e90867f8e040591c8c8d303135a9c Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Wed, 5 May 2021 15:20:32 +0200 Subject: [PATCH] Merge pull request #3978 from overleaf/jel-notifications-decaf Notifications decaf cleanup and remove eslint disable GitOrigin-RevId: 557a7c63aecda346501d56d1eb18935b12130e8a --- .../Notifications/NotificationsController.js | 26 ++---- .../Notifications/NotificationsHandler.js | 89 ++++++------------- services/web/app/src/router.js | 2 +- .../NotificationsControllerTests.js | 40 +++------ 4 files changed, 48 insertions(+), 109 deletions(-) diff --git a/services/web/app/src/Features/Notifications/NotificationsController.js b/services/web/app/src/Features/Notifications/NotificationsController.js index 539f6b62f5..68d920fcfe 100644 --- a/services/web/app/src/Features/Notifications/NotificationsController.js +++ b/services/web/app/src/Features/Notifications/NotificationsController.js @@ -1,24 +1,12 @@ -/* eslint-disable - camelcase, - node/handle-callback-err, - max-len, -*/ -// 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 NotificationsHandler = require('./NotificationsHandler') const AuthenticationController = require('../Authentication/AuthenticationController') const _ = require('underscore') module.exports = { getAllUnreadNotifications(req, res) { - const user_id = AuthenticationController.getLoggedInUserId(req) - return NotificationsHandler.getUserNotifications( - user_id, + const userId = AuthenticationController.getLoggedInUserId(req) + NotificationsHandler.getUserNotifications( + userId, function (err, unreadNotifications) { unreadNotifications = _.map( unreadNotifications, @@ -30,15 +18,15 @@ module.exports = { return notification } ) - return res.send(unreadNotifications) + res.send(unreadNotifications) } ) }, markNotificationAsRead(req, res) { - const user_id = AuthenticationController.getLoggedInUserId(req) - const { notification_id } = req.params - NotificationsHandler.markAsRead(user_id, notification_id, () => + const userId = AuthenticationController.getLoggedInUserId(req) + const { notificationId } = req.params + NotificationsHandler.markAsRead(userId, notificationId, () => res.sendStatus(200) ) }, diff --git a/services/web/app/src/Features/Notifications/NotificationsHandler.js b/services/web/app/src/Features/Notifications/NotificationsHandler.js index ce2deccda9..a83cb9e047 100644 --- a/services/web/app/src/Features/Notifications/NotificationsHandler.js +++ b/services/web/app/src/Features/Notifications/NotificationsHandler.js @@ -1,65 +1,46 @@ -/* eslint-disable - camelcase, - 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 - */ const settings = require('settings-sharelatex') const request = require('request') const logger = require('logger-sharelatex') +const _ = require('lodash') +const notificationsApi = _.get(settings, ['apis', 'notifications', 'url']) const oneSecond = 1000 const makeRequest = function (opts, callback) { - if ( - (settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined) == null - ) { - return callback(null, { statusCode: 200 }) + if (notificationsApi) { + request(opts, callback) } else { - return request(opts, callback) + callback(null, { statusCode: 200 }) } } module.exports = { - getUserNotifications(user_id, callback) { + getUserNotifications(userId, callback) { const opts = { - uri: `${ - settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined - }/user/${user_id}`, + uri: `${notificationsApi}/user/${userId}`, json: true, timeout: oneSecond, method: 'GET', } - return makeRequest(opts, function (err, res, unreadNotifications) { - const statusCode = res != null ? res.statusCode : 500 - if (err != null || statusCode !== 200) { - const e = new Error( - `something went wrong getting notifications, ${err}, ${statusCode}` + makeRequest(opts, function (err, res, unreadNotifications) { + const statusCode = res ? res.statusCode : 500 + if (err || statusCode !== 200) { + logger.err( + { err, statusCode }, + 'something went wrong getting notifications' ) - logger.err({ err }, 'something went wrong getting notifications') - return callback(null, []) + callback(null, []) } else { if (unreadNotifications == null) { unreadNotifications = [] } - return callback(null, unreadNotifications) + callback(null, unreadNotifications) } }) }, createNotification( - user_id, + userId, key, templateKey, messageOpts, @@ -77,63 +58,47 @@ module.exports = { templateKey, forceCreate, } - if (expiryDateTime != null) { + if (expiryDateTime) { payload.expires = expiryDateTime } const opts = { - uri: `${ - settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined - }/user/${user_id}`, + uri: `${notificationsApi}/user/${userId}`, timeout: oneSecond, method: 'POST', json: payload, } - return makeRequest(opts, callback) + makeRequest(opts, callback) }, - markAsReadWithKey(user_id, key, callback) { + markAsReadWithKey(userId, key, callback) { const opts = { - uri: `${ - settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined - }/user/${user_id}`, + uri: `${notificationsApi}/user/${userId}`, method: 'DELETE', timeout: oneSecond, json: { key, }, } - return makeRequest(opts, callback) + makeRequest(opts, callback) }, - markAsRead(user_id, notification_id, callback) { + markAsRead(userId, notificationId, callback) { const opts = { method: 'DELETE', - uri: `${ - settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined - }/user/${user_id}/notification/${notification_id}`, + uri: `${notificationsApi}/user/${userId}/notification/${notificationId}`, timeout: oneSecond, } - return makeRequest(opts, callback) + makeRequest(opts, callback) }, // removes notification by key, without regard for user_id, // should not be exposed to user via ui/router markAsReadByKeyOnly(key, callback) { const opts = { - uri: `${ - settings.apis.notifications != null - ? settings.apis.notifications.url - : undefined - }/key/${key}`, + uri: `${notificationsApi}/key/${key}`, method: 'DELETE', timeout: oneSecond, } - return makeRequest(opts, callback) + makeRequest(opts, callback) }, } diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 6c41857369..83ee901d9e 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -725,7 +725,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { NotificationsController.getAllUnreadNotifications ) webRouter.delete( - '/notifications/:notification_id', + '/notifications/:notificationId', AuthenticationController.requireLogin(), NotificationsController.markNotificationAsRead ) diff --git a/services/web/test/unit/src/Notifications/NotificationsControllerTests.js b/services/web/test/unit/src/Notifications/NotificationsControllerTests.js index 3dcd55ffb0..288b9aa19c 100644 --- a/services/web/test/unit/src/Notifications/NotificationsControllerTests.js +++ b/services/web/test/unit/src/Notifications/NotificationsControllerTests.js @@ -1,18 +1,4 @@ -/* eslint-disable - camelcase, - 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 SandboxedModule = require('sandboxed-module') -const assert = require('assert') const sinon = require('sinon') const modulePath = require('path').join( __dirname, @@ -20,8 +6,8 @@ const modulePath = require('path').join( ) describe('NotificationsController', function () { - const user_id = '123nd3ijdks' - const notification_id = '123njdskj9jlk' + const userId = '123nd3ijdks' + const notificationId = '123njdskj9jlk' beforeEach(function () { this.handler = { @@ -30,11 +16,11 @@ describe('NotificationsController', function () { } this.req = { params: { - notification_id, + notificationId, }, session: { user: { - _id: user_id, + _id: userId, }, }, i18n: { @@ -44,36 +30,36 @@ describe('NotificationsController', function () { this.AuthenticationController = { getLoggedInUserId: sinon.stub().returns(this.req.session.user._id), } - return (this.controller = SandboxedModule.require(modulePath, { + this.controller = SandboxedModule.require(modulePath, { requires: { './NotificationsHandler': this.handler, '../Authentication/AuthenticationController': this .AuthenticationController, }, - })) + }) }) it('should ask the handler for all unread notifications', function (done) { - const allNotifications = [{ _id: notification_id, user_id }] + const allNotifications = [{ _id: notificationId, user_id: userId }] this.handler.getUserNotifications = sinon .stub() .callsArgWith(1, null, allNotifications) - return this.controller.getAllUnreadNotifications(this.req, { + this.controller.getAllUnreadNotifications(this.req, { send: body => { body.should.deep.equal(allNotifications) - this.handler.getUserNotifications.calledWith(user_id).should.equal(true) - return done() + this.handler.getUserNotifications.calledWith(userId).should.equal(true) + done() }, }) }) it('should send a delete request when a delete has been received to mark a notification', function (done) { - return this.controller.markNotificationAsRead(this.req, { + this.controller.markNotificationAsRead(this.req, { sendStatus: () => { this.handler.markAsRead - .calledWith(user_id, notification_id) + .calledWith(userId, notificationId) .should.equal(true) - return done() + done() }, }) })