fix(email): add better password change link for certain emails #93

vladikoff · 2015-10-06T21:55:43Z

Fix #78

vladikoff · 2015-10-13T20:12:07Z

rfk · 2015-10-14T04:43:55Z

mailer.js

@@ -162,7 +169,8 @@ module.exports = function (log) {
        email: message.email,
        link: link,
        supportUrl: this.supportUrl,
-        supportLinkAttributes: this._supportLinkAttributes()
+        supportLinkAttributes: this._supportLinkAttributes(),
+        passwordChangeUrl: this._initiatePasswordChange()


naming nit: passwordChangeLinkAttributes or similar for consistency, since it's not just the URL?

rfk · 2015-10-14T04:45:07Z

mailer.js

@@ -321,7 +334,8 @@ module.exports = function (log) {
        link: link,
        oneClickLink: oneClickLink,
        supportUrl: this.supportUrl,
-        supportLinkAttributes: this._supportLinkAttributes()
+        supportLinkAttributes: this._supportLinkAttributes(),
+        passwordChangeUrl: this._initiatePasswordChange()


Wow that's a lot of identical changes...I wonder if we should factor these links out into some common set of attributes that are included in every render, so that we can manage them in a single place.

opened https://github.com/mozilla/fxa-auth-mailer/issues/94 so we can merge this and cut strings

rfk · 2015-10-14T04:47:09Z

@vladikoff travis bustage?

rfk · 2015-10-14T04:47:53Z

Also if I understand the rules correctly (thanks to a recent refresher from @shane-tomlinson) then we shouldn't merge this l10n-affecting string change until after we cut the next train...?

vladikoff · 2015-10-15T16:15:39Z

will fix the tests and ping @shane-tomlinson before cutting strings....

shane-tomlinson · 2015-10-15T16:23:13Z

Also if I understand the rules correctly (thanks to a recent refresher from @shane-tomlinson) then we shouldn't merge this l10n-affecting string change until after we cut the next train...?

We are in the "merge new strings!" window, until I go to sleep tonight.

Fixes #78

shane-tomlinson · 2015-10-15T17:09:56Z

test/local/mailer_tests.js

@@ -43,7 +43,7 @@ P.all(
          service: 'service',
        }

-        var supportHtmlLink = new RegExp('<a href="' + config.mail.supportUrl + '" style="color: #0095dd; text-decoration: none;">Mozilla Support</a>')
+        var supportHtmlLink = new RegExp('<a href="' + config.mail.supportUrl + '" style="color: #0095dd; text-decoration: none; font-family: sans-serif;">Mozilla Support</a>')


Should we add a "Change Password" link test too?

added to https://github.com/mozilla/fxa-auth-mailer/issues/94

shane-tomlinson · 2015-10-15T17:11:32Z

templates/password_reset.html

@@ -27,7 +27,7 @@ <h1 style="font-family: sans-serif; font-weight: normal; margin: 0 0 24px 0; tex
  <td border="0" cellpadding="0" cellspacing="0" height="100%" width="100%">
    <br/>
    <p class="secondary" style="font-family: sans-serif; font-weight: normal; margin: 0; text-align: center; color: #8A9BA8; font-size: 11px; line-height: 13px; width: 310px !important; word-wrap:break-word">
-      {{t "This is an automated email; if you received it in error, no action is required."}} {{{t "For more information, please visit <a %(supportLinkAttributes)s>Mozilla Support</a>"}}}
+      {{t "This is an automated email; if you did not authorize this action, then <a %(passwordChangeLinkAttributes)s>please change your password.</a>" }} {{{t "For more information, please visit <a %(supportLinkAttributes)s>Mozilla Support</a>"}}}


Ahh, since this now has HTML, you'll have to replace {{ with {{{or else the HTML will be escaped.

That's my theory anyways.

yea good note fixed via 83984f6

fix(email): add better password change link for certain emails

shane-tomlinson · 2015-10-15T17:13:02Z

mailer.js

@@ -130,7 +136,8 @@ module.exports = function (log) {
        link: link,
        oneClickLink: oneClickLink,
        supportUrl: this.supportUrl,
-        supportLinkAttributes: this._supportLinkAttributes()
+        supportLinkAttributes: this._supportLinkAttributes(),
+        passwordChangeLinkAttributes: this._initiatePasswordChange()


Should _initiatePasswordChange be renamed to _passwordChangeLinkAttributes to stay consistent?

added a note to https://github.com/mozilla/fxa-auth-mailer/issues/94#issuecomment-148462202

vladikoff added WIP waffle:progress labels Oct 6, 2015

vladikoff force-pushed the vladikoff:i78 branch from 26b66a7 to 498aed5 Oct 13, 2015

vladikoff removed the WIP label Oct 13, 2015

rfk reviewed Oct 14, 2015
View changes

rfk added i18n strings labels Oct 14, 2015

vladikoff force-pushed the vladikoff:i78 branch from 498aed5 to 34ce572 Oct 15, 2015

fix(emails): add better change password text

d5e2c0f

Fixes #78

vladikoff force-pushed the vladikoff:i78 branch from 34ce572 to d5e2c0f Oct 15, 2015

shane-tomlinson reviewed Oct 15, 2015
View changes

vladikoff added a commit that referenced this pull request Oct 15, 2015

Merge pull request #93 from vladikoff/i78

c3e0d6e

fix(email): add better password change link for certain emails

vladikoff merged commit c3e0d6e into mozilla:master Oct 15, 2015
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

vladikoff removed the waffle:progress label Oct 15, 2015

vladikoff deleted the vladikoff:i78 branch Oct 15, 2015

shane-tomlinson reviewed Oct 15, 2015
View changes

mozilla / fxa-auth-mailer Archived

fix(email): add better password change link for certain emails #93

fix(email): add better password change link for certain emails #93

vladikoff commented Oct 6, 2015

vladikoff commented Oct 13, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rfk commented Oct 14, 2015

rfk commented Oct 14, 2015

vladikoff commented Oct 15, 2015

shane-tomlinson commented Oct 15, 2015

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mozilla / fxa-auth-mailer Archived

fix(email): add better password change link for certain emails #93

fix(email): add better password change link for certain emails #93

Conversation

vladikoff commented Oct 6, 2015

vladikoff commented Oct 13, 2015

This comment has been minimized.

rfk Oct 14, 2015 Member

This comment has been minimized.

rfk Oct 14, 2015 Member

This comment has been minimized.

vladikoff Oct 15, 2015 Author Contributor

rfk commented Oct 14, 2015

rfk commented Oct 14, 2015

vladikoff commented Oct 15, 2015

shane-tomlinson commented Oct 15, 2015

This comment has been minimized.

shane-tomlinson Oct 15, 2015 Member

This comment has been minimized.

vladikoff Oct 15, 2015 Author Contributor

This comment has been minimized.

shane-tomlinson Oct 15, 2015 Member

This comment has been minimized.

shane-tomlinson Oct 15, 2015 Member

This comment has been minimized.

vladikoff Oct 15, 2015 Author Contributor

This comment has been minimized.

shane-tomlinson Oct 15, 2015 Member

This comment has been minimized.

vladikoff Oct 15, 2015 Author Contributor

rfk Oct 14, 2015
Member

rfk Oct 14, 2015
Member

vladikoff Oct 15, 2015
Author Contributor

shane-tomlinson Oct 15, 2015
Member

vladikoff Oct 15, 2015
Author Contributor

shane-tomlinson Oct 15, 2015
Member

shane-tomlinson Oct 15, 2015
Member

vladikoff Oct 15, 2015
Author Contributor

shane-tomlinson Oct 15, 2015
Member

vladikoff Oct 15, 2015
Author Contributor