Merge lp:~sinzui/launchpad/list-urls-bug-463444 into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my branch to fix team join emails have bad mailing list url.

    lp:~sinzui/launchpad/list-urls-bug-463444
    Diff size: 35
    Launchpad bug: https://bugs.launchpad.net/bugs/463444
    Test command: ./bin/test -vv -t teammembership-email-notification
    Pre-implementation: no one
    Target release: 3.1.10

= Fix team join emails have bad mailing list url =

The URL provided for subscribing to team emails is a launchpad.dev email.

== Rules ==

emailtemplates/team-list-subscribe-block has the server hard coded
teammembership-email-notification.txt will always pass.

    * Change the email template to use a %(editemails_url)s
      * Watch the test fail.
    * Add code to interpolar "+editemails" % canonical_url(person)
      * Watch the test pass.

== QA ==

    * Open the staging mailbox and mark all email read.
    * Add someone to your team on staging.
    * Update the mailbox and read the email to the new member.
    * Verify that the email's edit URL is:
      https://staging.launchpad.net/people/+me/+editemails

== Lint ==

Linting changed files:
  lib/canonical/launchpad/mailnotification.py
  lib/canonical/launchpad/emailtemplates/team-list-subscribe-block.txt

== Test ==

lp/registry/doc/teammembership-email-notification.txt did not change because
it was already testing for the correct result. The email template was
hard coded to always return the correct URL.

The test failed when the substitution variable was added. It passed after
a few trials make the URL from ILaunchpadRoot.

== Implementation ==

    * lib/canonical/launchpad/emailtemplates/team-list-subscribe-block.txt
      * Replaced the hard coded URL with a substiution variable.
    * lib/canonical/launchpad/mailnotification.py
      * Updated the code to create a URL from ILaunchpadRoot and to
        interpolate that into the team-list-subscribe-block text.

[Guilherme Salgado (salgado)]

This looks good, but you could use webapp.urlappend() to construct the URL to the +editemails page.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/emailtemplates/team-list-subscribe-block.txt'
--- lib/canonical/launchpad/emailtemplates/team-list-subscribe-block.txt	2009-08-24 03:53:19 +0000
+++ lib/canonical/launchpad/emailtemplates/team-list-subscribe-block.txt	2009-10-30 13:10:25 +0000
@@ -1,4 +1,4 @@
 
 If you would like to subscribe to the team list, use the link below
 to update your Mailing List Subscription preferences.
-  <http://launchpad.dev/people/+me/+editemails>
+  <%(editemails_url)s>
 
=== modified file 'lib/canonical/launchpad/mailnotification.py'
--- lib/canonical/launchpad/mailnotification.py	2009-08-24 03:53:19 +0000
+++ lib/canonical/launchpad/mailnotification.py	2009-10-30 13:10:25 +0000
@@ -36,6 +36,7 @@
     IStructuralSubscriptionTarget, ITeamMembershipSet, IUpstreamBugTask,
     TeamMembershipStatus)
 from lp.bugs.interfaces.bugchange import IBugChange
+from canonical.launchpad.interfaces.launchpad import ILaunchpadRoot
 from canonical.launchpad.interfaces.message import (
     IDirectEmailAuthorization, QuotaReachedError)
 from canonical.launchpad.interfaces.structuralsubscription import (
@@ -45,7 +46,8 @@
 from lp.services.mail.mailwrapper import MailWrapper
 from lp.services.mail.notificationrecipientset import (
     NotificationRecipientSet)
-from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.launchpad.webapp.url import urlappend
 
 
 CC = "CC"
@@ -74,6 +76,7 @@
     IBug.getBugNotificationRecipients().
     """
     implements(INotificationRecipientSet)
+
     def __init__(self, duplicateof=None):
         """Constructs a new BugNotificationRecipients instance.
 
@@ -191,7 +194,7 @@
 
 def format_rfc2822_date(date):
     """Formats a date according to RFC2822's desires."""
-    return formatdate(rfc822.mktime_tz(date.utctimetuple() + (0,)))
+    return formatdate(rfc822.mktime_tz(date.utctimetuple() + (0, )))
 
 
 class BugNotificationBuilder:
@@ -461,8 +464,7 @@
         get_bugmail_error_address(), [config.launchpad.errors_address],
         'Unhandled Email: %s' % file_alias_url,
         template % {'url': file_alias_url, 'error_msg': message},
-        headers={'X-Launchpad-Unhandled-Email': message}
-        )
+        headers={'X-Launchpad-Unhandled-Email': message})
 
 
 def generate_bug_add_email(bug, new_recipients=False, reason=None,
@@ -505,12 +507,12 @@
 
     mailwrapper = MailWrapper(width=72)
     content_substitutions = {
-        'visibility' : visibility,
-        'bug_url' : canonical_url(bug),
+        'visibility': visibility,
+        'bug_url': canonical_url(bug),
         'bug_info': "\n".join(bug_info),
         'bug_title': bug.title,
         'description': mailwrapper.format(bug.description),
-        'notification_rationale': reason
+        'notification_rationale': reason,
         }
 
     if new_recipients:
@@ -571,13 +573,11 @@
     # which begin with '?'.
     text_diff = [
         diff_line for diff_line in text_diff
-        if not diff_line.startswith('?')
-        ]
+        if not diff_line.startswith('?')]
     # Add a whitespace between the +/- and the text line.
     text_diff = [
         re.sub('^([\+\- ])(.*)', r'\1 \2', line)
-        for line in text_diff
-        ]
+        for line in text_diff]
     text_diff = '\n'.join(text_diff)
     return text_diff
 
@@ -585,9 +585,9 @@
 def _get_task_change_row(label, oldval_display, newval_display):
     """Return a row formatted for display in task change info."""
     return u"%(label)13s: %(oldval)s => %(newval)s\n" % {
-        'label' : label.capitalize(),
-        'oldval' : oldval_display,
-        'newval' : newval_display}
+        'label': label.capitalize(),
+        'oldval': oldval_display,
+        'newval': newval_display}
 
 
 def _get_task_change_values(task_change, displayattrname):
@@ -614,7 +614,7 @@
     """
     changes = {}
 
-    for field_name in ("title", "description",  "name", "private",
+    for field_name in ("title", "description", "name", "private",
                        "security_related", "duplicateof", "tags"):
         # fields for which we show old => new when their values change
         old_val = getattr(old_bug, field_name)
@@ -786,7 +786,7 @@
         bug=bug,
         bugurl=canonical_url(bug),
         user=IPerson(event.user),
-        attachment={'new' : bugattachment, 'old': None})
+        attachment={'new': bugattachment, 'old': None})
 
     add_bug_change_notifications(bug_delta)
 
@@ -799,7 +799,7 @@
         bug=bug,
         bugurl=canonical_url(bug),
         user=IPerson(event.user),
-        attachment={'old' : bugattachment, 'new': None})
+        attachment={'old': bugattachment, 'new': None})
 
     add_bug_change_notifications(bug_delta)
 
@@ -908,8 +908,12 @@
                 "You received this email because you are the new member.")
 
         if team.mailing_list is not None:
-            list_instructions = get_email_template(
+            template = get_email_template(
                 'team-list-subscribe-block.txt')
+            editemails_url = urlappend(
+                canonical_url(getUtility(ILaunchpadRoot)),
+                'people/+me/+editemails')
+            list_instructions = template % dict(editemails_url=editemails_url)
         else:
             list_instructions = ''
 
@@ -990,6 +994,7 @@
     """Format the email subject line for a specification."""
     return '[Blueprint %s] %s' % (spec.name, spec.title)
 
+
 @block_implicit_flushes
 def notify_specification_modified(spec, event):
     """Notify the related people that a specification has been modifed."""
@@ -1061,7 +1066,6 @@
         simple_sendmail_from_person(user, address, subject, body)
 
 
-
 @block_implicit_flushes
 def notify_specification_subscription_created(specsub, event):
     """Notify a user that they have been subscribed to a blueprint."""
@@ -1074,12 +1078,13 @@
         'You are now subscribed to the blueprint '
         '%(blueprint_name)s - %(blueprint_title)s.\n\n'
         '--\n  %(blueprint_url)s' %
-        {'blueprint_name' : spec.name,
-         'blueprint_title' : spec.title,
-         'blueprint_url' : canonical_url(spec)})
+        {'blueprint_name': spec.name,
+         'blueprint_title': spec.title,
+         'blueprint_url': canonical_url(spec)})
     for address in get_contact_email_addresses(person):
         simple_sendmail_from_person(user, address, subject, body)
 
+
 @block_implicit_flushes
 def notify_specification_subscription_modified(specsub, event):
     """Notify a subscriber to a blueprint that their
@@ -1103,10 +1108,10 @@
         '%(blueprint_name)s - %(blueprint_title)s '
         'has changed to [%(specsub_type)s].\n\n'
         '--\n  %(blueprint_url)s' %
-        {'blueprint_name' : spec.name,
-         'blueprint_title' : spec.title,
-         'specsub_type' : specsub_type,
-         'blueprint_url' : canonical_url(spec)})
+        {'blueprint_name': spec.name,
+         'blueprint_title': spec.title,
+         'specsub_type': specsub_type,
+         'blueprint_url': canonical_url(spec)})
     for address in get_contact_email_addresses(person):
         simple_sendmail_from_person(user, address, subject, body)
 
@@ -1184,6 +1189,7 @@
             template % replacements, force_wrap=True)
         simple_sendmail(from_address, address, subject, body)
 
+
 @block_implicit_flushes
 def notify_new_ppa_subscription(subscription, event):
     """Notification that a new PPA subscription can be activated."""
@@ -1290,7 +1296,7 @@
         u'',
         u'-- ',
         u'This message was sent from Launchpad by the user',
-        u'%s (%s)' % (sender_name , canonical_url(sender)),
+        u'%s (%s)' % (sender_name, canonical_url(sender)),
         u'using %s.',
         u'For more information see',
         u'https://help.launchpad.net/YourAccount/ContactingPeople',