diff --git a/services/notifications/.nvmrc b/services/notifications/.nvmrc new file mode 100644 index 0000000000..bf77d54968 --- /dev/null +++ b/services/notifications/.nvmrc @@ -0,0 +1 @@ +4.2 diff --git a/services/notifications/README.md b/services/notifications/README.md index 02e1a15e78..4b2b4184b0 100644 --- a/services/notifications/README.md +++ b/services/notifications/README.md @@ -2,3 +2,13 @@ notifications-sharelatex =============== An API for managing user notifications in ShareLaTeX + + +database indexes +================ + +For notification expiry to work, a TTL index on `notifications.expires` must be created: + +```javascript +db.notifications.createIndex({expires: 1}, {expireAfterSeconds: 10}) +``` diff --git a/services/notifications/app.coffee b/services/notifications/app.coffee index 8b78c4b8f8..4a8136cb92 100644 --- a/services/notifications/app.coffee +++ b/services/notifications/app.coffee @@ -24,6 +24,7 @@ app.post '/user/:user_id', controller.addNotification app.get '/user/:user_id', controller.getUserNotifications app.del '/user/:user_id/notification/:notification_id', controller.removeNotificationId app.del '/user/:user_id', controller.removeNotificationKey +app.del '/key/:key', controller.removeNotificationByKeyOnly app.get '/status', (req, res)-> res.send('notifications sharelatex up') diff --git a/services/notifications/app/coffee/Notifications.coffee b/services/notifications/app/coffee/Notifications.coffee index f9a6adc86a..e8edae5aab 100644 --- a/services/notifications/app/coffee/Notifications.coffee +++ b/services/notifications/app/coffee/Notifications.coffee @@ -22,25 +22,43 @@ module.exports = logger.log number:number, user_id:user_id, key:notification.key, "alredy has notification key for user" callback number else - doc = + doc = user_id: ObjectId(user_id) key: notification.key messageOpts: notification.messageOpts templateKey: notification.templateKey + # TTL index on the optional `expires` field, which should arrive as an iso date-string, corresponding to + # a datetime in the future when the document should be automatically removed. + # in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing + # see `README.md` for instruction on creating TTL index + if notification.expires? + try + doc.expires = new Date(notification.expires) + _testValue = doc.expires.toISOString() + catch err + logger.error {user_id, expires: notification.expires}, "error converting `expires` field to Date" + return callback(err) db.notifications.insert(doc, callback) removeNotificationId: (user_id, notification_id, callback)-> - searchOps = + searchOps = user_id:ObjectId(user_id) _id:ObjectId(notification_id) - updateOperation = + updateOperation = "$unset": {templateKey:true, messageOpts: true} db.notifications.update searchOps, updateOperation, callback removeNotificationKey: (user_id, notification_key, callback)-> - searchOps = + searchOps = user_id:ObjectId(user_id) key: notification_key - updateOperation = + updateOperation = + "$unset": {templateKey:true} + db.notifications.update searchOps, updateOperation, callback + + removeNotificationByKeyOnly: (notification_key, callback)-> + searchOps = + key: notification_key + updateOperation = "$unset": {templateKey:true} db.notifications.update searchOps, updateOperation, callback diff --git a/services/notifications/app/coffee/NotificationsController.coffee b/services/notifications/app/coffee/NotificationsController.coffee index 6ee7c68cbc..416cdeccc0 100644 --- a/services/notifications/app/coffee/NotificationsController.coffee +++ b/services/notifications/app/coffee/NotificationsController.coffee @@ -30,3 +30,10 @@ module.exports = metrics.inc "removeNotificationKey" Notifications.removeNotificationKey req.params.user_id, req.body.key, (err, notifications)-> res.send() + + removeNotificationByKeyOnly: (req, res)-> + notification_key = req.params.key + logger.log {notification_key}, "mark notification as read by key only" + metrics.inc "removeNotificationKey" + Notifications.removeNotificationByKeyOnly notification_key, (err, notifications)-> + res.send() diff --git a/services/notifications/test/unit/coffee/NotificationsControllerTest.coffee b/services/notifications/test/unit/coffee/NotificationsControllerTest.coffee index 286abdf00e..c7c8fdd987 100644 --- a/services/notifications/test/unit/coffee/NotificationsControllerTest.coffee +++ b/services/notifications/test/unit/coffee/NotificationsControllerTest.coffee @@ -24,7 +24,7 @@ describe 'Notifications Controller', -> describe "getUserNotifications", -> it 'should ask the notifications for the users notifications', (done)-> @notifications.getUserNotifications = sinon.stub().callsArgWith(1, null, @stubbedNotification) - req = + req = params: user_id: user_id @controller.getUserNotifications req, json:(result)=> @@ -35,7 +35,7 @@ describe 'Notifications Controller', -> describe "addNotification", -> it "should tell the notifications to add the notification for the user", (done)-> @notifications.addNotification = sinon.stub().callsArgWith(2) - req = + req = params: user_id: user_id body: @stubbedNotification @@ -46,7 +46,7 @@ describe 'Notifications Controller', -> describe "removeNotificationId", -> it "should tell the notifications to mark the notification Id as read", (done)-> @notifications.removeNotificationId = sinon.stub().callsArgWith(2) - req = + req = params: user_id: user_id notification_id: notification_id @@ -57,10 +57,20 @@ describe 'Notifications Controller', -> describe "removeNotificationKey", -> it "should tell the notifications to mark the notification Key as read", (done)-> @notifications.removeNotificationKey = sinon.stub().callsArgWith(2) - req = + req = params: user_id: user_id body: {key: notification_key} @controller.removeNotificationKey req, send:(result)=> @notifications.removeNotificationKey.calledWith(user_id, notification_key).should.equal true - done() \ No newline at end of file + done() + + describe "removeNotificationByKeyOnly", -> + it "should tell the notifications to mark the notification Key as read", (done)-> + @notifications.removeNotificationByKeyOnly = sinon.stub().callsArgWith(1) + req = + params: + key: notification_key + @controller.removeNotificationByKeyOnly req, send:(result)=> + @notifications.removeNotificationByKeyOnly.calledWith(notification_key).should.equal true + done() diff --git a/services/notifications/test/unit/coffee/NotificationsTests.coffee b/services/notifications/test/unit/coffee/NotificationsTests.coffee index db002962f1..f070889de4 100644 --- a/services/notifications/test/unit/coffee/NotificationsTests.coffee +++ b/services/notifications/test/unit/coffee/NotificationsTests.coffee @@ -1,5 +1,6 @@ sinon = require('sinon') chai = require('chai') +expect = chai.should should = chai.should() modulePath = "../../../app/js/Notifications.js" SandboxedModule = require('sandboxed-module') @@ -17,7 +18,6 @@ describe 'Notifications Tests', -> @insertStub = sinon.stub() @countStub = sinon.stub() @updateStub = sinon.stub() - @findStub.cock = "helllo" @mongojs = => notifications: update: self.mongojsUpdate @@ -28,7 +28,10 @@ describe 'Notifications Tests', -> @mongojs.ObjectId = ObjectId @notifications = SandboxedModule.require modulePath, requires: - 'logger-sharelatex': log:-> + 'logger-sharelatex': { + log:()-> + error:()-> + } 'settings-sharelatex': {} 'mongojs':@mongojs @@ -44,12 +47,26 @@ describe 'Notifications Tests', -> done() describe 'addNotification', -> - it 'should insert the notification into the collectionssss', (done)-> + beforeEach -> + @stubbedNotification = { + user_id: ObjectId(user_id), + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key" + } + @expectedDocument = { + user_id: @stubbedNotification.user_id, + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key" + } + + it 'should insert the notification into the collection', (done)-> @insertStub.callsArgWith(1, null) @countStub.callsArgWith(1, null, 0) @notifications.addNotification user_id, @stubbedNotification, (err)=> - assert.deepEqual(@insertStub.args[0][0], @stubbedNotification) + @insertStub.calledWith(@expectedDocument).should.equal true done() it 'should fail insert of existing notification key', (done)-> @@ -57,18 +74,71 @@ describe 'Notifications Tests', -> @countStub.callsArgWith(1, null, 1) @notifications.addNotification user_id, @stubbedNotification, (err)=> - @insertStub.calledWith(@stubbedNotification).should.equal false + @insertStub.calledWith(@expectedDocument).should.equal false done() + describe 'when the notification is set to expire', () -> + beforeEach -> + @stubbedNotification = { + user_id: ObjectId(user_id), + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key", + expires: '2922-02-13T09:32:56.289Z' + } + @expectedDocument = { + user_id: @stubbedNotification.user_id, + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key", + expires: new Date(@stubbedNotification.expires), + } + + it 'should add an `expires` Date field to the document', (done)-> + @insertStub.callsArgWith(1, null) + @countStub.callsArgWith(1, null, 0) + + @notifications.addNotification user_id, @stubbedNotification, (err)=> + @insertStub.callCount.should.equal 1 + @insertStub.calledWith(@expectedDocument).should.equal true + done() + + describe 'when the notification has a nonsensical expires field', () -> + beforeEach -> + @stubbedNotification = { + user_id: ObjectId(user_id), + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key", + expires: 'WAT' + } + @expectedDocument = { + user_id: @stubbedNotification.user_id, + key:"notification-key", + messageOpts:"some info", + templateKey:"template-key", + expires: new Date(@stubbedNotification.expires), + } + + it 'should produce an error', (done)-> + @insertStub.callsArgWith(1, null) + @countStub.callsArgWith(1, null, 0) + + @notifications.addNotification user_id, @stubbedNotification, (err)=> + (err instanceof Error).should.equal true + @insertStub.callCount.should.equal 0 + @insertStub.calledWith(@expectedDocument).should.equal false + done() + describe 'removeNotificationId', -> it 'should mark the notification id as read', (done)-> @updateStub.callsArgWith(2, null) @notifications.removeNotificationId user_id, notification_id, (err)=> - searchOps = + searchOps = user_id:ObjectId(user_id) _id:ObjectId(notification_id) - updateOperation = + updateOperation = "$unset": {templateKey:true, messageOpts:true} assert.deepEqual(@updateStub.args[0][0], searchOps) assert.deepEqual(@updateStub.args[0][1], updateOperation) @@ -79,10 +149,22 @@ describe 'Notifications Tests', -> @updateStub.callsArgWith(2, null) @notifications.removeNotificationKey user_id, notification_key, (err)=> - searchOps = + searchOps = user_id:ObjectId(user_id) key: notification_key - updateOperation = + updateOperation = + "$unset": {templateKey:true} + @updateStub.calledWith(searchOps, updateOperation).should.equal true + done() + + describe 'removeNotificationByKeyOnly', -> + it 'should mark the notification key as read', (done)-> + @updateStub.callsArgWith(2, null) + + @notifications.removeNotificationByKeyOnly notification_key, (err)=> + searchOps = + key: notification_key + updateOperation = "$unset": {templateKey:true} assert.deepEqual(@updateStub.args[0][0], searchOps) assert.deepEqual(@updateStub.args[0][1], updateOperation)