Code review comment for lp:~nhandler/launchpad/bugfix151113

Summary

Bug #151113 describes how there are two spacing issues in the signature of Blueprint e-mail notifications. The first issue is that the '--' separator does not have a trailing space. This causes certain e-mail clients to not recognize it as a signature. The second issue is that the text in the signature is unnecessarily indented by two spaces.

Proposed fix

Matthew Paul Thomas noted in the bug report that the indentation issue needs to be fixed in def notify_specification_subscription_created() and emailtemplates/specification-modified.txt. He also noted that the signature separator trailing space issue needs to be fixed in notify_specification_subscription_created()

Pre-implementation notes

It was agreed that the proposed fix was pretty straight-forward.

Implementation details

lib/canonical/launchpad/emailtemplates/specification-modified.txt:
  * Remove two space indentation of signature
lib/canonical/launchpad/mailnotification.py:
  * Add trailing space to signature separator
  * Remove two space indentation of signature

Tests

$ ./bin/test -vv -t specification-notifications.txt
Running tests at level 1
Running canonical.testing.layers.DatabaseFunctionalLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.003 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 0.452 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 4.370 seconds.
  Set up canonical.testing.layers.DatabaseFunctionalLayer in 0.000 seconds.
  Running:
 lib/lp/blueprints/tests/../doc/specification-notifications.txt
  Ran 12 tests with 0 failures and 0 errors in 1.406 seconds.
Tearing down left over layers:
  Tear down canonical.testing.layers.DatabaseFunctionalLayer in 0.000 seconds.
  Tear down canonical.testing.layers.DatabaseLayer in 0.018 seconds.
  Tear down canonical.testing.layers.FunctionalLayer ... not supported
  Tear down canonical.testing.layers.BaseLayer in 0.000 seconds.

Demo and Q/A

  * Log on as Sample Person (<email address hidden>:test)
  * Visit https://blueprints.launchpad.dev/ubuntu/+spec/media-integrity-check/+subscribe
  * Click the 'Suscribe' button
  * Verify that the spacing is correct in the email sent from Launchpad
  * Modify the blueprint
  * Verify that the spacing is correct in the email sent from Launchpad

lint

$ make lint
utilities/shhh.py PYTHONPATH= python2.4 bootstrap.py\
                --ez_setup-source=ez_setup.py \
  --download-base=download-cache/dist --eggs=eggs
= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/mailnotification.py
  lib/canonical/launchpad/emailtemplates/specification-modified.txt

diff

{{{
=== modified file 'lib/canonical/launchpad/emailtemplates/specification-modified.txt'
--- lib/canonical/launchpad/emailtemplates/specification-modified.txt 2007-07-09 08:44:46 +0000
+++ lib/canonical/launchpad/emailtemplates/specification-modified.txt 2009-11-16 02:18:50 +0000
@@ -3,5 +3,5 @@
 %(info_fields)s

 --
- %(spec_title)s
- %(spec_url)s
+%(spec_title)s
+%(spec_url)s

=== modified file 'lib/canonical/launchpad/mailnotification.py'
--- lib/canonical/launchpad/mailnotification.py 2009-11-05 19:59:52 +0000
+++ lib/canonical/launchpad/mailnotification.py 2009-11-16 02:18:50 +0000
@@ -1077,7 +1077,7 @@
     body = mailwrapper.format(
         'You are now subscribed to the blueprint '
         '%(blueprint_name)s - %(blueprint_title)s.\n\n'
- '--\n %(blueprint_url)s' %
+ '-- \n%(blueprint_url)s' %
         {'blueprint_name': spec.name,
          'blueprint_title': spec.title,
          'blueprint_url': canonical_url(spec)})
}}}