Merge pull request #3978 from overleaf/jel-notifications-decaf

Notifications decaf cleanup and remove eslint disable

GitOrigin-RevId: 557a7c63aecda346501d56d1eb18935b12130e8a

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)
     )
   },

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)
   },
 }

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
   )

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()
       },
     })
   })