Merge branch 'master' into pr-change-free-history-limits

services/web/app/coffee/Features/Authentication/AuthenticationController.coffee

@@ -84,7 +84,7 @@ module.exports = AuthenticationController =
 	doPassportLogin: (req, username, password, done) ->
 		email = username.toLowerCase()
 		Modules = require "../../infrastructure/Modules"
-		Modules.hooks.fire 'preDoPassportLogin', email, (err, infoList) ->
+		Modules.hooks.fire 'preDoPassportLogin', req, email, (err, infoList) ->
 			return next(err) if err?
 			info = infoList.find((i) => i?)
 			if info?

services/web/app/coffee/Features/Compile/ClsiCookieManager.coffee

@@ -49,7 +49,7 @@ module.exports = (backendGroup)->
 			return callback()
 		serverId = @_parseServerIdFromResponse(response)
 		if !serverId? # We don't get a cookie back if it hasn't changed
-			return callback()
+			return rclient.expire(@buildKey(project_id), Settings.clsiCookie.ttl, callback)
 		if rclient_secondary?
 			@_setServerIdInRedis rclient_secondary, project_id, serverId
 		@_setServerIdInRedis rclient, project_id, serverId, (err) ->

services/web/app/coffee/Features/Institutions/InstitutionsAPI.coffee

@@ -80,6 +80,8 @@ makeAffiliationRequest = (requestOptions, callback = (error) ->) ->
 				errorMessage = "#{response.statusCode}: #{body.errors}"
 			else
 				errorMessage = "#{requestOptions.defaultErrorMessage}: #{response.statusCode}"
+
+			logger.err path: requestOptions.path, body: requestOptions.body, errorMessage
 			return callback(new Error(errorMessage))
 
 		callback(null, body)

services/web/app/coffee/Features/User/UserCreator.coffee

@@ -6,15 +6,18 @@ metrics = require('metrics-sharelatex')
 
 module.exports = UserCreator =
 
-	createNewUser: (opts, callback)->
-		logger.log opts:opts, "creating new user"
+	createNewUser: (attributes, options, callback = (error, user) ->)->
+		if arguments.length == 2
+			callback = options
+			options = {}
+		logger.log user: attributes, "creating new user"
 		user = new User()
 
-		username = opts.email.match(/^[^@]*/)
-		if !opts.first_name? or opts.first_name == ""
-			opts.first_name = username[0]
+		username = attributes.email.match(/^[^@]*/)
+		if !attributes.first_name? or attributes.first_name == ""
+			attributes.first_name = username[0]
 
-		for key, value of opts
+		for key, value of attributes
 			user[key] = value
 			
 		user.ace.syntaxValidation = true
@@ -27,6 +30,7 @@ module.exports = UserCreator =
 		user.save (err)->
 			callback(err, user)
 
+			return if options?.skip_affiliation
 			# call addaffiliation after the main callback so it runs in the
 			# background. There is no guaranty this will run so we must no rely on it
 			addAffiliation user._id, user.email, (error) ->

services/web/app/views/subscriptions/plans.pug

@@ -14,5 +14,5 @@ block content
 		.content.plans(ng-controller="PlansController")
 			.container(class="more-details" ng-cloak ng-if="plansVariant === 'more-details'")
 				include _plans_page_details_more
-			.container(ng-cloak ng-if="plansVariant === 'default' || !shouldABTestPlans")
+			.container(ng-cloak ng-if="plansVariant === 'default' || !shouldABTestPlans || timeout")
 				include _plans_page_details_less

services/web/app/views/user/settings.pug

@@ -168,6 +168,11 @@ block content
 										i.fa.fa-check
 										| #{translate("unsubscribed")}
 
+									if !settings.overleaf && user.overleaf
+										p
+											| Please note: If you have linked your account with Overleaf
+											| v2, then deleting your ShareLaTeX account will also delete
+											| account and all of it's associated projects and data.
 									p #{translate("need_to_leave")} 
 										a(href, ng-click="deleteAccount()")  #{translate("delete_your_account")}
 

services/web/config/settings.defaults.coffee

@@ -135,6 +135,7 @@ module.exports = settings =
 			url: "http://#{process.env['FILESTORE_HOST'] or 'localhost'}:3009"
 		clsi:
 			url: "http://#{process.env['CLSI_HOST'] or 'localhost'}:3013"
+			# url: "http://#{process.env['CLSI_LB_HOST']}:3014"
 			backendGroupName: undefined
 		templates:
 			url: "http://#{process.env['TEMPLATES_HOST'] or 'localhost'}:3007"
@@ -340,7 +341,7 @@ module.exports = settings =
 	# disablePerUserCompiles: true
 
 	# Domain the client (pdfjs) should download the compiled pdf from
-	# pdfDownloadDomain: "http://compiles.sharelatex.test:3014"
+	# pdfDownloadDomain: "http://clsi-lb:3014"
 
 	# Maximum size of text documents in the real-time editing system.
 	max_doc_length: 2 * 1024 * 1024 # 2mb

services/web/public/coffee/ide/pdf/controllers/PdfController.coffee

@@ -225,7 +225,8 @@ define [
 			}, {params: params}
 
 		buildPdfDownloadUrl = (pdfDownloadDomain, path)->
-			if pdfDownloadDomain?
+			 #we only download builds from compiles server for security reasons
+			if pdfDownloadDomain? and path.indexOf("build") != -1
 				return "#{pdfDownloadDomain}#{path}"
 			else
 				return path
@@ -378,14 +379,15 @@ define [
 						compileGroup:ide.compileGroup
 						clsiserverid:ide.clsiServerId
 				if file?.url?  # FIXME clean this up when we have file.urls out consistently
-					opts.url = buildPdfDownloadUrl options.pdfDownloadDomain, file.url
+					opts.url = file.url
 				else if file?.build?
-					opts.url = buildPdfDownloadUrl options.pdfDownloadDomain, "/project/#{$scope.project_id}/build/#{file.build}/output/#{name}"
+					opts.url = "/project/#{$scope.project_id}/build/#{file.build}/output/#{name}"
 				else
-					opts.url = buildPdfDownloadUrl options.pdfDownloadDomain,  "/project/#{$scope.project_id}/output/#{name}"
+					opts.url = "/project/#{$scope.project_id}/output/#{name}"
 				# check if we need to bust cache (build id is unique so don't need it in that case)
 				if not file?.build?
 					opts.params.cache_bust = "#{Date.now()}"
+				opts.url = buildPdfDownloadUrl options.pdfDownloadDomain, opts.url
 				return $http(opts)
 
 			# accumulate the log entries

services/web/public/coffee/main/account-settings.coffee

@@ -1,7 +1,7 @@
 define [
 	"base"
 ], (App) ->
-	App.controller "AccountSettingsController", ["$scope", "$http", "$modal", "event_tracking", ($scope, $http, $modal, event_tracking) ->
+	App.controller "AccountSettingsController", ["$scope", "$http", "$modal", "event_tracking", "UserAffiliationsDataService", ($scope, $http, $modal, event_tracking, UserAffiliationsDataService) ->
 		$scope.subscribed = true
 
 		$scope.unsubscribe = () ->
@@ -21,8 +21,14 @@ define [
 		$scope.deleteAccount = () ->
 			modalInstance = $modal.open(
 				templateUrl: "deleteAccountModalTemplate"
-				controller: "DeleteAccountModalController",
-				scope: $scope
+				controller: "DeleteAccountModalController"
+				resolve:
+					userDefaultEmail: () ->
+						UserAffiliationsDataService
+							.getUserDefaultEmail()
+							.then (defaultEmailDetails) ->
+								return defaultEmailDetails?.email or null
+							.catch () -> null
 			)
 
 		$scope.upgradeIntegration = (service) ->
@@ -30,8 +36,8 @@ define [
 	]
 
 	App.controller "DeleteAccountModalController", [
-		"$scope", "$modalInstance", "$timeout", "$http",
-		($scope,   $modalInstance,   $timeout,   $http) ->
+		"$scope", "$modalInstance", "$timeout", "$http", "userDefaultEmail",
+		($scope,   $modalInstance,   $timeout,   $http,   userDefaultEmail) ->
 			$scope.state =
 				isValid : false
 				deleteText: ""
@@ -46,7 +52,7 @@ define [
 				, 700
 
 			$scope.checkValidation = ->
-				$scope.state.isValid = $scope.state.deleteText == $scope.email and $scope.state.password.length > 0
+				$scope.state.isValid = userDefaultEmail? and $scope.state.deleteText == userDefaultEmail and $scope.state.password.length > 0
 
 			$scope.delete = () ->
 				$scope.state.inflight = true

services/web/public/coffee/main/affiliations/factories/UserAffiliationsDataService.coffee

@@ -31,6 +31,10 @@ define [
 			$http.get "/user/emails"
 				.then (response) -> response.data
 
+		getUserDefaultEmail = () ->
+			getUserEmails().then (userEmails) ->
+				_.find userEmails, (userEmail) -> userEmail.default
+
 		getUniversitiesFromCountry = (country) ->
 			if universities[country.code]?
 				universitiesFromCountry = universities[country.code]
@@ -118,6 +122,7 @@ define [
 			getDefaultRoleHints
 			getDefaultDepartmentHints
 			getUserEmails
+			getUserDefaultEmail
 			getUniversitiesFromCountry
 			getUniversityDomainFromPartialDomainInput
 			getUniversityDetails

services/web/public/coffee/main/new-subscription.coffee

@@ -4,20 +4,21 @@ define [
 	"libs/recurly-4.8.5"
 ], (App)->
 
-	App.controller "NewSubscriptionController", ($scope, MultiCurrencyPricing, abTestManager, $http, sixpack, event_tracking, ccUtils)->
+	App.controller "NewSubscriptionController", ($scope, MultiCurrencyPricing, abTestManager, $http, sixpack, event_tracking, ccUtils, ipCookie)->
 		throw new Error("Recurly API Library Missing.")  if typeof recurly is "undefined"
 
 		$scope.currencyCode = MultiCurrencyPricing.currencyCode
 		$scope.plans = MultiCurrencyPricing.plans
 		$scope.planCode = window.plan_code
+		$scope.plansVariant = ipCookie('plansVariant')
 
 		$scope.switchToStudent = ()->
 			currentPlanCode = window.plan_code
 			planCode = currentPlanCode.replace('collaborator', 'student')
-			event_tracking.sendMB 'subscription-form-switch-to-student', { plan: window.plan_code }
+			event_tracking.sendMB 'subscription-form-switch-to-student', { plan: window.plan_code, variant: $scope.plansVariant }
 			window.location = "/user/subscription/new?planCode=#{planCode}&currency=#{$scope.currencyCode}&cc=#{$scope.data.coupon}"
 
-		event_tracking.sendMB "subscription-form", { plan : window.plan_code }
+		event_tracking.sendMB "subscription-form", { plan : window.plan_code, variant: $scope.plansVariant }
 
 		$scope.paymentMethod =
 			value: "credit_card"
@@ -143,13 +144,14 @@ define [
 					currencyCode	: postData.subscriptionDetails.currencyCode,
 					plan_code		: postData.subscriptionDetails.plan_code,
 					coupon_code		: postData.subscriptionDetails.coupon_code,
-					isPaypal		: postData.subscriptionDetails.isPaypal
+					isPaypal		: postData.subscriptionDetails.isPaypal,
+					variant			: $scope.plansVariant
 				}
 
 
 				$http.post("/user/subscription/create", postData)
 					.then ()->
-						event_tracking.sendMB "subscription-submission-success"
+						event_tracking.sendMB "subscription-submission-success", { variant: $scope.plansVariant }
 						window.location.href = "/user/subscription/thank-you"
 					.catch ()->
 						$scope.processing = false
@@ -235,6 +237,3 @@ define [
 			{code:'WK',name:'Wake Island'},{code:'WF',name:'Wallis and Futuna'},{code:'EH',name:'Western Sahara'},{code:'YE',name:'Yemen'},
 			{code:'ZM',name:'Zambia'},{code:'AX',name:'Åland Islandscode:'}
 		]
-
-		sixpack.participate 'plans', ['default', 'more-details'], (chosenVariation, rawResponse)->
-			$scope.plansVariant = chosenVariation

services/web/public/coffee/main/plans.coffee

@@ -145,15 +145,21 @@ define [
 		}
 
 
-	App.controller "PlansController", ($scope, $modal, event_tracking, abTestManager, MultiCurrencyPricing, $http, sixpack, $filter) ->
+	App.controller "PlansController", ($scope, $modal, event_tracking, abTestManager, MultiCurrencyPricing, $http, sixpack, $filter, ipCookie) ->
 
 		$scope.showPlans = false
 		$scope.shouldABTestPlans = window.shouldABTestPlans
 
 		if $scope.shouldABTestPlans
 			sixpack.participate 'plans-details', ['default', 'more-details'], (chosenVariation, rawResponse)->
+				if rawResponse?.status != 'failed' 
 					$scope.plansVariant = chosenVariation
+					expiration = new Date();
+					expiration.setDate(expiration.getDate() + 5);
+					ipCookie('plansVariant', chosenVariation, {expires: expiration})
 					event_tracking.send 'subscription-funnel', 'plans-page-loaded', chosenVariation
+				else
+					$scope.timeout = true
 
 		$scope.showPlans = true
 
@@ -184,9 +190,9 @@ define [
 			if $scope.ui.view == "annual"
 				plan = "#{plan}_annual"
 			plan = eventLabel(plan, location)
-			event_tracking.sendMB 'plans-page-start-trial', {plan}
+			event_tracking.sendMB 'plans-page-start-trial', {plan, variant: $scope.plansVariant}
 			event_tracking.send 'subscription-funnel', 'sign_up_now_button', plan
-			if $scope.shouldABTestPlans
+			if $scope.plansVariant
 				sixpack.convert 'plans-details'
 
 		$scope.switchToMonthly = (e, location) ->

services/web/public/stylesheets/app/editor/review-panel.less

@@ -922,7 +922,7 @@
 	}
 }
 
-.review-icon {
+.review-icon when (@is-overleaf = false) {
 	display: inline-block;
 	background: url('/img/review-icon-sprite.png') top/30px no-repeat;
 	width: 30px;
@@ -945,10 +945,17 @@
 	}
 }
 
-.review-icon when (@is-overleaf) {
-	background-position-y: -60px;
-	.toolbar .btn-full-height:hover & {
-		background-position-y: -60px;
+.review-icon when (@is-overleaf = true) {
+	display: inline-block;
+	background: url('/img/review-icon-sprite-ol.png') top/30px no-repeat;
+	width: 30px;
+
+	&::before {
+		content: '\00a0'; // Non-breakable space. A non-breakable character here makes this icon work like font-awesome.
+	}
+	
+	@media (min-device-pixel-ratio: 1.5), (min-resolution: 144dpi) {
+		background-image: url('/img/review-icon-sprite-ol@2x.png');
 	}
 }
 

services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee

@@ -214,7 +214,7 @@ describe "AuthenticationController", ->
 		beforeEach ->
 			@AuthenticationController._recordFailedLogin = sinon.stub()
 			@AuthenticationController._recordSuccessfulLogin = sinon.stub()
-			@Modules.hooks.fire = sinon.stub().callsArgWith(2, null, [])
+			@Modules.hooks.fire = sinon.stub().callsArgWith(3, null, [])
 			# @AuthenticationController.establishUserSession = sinon.stub().callsArg(2)
 			@req.body =
 				email: @email
@@ -225,7 +225,7 @@ describe "AuthenticationController", ->
 
 		describe "when the preDoPassportLogin hooks produce an info object", ->
 			beforeEach ->
-				@Modules.hooks.fire = sinon.stub().callsArgWith(2, null, [null, {redir: '/somewhere'}, null])
+				@Modules.hooks.fire = sinon.stub().callsArgWith(3, null, [null, {redir: '/somewhere'}, null])
 
 			it "should stop early and call done with this info object", (done) ->
 				@AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb)

services/web/test/unit/coffee/User/UserCreatorTests.coffee

@@ -20,7 +20,7 @@ describe "UserCreator", ->
 		@addAffiliation = sinon.stub().yields()
 		@UserCreator = SandboxedModule.require modulePath, requires:
 			"../../models/User": User:@UserModel
-			"logger-sharelatex":{log:->}
+			"logger-sharelatex":{ log: sinon.stub(), err: sinon.stub() }
 			'metrics-sharelatex': {timeAsyncMethod: ()->}
 			"../Institutions/InstitutionsAPI": addAffiliation: @addAffiliation
 
@@ -88,3 +88,11 @@ describe "UserCreator", ->
 				process.nextTick () =>
 					sinon.assert.calledWith(@addAffiliation, user._id, user.email)
 					done()
+
+		it "should not add affiliation if skipping", (done)->
+			attributes =  email: @email
+			options = skip_affiliation: true
+			@UserCreator.createNewUser attributes, options, (err, user) =>
+				process.nextTick () =>
+					sinon.assert.notCalled(@addAffiliation)
+					done()