From 025863d8aefe21a4f58b34b09859cc7e7d0c4d53 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Thu, 16 Mar 2023 08:38:09 -0400 Subject: [PATCH] Merge pull request #12208 from overleaf/em-camel-case-notifications Camel case variables in notifications GitOrigin-RevId: da1fb77f42e4ba58b6b3833aa05ddf76e1d613f6 --- .../app/js/HealthCheckController.js | 31 ++++++----- .../notifications/app/js/Notifications.js | 37 +++++++------ .../app/js/NotificationsController.js | 19 +++---- .../unit/js/NotificationsControllerTest.js | 33 ++++++------ .../test/unit/js/NotificationsTests.js | 53 +++++++++---------- 5 files changed, 83 insertions(+), 90 deletions(-) diff --git a/services/notifications/app/js/HealthCheckController.js b/services/notifications/app/js/HealthCheckController.js index fa3c228b9f..58ea0eb926 100644 --- a/services/notifications/app/js/HealthCheckController.js +++ b/services/notifications/app/js/HealthCheckController.js @@ -1,5 +1,4 @@ /* eslint-disable - camelcase, no-dupe-keys, */ // TODO: This file was created by bulk-decaffeinate. @@ -19,27 +18,27 @@ const logger = require('@overleaf/logger') module.exports = { check(callback) { - const user_id = ObjectId() + const userId = ObjectId() const cleanupNotifications = callback => - db.notifications.remove({ user_id }, callback) + db.notifications.remove({ user_id: userId }, callback) - let notification_key = `smoke-test-notification-${ObjectId()}` + let notificationKey = `smoke-test-notification-${ObjectId()}` const getOpts = endPath => ({ - url: `http://localhost:${port}/user/${user_id}${endPath}`, + url: `http://localhost:${port}/user/${userId}${endPath}`, timeout: 5000, }) logger.debug( - { user_id, opts: getOpts(), key: notification_key, user_id }, + { userId, opts: getOpts(), key: notificationKey, userId }, 'Health Check: running' ) const jobs = [ function (cb) { const opts = getOpts('/') opts.json = { - key: notification_key, + key: notificationKey, messageOpts: '', templateKey: 'f4g5', - user_id, + user_id: userId, } return request.post(opts, cb) }, @@ -57,14 +56,14 @@ module.exports = { } const hasNotification = body.some( notification => - notification.key === notification_key && - notification.user_id === user_id.toString() + notification.key === notificationKey && + notification.user_id === userId.toString() ) if (hasNotification) { return cb(null, body) } else { logger.err( - { body, notification_key }, + { body, notificationKey }, 'Health Check: notification not in response' ) return cb(new Error('notification not found in response')) @@ -77,11 +76,11 @@ module.exports = { logger.err({ err }, 'Health Check: error running health check') return cleanupNotifications(() => callback(err)) } else { - const notification_id = body[1][0]._id - notification_key = body[1][0].key - let opts = getOpts(`/notification/${notification_id}`) + const notificationId = body[1][0]._id + notificationKey = body[1][0].key + let opts = getOpts(`/notification/${notificationId}`) logger.debug( - { notification_id, notification_key }, + { notificationId, notificationKey }, 'Health Check: doing cleanup' ) return request.del(opts, function (err, res, body) { @@ -94,7 +93,7 @@ module.exports = { return callback(err) } opts = getOpts('') - opts.json = { key: notification_key } + opts.json = { key: notificationKey } return request.del(opts, function (err, res, body) { if (err != null) { logger.err( diff --git a/services/notifications/app/js/Notifications.js b/services/notifications/app/js/Notifications.js index e9a9ae1eee..837a5a49fd 100644 --- a/services/notifications/app/js/Notifications.js +++ b/services/notifications/app/js/Notifications.js @@ -1,5 +1,4 @@ /* eslint-disable - camelcase, no-unused-vars, */ // TODO: This file was created by bulk-decaffeinate. @@ -16,23 +15,23 @@ const { db, ObjectId } = require('./mongodb') const metrics = require('@overleaf/metrics') module.exports = Notifications = { - getUserNotifications(user_id, callback) { + getUserNotifications(userId, callback) { if (callback == null) { callback = function () {} } const query = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), templateKey: { $exists: true }, } db.notifications.find(query).toArray(callback) }, - _countExistingNotifications(user_id, notification, callback) { + _countExistingNotifications(userId, notification, callback) { if (callback == null) { callback = function () {} } const query = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: notification.key, } return db.notifications.count(query, function (err, count) { @@ -43,9 +42,9 @@ module.exports = Notifications = { }) }, - addNotification(user_id, notification, callback) { + addNotification(userId, notification, callback) { return this._countExistingNotifications( - user_id, + userId, notification, function (err, count) { if (err != null) { @@ -55,7 +54,7 @@ module.exports = Notifications = { return callback() } const doc = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: notification.key, messageOpts: notification.messageOpts, templateKey: notification.templateKey, @@ -71,7 +70,7 @@ module.exports = Notifications = { } catch (error) { err = error logger.error( - { user_id, expires: notification.expires }, + { userId, expires: notification.expires }, 'error converting `expires` field to Date' ) return callback(err) @@ -87,26 +86,26 @@ module.exports = Notifications = { ) }, - removeNotificationId(user_id, notification_id, callback) { + removeNotificationId(userId, notificationId, callback) { const searchOps = { - user_id: ObjectId(user_id), - _id: ObjectId(notification_id), + user_id: ObjectId(userId), + _id: ObjectId(notificationId), } const updateOperation = { $unset: { templateKey: true, messageOpts: true } } db.notifications.updateOne(searchOps, updateOperation, callback) }, - removeNotificationKey(user_id, notification_key, callback) { + removeNotificationKey(userId, notificationKey, callback) { const searchOps = { - user_id: ObjectId(user_id), - key: notification_key, + user_id: ObjectId(userId), + key: notificationKey, } const updateOperation = { $unset: { templateKey: true } } db.notifications.updateOne(searchOps, updateOperation, callback) }, - removeNotificationByKeyOnly(notification_key, callback) { - const searchOps = { key: notification_key } + removeNotificationByKeyOnly(notificationKey, callback) { + const searchOps = { key: notificationKey } const updateOperation = { $unset: { templateKey: true } } db.notifications.updateOne(searchOps, updateOperation, callback) }, @@ -128,8 +127,8 @@ module.exports = Notifications = { }, // hard delete of doc, rather than removing the templateKey - deleteNotificationByKeyOnly(notification_key, callback) { - const searchOps = { key: notification_key } + deleteNotificationByKeyOnly(notificationKey, callback) { + const searchOps = { key: notificationKey } db.notifications.deleteOne(searchOps, callback) }, } diff --git a/services/notifications/app/js/NotificationsController.js b/services/notifications/app/js/NotificationsController.js index 706c03525b..89b7fa97d6 100644 --- a/services/notifications/app/js/NotificationsController.js +++ b/services/notifications/app/js/NotificationsController.js @@ -1,6 +1,3 @@ -/* eslint-disable - camelcase, -*/ // TODO: This file was created by bulk-decaffeinate. // Fix any style issues and re-enable lint. /* @@ -16,7 +13,7 @@ const metrics = require('@overleaf/metrics') module.exports = { getUserNotifications(req, res, next) { logger.debug( - { user_id: req.params.user_id }, + { userId: req.params.user_id }, 'getting user unread notifications' ) metrics.inc('getUserNotifications') @@ -31,7 +28,7 @@ module.exports = { addNotification(req, res) { logger.debug( - { user_id: req.params.user_id, notification: req.body }, + { userId: req.params.user_id, notification: req.body }, 'adding notification' ) metrics.inc('addNotification') @@ -51,8 +48,8 @@ module.exports = { removeNotificationId(req, res, next) { logger.debug( { - user_id: req.params.user_id, - notification_id: req.params.notification_id, + userId: req.params.user_id, + notificationId: req.params.notification_id, }, 'mark id notification as read' ) @@ -69,7 +66,7 @@ module.exports = { removeNotificationKey(req, res, next) { logger.debug( - { user_id: req.params.user_id, notification_key: req.body.key }, + { userId: req.params.user_id, notificationKey: req.body.key }, 'mark key notification as read' ) metrics.inc('removeNotificationKey') @@ -84,10 +81,10 @@ module.exports = { }, removeNotificationByKeyOnly(req, res, next) { - const notification_key = req.params.key - logger.debug({ notification_key }, 'mark notification as read by key only') + const notificationKey = req.params.key + logger.debug({ notificationKey }, 'mark notification as read by key only') metrics.inc('removeNotificationKey') - return Notifications.removeNotificationByKeyOnly(notification_key, err => { + return Notifications.removeNotificationByKeyOnly(notificationKey, err => { if (err) return next(err) res.sendStatus(200) }) diff --git a/services/notifications/test/unit/js/NotificationsControllerTest.js b/services/notifications/test/unit/js/NotificationsControllerTest.js index f359eb6954..eefeb994f2 100644 --- a/services/notifications/test/unit/js/NotificationsControllerTest.js +++ b/services/notifications/test/unit/js/NotificationsControllerTest.js @@ -1,5 +1,4 @@ /* eslint-disable - camelcase, no-return-assign, no-unused-vars, */ @@ -15,9 +14,9 @@ const modulePath = '../../../app/js/NotificationsController.js' const SandboxedModule = require('sandboxed-module') const assert = require('assert') -const user_id = '51dc93e6fb625a261300003b' -const notification_id = 'fb625a26f09d' -const notification_key = 'my-notification-key' +const userId = '51dc93e6fb625a261300003b' +const notificationId = 'fb625a26f09d' +const notificationKey = 'my-notification-key' describe('Notifications Controller', function () { beforeEach(function () { @@ -34,7 +33,7 @@ describe('Notifications Controller', function () { return (this.stubbedNotification = [ { - key: notification_key, + key: notificationKey, messageOpts: 'some info', templateKey: 'template-key', }, @@ -48,14 +47,14 @@ describe('Notifications Controller', function () { .callsArgWith(1, null, this.stubbedNotification) const req = { params: { - user_id, + user_id: userId, }, } return this.controller.getUserNotifications(req, { json: result => { result.should.equal(this.stubbedNotification) this.notifications.getUserNotifications - .calledWith(user_id) + .calledWith(userId) .should.equal(true) return done() }, @@ -68,14 +67,14 @@ describe('Notifications Controller', function () { this.notifications.addNotification = sinon.stub().callsArgWith(2) const req = { params: { - user_id, + user_id: userId, }, body: this.stubbedNotification, } return this.controller.addNotification(req, { sendStatus: code => { this.notifications.addNotification - .calledWith(user_id, this.stubbedNotification) + .calledWith(userId, this.stubbedNotification) .should.equal(true) code.should.equal(200) return done() @@ -89,14 +88,14 @@ describe('Notifications Controller', function () { this.notifications.removeNotificationId = sinon.stub().callsArgWith(2) const req = { params: { - user_id, - notification_id, + user_id: userId, + notification_id: notificationId, }, } return this.controller.removeNotificationId(req, { sendStatus: code => { this.notifications.removeNotificationId - .calledWith(user_id, notification_id) + .calledWith(userId, notificationId) .should.equal(true) code.should.equal(200) return done() @@ -110,14 +109,14 @@ describe('Notifications Controller', function () { this.notifications.removeNotificationKey = sinon.stub().callsArgWith(2) const req = { params: { - user_id, + user_id: userId, }, - body: { key: notification_key }, + body: { key: notificationKey }, } return this.controller.removeNotificationKey(req, { sendStatus: code => { this.notifications.removeNotificationKey - .calledWith(user_id, notification_key) + .calledWith(userId, notificationKey) .should.equal(true) code.should.equal(200) return done() @@ -133,13 +132,13 @@ describe('Notifications Controller', function () { .callsArgWith(1) const req = { params: { - key: notification_key, + key: notificationKey, }, } return this.controller.removeNotificationByKeyOnly(req, { sendStatus: code => { this.notifications.removeNotificationByKeyOnly - .calledWith(notification_key) + .calledWith(notificationKey) .should.equal(true) code.should.equal(200) return done() diff --git a/services/notifications/test/unit/js/NotificationsTests.js b/services/notifications/test/unit/js/NotificationsTests.js index e8f46c8710..54eff4d640 100644 --- a/services/notifications/test/unit/js/NotificationsTests.js +++ b/services/notifications/test/unit/js/NotificationsTests.js @@ -1,5 +1,4 @@ /* eslint-disable - camelcase, no-dupe-keys, no-return-assign, no-unused-vars, @@ -18,9 +17,9 @@ const SandboxedModule = require('sandboxed-module') const assert = require('assert') const { ObjectId } = require('mongodb') -const user_id = '51dc93e6fb625a261300003b' -const notification_id = 'fb625a26f09d' -const notification_key = 'notification-key' +const userId = '51dc93e6fb625a261300003b' +const notificationId = 'fb625a26f09d' +const notificationKey = 'notification-key' describe('Notifications Tests', function () { beforeEach(function () { @@ -47,7 +46,7 @@ describe('Notifications Tests', function () { }) this.stubbedNotification = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: 'notification-key', messageOpts: 'some info', templateKey: 'template-key', @@ -59,12 +58,12 @@ describe('Notifications Tests', function () { return it('should find all notifications and return i', function (done) { this.findToArrayStub.callsArgWith(0, null, this.stubbedNotificationArray) return this.notifications.getUserNotifications( - user_id, + userId, (err, notifications) => { if (err) return done(err) notifications.should.equal(this.stubbedNotificationArray) assert.deepEqual(this.findStub.args[0][0], { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), templateKey: { $exists: true }, }) return done() @@ -76,7 +75,7 @@ describe('Notifications Tests', function () { describe('addNotification', function () { beforeEach(function () { this.stubbedNotification = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: 'notification-key', messageOpts: 'some info', templateKey: 'template-key', @@ -97,7 +96,7 @@ describe('Notifications Tests', function () { it('should insert the notification into the collection', function (done) { return this.notifications.addNotification( - user_id, + userId, this.stubbedNotification, err => { expect(err).not.to.exist @@ -119,7 +118,7 @@ describe('Notifications Tests', function () { it('should fail to insert', function (done) { return this.notifications.addNotification( - user_id, + userId, this.stubbedNotification, err => { expect(err).not.to.exist @@ -132,7 +131,7 @@ describe('Notifications Tests', function () { return it('should update the key if forceCreate is true', function (done) { this.stubbedNotification.forceCreate = true return this.notifications.addNotification( - user_id, + userId, this.stubbedNotification, err => { expect(err).not.to.exist @@ -151,7 +150,7 @@ describe('Notifications Tests', function () { describe('when the notification is set to expire', function () { beforeEach(function () { this.stubbedNotification = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: 'notification-key', messageOpts: 'some info', templateKey: 'template-key', @@ -172,7 +171,7 @@ describe('Notifications Tests', function () { return it('should add an `expires` Date field to the document', function (done) { return this.notifications.addNotification( - user_id, + userId, this.stubbedNotification, err => { expect(err).not.to.exist @@ -191,7 +190,7 @@ describe('Notifications Tests', function () { return describe('when the notification has a nonsensical expires field', function () { beforeEach(function () { this.stubbedNotification = { - user_id: ObjectId(user_id), + user_id: ObjectId(userId), key: 'notification-key', messageOpts: 'some info', templateKey: 'template-key', @@ -208,7 +207,7 @@ describe('Notifications Tests', function () { return it('should produce an error', function (done) { return this.notifications.addNotification( - user_id, + userId, this.stubbedNotification, err => { ;(err instanceof Error).should.equal(true) @@ -225,13 +224,13 @@ describe('Notifications Tests', function () { this.updateOneStub.callsArgWith(2, null) return this.notifications.removeNotificationId( - user_id, - notification_id, + userId, + notificationId, err => { if (err) return done(err) const searchOps = { - user_id: ObjectId(user_id), - _id: ObjectId(notification_id), + user_id: ObjectId(userId), + _id: ObjectId(notificationId), } const updateOperation = { $unset: { templateKey: true, messageOpts: true }, @@ -249,13 +248,13 @@ describe('Notifications Tests', function () { this.updateOneStub.callsArgWith(2, null) return this.notifications.removeNotificationKey( - user_id, - notification_key, + userId, + notificationKey, err => { if (err) return done(err) const searchOps = { - user_id: ObjectId(user_id), - key: notification_key, + user_id: ObjectId(userId), + key: notificationKey, } const updateOperation = { $unset: { templateKey: true }, @@ -273,10 +272,10 @@ describe('Notifications Tests', function () { this.updateOneStub.callsArgWith(2, null) return this.notifications.removeNotificationByKeyOnly( - notification_key, + notificationKey, err => { if (err) return done(err) - const searchOps = { key: notification_key } + const searchOps = { key: notificationKey } const updateOperation = { $unset: { templateKey: true } } assert.deepEqual(this.updateOneStub.args[0][0], searchOps) assert.deepEqual(this.updateOneStub.args[0][1], updateOperation) @@ -291,10 +290,10 @@ describe('Notifications Tests', function () { this.deleteOneStub.callsArgWith(1, null) return this.notifications.deleteNotificationByKeyOnly( - notification_key, + notificationKey, err => { if (err) return done(err) - const searchOps = { key: notification_key } + const searchOps = { key: notificationKey } assert.deepEqual(this.deleteOneStub.args[0][0], searchOps) return done() }