From 75521a4c86d0922fa099bf8ae83b5bcb01edd45a Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 28 Aug 2018 10:47:33 +0100 Subject: [PATCH 1/3] add error handling for changing email in newsletter of user who has not subscribed --- .../app/coffee/Features/Newsletter/NewsletterManager.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee index 5fb2e00eb7..eee7258985 100644 --- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee +++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee @@ -39,8 +39,11 @@ module.exports = delete options.body.status options.body.email_address = newEmail mailchimp.request options, (err)-> + if err? and err?.message?.indexOf("merge fields were invalid") + logger.log {oldEmail, newEmail}, "unable to change email in newsletter as user has not subscribed" + return callback() # if the user has unsubscribed mailchimp will error on email address change - if err? and err?.message.indexOf("could not be validated") == -1 + else if err? and err?.message?.indexOf("could not be validated") == -1 logger.err err:err, "error changing email in newsletter" return callback(err) else From 423bc9312d26aef6f1e6d3ef69cbd790edb663b4 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 28 Aug 2018 12:32:20 +0100 Subject: [PATCH 2/3] only set status and merge fields if required Not strictly nessaserry but it is a bit safer also improve error reporting for change email --- .../Newsletter/NewsletterManager.coffee | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee index eee7258985..1195796ec3 100644 --- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee +++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee @@ -16,10 +16,10 @@ module.exports = subscribe: (user, callback = () ->)-> options = buildOptions(user, true) - logger.log options:options, user:user, email:user.email, "trying to subscribe user to the mailing list" + logger.log options:options, user:user, email:user.email, "subscribing user to the mailing list" mailchimp.request options, (err)-> if err? - logger.err err:err, "error subscribing person to newsletter" + logger.err err:err, user:user, "error subscribing person to newsletter" else logger.log user:user, "finished subscribing user to the newsletter" callback(err) @@ -29,7 +29,7 @@ module.exports = options = buildOptions(user, false) mailchimp.request options, (err)-> if err? - logger.err err:err, "error unsubscribing person to newsletter" + logger.err err:err, user:user, "error unsubscribing person to newsletter" else logger.log user:user, "finished unsubscribing user to the newsletter" callback(err) @@ -39,12 +39,15 @@ module.exports = delete options.body.status options.body.email_address = newEmail mailchimp.request options, (err)-> - if err? and err?.message?.indexOf("merge fields were invalid") - logger.log {oldEmail, newEmail}, "unable to change email in newsletter as user has not subscribed" + if err? and err?.message?.indexOf("merge fields were invalid") != -1 + logger.log {oldEmail, newEmail}, "unable to change email in newsletter, user has never subscribed" return callback() - # if the user has unsubscribed mailchimp will error on email address change - else if err? and err?.message?.indexOf("could not be validated") == -1 - logger.err err:err, "error changing email in newsletter" + else if err? and err?.message?.indexOf("could not be validated") != -1 + logger.log {oldEmail, newEmail}, + "unable to change email in newsletter, user has previously unsubscribed or new email already exist on list" + return callback(err) + else if err? + logger.err {err, oldEmail, newEmail}, "error changing email in newsletter" return callback(err) else logger.log "finished changing email in the newsletter" @@ -54,18 +57,24 @@ hashEmail = (email)-> crypto.createHash('md5').update(email.toLowerCase()).digest("hex") buildOptions = (user, is_subscribed)-> - 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 - MONGO_ID:user._id + + status = if is_subscribed then "subscribed" else "unsubscribed" + if is_subscribed? + opts.body.status = status + opts.body.status_if_new = status + + if user._id? + opts.body.merge_fields = + FNAME: user.first_name + LNAME: user.last_name + MONGO_ID:user._id + + console.log opts return opts From 32749267b85d98ee9b2aaa768b8f848943b63fc8 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 28 Aug 2018 12:46:49 +0100 Subject: [PATCH 3/3] improve logging on change email in newsletter --- .../web/app/coffee/Features/Newsletter/NewsletterManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee index 1195796ec3..f64b8b7d63 100644 --- a/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee +++ b/services/web/app/coffee/Features/Newsletter/NewsletterManager.coffee @@ -38,6 +38,7 @@ module.exports = options = buildOptions({email:oldEmail}) delete options.body.status options.body.email_address = newEmail + logger.log {oldEmail, newEmail, options}, "changing email in newsletter" mailchimp.request options, (err)-> if err? and err?.message?.indexOf("merge fields were invalid") != -1 logger.log {oldEmail, newEmail}, "unable to change email in newsletter, user has never subscribed" @@ -75,6 +76,5 @@ buildOptions = (user, is_subscribed)-> LNAME: user.last_name MONGO_ID:user._id - console.log opts return opts