From d722f47b0f62e725596f7a28df31dddcb66af32f Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 8 Mar 2017 17:51:35 +0000 Subject: [PATCH 1/5] add indentify option and uuid for users not logged in --- .../coffee/Features/Analytics/AnalyticsController.coffee | 4 +++- .../app/coffee/Features/Analytics/AnalyticsManager.coffee | 8 ++++++++ .../coffee/Features/StaticPages/StaticPagesRouter.coffee | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee index f9431a0f5c..a4267e28df 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee @@ -1,7 +1,9 @@ AnalyticsManager = require "./AnalyticsManager" +AuthenticationController = require("../Authentication/AuthenticationController") module.exports = AnalyticsController = recordEvent: (req, res, next) -> - AnalyticsManager.recordEvent req.session?.user?._id, req.params.event, req.body, (error) -> + user_id = AuthenticationController.getLoggedInUserId(req) or req.sessionID + AnalyticsManager.recordEvent user_id, req.params.event, req.body, (error) -> return next(error) if error? res.send 204 diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 9f34d6d9d1..9165678070 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -16,6 +16,14 @@ makeRequest = (opts, callback)-> module.exports = + idendifyUser: (user_id, old_user_id, callback)-> + opts = + body: + old_user_id:old_user_id + json:true + method:"POST" + timeout:1000 + url: "/user/#{user_id}/idendify" recordEvent: (user_id, event, segmentation = {}, callback = (error) ->) -> if user_id+"" == settings.smokeTest?.userId+"" diff --git a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee index f1f55814c7..66c210491b 100644 --- a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee +++ b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee @@ -9,6 +9,8 @@ module.exports = webRouter.get '/tos', HomeController.externalPage("tos", "Terms of Service") webRouter.get '/about', HomeController.externalPage("about", "About Us") + webRouter.get '/review', HomeController.externalPage("review", "About Us") + webRouter.get '/security', HomeController.externalPage("security", "Security") webRouter.get '/privacy_policy', HomeController.externalPage("privacy", "Privacy Policy") webRouter.get '/planned_maintenance', HomeController.externalPage("planned_maintenance", "Planned Maintenance") From f910bb58de378609aea158fb0806a38cc7c44064 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 22 Mar 2017 13:11:45 +0000 Subject: [PATCH 2/5] add tests for AnalyticsController --- .../Analytics/AnalyticsControllerTests.coffee | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee diff --git a/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee b/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee new file mode 100644 index 0000000000..2c52b8b5f0 --- /dev/null +++ b/services/web/test/UnitTests/coffee/Analytics/AnalyticsControllerTests.coffee @@ -0,0 +1,44 @@ +should = require('chai').should() +SandboxedModule = require('sandboxed-module') +assert = require('assert') +path = require('path') +modulePath = path.join __dirname, '../../../../app/js/Features/Analytics/AnalyticsController' +sinon = require("sinon") +expect = require("chai").expect + + +describe 'AnalyticsController', -> + + beforeEach -> + @AuthenticationController = + getLoggedInUserId: sinon.stub() + + @AnalyticsManager = + recordEvent: sinon.stub().callsArgWith(3) + + @req = + params: + event:"i_did_something" + body:"stuff" + sessionID: "sessionIDHere" + + @res = + send:-> + @controller = SandboxedModule.require modulePath, requires: + "./AnalyticsManager":@AnalyticsManager + "../Authentication/AuthenticationController":@AuthenticationController + "logger-sharelatex": + log:-> + + describe "recordEvent", -> + + it "should use the user_id", (done)-> + @AuthenticationController.getLoggedInUserId.returns("1234") + @controller.recordEvent @req, @res + @AnalyticsManager.recordEvent.calledWith("1234", @req.params["event"], @req.body).should.equal true + done() + + it "should use the session id", (done)-> + @controller.recordEvent @req, @res + @AnalyticsManager.recordEvent.calledWith(@req.sessionID, @req.params["event"], @req.body).should.equal true + done() From ebdce6169efae75ed5b94e7a0d160dceb3305526 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 22 Mar 2017 15:50:49 +0000 Subject: [PATCH 3/5] idendifyUser on login --- .../app/coffee/Features/Analytics/AnalyticsManager.coffee | 5 +++-- .../Features/Authentication/AuthenticationController.coffee | 1 + .../Authentication/AuthenticationControllerTests.coffee | 5 +++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 9165678070..003a59de2d 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -16,14 +16,15 @@ makeRequest = (opts, callback)-> module.exports = - idendifyUser: (user_id, old_user_id, callback)-> + idendifyUser: (user_id, old_user_id, callback = (error)->)-> opts = body: old_user_id:old_user_id json:true method:"POST" timeout:1000 - url: "/user/#{user_id}/idendify" + url: "/user/#{user_id}/identify" + makeRequest opts, callback recordEvent: (user_id, event, segmentation = {}, callback = (error) ->) -> if user_id+"" == settings.smokeTest?.userId+"" diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 485b046a85..b78afef5fb 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -87,6 +87,7 @@ module.exports = AuthenticationController = LoginRateLimiter.recordSuccessfulLogin(email) AuthenticationController._recordSuccessfulLogin(user._id) Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip}) + Analytics.idendifyUser(user._id, req.sessionID) logger.log email: email, user_id: user._id.toString(), "successful log in" req.session.justLoggedIn = true # capture the request ip for use when creating the session diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 94e930c7b1..6230696d99 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -254,6 +254,8 @@ describe "AuthenticationController", -> @cb = sinon.stub() @LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true) @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) + @req.sessionID = Math.random() + @AnalyticsManager.idendifyUser = sinon.stub() @AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb) it "should attempt to authorise the user", -> @@ -261,6 +263,9 @@ describe "AuthenticationController", -> .calledWith(email: @email.toLowerCase(), @password) .should.equal true + it "should call idendifyUser", -> + @AnalyticsManager.idendifyUser.calledWith(@user._id, @req.sessionID).should.equal true + it "should setup the user data in the background", -> @UserHandler.setupLoginData.calledWith(@user).should.equal true From cff922a0f548040ba012dbc28d0872cf596fcf72 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 22 Mar 2017 16:01:26 +0000 Subject: [PATCH 4/5] idendify -> identify --- .../app/coffee/Features/Analytics/AnalyticsManager.coffee | 2 +- .../Features/Authentication/AuthenticationController.coffee | 2 +- .../Authentication/AuthenticationControllerTests.coffee | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 003a59de2d..be6e011288 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -16,7 +16,7 @@ makeRequest = (opts, callback)-> module.exports = - idendifyUser: (user_id, old_user_id, callback = (error)->)-> + identifyUser: (user_id, old_user_id, callback = (error)->)-> opts = body: old_user_id:old_user_id diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index b78afef5fb..3c43323887 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -87,7 +87,7 @@ module.exports = AuthenticationController = LoginRateLimiter.recordSuccessfulLogin(email) AuthenticationController._recordSuccessfulLogin(user._id) Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip}) - Analytics.idendifyUser(user._id, req.sessionID) + Analytics.identifyUser(user._id, req.sessionID) logger.log email: email, user_id: user._id.toString(), "successful log in" req.session.justLoggedIn = true # capture the request ip for use when creating the session diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 6230696d99..f44968a0e5 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -255,7 +255,7 @@ describe "AuthenticationController", -> @LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true) @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) @req.sessionID = Math.random() - @AnalyticsManager.idendifyUser = sinon.stub() + @AnalyticsManager.identifyUser = sinon.stub() @AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb) it "should attempt to authorise the user", -> @@ -263,8 +263,8 @@ describe "AuthenticationController", -> .calledWith(email: @email.toLowerCase(), @password) .should.equal true - it "should call idendifyUser", -> - @AnalyticsManager.idendifyUser.calledWith(@user._id, @req.sessionID).should.equal true + it "should call identifyUser", -> + @AnalyticsManager.identifyUser.calledWith(@user._id, @req.sessionID).should.equal true it "should setup the user data in the background", -> @UserHandler.setupLoginData.calledWith(@user).should.equal true From ed4a3219068e7597cf642bcf4ca230b5dd8c9f0d Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 23 Mar 2017 15:39:12 +0000 Subject: [PATCH 5/5] remove extra debug route --- .../app/coffee/Features/StaticPages/StaticPagesRouter.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee index 66c210491b..47360f1728 100644 --- a/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee +++ b/services/web/app/coffee/Features/StaticPages/StaticPagesRouter.coffee @@ -9,7 +9,6 @@ module.exports = webRouter.get '/tos', HomeController.externalPage("tos", "Terms of Service") webRouter.get '/about', HomeController.externalPage("about", "About Us") - webRouter.get '/review', HomeController.externalPage("review", "About Us") webRouter.get '/security', HomeController.externalPage("security", "Security") webRouter.get '/privacy_policy', HomeController.externalPage("privacy", "Privacy Policy") @@ -20,4 +19,4 @@ module.exports = webRouter.get '/dropbox', HomeController.externalPage("dropbox", "Dropbox and ShareLaTeX") webRouter.get '/university', UniversityController.getIndexPage - webRouter.get '/university/*', UniversityController.getPage \ No newline at end of file + webRouter.get '/university/*', UniversityController.getPage