From 17a63258c6ee4ea24e5d10bad4c3bf85d91fdf01 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Sat, 4 Aug 2018 17:30:24 +0100
Subject: [PATCH 1/8] changed newsletter to use mailchimp

---
 .../Newsletter/NewsletterManager.coffee       | 51 ++++++++++++-------
 services/web/config/settings.defaults.coffee  |  6 +--
 services/web/package.json                     |  1 +
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
index 8d7eee43bf..7baa317632 100644
--- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
+++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
@@ -1,37 +1,52 @@
 async = require('async')
-Request = require('request')
 logger = require 'logger-sharelatex'
 Settings = require 'settings-sharelatex'
+crypto = require('crypto')
+Mailchimp = require('mailchimp-api-v3')
+mailchimp = new Mailchimp(Settings.mailchimp?.api_key) if Settings.mailchimp?
 
 module.exports =
 	subscribe: (user, callback = () ->)->
-		if !Settings.markdownmail?
+		if !Settings.mailchimp?
 			logger.warn "No newsletter provider configured so not subscribing user"
 			return callback()
-		logger.log user:user, email:user.email, "trying to subscribe user to the mailing list"
 		options = buildOptions(user, true)
-		Request.post options, (err, response, body)->
-			logger.log body:body, user:user, "finished attempting to subscribe the user to the news letter"
+		logger.log options:options, user:user, email:user.email, "trying to subscribe user to the mailing list"
+		mailchimp.request options, (err)->
+			if err?
+				logger.err err:err, "error subscribing person to newsletter"
+			else
+				logger.log user:user, "finished subscribing user to the newsletter"
 			callback(err)
 
 	unsubscribe: (user, callback = () ->)->
-		if !Settings.markdownmail?
+		if !Settings.mailchimp?
 			logger.warn "No newsletter provider configured so not unsubscribing user"
 			return callback()
 		logger.log user:user, email:user.email, "trying to unsubscribe user to the mailing list"
 		options = buildOptions(user, false)
-		Request.post options, (err, response, body)->
-			logger.log err:err, body:body, email:user.email, "compled newsletter unsubscribe attempt"
+		mailchimp.request options, (err)->
+			if err?
+				logger.err err:err, "error unsubscribing person to newsletter"
+			else
+				logger.log user:user, "finished unsubscribing user to the newsletter"
 			callback(err)
 
+hashEmail = (email)->
+	crypto.createHash('md5').update(email.toLowerCase()).digest("hex")
+
 buildOptions = (user, is_subscribed)->
-	options =
-		json:
-			secret_token: Settings.markdownmail.secret 
-			name: "#{user.first_name} #{user.last_name}"
-			email: user.email
-			subscriber_list_id: Settings.markdownmail.list_id
-			is_subscribed: is_subscribed
-		url: "https://www.markdownmail.io/lists/subscribe"
-		timeout: 30 * 1000
-	return options
\ No newline at end of file
+	status =  if is_subscribed then "subscribed" else "unsubscribed"
+	subscriber_hash = hashEmail(user.email)
+	opts =
+		method: "PUT"
+		path: "/lists/#{Settings.mailchimp?.list_id}/members/#{subscriber_hash}"
+		body:
+			status_if_new: status
+			status: status
+			email_address:user.email
+			merge_fields:
+				FNAME: user.first_name
+				LNAME: user.last_name
+	return opts
+
diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee
index 1306ce91c2..827f4f2959 100644
--- a/services/web/config/settings.defaults.coffee
+++ b/services/web/config/settings.defaults.coffee
@@ -277,10 +277,10 @@ module.exports = settings =
 	# Third party services
 	# --------------------
 	#
-	# ShareLaTeX's regular newsletter is managed by Markdown mail. Add your
+	# ShareLaTeX's regular newsletter is managed by mailchimp. Add your
 	# credentials here to integrate with this.
-	# markdownmail:
-	# 	secret: ""
+	# mailchimp:
+	# 	api_key: ""
 	# 	list_id: ""
 	#
 	# Fill in your unique token from various analytics services to enable
diff --git a/services/web/package.json b/services/web/package.json
index ed0eb06455..1dce008612 100644
--- a/services/web/package.json
+++ b/services/web/package.json
@@ -59,6 +59,7 @@
     "lodash": "^4.13.1",
     "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#master",
     "lynx": "0.1.1",
+    "mailchimp-api-v3": "^1.12.0",
     "marked": "^0.3.5",
     "method-override": "^2.3.3",
     "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1",

From fa37caef58d84d645b40aa57e8d2dc1c6b2fabb5 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Mon, 6 Aug 2018 12:37:18 +0100
Subject: [PATCH 2/8] remove null check on mailchimp at top of file

I didn't like the if statment being after the require, mailchimp
could also be null and called elsewhere by acident
---
 .../app/coffee/Features/Newsletter/NewsletterManager.coffee    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
index 7baa317632..4850554303 100644
--- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
+++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
@@ -3,7 +3,6 @@ logger = require 'logger-sharelatex'
 Settings = require 'settings-sharelatex'
 crypto = require('crypto')
 Mailchimp = require('mailchimp-api-v3')
-mailchimp = new Mailchimp(Settings.mailchimp?.api_key) if Settings.mailchimp?
 
 module.exports =
 	subscribe: (user, callback = () ->)->
@@ -12,6 +11,7 @@ module.exports =
 			return callback()
 		options = buildOptions(user, true)
 		logger.log options:options, user:user, email:user.email, "trying to subscribe user to the mailing list"
+		mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
 		mailchimp.request options, (err)->
 			if err?
 				logger.err err:err, "error subscribing person to newsletter"
@@ -25,6 +25,7 @@ module.exports =
 			return callback()
 		logger.log user:user, email:user.email, "trying to unsubscribe user to the mailing list"
 		options = buildOptions(user, false)
+		mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
 		mailchimp.request options, (err)->
 			if err?
 				logger.err err:err, "error unsubscribing person to newsletter"

From 6cf1f71604923bd042c505e248be845f6c724fff Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Mon, 6 Aug 2018 17:43:03 +0100
Subject: [PATCH 3/8] add newsletter checkbox to user register forms

---
 .../coffee/Features/User/UserRegistrationHandler.coffee   | 5 +++--
 services/web/public/stylesheets/app/homepage.less         | 4 ++++
 .../unit/coffee/User/UserRegistrationHandlerTests.coffee  | 8 +++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
index df7fe93218..848434271f 100644
--- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
+++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
@@ -1,4 +1,4 @@
-sanitize = require('sanitizer')
+resanitize = require('sanitizer')
 User = require("../../models/User").User
 UserCreator = require("./UserCreator")
 UserGetter = require("./UserGetter")
@@ -54,7 +54,8 @@ module.exports = UserRegistrationHandler =
 					(cb)-> User.update {_id: user._id}, {"$set":{holdingAccount:false}}, cb
 					(cb)-> AuthenticationManager.setUserPassword user._id, userDetails.password, cb
 					(cb)->
-						NewsLetterManager.subscribe user, ->
+						if userDetails.subscribeToNewsletter == "true"
+							NewsLetterManager.subscribe user, ->
 						cb() #this can be slow, just fire it off
 				], (err)->
 					logger.log user: user, "registered"
diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less
index d502855b72..22efbc847d 100644
--- a/services/web/public/stylesheets/app/homepage.less
+++ b/services/web/public/stylesheets/app/homepage.less
@@ -47,7 +47,11 @@
 		}
 		.form-group {
 			margin-left: @line-height-computed / 2;
+			input[type="checkbox"] {
+				margin-right: 5px;
+			}
 		}
+
 	}
 	.screenshot {
 		height: 550px;
diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee
index e9a6c568dd..f8bcce30ce 100644
--- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee
+++ b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee
@@ -132,11 +132,17 @@ describe "UserRegistrationHandler", ->
 					@AuthenticationManager.setUserPassword.calledWith(@user._id, @passingRequest.password).should.equal true
 					done()			
 
-			it "should add the user to the news letter manager", (done)->
+			it "should add the user to the newsletter if accepted terms", (done)->
+				@passingRequest.subscribeToNewsletter = "true"
 				@handler.registerNewUser @passingRequest, (err)=>
 					@NewsLetterManager.subscribe.calledWith(@user).should.equal true
 					done()
 
+			it "should not add the user to the newsletter if not accepted terms", (done)->
+				@handler.registerNewUser @passingRequest, (err)=>
+					@NewsLetterManager.subscribe.calledWith(@user).should.equal false
+					done()
+
 			it "should track the registration event", (done)->
 				@handler.registerNewUser @passingRequest, (err)=>
 					@AnalyticsManager.recordEvent

From 88f2b3670bbe65f602a2b7b1c93be262a2c07b33 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Tue, 7 Aug 2018 16:48:35 +0100
Subject: [PATCH 4/8] remove unneed style

---
 services/web/public/stylesheets/app/homepage.less | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less
index 22efbc847d..d502855b72 100644
--- a/services/web/public/stylesheets/app/homepage.less
+++ b/services/web/public/stylesheets/app/homepage.less
@@ -47,11 +47,7 @@
 		}
 		.form-group {
 			margin-left: @line-height-computed / 2;
-			input[type="checkbox"] {
-				margin-right: 5px;
-			}
 		}
-
 	}
 	.screenshot {
 		height: 550px;

From 6208e9f2d0cbf673ab1b69c0e2a48b915c2a9e4f Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Tue, 7 Aug 2018 21:38:31 +0100
Subject: [PATCH 5/8] add changeEmail function to newsletter manager

not actually called
---
 .../Newsletter/NewsletterManager.coffee       | 30 ++++++++++++++-----
 .../coffee/Features/User/UserUpdater.coffee   |  1 +
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
index 4850554303..0c1a98a6b1 100644
--- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
+++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
@@ -4,14 +4,19 @@ Settings = require 'settings-sharelatex'
 crypto = require('crypto')
 Mailchimp = require('mailchimp-api-v3')
 
+
+if !Settings.mailchimp?.api_key?
+	logger.warn "No newsletter provider configured so not chaning email on user"
+	mailchimp =
+		request: (opts, cb)-> cb()
+else
+	mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
+
 module.exports =
+
 	subscribe: (user, callback = () ->)->
-		if !Settings.mailchimp?
-			logger.warn "No newsletter provider configured so not subscribing user"
-			return callback()
 		options = buildOptions(user, true)
 		logger.log options:options, user:user, email:user.email, "trying to subscribe user to the mailing list"
-		mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
 		mailchimp.request options, (err)->
 			if err?
 				logger.err err:err, "error subscribing person to newsletter"
@@ -20,12 +25,8 @@ module.exports =
 			callback(err)
 
 	unsubscribe: (user, callback = () ->)->
-		if !Settings.mailchimp?
-			logger.warn "No newsletter provider configured so not unsubscribing user"
-			return callback()
 		logger.log user:user, email:user.email, "trying to unsubscribe user to the mailing list"
 		options = buildOptions(user, false)
-		mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
 		mailchimp.request options, (err)->
 			if err?
 				logger.err err:err, "error unsubscribing person to newsletter"
@@ -33,6 +34,19 @@ module.exports =
 				logger.log user:user, "finished unsubscribing user to the newsletter"
 			callback(err)
 
+	changeEmail: (oldEmail, newEmail, callback = ()->)->
+		options = buildOptions({email:oldEmail})
+		delete options.body.status
+		options.body.email_address = newEmail
+		mailchimp.request options, (err)->
+			# if the user has unsubscribed mailchimp will error on email address change
+			if err? and err?.message.indexOf("could not be validated") == -1
+				logger.err err:err, "error changing email in newsletter"
+				return callback(err)
+			else
+				logger.log "finished changing email in the newsletter"
+				return callback()
+
 hashEmail = (email)->
 	crypto.createHash('md5').update(email.toLowerCase()).digest("hex")
 
diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee
index 8d5e3658f9..7d828366e7 100644
--- a/services/web/app/coffee/Features/User/UserUpdater.coffee
+++ b/services/web/app/coffee/Features/User/UserUpdater.coffee
@@ -11,6 +11,7 @@ EmailHelper = require "../Helpers/EmailHelper"
 Errors = require "../Errors/Errors"
 Settings = require "settings-sharelatex"
 request = require 'request'
+NewsletterManager = require "../Newsletter/NewsletterManager"
 
 module.exports = UserUpdater =
 	updateUser: (query, update, callback = (error) ->) ->

From 687637eec71bacd2aa8b8a92ced6330aa2d9c728 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Wed, 8 Aug 2018 13:15:51 +0100
Subject: [PATCH 6/8] change email address in newsletter when changing default
 email

put mongo_id into mailchimp merge fields
---
 .../Newsletter/NewsletterManager.coffee       |  2 +-
 .../coffee/Features/User/UserUpdater.coffee   | 22 ++++++++++------
 .../unit/coffee/User/UserUpdaterTests.coffee  | 26 ++++++++++++++++---
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
index 0c1a98a6b1..431eb214c3 100644
--- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
+++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
@@ -4,7 +4,6 @@ Settings = require 'settings-sharelatex'
 crypto = require('crypto')
 Mailchimp = require('mailchimp-api-v3')
 
-
 if !Settings.mailchimp?.api_key?
 	logger.warn "No newsletter provider configured so not chaning email on user"
 	mailchimp =
@@ -63,5 +62,6 @@ buildOptions = (user, is_subscribed)->
 			merge_fields:
 				FNAME: user.first_name
 				LNAME: user.last_name
+				MONGO_ID:user._id
 	return opts
 
diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee
index 7d828366e7..5770c7b834 100644
--- a/services/web/app/coffee/Features/User/UserUpdater.coffee
+++ b/services/web/app/coffee/Features/User/UserUpdater.coffee
@@ -100,15 +100,21 @@ module.exports = UserUpdater =
 	setDefaultEmailAddress: (userId, email, callback) ->
 		email = EmailHelper.parseEmail(email)
 		return callback(new Error('invalid email')) if !email?
-		query = _id: userId, 'emails.email': email
-		update = $set: email: email
-		@updateUser query, update, (error, res) ->
-			if error?
-				logger.err error:error, 'problem setting default emails'
+		UserGetter.getUserEmail userId, (error, oldEmail) =>
+			if err?
 				return callback(error)
-			if res.n == 0 # TODO: Check n or nMatched?
-				return callback(new Error('Default email does not belong to user'))
-			callback()
+			query = _id: userId, 'emails.email': email
+			update = $set: email: email
+			@updateUser query, update, (error, res) ->
+				if error?
+					logger.err error:error, 'problem setting default emails'
+					return callback(error)
+				else if res.n == 0 # TODO: Check n or nMatched?
+					return callback(new Error('Default email does not belong to user'))
+				else
+					NewsletterManager.changeEmail oldEmail, email, callback
+
+
 
 	updateV1AndSetDefaultEmailAddress: (userId, email, callback) ->
 		@updateEmailAddressInV1 userId, email, (error) =>
diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee
index f2ac951727..17f691edba 100644
--- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee
+++ b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee
@@ -18,21 +18,27 @@ describe "UserUpdater", ->
 			getUserEmail: sinon.stub()
 			getUserByAnyEmail: sinon.stub()
 			ensureUniqueEmailAddress: sinon.stub()
-		@logger = err: sinon.stub(), log: ->
+		@logger = 
+			err: sinon.stub()
+			log: ->
+			warn: ->
 		@addAffiliation = sinon.stub().yields()
 		@removeAffiliation = sinon.stub().callsArgWith(2, null)
 		@refreshFeatures = sinon.stub().yields()
+		@NewsletterManager = 
+			changeEmail:sinon.stub()
 		@UserUpdater = SandboxedModule.require modulePath, requires:
 			"logger-sharelatex": @logger
+			"../../infrastructure/mongojs":@mongojs
+			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
 			"./UserGetter": @UserGetter
 			'../Institutions/InstitutionsAPI':
 				addAffiliation: @addAffiliation
 				removeAffiliation: @removeAffiliation
 			'../Subscription/FeaturesUpdater': refreshFeatures: @refreshFeatures
-			"../../infrastructure/mongojs":@mongojs
-			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
 			"settings-sharelatex": @settings = {}
 			"request": @request = {}
+			"../Newsletter/NewsletterManager": @NewsletterManager
 
 		@stubbedUser = 
 			_id: "3131231"
@@ -174,6 +180,10 @@ describe "UserUpdater", ->
 				done()
 
 	describe 'setDefaultEmailAddress', ->
+		beforeEach ->
+			@UserGetter.getUserEmail.callsArgWith(1, null, @stubbedUser.email)
+			@NewsletterManager.changeEmail.callsArgWith(2, null)
+
 		it 'set default', (done)->
 			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 1)
 
@@ -185,6 +195,16 @@ describe "UserUpdater", ->
 				).should.equal true
 				done()
 
+		it 'set changed the email in newsletter', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 1)
+
+			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.not.exist(err)
+				@NewsletterManager.changeEmail.calledWith(
+					@stubbedUser.email, @newEmail
+				).should.equal true
+				done()
+
 		it 'handle error', (done)->
 			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, new Error('nope'))
 

From c68366155e0e2e56c47ba9c0c9492b4aa0d440f3 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Wed, 8 Aug 2018 14:14:56 +0100
Subject: [PATCH 7/8] remove sanitize = require('sanitizer') not used anywhere

---
 .../web/app/coffee/Features/User/UserRegistrationHandler.coffee  | 1 -
 1 file changed, 1 deletion(-)

diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
index 848434271f..1291142dab 100644
--- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
+++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
@@ -1,4 +1,3 @@
-resanitize = require('sanitizer')
 User = require("../../models/User").User
 UserCreator = require("./UserCreator")
 UserGetter = require("./UserGetter")

From df161d3ece9f11673e7cdc04fd6ea6ec36e02528 Mon Sep 17 00:00:00 2001
From: Henry Oswald <henry.oswald@sharelatex.com>
Date: Wed, 8 Aug 2018 14:32:36 +0100
Subject: [PATCH 8/8] change newsletter log to info on process boot

---
 .../app/coffee/Features/Newsletter/NewsletterManager.coffee    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
index 431eb214c3..5fb2e00eb7 100644
--- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
+++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee
@@ -5,10 +5,11 @@ crypto = require('crypto')
 Mailchimp = require('mailchimp-api-v3')
 
 if !Settings.mailchimp?.api_key?
-	logger.warn "No newsletter provider configured so not chaning email on user"
+	logger.info "Using newsletter provider: none"
 	mailchimp =
 		request: (opts, cb)-> cb()
 else
+	logger.info "Using newsletter provider: mailchimp"
 	mailchimp = new Mailchimp(Settings.mailchimp?.api_key)
 
 module.exports =