From 5edead4446eb3583fce8a8971b3a0b29a87da42d Mon Sep 17 00:00:00 2001 From: Hugh O'Brien Date: Mon, 15 Apr 2019 16:56:10 +0100 Subject: [PATCH] Merge pull request #1694 from sharelatex/hb-clear-ip-notifications Remove notification for university enrolment on affiliation add GitOrigin-RevId: 67df66ad6c6e9e16b6573fa15eb34293adaa0287 --- .../AuthenticationController.coffee | 2 +- .../Institutions/InstitutionsAPI.coffee | 13 ++++++++++--- .../Notifications/NotificationsBuilder.coffee | 17 ++++++++--------- .../Institutions/InstitutionsAPITests.coffee | 4 ++++ .../NotificationsBuilderTests.coffee | 5 ++--- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index cf162d2581..859ae2b099 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -132,7 +132,7 @@ module.exports = AuthenticationController = ipMatchCheck: (req, user) -> if req.ip != user.lastLoginIp - NotificationsBuilder.ipMatcherAffiliation(user._id, req.ip).create() + NotificationsBuilder.ipMatcherAffiliation(user._id).create(req.ip) UserUpdater.updateUser user._id.toString(), { $set: { "lastLoginIp": req.ip } } diff --git a/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee b/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee index dfc04e63b1..e996c5020f 100644 --- a/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee +++ b/services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee @@ -2,6 +2,7 @@ logger = require("logger-sharelatex") metrics = require("metrics-sharelatex") settings = require "settings-sharelatex" request = require "request" +NotificationsBuilder = require("../Notifications/NotificationsBuilder") module.exports = InstitutionsAPI = getInstitutionAffiliations: (institutionId, callback = (error, body) ->) -> @@ -26,7 +27,6 @@ module.exports = InstitutionsAPI = defaultErrorMessage: "Couldn't get user affiliations" }, (error, body) -> callback(error, body or []) - addAffiliation: (userId, email, affiliationOptions, callback) -> unless callback? # affiliationOptions is optional callback = affiliationOptions @@ -38,8 +38,15 @@ module.exports = InstitutionsAPI = path: "/api/v2/users/#{userId.toString()}/affiliations" body: { email, university, department, role, confirmedAt } defaultErrorMessage: "Couldn't create affiliation" - }, callback - + }, (error, body) -> + if error + return callback(error, body) + # have notifications delete any ip matcher notifications for this university + logger.log university + NotificationsBuilder.ipMatcherAffiliation(userId).read university?.id, (err) -> + if err + logger.err err:err, "Something went wrong marking ip notifications read" + callback(error, body) removeAffiliation: (userId, email, callback = (error) ->) -> makeAffiliationRequest { diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index abac3f02a7..b787bcf91c 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -37,11 +37,9 @@ module.exports = read: (callback=()->) -> NotificationsHandler.markAsReadByKeyOnly @key, callback - ipMatcherAffiliation: (userId, ip) -> - key: "ip-matched-affiliation-#{ip}" - create: (callback=()->) -> + ipMatcherAffiliation: (userId) -> + create: (ip, callback=()->) -> return null unless settings?.apis?.v1?.url # service is not configured - _key = @key request { method: 'GET' url: "#{settings.apis.v1.url}/api/v2/users/#{userId}/ip_matcher" @@ -53,12 +51,13 @@ module.exports = return error if error? return null unless response.statusCode == 200 + key = "ip-matched-affiliation-#{body.id}" messageOpts = - university_id: body.id university_name: body.name content: body.enrolment_ad_html - logger.log user_id:userId, key:_key, "creating notification key for user" - NotificationsHandler.createNotification userId, _key, "notification_ip_matched_affiliation", messageOpts, null, false, callback + logger.log user_id:userId, key:key, "creating notification key for user" + NotificationsHandler.createNotification userId, key, "notification_ip_matched_affiliation", messageOpts, null, false, callback - read: (callback = ->)-> - NotificationsHandler.markAsReadWithKey userId, @key, callback + read: (university_id, callback = ->)-> + key = "ip-matched-affiliation-#{university_id}" + NotificationsHandler.markAsReadWithKey userId, key, callback diff --git a/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee b/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee index 0c4e7ae445..30faf918f2 100644 --- a/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee +++ b/services/web/test/unit/coffee/Institutions/InstitutionsAPITests.coffee @@ -13,11 +13,14 @@ describe "InstitutionsAPI", -> @logger = err: sinon.stub(), log: -> @settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } @request = sinon.stub() + @ipMatcherNotification = read: @markAsReadIpMatcher = sinon.stub().callsArgWith(1, null) @InstitutionsAPI = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger "metrics-sharelatex": timeAsyncMethod: sinon.stub() 'settings-sharelatex': @settings 'request': @request + "../Notifications/NotificationsBuilder": + ipMatcherAffiliation: sinon.stub().returns(@ipMatcherNotification) @stubbedUser = _id: "3131231" @@ -126,6 +129,7 @@ describe "InstitutionsAPI", -> body.department.should.equal affiliationOptions.department body.role.should.equal affiliationOptions.role body.confirmedAt.should.equal affiliationOptions.confirmedAt + @markAsReadIpMatcher.calledOnce.should.equal true done() it 'handle error', (done)-> diff --git a/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee index 941e26df1e..e2d78ec139 100644 --- a/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee +++ b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee @@ -25,15 +25,14 @@ describe 'NotificationsBuilder', -> it 'should call v1 and create affiliation notifications', (done)-> ip = '192.168.0.1' - @controller.ipMatcherAffiliation(user_id, ip).create (callback)=> + @controller.ipMatcherAffiliation(user_id).create ip, (callback)=> @request.calledOnce.should.equal true expectedOpts = - university_id: @body.id university_name: @body.name content: @body.enrolment_ad_html @handler.createNotification.calledWith( user_id, - "ip-matched-affiliation-#{ip}", + "ip-matched-affiliation-#{@body.id}", "notification_ip_matched_affiliation", expectedOpts ).should.equal true