Merge lp:~danilo/launchpad/bug-410579 into lp:launchpad

[Данило Шеган]

[Данило Шеган (danilo)]

= Summary =

This branch fixes #92751 (#410579 is a duplicate which I originally
started on), doing a bunch of drive-by cleanups along the way.

Our current download email for translations is very terse and not very
informative. It goes something like the following:

Subject: Translation download request: danilo
> Hello Данило Шеган,
>
> The translation files you requested from Launchpad are ready for
> download from the following location:
>
> http://launchpadlibrarian.net/30048422/po_kio4-es.po

== Proposed fix ==

We should include more information, like what the request is for, and
how download link will expire in 1 week. We should also include
information on how you can re-request the download if you need to.

Subject should not mention your username, but instead a short
description of what the download is about.

== Implementation details ==

The branch has quickly gotten very big (~900 lines), but it's mostly
because of textual changes to email templates. I hope it's not a problem.

 * scripts/po_export_queue.py:
   This is where the core change is: everything related to emailing is moved into
   ExportResult implementation, which generates appropriate emails.

   Two methods are of notice:

     * _getExportRequestOrigin(): figures out where export request was most
       likely made;
       TranslationImportQueue keeps only a list of POTemplate and POFile objects
       and depending this method figures out the place where the request was
       "most likely" made (i.e. context where you could have requested these
       files, but then trying for the most relevant context as well).

       For instance, if we've got two pofiles on a single template, request was
       likely made on a potemplate object, so we return that (for nicer URLs
       and emails subjects though, if there's only one template in
       a productseries/sourcepackage, we return them instead).

     * _getShortRequestName(request) takes the context returned by previous method
       and returns a nice short description in the form of 'Evolution trunk' or
       'Ubuntu Karmic gimp-2.0 - Spanish translation' suitable for use as email
       subjects

   These are not unit tested, though they could be, simply because they are
   sufficiently well exercised by existing doctests, and because if these start
   failing, it'll be easy to notice (so, I'd rather not increase CPU cycles
   we spend on tests). They are mostly for 'nicer UI output', so their correctness
   is not critical to the operation.

   Other changes in this file are mostly for using emailtemplates instead of hardcoding text.

 * emailtemplates/poexport-*.txt:
   All the email templates have been moved out of the code and into separate
   template files. Some 75 lines of diff are basically

 * doc/poexport-request.txt,
   doc/poexport-request-productseries.txt,
   doc/poexport-queue.txt:
   There are a lot of changes to tests that test how emails are generated:
   basically, half of the branch is there (440 lines). The changes there
   include introducing a shared helper print_mail_subject_and_body method,
   print subject along with body of the email, and changes to test new
   e...

[Barry Warsaw (barry)]

On Aug 14, 2009, at 06:35 PM, Данило Шеган wrote:

> Данило Шеган has proposed merging lp:~danilo/launchpad/
> bug-410579 into lp:launchpad/devel.
>
> We should include more information, like what the request is for, and
> how download link will expire in 1 week. We should also include
> information on how you can re-request the download if you need to.

Thanks for working on this Danilo. It looks like a very useful change
that
will help our users!

> Subject should not mention your username, but instead a short
> description of what the download is about.
>
> == Implementation details ==
>
> The branch has quickly gotten very big (~900 lines), but it's mostly
> because of textual changes to email templates. I hope it's not a
> problem.
>
> * scripts/po_export_queue.py:
> This is where the core change is: everything related to emailing is
> moved into
> ExportResult implementation, which generates appropriate emails.
>
> Two methods are of notice:
>
> * _getExportRequestOrigin(): figures out where export request was
> most
> likely made;
> TranslationImportQueue keeps only a list of POTemplate and
> POFile objects
> and depending this method figures out the place where the
> request was
> "most likely" made (i.e. context where you could have requested
> these
> files, but then trying for the most relevant context as well).
>
> For instance, if we've got two pofiles on a single template,
> request was
> likely made on a potemplate object, so we return that (for
> nicer URLs
> and emails subjects though, if there's only one template in
> a productseries/sourcepackage, we return them instead).
>
> * _getShortRequestName(request) takes the context returned by
> previous method
> and returns a nice short description in the form of 'Evolution
> trunk' or
> 'Ubuntu Karmic gimp-2.0 - Spanish translation' suitable for use
> as email
> subjects
>
> These are not unit tested, though they could be, simply because
> they are
> sufficiently well exercised by existing doctests, and because if
> these start
> failing, it'll be easy to notice (so, I'd rather not increase CPU
> cycles
> we spend on tests). They are mostly for 'nicer UI output', so
> their correctness
> is not critical to the operation.

Do you really think it would take up that much more cycles to unit
test these
two methods? I'm generally not in favor of these kinds of indirect
test when
they can be reasonably avoided because that makes it much more
difficult to
change the methods later. You never really quite know what the effect
will
be, so you end up having to run the full test suite "just in case".
With a
unit test, you at least know you need to run that test if you change the
method, and can presume that if those unit tests pass, everything else
implicitly testing them will too.

>
> Other changes in this file are mostly for using emailtemplates
> instead of hardcoding text.
>
> * emailtemplates/poexport-*.txt:
> All the email templates have been moved out of the code and into
> separate
> template files. Some 75 lines of diff are basically
>
> * doc/poexpor...

[Barry Warsaw (barry)]

merge-conditional

review: Approve

[Данило Шеган (danilo)]

Barry, thanks for the review.

I've applied most of your recommendations. The ones I didn't were about
'positive tests' (i.e. is 'is not None' really a negative test?) and I
haven't provided unit tests. Still, I've modified doctests to use
mail_helpers, so that resulted in a nice big incremental diff.

У пет, 14. 08 2009. у 16:05 -0400, Barry Warsaw пише:
> > * doc/poexport-request.txt,
> > doc/poexport-request-productseries.txt,
> > doc/poexport-queue.txt:
> > There are a lot of changes to tests that test how emails are generated:
> > basically, half of the branch is there (440 lines). The changes there
> > include introducing a shared helper print_mail_subject_and_body method,
> > print subject along with body of the email, and changes to test new
> > email contents and subjects. Some minor changes to how ExportResult is
> > constructed were necessary as well to conform with the new API (i.e. pass
> > person, requested exports and logger).
>
> Do you know about lp.testing.mail_helpers.print_emails()? I generally use
> that plus ellipses to cut down on the output. Is there a reason why you'd
> want print_mail_subject_and_body instead?

Yes, I've looked through existing helpers. I've already struggled to
keep the diff size reasonable, and this would have made it only worse.
Though, I am happy to migrate existing tests to the use them, and I did
so. Note that only this change means 640 lines of diff in the
incremental diff ;)

> > To not increase size of the diff, some bits of the test are left
> > indented with 2 spaces. I'd be happy to fix before submitting.
>
> Sure, thanks! rs=me on those additional cleanups.

Thanks.

> For the most part, the branch looks fine. I do have a few questions and
> suggestions, but I'll mark this as merge-conditional on the hope that those
> are easy to address or answer. I'm happy to answer any questions or chat
> about my suggestions with you on Monday.

Great, thanks.

> === added file 'lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt'
> --- lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 1970-01-01 00:00:00 +0000
> +++ lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 2009-08-14 16:33:57 +0000
> > @@ -0,0 +1,16 @@
> > +Hello admins,
>
> Maybe "Hello Launchpad administrators," ?

Sure, fixed. Note that these emails only ended up on our internal
error-reports list, but no reason not to make them even nicer :)

> > +--
>
> Note that the signature separator is actually "-- " yep with a trailing space!
> I know that kind of screws the no-trailing-whitespace thing, but there you
> have it. ;)

Ah, interesting. Fixed all over the place.

> > +Automatic message from Launchpad.net — https://launchpad.net/
>
> Wow, this is like a failure failure. :)

My mail client linkifies the second, but not the first. I still prefer
the former in an email message, so I made it

  "Automatic message from Launchpad.net."

in all the email templates.

> === modified file 'lib/lp/translations/scripts/po_export_queue.py'
> --- lib/lp/translations/scripts/po_export_queue.py 2009-07-17 00:26:05 +0000
> +++ lib/lp/...

[Barry Warsaw (barry)]

On Aug 17, 2009, at 01:48 PM, Данило Шеган wrote:

>I've applied most of your recommendations. The ones I didn't were about
>'positive tests' (i.e. is 'is not None' really a negative test?) and I
>haven't provided unit tests. Still, I've modified doctests to use
>mail_helpers, so that resulted in a nice big incremental diff.

Indeed, that looks fantastic. Thanks!

>У пет, 14. 08 2009. у 16:05 -0400, Barry Warsaw пише:
>> > * doc/poexport-request.txt,
>> > doc/poexport-request-productseries.txt,
>> > doc/poexport-queue.txt:
>> > There are a lot of changes to tests that test how emails are generated:
>> > basically, half of the branch is there (440 lines). The changes there
>> > include introducing a shared helper print_mail_subject_and_body method,
>> > print subject along with body of the email, and changes to test new
>> > email contents and subjects. Some minor changes to how ExportResult is
>> > constructed were necessary as well to conform with the new API (i.e. pass
>> > person, requested exports and logger).
>>
>> Do you know about lp.testing.mail_helpers.print_emails()? I generally use
>> that plus ellipses to cut down on the output. Is there a reason why you'd
>> want print_mail_subject_and_body instead?
>
>Yes, I've looked through existing helpers. I've already struggled to
>keep the diff size reasonable, and this would have made it only worse.
>Though, I am happy to migrate existing tests to the use them, and I did
>so. Note that only this change means 640 lines of diff in the
>incremental diff ;)

That's the way to do it! :) Really, it looks great now.

>> === added file 'lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt'
>> --- lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 1970-01-01 00:00:00 +0000
>> +++ lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 2009-08-14 16:33:57 +0000
>> > @@ -0,0 +1,16 @@
>> > +Hello admins,
>>
>> Maybe "Hello Launchpad administrators," ?
>
>Sure, fixed. Note that these emails only ended up on our internal
>error-reports list, but no reason not to make them even nicer :)

If we can't be nice to ourselves, we can't be nice to our users :)

>> > +Automatic message from Launchpad.net — https://launchpad.net/
>>
>> Wow, this is like a failure failure. :)
>
>My mail client linkifies the second, but not the first. I still prefer
>the former in an email message, so I made it
>
> "Automatic message from Launchpad.net."
>
>in all the email templates.

Y'know, I didn't really notice that that was an em-dash. I thought it was a
plain old '-' but I guess that was just my editor's font playing tricks on
me. In any case, I think keeping it pure ASCII is a good thing (as you saw by
removing all those utf-8 conversions).

But I wasn't actually complaining about it, I was just observing that the
message appeared to be sent when a failure message failed. Still, I'm glad my
Friday-addled brain was able to reach through the fog and offer a not-horrible
suggestion. ;)

>> Is it really more convenient to access the logger through the instance rather
>> than use the module global? I can almost promise you...

=== modified file 'lib/lp/translations/doc/poexport-queue.txt'
--- lib/lp/translations/doc/poexport-queue.txt	2009-07-02 17:16:50 +0000
+++ lib/lp/translations/doc/poexport-queue.txt	2009-08-14 17:46:32 +0000
@@ -9,17 +9,14 @@
 ExportResult class is used to control the list of exported files that succeed
 and the ones that failed with the error associated.
 
-  >>> import transaction
-  >>> from zope.component import getUtility
-  >>> from canonical.launchpad.interfaces import IPersonSet
-  >>> from lp.services.mail.stub import test_emails
-  >>> from lp.translations.scripts.po_export_queue import ExportResult
-
-  # Function to print mail content without headers.
-  >>> def print_mail_body(body):
-  ...     lines = body[body.index('\n\n')+2:].split('\n')
-  ...     for line in lines:
-  ...         print ">", line
+    >>> import transaction
+    >>> from zope.component import getUtility
+    >>> from canonical.launchpad.interfaces import IPersonSet
+    >>> from lp.services.mail.stub import test_emails
+    >>> from lp.translations.scripts.po_export_queue import ExportResult
+    >>> from lp.translations.tests.helpers import print_mail_subject_and_body
+    >>> import logging
+    >>> logger = logging.getLogger()
 
 When there is an error, the system will notify it.
 
@@ -29,10 +26,10 @@
     >>> from lp.translations.model.potemplate import POTemplate
     >>> potemplate = POTemplate.get(1)
     >>> pofile = potemplate.getPOFileByLang('es')
+    >>> personset = getUtility(IPersonSet)
+    >>> carlos = personset.getByName('carlos')
 
-    >>> result = ExportResult('testing export')
-    >>> result.object_names.append(potemplate.displayname)
-    >>> result.object_names.append(pofile.title)
+    >>> result = ExportResult(carlos, [potemplate, pofile], logger)
 
 Record the error.
 
@@ -50,9 +47,7 @@
 given that we set a URL, the notification will detect the programming error.
 In this example, 'carlos' will be the one that did the request.
 
-  >>> personset = getUtility(IPersonSet)
-  >>> carlos = personset.getByName('carlos')
-  >>> result.notify(carlos)
+  >>> result.notify()
   Traceback (most recent call last):
   ...
   AssertionError: We cannot have a URL for the export and a failure.
@@ -60,15 +55,15 @@
 In this case, it should be None, so the notify works.
 
   >>> result.url = None
-  >>> result.notify(carlos)
+  >>> result.notify()
 
   # We need to commit the transaction to get the emails in the queue.
   >>> transaction.commit()
 
 As usual, when there is an error, two emails should be sent:
 
-  >>> len(test_emails) == 2
-  True
+  >>> len(test_emails)
+  2
 
   >>> while len(test_emails) > 0:
   ...     from_addrs, to_addrs, body = test_emails.pop()
@@ -83,38 +78,55 @@
 
   >>> print carlos_addrs
   ['[e-mail]']
-  >>> print_mail_body(carlos_body)
-  > 
+  >>> print_mail_subject_and_body(carlos_body)
+  Subject: Launchpad translation download: Evolution trunk -
+  evolution-2.2 template
   > Hello Carlos Perell=C3=B3 Mar=C3=ADn,
-  > 
+  >
   > Launchpad encountered problems exporting the files you requested.
   > The Launchpad Translations team has been notified of this problem.
   > Please reply to this email for further assistance.
   >
+  > If you want to retry your request, you can do so at
+  >
+  >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+  > port.
+  >
+  > --
+  > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+  >
 
 And the other to the admins.  This one lists the files that were being
 exported as context to help tracking down any bugs.
 
   >>> print admins_addrs
   ['[e-mail]']
-  >>> print_mail_body(admins_body)
-  > 
+  >>> print_mail_subject_and_body(admins_body)
+  Subject: Launchpad translation download errors: Evolution trunk -
+  evolution-2.2 template
   > Hello admins,
-  > 
+  >
   > Launchpad encountered problems exporting translation files
-  > requested by Carlos Perell=C3=B3 Mar=C3=ADn.
+  > requested by Carlos Perell=C3=B3 Mar=C3=ADn (carlos) at
+  >
+  >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+  > port
   >
   > This means we have a bug in Launchpad that needs to be fixed
-  > before this export can proceed.  Here is the list of failed
-  > files and the error we got:
-  > 
+  > before this export can proceed.  Here is the error we got:
+  >
   > Traceback (most recent call last):
   ...
   > AssertionError: It's just an error for testing purposes
   >
-  > The failed request involved these objects:
-  > evolution-2.2 in Evolution trunk
-  > Spanish (es) translation of evolution-2.2 in Evolution trunk
+  >
+  > Failed export request included:
+  >   * evolution-2.2 in Evolution trunk
+  >   * Spanish (es) translation of evolution-2.2 in Evolution trunk
+  >
+  > --
+  > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+  >
 
 As a special case, some error messages are poisoned with non-ASCII characters
 and can't be reported without triggering an error themselves.  Those are
@@ -124,10 +136,10 @@
   ...     raise AssertionError, "Really nasty \xc3 non-ASCII error!"
   ... except AssertionError:
   ...     result.addFailure()
-  >>> result.notify(carlos)
+  >>> result.notify()
   >>> transaction.commit()
-  >>> len(test_emails) == 2
-  True
+  >>> len(test_emails)
+  2
 
   >>> carlos_body = None
   >>> admins_body = None
@@ -140,39 +152,57 @@
 
 The user's notification looks no different from that for an ordinary error.
 
-  >>> print_mail_body(carlos_body)
-  > 
+  >>> print_mail_subject_and_body(carlos_body)
+  Subject: Launchpad translation download: Evolution trunk -
+  evolution-2.2 template
   > Hello Carlos Perell=C3=B3 Mar=C3=ADn,
-  > 
+  >
   > Launchpad encountered problems exporting the files you requested.
   > The Launchpad Translations team has been notified of this problem.
   > Please reply to this email for further assistance.
   >
+  > If you want to retry your request, you can do so at
+  >
+  >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+  > port.
+  >
+  > --
+  > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+  >
 
 The one for the administrators, however, does not include the unprintable
 exception text.
 
-  >>> print_mail_body(admins_body)
-  > 
+  >>> print_mail_subject_and_body(admins_body)
+  Subject: Launchpad translation download errors: Evolution trunk -
+  evolution-2.2 template
   > Hello admins,
-  > 
+  >
   > A UnicodeDecodeError occurred while trying to notify you of a
   > failure during a translation export requested by Carlos ...
-  >
-  > The failed request involved these objects:
-  > evolution-2.2 in Evolution trunk
-  > Spanish (es) translation of evolution-2.2 in Evolution trunk
+  > (carlos) at
+  >
+  >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+  > port
+  >
+  > Failed export request included:
+  >   * evolution-2.2 in Evolution trunk
+  >   * Spanish (es) translation of evolution-2.2 in Evolution trunk
+  >
+  > --
+  > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+  >
 
 
 Finally, there is the case when there are no errors at all. This is the usual
 case.
 
-  >>> result = ExportResult('testing export')
+  >>> result = ExportResult(carlos, [potemplate, pofile], logger)
 
 As noted before, result.url should be set to the URL where the user can
 download the requested files. If we don't set it, the export will fail:
 
-  >>> result.notify(carlos)
+  >>> result.notify()
   Traceback (most recent call last):
   ...
   AssertionError: On success, an exported URL is expected.
@@ -183,28 +213,41 @@
 
 And notify the user.
 
-  >>> result.notify(carlos)
+  >>> result.notify()
 
   # We need to commit the transaction to get the email in the queue.
   >>> transaction.commit()
 
 In this case, there are no errors, so we should get just a single email
 
-  >>> len(test_emails) == 1
-  True
+  >>> len(test_emails)
+  1
 
   >>> from_addrs, to_addrs, body = test_emails.pop()
 
   >>> print to_addrs
   ['[e-mail]']
-  >>> print_mail_body(body)
-  > 
+  >>> print_mail_subject_and_body(body)
+  Subject: Launchpad translation download: Evolution trunk -
+  evolution-2.2 template
   > Hello Carlos Perell=C3=B3 Mar=C3=ADn,
-  > 
+  >
   > The translation files you requested from Launchpad are ready for
   > download from the following location:
-  > 
-  >     http://someplace.com/somefile.tar.gz
+  >
+  >   http://someplace.com/somefile.tar.gz
+  >
+  > Note: this link will expire in about 1 week.  If you want to
+  > download these translations again, you will have to request
+  > them again at
+  >
+  >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+  > port
+  >
+  > --
+  > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+  >
+
 
 
 == process_queue() ==
@@ -236,7 +279,6 @@
 
 Once the queue is processed, the queue is empty again.
 
-    >>> import logging
     >>> process_queue(transaction, logging.getLogger())
     INFO:...Stored file at http://.../po_evolution-2.2.pot
 
@@ -253,14 +295,26 @@
     >>> from_addrs, to_addrs, body = test_emails.pop()
     >>> print to_addrs
     ['[e-mail]']
-    >>> print_mail_body(body)
-    > 
+    >>> print_mail_subject_and_body(body)
+    Subject: Launchpad translation download: Evolution trunk -
+    evolution-2.2 template
     > Hello Carlos Perell=C3=B3 Mar=C3=ADn,
-    > 
+    >
     > The translation files you requested from Launchpad are ready for
     > download from the following location:
-    > 
+    >
     >     http://localhost:58000/.../po_evolution-2.2.pot
+    >
+    > Note: this link will expire in about 1 week.  If you want to
+    > download these translations again, you will have to request
+    > them again at
+    >
+    >   http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex=
+    > port
+    >
+    > --
+    > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/
+    >
 
 Let's have a closer look at what is being exported. Usually all messages are
 exported but not all messages are equal. Some messages have been imported
 
=== modified file 'lib/lp/translations/doc/poexport-request-productseries.txt'
--- lib/lp/translations/doc/poexport-request-productseries.txt	2009-07-23 17:49:31 +0000
+++ lib/lp/translations/doc/poexport-request-productseries.txt	2009-08-14 17:46:32 +0000
@@ -56,23 +56,28 @@
 
 The user receives a confirmation email.
 
-    >>> def print_mail_body(body):
-    ...     lines = body[body.index('\n\n')+2:].split('\n')
-    ...     for line in lines:
-    ...         print ">", line
-
+    >>> from lp.translations.tests.helpers import print_mail_subject_and_body
     >>> from lp.services.mail.stub import test_emails
     >>> len(test_emails)
     1
     >>> from_addr, to_addrs, body = test_emails.pop()
-    >>> print_mail_body(body)
-    >
+    >>> print_mail_subject_and_body(body)
+    Subject: Launchpad translation download: Evolution trunk
     > Hello ...,
     >
     > The translation files you requested from Launchpad are ready for
     > download from the following location:
     >
-    >     http://localhost:58000/.../launchpad-export.tar.gz
+    >   http://localhost:58000/.../launchpad-export.tar.gz
+    >
+    > Note: this link will expire in about 1 week.  If you want to
+    > download these translations again, you will have to request
+    > them again at
+    >
+    >   http://translations.launchpad.dev/evolution/trunk/+export
+    >
+    > --
+    > Automatic message from Launchpad.net...
 
 The email contains a URL linking to where the exported file can be downloaded.
 
 
=== modified file 'lib/lp/translations/doc/poexport-request.txt'
--- lib/lp/translations/doc/poexport-request.txt	2009-07-23 17:49:31 +0000
+++ lib/lp/translations/doc/poexport-request.txt	2009-08-14 17:46:32 +0000
@@ -68,25 +68,31 @@
 
 The user receives a confirmation email.
 
-    >>> def print_mail_body(body):
-    ...     lines = body[body.index('\n\n')+2:].split('\n')
-    ...     for line in lines:
-    ...         print ">", line
-
+    >>> from lp.translations.tests.helpers import print_mail_subject_and_body
     >>> from lp.services.mail.stub import test_emails
     >>> len(test_emails)
     1
     >>> from_addr, to_addrs, body = test_emails.pop()
     >>> to_addrs
     ['[e-mail]']
-    >>> print_mail_body(body)
-    >
+    >>> print_mail_subject_and_body(body)
+    Subject: Launchpad translation download: Ubuntu Hoary pmount -
+    pmount template
     > Hello Mark Shuttleworth,
     >
     > The translation files you requested from Launchpad are ready for
     > download from the following location:
     >
     >     http://localhost:58000/.../launchpad-export.tar.gz
+    >
+    > Note: this link will expire in about 1 week.  If you want to
+    > download these translations again, you will have to request
+    > them again at
+    >
+    >   http://translations.launchpad.../hoary/+source/pmount/+pots/p...
+    >
+    > --
+    > Automatic message from Launchpad.net...
 
 The email contains a URL linking to where the exported file can be downloaded.
 
@@ -197,14 +203,26 @@
     >>> from_addr, to_addrs, body = test_emails.pop()
     >>> to_addrs
     ['[e-mail]']
-    >>> print_mail_body(body)
-    > 
+    >>> print_mail_subject_and_body(body)
+    Subject: Launchpad translation download: Ubuntu Hoary pmount - Czech
+    translation of pmount
     > Hello Mark Shuttleworth,
-    > 
+    >
     > The translation files you requested from Launchpad are ready for
     > download from the following location:
-    > 
-    >     http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo
+    >
+    >   http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo
+    >
+    > Note: this link will expire in about 1 week.  If you want to
+    > download these translations again, you will have to request
+    > them again at
+    >
+    >   http://translations.launchpad.../pmount/+pots/pmoun=
+    > t/cs/+export
+    >
+    > --
+    > Automatic message from Launchpad.net...
+
     >>> url = extract_url(body)
 
 Check whether we generated a good .mo file.
 
=== added file 'lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt'
--- lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt	2009-08-14 16:33:57 +0000
@@ -0,0 +1,16 @@
+Hello admins,
+
+Launchpad encountered problems exporting translation files
+requested by %(person)s (%(person_id)s) at
+
+  %(request_url)s
+
+This means we have a bug in Launchpad that needs to be fixed
+before this export can proceed.  Here is the error we got:
+
+%(failure_message)s
+
+%(failed_requests)s
+
+--
+Automatic message from Launchpad.net — https://launchpad.net/
 
=== added file 'lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt'
--- lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt	2009-08-14 16:33:57 +0000
@@ -0,0 +1,12 @@
+Hello admins,
+
+A UnicodeDecodeError occurred while trying to notify you of a
+failure during a translation export requested by %(person)s
+(%(person_id)s) at
+
+  %(request_url)s
+
+%(failed_requests)s
+
+--
+Automatic message from Launchpad.net — https://launchpad.net/
 
=== added file 'lib/lp/translations/emailtemplates/poexport-failure.txt'
--- lib/lp/translations/emailtemplates/poexport-failure.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/emailtemplates/poexport-failure.txt	2009-08-14 16:33:57 +0000
@@ -0,0 +1,12 @@
+Hello %(person)s,
+
+Launchpad encountered problems exporting the files you requested.
+The Launchpad Translations team has been notified of this problem.
+Please reply to this email for further assistance.
+
+If you want to retry your request, you can do so at
+
+  %(request_url)s.
+
+--
+Automatic message from Launchpad.net — https://launchpad.net/
 
=== added file 'lib/lp/translations/emailtemplates/poexport-success.txt'
--- lib/lp/translations/emailtemplates/poexport-success.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/emailtemplates/poexport-success.txt	2009-08-14 16:33:57 +0000
@@ -0,0 +1,15 @@
+Hello %(person)s,
+
+The translation files you requested from Launchpad are ready for
+download from the following location:
+
+  %(download_url)s
+
+Note: this link will expire in about 1 week.  If you want to
+download these translations again, you will have to request
+them again at
+
+  %(request_url)s
+
+--
+Automatic message from Launchpad.net — https://launchpad.net/
 
=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py	2009-07-17 00:26:05 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py	2009-08-14 16:35:06 +0000
@@ -457,7 +457,7 @@
     file_type = Choice(
         title=_("File Type"),
         description=_(
-            "The type of the file being imported imported."),
+            "The type of the file being imported."),
         required=True,
         vocabulary = TranslationFileType)
 
 
=== modified file 'lib/lp/translations/scripts/po_export_queue.py'
--- lib/lp/translations/scripts/po_export_queue.py	2009-07-17 00:26:05 +0000
+++ lib/lp/translations/scripts/po_export_queue.py	2009-08-14 18:02:09 +0000
@@ -10,7 +10,6 @@
 
 import os
 import psycopg2
-import textwrap
 import traceback
 from StringIO import StringIO
 from zope.component import getAdapter, getUtility
@@ -18,9 +17,13 @@
 from canonical.config import config
 from canonical.launchpad import helpers
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from canonical.launchpad.webapp import canonical_url
+from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.translations.interfaces.poexportrequest import (
     IPOExportRequestSet)
 from lp.translations.interfaces.potemplate import IPOTemplate
+from lp.translations.interfaces.pofile import IPOFile
 from lp.translations.interfaces.translationcommonformat import (
     ITranslationFileData)
 from lp.translations.interfaces.translationexporter import (
@@ -35,39 +38,210 @@
 
     This class has three main attributes:
 
-     - name: A short identifying string for this export.
+     - person: A person requesting this export.
      - url: The Librarian URL for any successfully exported files.
-     - failure: Failure got while exporting.
+     - failure: Failure gotten while exporting.
     """
 
-    def __init__(self, name):
-        self.name = name
+    def __init__(self, person, requested_exports, logger):
+        self.person = person
         self.url = None
         self.failure = None
-        self.object_names = []
-
-    def _getFailureEmailBody(self, person):
+        self.logger = logger
+
+        self.requested_exports = list(requested_exports)
+        export_requested_at = self._getExportRequestOrigin()
+        self.name = self._getShortRequestName(export_requested_at)
+
+        self.request_url = canonical_url(
+            export_requested_at,
+            rootsite='translations') + '/+export'
+
+    def _getShortRequestName(self, request):
+        """Return a short request name for use in email subjects."""
+        if IPOFile.providedBy(request):
+            title = '%s translation of %s' % (
+                request.language.englishname,
+                request.potemplate.name)
+            productseries = request.potemplate.productseries
+            distroseries = request.potemplate.distroseries
+            sourcepackagename = request.potemplate.sourcepackagename
+        elif IPOTemplate.providedBy(request):
+            title = '%s template' % (request.name)
+            productseries = request.productseries
+            distroseries = request.distroseries
+            sourcepackagename = request.sourcepackagename
+        elif IProductSeries.providedBy(request):
+            title = None
+            productseries = request
+            distroseries = None
+            sourcepackagename = None
+        elif ISourcePackage.providedBy(request):
+            title = None
+            productseries = None
+            distroseries = request.distroseries
+            sourcepackagename = request.sourcepackagename
+        else:
+            raise AssertionError(
+                "We can not figure out short name for this translation "
+                "export origin.")
+
+        if productseries is not None:
+            root = '%s %s' % (
+                productseries.product.displayname,
+                productseries.name)
+        else:
+            root = '%s %s %s' % (
+                distroseries.distribution.displayname,
+                distroseries.displayname,
+                sourcepackagename.name)
+        if title is not None:
+            return '%s - %s' % (root, title)
+        else:
+            return root
+
+    def _getExportRequestOrigin(self):
+        """Figure out where an export request was made."""
+        # Determine all objects that export request could have
+        # originated on.
+        export_requested_at = None
+        pofiles = set()
+        implicit_potemplates = set()
+        direct_potemplates = set()
+        productseries = set()
+        sourcepackages = set()
+
+        last_template_name = None
+        for request in self.requested_exports:
+            if IPOTemplate.providedBy(request):
+                # If we are exporting a template, add it to
+                # the list of directly requested potemplates.
+                potemplate = request
+                direct_potemplates.add(potemplate)
+            else:
+                # Otherwise, we are exporting a POFile.
+                potemplate = request.potemplate
+                implicit_potemplates.add(potemplate)
+                pofiles.add(request)
+            if potemplate.displayname != last_template_name:
+                self.logger.debug(
+                    'Exporting objects for %s, related to template %s'
+                    % (self.person.displayname, potemplate.displayname))
+                last_template_name = potemplate.displayname
+
+            # Determine productseries or sourcepackage for any
+            # productseries/sourcepackage an export was requested at.
+            if potemplate.productseries is not None:
+                productseries.add(potemplate.productseries)
+            elif potemplate.sourcepackagename is not None:
+                sourcepackage = potemplate.distroseries.getSourcePackage(
+                    potemplate.sourcepackagename)
+                sourcepackages.add(sourcepackage)
+            else:
+                pass
+
+        if len(pofiles) == 1 and len(direct_potemplates) == 0:
+            # One POFile was requested.
+            export_requested_at = pofiles.pop()
+        elif len(pofiles) == 0 and len(direct_potemplates) == 1:
+            # A POTemplate was requested.
+            export_requested_at = direct_potemplates.pop()
+        elif len(pofiles) + len(direct_potemplates) >= 2:
+            # More than one file was requested.
+            all_potemplates = implicit_potemplates.union(direct_potemplates)
+            if len(all_potemplates) == 1:
+                # It's all part of a single POTemplate.
+                export_requested_at = all_potemplates.pop()
+            else:
+                # More than one POTemplate: request was made on
+                # either ProductSeries or SourcePackage.
+                if len(sourcepackages) > 0:
+                    export_requested_at = sourcepackages.pop()
+                elif len(productseries) > 0:
+                    export_requested_at = productseries.pop()
+
+        if IPOTemplate.providedBy(export_requested_at):
+            if len(sourcepackages) > 0:
+                sp = sourcepackages.pop()
+                if sp.getCurrentTranslationTemplates().count() == 1:
+                    export_requested_at = sp
+            elif len(productseries) > 0:
+                ps = productseries.pop()
+                if ps.getCurrentTranslationTemplates().count() == 1:
+                    export_requested_at = ps
+
+        return export_requested_at
+
+
+    def _getRequestedExportsNames(self):
+        requested_names = []
+        for translation_object in self.requested_exports:
+            if IPOTemplate.providedBy(translation_object):
+                request_name = translation_object.displayname
+            else:
+                request_name = translation_object.title
+            requested_names.append(request_name)
+
+        return requested_names
+
+    def _getFailureEmailBody(self):
         """Send an email notification about the export failing."""
-        return textwrap.dedent('''
-            Hello %s,
-
-            Launchpad encountered problems exporting the files you requested.
-            The Launchpad Translations team has been notified of this problem.
-            Please reply to this email for further assistance.
-            ''' % person.displayname)
-
-    def _getSuccessEmailBody(self, person):
+        template = helpers.get_email_template(
+            'poexport-failure.txt', 'translations').decode('utf-8')
+        return template % {
+            'person' : self.person.displayname,
+            'request_url' : self.request_url,
+            }
+
+    def _getFailedRequestsDescription(self):
+        """Return a printable description of failed export requests."""
+        failed_requests = self._getRequestedExportsNames()
+        if len(failed_requests) > 0:
+            failed_requests_text = 'Failed export request included:\n'
+            failed_requests_text += '\n'.join(
+                ['  * ' + request for request in failed_requests])
+        else:
+            failed_requests_text = 'There were no export requests.'
+        return failed_requests_text
+
+    def _getAdminFailureNotificationEmailBody(self):
+        """Send an email notification about failed export to admins."""
+        template = helpers.get_email_template(
+            'poexport-failure-admin-notification.txt',
+            'translations').decode('utf-8')
+        failed_requests = self._getFailedRequestsDescription()
+        return template % {
+            'person' : self.person.displayname,
+            'person_id' : self.person.name,
+            'request_url' : self.request_url,
+            'failure_message' : self.failure,
+            'failed_requests' : failed_requests,
+            }
+
+    def _getUnicodeDecodeErrorEmailBody(self):
+        """Send an email notification to admins about UnicodeDecodeError."""
+        template = helpers.get_email_template(
+            'poexport-failure-unicodedecodeerror.txt',
+            'translations').decode('utf-8')
+        failed_requests = self._getFailedRequestsDescription()
+        return template % {
+            'person' : self.person.displayname,
+            'person_id' : self.person.name,
+            'request_url' : self.request_url,
+            'failed_requests' : failed_requests,
+            }
+
+    def _getSuccessEmailBody(self):
         """Send an email notification about the export working."""
-        return textwrap.dedent('''
-            Hello %s,
-
-            The translation files you requested from Launchpad are ready for
-            download from the following location:
-
-            \t%s''' % (person.displayname, self.url)
-            )
-
-    def notify(self, person):
+        template = helpers.get_email_template(
+            'poexport-success.txt', 'translations').decode('utf-8')
+        return template % {
+            'person' : self.person.displayname,
+            'download_url' : self.url,
+            'request_url' : self.request_url,
+            }
+
+    def notify(self):
         """Send a notification email to the given person about the export.
 
         If there is a failure, a copy of the email is also sent to the
@@ -76,22 +250,22 @@
         if self.failure is None and self.url is not None:
             # There is no failure, so we have a full export without
             # problems.
-            body = self._getSuccessEmailBody(person)
+            body = self._getSuccessEmailBody()
         elif self.failure is not None and self.url is None:
-            body = self._getFailureEmailBody(person)
+            body = self._getFailureEmailBody()
         elif self.failure is not None and self.url is not None:
             raise AssertionError(
                 'We cannot have a URL for the export and a failure.')
         else:
             raise AssertionError('On success, an exported URL is expected.')
 
-        recipients = list(helpers.get_contact_email_addresses(person))
+        recipients = list(helpers.get_contact_email_addresses(self.person))
 
         for recipient in [str(recipient) for recipient in recipients]:
             simple_sendmail(
                 from_addr=config.rosetta.admin_email,
                 to_addrs=[recipient],
-                subject='Translation download request: %s' % self.name,
+                subject='Launchpad translation download: %s' % self.name,
                 body=body)
 
         if self.failure is None:
@@ -99,42 +273,20 @@
             return
 
         # The export process had errors that we should notify admins about.
-        if self.object_names:
-            names = '\n'.join(self.object_names)
-            template_sentence = "\n" + textwrap.dedent(
-                "The failed request involved these objects:\n%s" % names)
-        else:
-            template_sentence = ""
-
         try:
-            admins_email_body = textwrap.dedent('''
-                Hello admins,
-
-                Launchpad encountered problems exporting translation files
-                requested by %s.
-
-                This means we have a bug in Launchpad that needs to be fixed
-                before this export can proceed.  Here is the list of failed
-                files and the error we got:
-
-                %s%s''') % (
-                    person.displayname, self.failure, template_sentence)
+            admins_email_body = self._getAdminFailureNotificationEmailBody()
         except UnicodeDecodeError:
             # Unfortunately this happens sometimes: invalidly-encoded data
             # makes it into the exception description, possibly from error
             # messages printed by msgfmt.  Before we can fix that, we need to
             # know what exports suffer from this problem.
-            admins_email_body = textwrap.dedent('''
-                Hello admins,
-
-                A UnicodeDecodeError occurred while trying to notify you of a
-                failure during a translation export requested by %s.
-                %s''') % (person.displayname, template_sentence)
+            admins_email_body = self._getUnicodeDecodeErrorEmailBody()
 
         simple_sendmail(
             from_addr=config.rosetta.admin_email,
             to_addrs=[config.launchpad.errors_address],
-            subject='Translation download errors: %s' % self.name,
+            subject=(
+                'Launchpad translation download errors: %s' % self.name),
             body=admins_email_body)
 
     def addFailure(self):
@@ -174,26 +326,11 @@
     translation_format_exporter = (
         translation_exporter.getExporterProducingTargetFileFormat(format))
 
-    result = ExportResult(person.name)
-    translation_file_list = list(objects)
-    last_template_name = None
-    for obj in translation_file_list:
-        if IPOTemplate.providedBy(obj):
-            template_name = obj.displayname
-            object_name = template_name
-        else:
-            template_name = obj.potemplate.displayname
-            object_name = obj.title
-        result.object_names.append(object_name)
-        if template_name != last_template_name:
-            logger.debug(
-                'Exporting objects for %s, related to template %s'
-                % (person.displayname, template_name))
-            last_template_name = template_name
+    result = ExportResult(person, objects, logger)
 
     try:
         exported_file = translation_format_exporter.exportTranslationFiles(
-            generate_translationfiledata(translation_file_list, format))
+            generate_translationfiledata(list(objects), format))
     except (KeyboardInterrupt, SystemExit):
         # We should never catch KeyboardInterrupt or SystemExit.
         raise
@@ -227,7 +364,7 @@
         result.url = alias.http_url
         logger.info("Stored file at %s" % result.url)
 
-    result.notify(person)
+    result.notify()
 
 
 def process_queue(transaction_manager, logger):
 
=== added file 'lib/lp/translations/tests/helpers.py'
--- lib/lp/translations/tests/helpers.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/tests/helpers.py	2009-08-14 17:46:32 +0000
@@ -0,0 +1,19 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helper module reused in different tests."""
+
+__metaclass__ = type
+
+__all__ = [
+    'print_mail_subject_and_body'
+    ]
+
+import email
+
+def print_mail_subject_and_body(contents):
+    msg = email.message_from_string(contents)
+    body = msg.get_payload()
+    print 'Subject: %s' % (msg['subject'])
+    for line in body.split('\n'):
+        print ">", line