Merge lp:~jtv/launchpad/bug-448019 into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 448019 =

See the bug report for what the problem is, what it costs us, and how I
fixed it. I hope it will also take a lot of wind out of the sails of
bug 447754. This is pretty much a 5-minute fix that'll save us some
real operational time, even if it's not perfect.

In a nutshell, when a translations import approval hits a conflict, this
branch logs not just the translation domain that multiple templates lay
a claim to, as we do now; but also the display names of those templates.
For Ubuntu packages that will include the Ubuntu release series, source
package name, and template name (which is a slightly different thing
from its domain).

== Tests ==

{{{
./bin/test -vv -t potemplate.txt
}}}

Since the logging already happened, and was already tested, I can just
update an existing test. That test is actually for products rather than
packages, so not a great illustration of the real-world problem we see
happening. This is not the place to second-guess POTemplate.displayname
though.

== Demo and Q/A ==

Q/A is annoyingly hard to set up. Only Soyuz does uploads without
specifying a source package name. Luckily we just added a script to
re-upload a package's translations, and there's an obscure case in the
approver that we can use to tickle the problem.

On staging (seriously kids, don't try this on production) do the
following:
 * Pick some arbitrary template in Intrepid, and on its +admin page,
   set its translation domain to "k3b". There are now two templates in
   separate Intrepid source packages with that domain.
 * Run scripts/rosetta/reupload-translations.py -s intrepid -p k3b-i18n
   to re-upload the latest translations tarball for this package.
 * Run cronscripts/rosetta-approve-imports.py -vvv

That last script run will try to approve queue entries for k3b-i18n. A
special case in the approver says that uploads for this package are to
be redirected to other packages, based on template domain matching. The
attempt to do this will run into the clash between the two k3b templates
and log a more helpful message than before.

No lint. Thank you for reviewing!

Jeroen

[Eleanor Berger (intellectronica)]

review: Approve (code)

=== modified file 'lib/lp/translations/doc/potemplate.txt'
--- lib/lp/translations/doc/potemplate.txt	2009-07-23 17:49:31 +0000
+++ lib/lp/translations/doc/potemplate.txt	2009-10-10 18:50:26 +0000
@@ -135,18 +135,21 @@
 as an extra parameter. The default value is "None" which disables the filter
 and both current and disabled templates are returned.
 
-For getPOTemplateByTranslationDomain, this case is an error because only one
-template per translation domain is allowed.
+For getPOTemplateByTranslationDomain, this case is an error.  The
+approver needs a unique domain to identify the right template to upload
+to.
 
     >>> potemplatesubset = potemplate_set.getSubset(
     ...     productseries=productseries)
     >>> potemplate = potemplatesubset.getPOTemplateByTranslationDomain(
     ...     'foodomain')
-    WARNING:lp.translations.model.potemplate:Found...
-    multiple templates with translation domain 'foodomain'...
-    There should be only one.
-    >>> potemplate is None
-    True
+    WARNING:lp.translations.model.potemplate:Found 2 competing templates
+    with translation domain 'foodomain':
+    "current-template in Fooproduct fooseries";
+    "disabled-template in Fooproduct fooseries".
+
+    >>> print potemplate
+    None
 
 Setting the filter to "True" will only return current (enabled) templates.
 
 
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2009-09-24 09:10:43 +0000
+++ lib/lp/translations/model/potemplate.py	2009-10-10 18:50:26 +0000
@@ -1106,10 +1106,12 @@
         elif len(result) == 1:
             return result[0]
         else:
+            templates = ['"%s"' % template.displayname for template in result]
+            templates.sort()
             log.warn(
-                "Found multiple templates with translation domain '%s'.  "
-                "There should be only one."
-                % translation_domain)
+                "Found %d competing templates with translation domain '%s': "
+                "%s."
+                % (len(templates), translation_domain, '; '.join(templates)))
             return None
 
     def getPOTemplateByPath(self, path):