Merge pull request #21 from overleaf/lint-format

Lint & Format

services/notifications/Makefile

@@ -17,13 +17,13 @@ clean:
 	docker rmi gcr.io/overleaf-ops/$(PROJECT_NAME):$(BRANCH_NAME)-$(BUILD_NUMBER)
 
 format:
-	$(DOCKER_COMPOSE) run --rm test_unit npm run format || true
+	$(DOCKER_COMPOSE) run --rm test_unit npm run format
 
 format_fix:
 	$(DOCKER_COMPOSE) run --rm test_unit npm run format:fix
 
 lint:
-	$(DOCKER_COMPOSE) run --rm test_unit npm run lint || true
+	$(DOCKER_COMPOSE) run --rm test_unit npm run lint
 
 test: format lint test_unit test_acceptance
 

services/notifications/app.js

@@ -16,9 +16,6 @@ const methodOverride = require('method-override')
 const bodyParser = require('body-parser')
 const errorHandler = require('errorhandler')
 const controller = require('./app/js/NotificationsController')
-const mongojs = require('mongojs')
-const db = mongojs(Settings.mongo.url, ['notifications'])
-const Path = require('path')
 
 metrics.memory.monitor(logger)
 
@@ -43,7 +40,7 @@ app.delete('/key/:key', controller.removeNotificationByKeyOnly)
 app.get('/status', (req, res) => res.send('notifications sharelatex up'))
 
 app.get('/health_check', (req, res) =>
-  HealthCheckController.check(function(err) {
+  HealthCheckController.check(function (err) {
     if (err != null) {
       logger.err({ err }, 'error performing health check')
       return res.sendStatus(500)
@@ -58,12 +55,12 @@ app.get('*', (req, res) => res.sendStatus(404))
 const host =
   __guard__(
     Settings.internal != null ? Settings.internal.notifications : undefined,
-    x => x.host
+    (x) => x.host
   ) || 'localhost'
 const port =
   __guard__(
     Settings.internal != null ? Settings.internal.notifications : undefined,
-    x1 => x1.port
+    (x1) => x1.port
   ) || 3042
 app.listen(port, host, () =>
   logger.info(`notifications starting up, listening on ${host}:${port}`)

services/notifications/app/js/HealthCheckController.js

@@ -28,11 +28,11 @@ const db = mongojs(Settings.mongo != null ? Settings.mongo.url : undefined, [
 module.exports = {
   check(callback) {
     const user_id = ObjectId()
-    const cleanupNotifications = callback =>
+    const cleanupNotifications = (callback) =>
       db.notifications.remove({ user_id }, callback)
 
     let notification_key = `smoke-test-notification-${ObjectId()}`
-    const getOpts = endPath => ({
+    const getOpts = (endPath) => ({
       url: `http://localhost:${port}/user/${user_id}${endPath}`,
       timeout: 5000
     })
@@ -41,7 +41,7 @@ module.exports = {
       'Health Check: running'
     )
     const jobs = [
-      function(cb) {
+      function (cb) {
         const opts = getOpts('/')
         opts.json = {
           key: notification_key,
@@ -51,10 +51,10 @@ module.exports = {
         }
         return request.post(opts, cb)
       },
-      function(cb) {
+      function (cb) {
         const opts = getOpts('/')
         opts.json = true
-        return request.get(opts, function(err, res, body) {
+        return request.get(opts, function (err, res, body) {
           if (err != null) {
             logger.err({ err }, 'Health Check: error getting notification')
             return callback(err)
@@ -65,7 +65,7 @@ module.exports = {
           }
           const hasNotification = _.some(
             body,
-            notification =>
+            (notification) =>
               notification.key === notification_key &&
               notification.user_id === user_id.toString()
           )
@@ -81,7 +81,7 @@ module.exports = {
         })
       }
     ]
-    return async.series(jobs, function(err, body) {
+    return async.series(jobs, function (err, body) {
       if (err != null) {
         logger.err({ err }, 'Health Check: error running health check')
         return cleanupNotifications(() => callback(err))
@@ -93,7 +93,7 @@ module.exports = {
           { notification_id, notification_key },
           'Health Check: doing cleanup'
         )
-        return request.del(opts, function(err, res, body) {
+        return request.del(opts, function (err, res, body) {
           if (err != null) {
             logger.err(
               err,
@@ -104,7 +104,7 @@ module.exports = {
           }
           opts = getOpts('')
           opts.json = { key: notification_key }
-          return request.del(opts, function(err, res, body) {
+          return request.del(opts, function (err, res, body) {
             if (err != null) {
               logger.err(
                 err,

services/notifications/app/js/Notifications.js

@@ -24,7 +24,7 @@ const metrics = require('metrics-sharelatex')
 module.exports = Notifications = {
   getUserNotifications(user_id, callback) {
     if (callback == null) {
-      callback = function(err, notifications) {}
+      callback = function (err, notifications) {}
     }
     const query = {
       user_id: ObjectId(user_id),
@@ -37,13 +37,13 @@ module.exports = Notifications = {
 
   _countExistingNotifications(user_id, notification, callback) {
     if (callback == null) {
-      callback = function(err, count) {}
+      callback = function (err, count) {}
     }
     const query = {
       user_id: ObjectId(user_id),
       key: notification.key
     }
-    return db.notifications.count(query, function(err, count) {
+    return db.notifications.count(query, function (err, count) {
       if (err != null) {
         return callback(err)
       }
@@ -52,7 +52,7 @@ module.exports = Notifications = {
   },
 
   addNotification(user_id, notification, callback) {
-    return this._countExistingNotifications(user_id, notification, function(
+    return this._countExistingNotifications(user_id, notification, function (
       err,
       count
     ) {
@@ -87,7 +87,7 @@ module.exports = Notifications = {
       }
       return db.notifications.update(
         { user_id: doc.user_id, key: notification.key },
-        { $set : { ...doc } },
+        { $set: doc },
         { upsert: true },
         callback
       )
@@ -124,6 +124,6 @@ module.exports = Notifications = {
     return db.notifications.remove(searchOps, { justOne: true }, callback)
   }
 }
-;['getUserNotifications', 'addNotification'].map(method =>
+;['getUserNotifications', 'addNotification'].map((method) =>
   metrics.timeAsyncMethod(Notifications, method, 'mongo.Notifications', logger)
 )

services/notifications/app/js/NotificationsController.js

@@ -33,16 +33,17 @@ module.exports = {
       'adding notification'
     )
     metrics.inc('addNotification')
-    return Notifications.addNotification(req.params.user_id, req.body, function(
-      err,
-      notifications
-    ) {
-      if (err != null) {
-        return res.sendStatus(500)
-      } else {
-        return res.sendStatus(200)
+    return Notifications.addNotification(
+      req.params.user_id,
+      req.body,
+      function (err, notifications) {
+        if (err != null) {
+          return res.sendStatus(500)
+        } else {
+          return res.sendStatus(200)
+        }
       }
-    })
+    )
   },
 
   removeNotificationId(req, res) {

services/notifications/config/settings.defaults.js

@@ -1,5 +1,4 @@
-let Settings
-module.exports = Settings = {
+module.exports = {
   internal: {
     notifications: {
       port: 3042,

services/notifications/test/unit/js/NotificationsControllerTest.js

@@ -21,8 +21,8 @@ const user_id = '51dc93e6fb625a261300003b'
 const notification_id = 'fb625a26f09d'
 const notification_key = 'my-notification-key'
 
-describe('Notifications Controller', function() {
-  beforeEach(function() {
+describe('Notifications Controller', function () {
+  beforeEach(function () {
     const self = this
     this.notifications = {}
     this.controller = SandboxedModule.require(modulePath, {
@@ -44,8 +44,8 @@ describe('Notifications Controller', function() {
     ])
   })
 
-  describe('getUserNotifications', function() {
-    return it('should ask the notifications for the users notifications', function(done) {
+  describe('getUserNotifications', function () {
+    return it('should ask the notifications for the users notifications', function (done) {
       this.notifications.getUserNotifications = sinon
         .stub()
         .callsArgWith(1, null, this.stubbedNotification)
@@ -55,7 +55,7 @@ describe('Notifications Controller', function() {
         }
       }
       return this.controller.getUserNotifications(req, {
-        json: result => {
+        json: (result) => {
           result.should.equal(this.stubbedNotification)
           this.notifications.getUserNotifications
             .calledWith(user_id)
@@ -66,8 +66,8 @@ describe('Notifications Controller', function() {
     })
   })
 
-  describe('addNotification', function() {
-    return it('should tell the notifications to add the notification for the user', function(done) {
+  describe('addNotification', function () {
+    return it('should tell the notifications to add the notification for the user', function (done) {
       this.notifications.addNotification = sinon.stub().callsArgWith(2)
       const req = {
         params: {
@@ -76,7 +76,7 @@ describe('Notifications Controller', function() {
         body: this.stubbedNotification
       }
       return this.controller.addNotification(req, {
-        sendStatus: code => {
+        sendStatus: (code) => {
           this.notifications.addNotification
             .calledWith(user_id, this.stubbedNotification)
             .should.equal(true)
@@ -87,8 +87,8 @@ describe('Notifications Controller', function() {
     })
   })
 
-  describe('removeNotificationId', function() {
-    return it('should tell the notifications to mark the notification Id as read', function(done) {
+  describe('removeNotificationId', function () {
+    return it('should tell the notifications to mark the notification Id as read', function (done) {
       this.notifications.removeNotificationId = sinon.stub().callsArgWith(2)
       const req = {
         params: {
@@ -97,7 +97,7 @@ describe('Notifications Controller', function() {
         }
       }
       return this.controller.removeNotificationId(req, {
-        sendStatus: code => {
+        sendStatus: (code) => {
           this.notifications.removeNotificationId
             .calledWith(user_id, notification_id)
             .should.equal(true)
@@ -108,8 +108,8 @@ describe('Notifications Controller', function() {
     })
   })
 
-  describe('removeNotificationKey', function() {
-    return it('should tell the notifications to mark the notification Key as read', function(done) {
+  describe('removeNotificationKey', function () {
+    return it('should tell the notifications to mark the notification Key as read', function (done) {
       this.notifications.removeNotificationKey = sinon.stub().callsArgWith(2)
       const req = {
         params: {
@@ -118,7 +118,7 @@ describe('Notifications Controller', function() {
         body: { key: notification_key }
       }
       return this.controller.removeNotificationKey(req, {
-        sendStatus: code => {
+        sendStatus: (code) => {
           this.notifications.removeNotificationKey
             .calledWith(user_id, notification_key)
             .should.equal(true)
@@ -129,8 +129,8 @@ describe('Notifications Controller', function() {
     })
   })
 
-  return describe('removeNotificationByKeyOnly', function() {
-    return it('should tell the notifications to mark the notification Key as read', function(done) {
+  return describe('removeNotificationByKeyOnly', function () {
+    return it('should tell the notifications to mark the notification Key as read', function (done) {
       this.notifications.removeNotificationByKeyOnly = sinon
         .stub()
         .callsArgWith(1)
@@ -140,7 +140,7 @@ describe('Notifications Controller', function() {
         }
       }
       return this.controller.removeNotificationByKeyOnly(req, {
-        sendStatus: code => {
+        sendStatus: (code) => {
           this.notifications.removeNotificationByKeyOnly
             .calledWith(notification_key)
             .should.equal(true)

services/notifications/test/unit/js/NotificationsTests.js

@@ -25,8 +25,8 @@ const user_id = '51dc93e6fb625a261300003b'
 const notification_id = 'fb625a26f09d'
 const notification_key = 'notification-key'
 
-describe('Notifications Tests', function() {
-  beforeEach(function() {
+describe('Notifications Tests', function () {
+  beforeEach(function () {
     const self = this
     this.findStub = sinon.stub()
     this.insertStub = sinon.stub()
@@ -71,8 +71,8 @@ describe('Notifications Tests', function() {
     return (this.stubbedNotificationArray = [this.stubbedNotification])
   })
 
-  describe('getUserNotifications', function() {
-    return it('should find all notifications and return i', function(done) {
+  describe('getUserNotifications', function () {
+    return it('should find all notifications and return i', function (done) {
       this.findStub.callsArgWith(1, null, this.stubbedNotificationArray)
       return this.notifications.getUserNotifications(
         user_id,
@@ -88,8 +88,8 @@ describe('Notifications Tests', function() {
     })
   })
 
-  describe('addNotification', function() {
-    beforeEach(function() {
+  describe('addNotification', function () {
+    beforeEach(function () {
       this.stubbedNotification = {
         user_id: ObjectId(user_id),
         key: 'notification-key',
@@ -110,11 +110,11 @@ describe('Notifications Tests', function() {
       return this.countStub.yields(null, 0)
     })
 
-    it('should insert the notification into the collection', function(done) {
+    it('should insert the notification into the collection', function (done) {
       return this.notifications.addNotification(
         user_id,
         this.stubbedNotification,
-        err => {
+        (err) => {
           expect(err).not.exists
           sinon.assert.calledWith(
             this.updateStub,
@@ -127,16 +127,16 @@ describe('Notifications Tests', function() {
       )
     })
 
-    describe('when there is an existing notification', function(done) {
-      beforeEach(function() {
+    describe('when there is an existing notification', function (done) {
+      beforeEach(function () {
         return this.countStub.yields(null, 1)
       })
 
-      it('should fail to insert', function(done) {
+      it('should fail to insert', function (done) {
         return this.notifications.addNotification(
           user_id,
           this.stubbedNotification,
-          err => {
+          (err) => {
             expect(err).not.exists
             sinon.assert.notCalled(this.updateStub)
             return done()
@@ -144,12 +144,12 @@ describe('Notifications Tests', function() {
         )
       })
 
-      return it('should update the key if forceCreate is true', function(done) {
+      return it('should update the key if forceCreate is true', function (done) {
         this.stubbedNotification.forceCreate = true
         return this.notifications.addNotification(
           user_id,
           this.stubbedNotification,
-          err => {
+          (err) => {
             expect(err).not.exists
             sinon.assert.calledWith(
               this.updateStub,
@@ -163,8 +163,8 @@ describe('Notifications Tests', function() {
       })
     })
 
-    describe('when the notification is set to expire', function() {
-      beforeEach(function() {
+    describe('when the notification is set to expire', function () {
+      beforeEach(function () {
         this.stubbedNotification = {
           user_id: ObjectId(user_id),
           key: 'notification-key',
@@ -185,11 +185,11 @@ describe('Notifications Tests', function() {
         })
       })
 
-      return it('should add an `expires` Date field to the document', function(done) {
+      return it('should add an `expires` Date field to the document', function (done) {
         return this.notifications.addNotification(
           user_id,
           this.stubbedNotification,
-          err => {
+          (err) => {
             expect(err).not.exists
             sinon.assert.calledWith(
               this.updateStub,
@@ -203,8 +203,8 @@ describe('Notifications Tests', function() {
       })
     })
 
-    return describe('when the notification has a nonsensical expires field', function() {
-      beforeEach(function() {
+    return describe('when the notification has a nonsensical expires field', function () {
+      beforeEach(function () {
         this.stubbedNotification = {
           user_id: ObjectId(user_id),
           key: 'notification-key',
@@ -221,11 +221,11 @@ describe('Notifications Tests', function() {
         })
       })
 
-      return it('should produce an error', function(done) {
+      return it('should produce an error', function (done) {
         return this.notifications.addNotification(
           user_id,
           this.stubbedNotification,
-          err => {
+          (err) => {
             ;(err instanceof Error).should.equal(true)
             sinon.assert.notCalled(this.updateStub)
             return done()
@@ -235,14 +235,14 @@ describe('Notifications Tests', function() {
     })
   })
 
-  describe('removeNotificationId', function() {
-    return it('should mark the notification id as read', function(done) {
+  describe('removeNotificationId', function () {
+    return it('should mark the notification id as read', function (done) {
       this.updateStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationId(
         user_id,
         notification_id,
-        err => {
+        (err) => {
           const searchOps = {
             user_id: ObjectId(user_id),
             _id: ObjectId(notification_id)
@@ -258,14 +258,14 @@ describe('Notifications Tests', function() {
     })
   })
 
-  describe('removeNotificationKey', function() {
-    return it('should mark the notification key as read', function(done) {
+  describe('removeNotificationKey', function () {
+    return it('should mark the notification key as read', function (done) {
       this.updateStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationKey(
         user_id,
         notification_key,
-        err => {
+        (err) => {
           const searchOps = {
             user_id: ObjectId(user_id),
             key: notification_key
@@ -281,13 +281,13 @@ describe('Notifications Tests', function() {
     })
   })
 
-  describe('removeNotificationByKeyOnly', function() {
-    return it('should mark the notification key as read', function(done) {
+  describe('removeNotificationByKeyOnly', function () {
+    return it('should mark the notification key as read', function (done) {
       this.updateStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationByKeyOnly(
         notification_key,
-        err => {
+        (err) => {
           const searchOps = { key: notification_key }
           const updateOperation = { $unset: { templateKey: true } }
           assert.deepEqual(this.updateStub.args[0][0], searchOps)
@@ -298,13 +298,13 @@ describe('Notifications Tests', function() {
     })
   })
 
-  return describe('deleteNotificationByKeyOnly', function() {
-    return it('should completely remove the notification', function(done) {
+  return describe('deleteNotificationByKeyOnly', function () {
+    return it('should completely remove the notification', function (done) {
       this.removeStub.callsArgWith(2, null)
 
       return this.notifications.deleteNotificationByKeyOnly(
         notification_key,
-        err => {
+        (err) => {
           const searchOps = { key: notification_key }
           const opts = { justOne: true }
           assert.deepEqual(this.removeStub.args[0][0], searchOps)