Code review comment for lp:~henninge/launchpad/bug-488765-oops-translations

Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 488765 =

A product has an attribute primary_translatable that returns the product series or source package that should be translated. Returning source packages here seems to be a newer feature that broke a product's +translations page if the primary_translatable was a source package because the template assumed it's a product series.

== Proposed fx ==

I filed bug 507534 about that behaviour and fixed the page by filtering out source packages in the view so that only product series are displayed by the template.

I also discovered that bug 371632 can be fixed with this.

== Implementation details ==

lib/lp/translations/browser/product.py

 * The view already had a property "primary_translatable" that was obviously unused and returned a dictionary. I re-used it to return the primary_translatable of the context or None if it is not a product series.

lib/lp/translations/stories/standalone/xx-product-translations.txt

 * The part that only checked for up/download links was expanded to check for the whole recommendation section. This way it can be tested for complete absence if no primary_translatable is available.

 * This test did reproducce the error because evolution is linked to a translatable source package in the sample data, so Product.primary_translatable returns a source package when all templates in the product series have been disabled.

 * Disabling all the templates is a bit of noise but needed to reproduce the error.

 * Appended a test to show the notice when no translations are available.

lib/lp/translations/templates/product-translations.pt

 * Use view/primary_translatable instead of context's.

 * Changed some ids and classes needed for testing.

 * Added notice for when no translations are available.

 * Changed conditions so that page is never empty.

== Test ==

bin/test -vvct product-translations

== Demo/QA ==

On launchpad.dev:
 1. Disable all templates for evolution trunk.
 2. Got to https://translations.launchpad.dev/evolution
 3. You should not get an oops but a notice that there are no translations for this project.

To reproduce on staging you'll need a project that is linked to a source pacakage with translations.

= Launchpad lint =

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

Linting changed files:
  lib/lp/translations/browser/product.py
  lib/lp/translations/stories/standalone/xx-product-translations.txt
  lib/lp/translations/templates/product-translations.pt

« Back to merge proposal