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

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 416434: Stuff to Translate =

This branch is over the review limit, for which I apologize.

We're turning the Person page for Translations into a proper personal
translations dashboard. We've already added a listing of translations
the user could review; this branch adds a listing of ones that the user
could help complete. We spec'ed this out during the Buenos Aires UI
sprint and prototyped it at the Barcelona coding sprint.

== Implementation details ==

As one minor cleanup, PersonTranslationView.history_horizon, the cutoff
date beyond which a person's translation contributions are no longer
considered for these listings, is now initialized when the view is. It
used to be initialized lazily by _setHistoryHorizon, but this just
turned into too many call sites to be sane.

In a previous branch, the view started deciding that a user was a
translator by looking at translations karma. This turned out to be a
bitch to test properly because of restrictive database permissions. So
instead of that it now checks for POFileTranslator entries, using the
same "history horizon" cutoff date. It's cleaner this way, really.

You may notice that I've had to abbreviate property names. In the unit
tests for PersonTranslationsView, I had to resort to p_n_p_to_review and
p_n_p_to_translate for projects_and_packages_to_review and
projects_and_packages_to_translate, respectively.

The Storm code for all this gets pretty complex. It was necessary to
implement things like POFile.untranslatedCount or translations access
levels in database code, when we normally compute those only in Python.
I've reused that code between stuff-to-translate and stuff-to-review
where possible.

Two Storm bugs were discovered while developing this; one, problems with
negative slicing/indexing of result sets, turned out to be already known
with the Storm developers. The code has workarounds for both.

== Tests ==
{{{
./bin/test -vv -t persontranslationview -t translationsperson \
    -t translations-to-complete
}}}

== Demo and Q/A ==

If you have done any translation recently, go to your personal page in
the Translations tab. It will list translations that you could help
translate. The list comprises three sections that by design are not
distinguished. At the top, up to to three will be taken from the ones
that you have worked on that have large numbers of untranslated strings.
After that, up to three will be shown that you have worked on and that
have very few untranslated strings. And finally the list is rounded out
to a maximum of 10 entries by translations that you haven't worked on,
or at least not recently.

You will not see these lists when you look at other people's pages.

== Lint ==

No lint warnings. (Well there were some unused imports, but I've fixed
them now).

Jeroen

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeroen T. Vermeulen wrote:
> Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-416434 into lp:launchpad/devel.
>
> Requested reviews:
> Canonical Launchpad Engineering (launchpad)
>
> = Bug 416434: Stuff to Translate =
>
> This branch is over the review limit, for which I apologize.
>
> We're turning the Person page for Translations into a proper personal
> translations dashboard. We've already added a listing of translations
> the user could review; this branch adds a listing of ones that the user
> could help complete. We spec'ed this out during the Buenos Aires UI
> sprint and prototyped it at the Barcelona coding sprint.

 merge approved

As we discussed on IRC:

Please stop assigning history_horizon = None as a class variable and
then conditionally re-initializing it as an instance variable in the
constructor. Just initialize it in the constructor unconditionally.

Please change the description of _queryTranslatableFiles's worked_on
parameter to:
:param worked_on: If True, get `POFile`s that the person has been
working on recently (where "recently" is defined as `no_older_than`).
If False, get ones that the person has not been working on recently
(those that the person has never worked on, or last worked on before
`no_older_than`)

Otherwise, this is good to land.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqcEsYACgkQ0F+nu1YWqI1D/wCeKOWHPy8DGQLJR8mdQxyu/6F3
+PMAoIGPGrY+VMhzYZuf2KsPmyV2WV58
=HjAd
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'lib/lp/translations/browser/configure.zcml'
--- lib/lp/translations/browser/configure.zcml	2009-08-28 22:27:49 +0000
+++ lib/lp/translations/browser/configure.zcml	2009-08-31 06:58:44 +0000
@@ -737,6 +737,13 @@
             permission="zope.Public"
             template="../templates/person-translations-to-review-table.pt"
             layer="canonical.launchpad.layers.TranslationsLayer"/>
+        <browser:page
+            for="lp.registry.interfaces.person.IPerson"
+            name="+translations-to-complete-table"
+            class="lp.translations.browser.person.PersonTranslationView"
+            permission="zope.Public"
+            template="../templates/person-translations-to-complete-table.pt"
+            layer="canonical.launchpad.layers.TranslationsLayer"/>
 
 
 <!-- Product views -->

=== modified file 'lib/lp/translations/browser/person.py'
--- lib/lp/translations/browser/person.py	2009-08-26 09:34:50 +0000
+++ lib/lp/translations/browser/person.py	2009-08-31 14:11:15 +0000
@@ -69,7 +69,6 @@
 
 class ReviewLinksAggregator(WorkListLinksAggregator):
     """A `TranslationLinksAggregator` for translations to review."""
-
     # Link to unreviewed suggestions.
     pofile_link_suffix = '/+translate?show=new_suggestions'
 
@@ -79,6 +78,17 @@
         return pofile.unreviewedCount()
 
 
+class TranslateLinksAggregator(WorkListLinksAggregator):
+    """A `TranslationLinksAggregator` for translations to complete."""
+    # Link to untranslated strings.
+    pofile_link_suffix = '/+translate?show=untranslated'
+
+    # Strings that need work are untranslated ones.
+    def countStrings(self, pofile):
+        """See `WorkListLinksAggregator.countStrings`."""
+        return pofile.untranslatedCount()
+
+
 def person_is_reviewer(person):
     """Is `person` a translations reviewer?"""
     groups = ITranslationsPerson(person).translation_groups
@@ -118,6 +128,12 @@
 
     history_horizon = None
 
+    def __init__(self, *args, **kwargs):
+        super(PersonTranslationView, self).__init__(*args, **kwargs)
+        if self.history_horizon is None:
+            now = datetime.now(pytz.timezone('UTC'))
+            self.history_horizon = now - timedelta(90, 0, 0)
+
     @cachedproperty
     def batchnav(self):
         translations_person = ITranslationsPerson(self.context)
@@ -156,7 +172,9 @@
     @property
     def person_is_translator(self):
         """Is this person active in translations?"""
-        return self.context.hasKarma('translations')
+        person = ITranslationsPerson(self.context)
+        history = person.getTranslationHistory(self.history_horizon).any()
+        return history is not None
 
     @property
     def person_includes_me(self):
@@ -180,12 +198,6 @@
         return not (
             translationmessage.potmsgset.hide_translations_from_anonymous)
 
-    def _setHistoryHorizon(self):
-        """If not already set, set `self.history_horizon`."""
-        if self.history_horizon is None:
-            now = datetime.now(pytz.timezone('UTC'))
-            self.history_horizon = now - timedelta(90, 0, 0)
-
     def _getTargetsForReview(self, max_fetch=None):
         """Query and aggregate the top targets for review.
 
@@ -195,7 +207,6 @@
             Multiple `POFile`s may be aggregated together into a single
             target.
         """
-        self._setHistoryHorizon()
         person = ITranslationsPerson(self.context)
         pofiles = person.getReviewableTranslationFiles(
             no_older_than=self.history_horizon)
@@ -214,23 +225,77 @@
             Multiple `POFile`s may be aggregated together into a single
             target.
         """
-        self._setHistoryHorizon()
         person = ITranslationsPerson(self.context)
         pofiles = person.suggestReviewableTranslationFiles(
             no_older_than=self.history_horizon)[:max_fetch]
 
         return ReviewLinksAggregator().aggregate(pofiles)
 
+    def _getTargetsForTranslation(self, max_fetch=None):
+        """Get translation targets for this person to translate.
+
+        Results are ordered from most to fewest untranslated messages.
+        """
+        person = ITranslationsPerson(self.context)
+        urgent_first = (max_fetch >= 0)
+        pofiles = person.getTranslatableFiles(
+            no_older_than=self.history_horizon, urgent_first=urgent_first)
+
+        if max_fetch is not None:
+            pofiles = pofiles[:abs(max_fetch)]
+
+        return TranslateLinksAggregator().aggregate(pofiles)
+
+    def _suggestTargetsForTranslation(self, max_fetch=None):
+        """Suggest translations this person could be helping complete."""
+        person = ITranslationsPerson(self.context)
+        pofiles = person.suggestTranslatableFiles(
+            no_older_than=self.history_horizon)
+
+        return TranslateLinksAggregator().aggregate(pofiles)
+
     @cachedproperty
     def all_projects_and_packages_to_review(self):
         """Top projects and packages for this person to review."""
         return self._getTargetsForReview()
 
+    def _addToTargetsList(self, existing_targets, new_targets, max_items,
+                          max_overall):
+        """Add `new_targets` to `existing_targets` list.
+
+        This is for use in showing top-10 ists of translations a user
+        should help review or complete.
+
+        :param existing_targets: Translation targets that are already
+            being listed.
+        :param new_targets: Translation targets to add.  Ones that were
+            already in `existing_targets` will not be added again.
+        :param max_items: Maximum number of targets from `new_targets`
+            to add.
+        :param max_overall: Maximum overall size of the resulting list.
+            What happens if `existing_targets` already exceeds this size
+            is none of your business.
+        :return: A list of translation targets containing all of
+            `existing_targets`, followed by as many from `new_targets`
+            as there is room for.
+        """
+        remaining_slots = max_overall - len(existing_targets)
+        maximum_addition = min(max_items, remaining_slots)
+        if remaining_slots <= 0:
+            return existing_targets
+
+        known_targets = set([item['target'] for item in existing_targets])
+        really_new = [
+            item
+            for item in new_targets
+            if item['target'] not in known_targets
+            ]
+
+        return existing_targets + really_new[:maximum_addition]
+
     @property
     def top_projects_and_packages_to_review(self):
         """Suggest translations for this person to review."""
-        self._setHistoryHorizon()
-
         # Maximum number of projects/packages to list that this person
         # has recently worked on.
         max_known_targets = 9
@@ -241,22 +306,54 @@
         # worked on.  Aggregation may reduce the number we get, so ask
         # the database for a few extra.
         fetch = 5 * max_known_targets
-        recent = self._getTargetsForReview(fetch)[:max_known_targets]
-
-        # Fill out the list with other translations that the person
-        # could also be reviewing.
-        empty_slots = list_length - len(recent)
-        fetch = 5 * empty_slots
-        random_suggestions = self._suggestTargetsForReview(fetch)
-        random_suggestions = random_suggestions[:empty_slots]
-
-        return recent + random_suggestions
+        recent = self._getTargetsForReview(fetch)
+        overall = self._addToTargetsList(
+            [], recent, max_known_targets, list_length)
+
+        # Fill out the list with other, randomly suggested translations
+        # that the person could also be reviewing.
+        fetch = 5 * (list_length - len(overall))
+        suggestions = self._suggestTargetsForReview(fetch)
+        overall = self._addToTargetsList(
+            overall, suggestions, list_length, list_length)
+
+        return overall
 
     @cachedproperty
     def num_projects_and_packages_to_review(self):
         """How many translations do we suggest for reviewing?"""
         return len(self.all_projects_and_packages_to_review)
 
+    @property
+    def top_projects_and_packages_to_translate(self):
+        """Suggest translations for this person to help complete."""
+        # Maximum number of translations to list that need the most work
+        # done.
+        max_urgent_targets = 3
+        # Maximum number of translations to list that are almost
+        # complete.
+        max_almost_complete_targets = 3
+        # Length of overall list to display.
+        list_length = 10
+
+        fetch = 5 * max_urgent_targets
+        urgent = self._getTargetsForTranslation(fetch)
+        overall = self._addToTargetsList(
+            [], urgent, max_urgent_targets, list_length)
+
+        fetch = 5 * max_almost_complete_targets
+        almost_complete = self._getTargetsForTranslation(-fetch)
+        overall = self._addToTargetsList(
+            overall, almost_complete, max_almost_complete_targets,
+            list_length)
+
+        fetch = 5 * (list_length - len(overall))
+        suggestions = self._suggestTargetsForTranslation(fetch)
+        overall = self._addToTargetsList(
+            overall, suggestions, list_length, list_length)
+
+        return overall
+
 
 class PersonTranslationRelicensingView(LaunchpadFormView):
     """View for Person's translation relicensing page."""

=== modified file 'lib/lp/translations/browser/tests/test_persontranslationview.py'
--- lib/lp/translations/browser/tests/test_persontranslationview.py	2009-08-25 10:26:28 +0000
+++ lib/lp/translations/browser/tests/test_persontranslationview.py	2009-08-31 14:51:57 +0000
@@ -26,14 +26,16 @@
         super(TestPersonTranslationView, self).setUp()
         person = removeSecurityProxy(self.factory.makePerson())
         self.view = PersonTranslationView(person, LaunchpadTestRequest())
+        self.translationgroup = None
+        self.dutch = LanguageSet().getLanguageByCode('nl')
+        self.view.context.addLanguage(self.dutch)
 
     def _makeReviewer(self):
         """Set up the person we're looking at as a Dutch reviewer."""
         owner = self.factory.makePerson()
         self.translationgroup = self.factory.makeTranslationGroup(owner=owner)
-        dutch = LanguageSet().getLanguageByCode('nl')
         TranslatorSet().new(
-            translationgroup=self.translationgroup, language=dutch,
+            translationgroup=self.translationgroup, language=self.dutch,
             translator=self.view.context)
 
     def _makePOFiles(self, count, previously_worked_on):
@@ -46,8 +48,10 @@
         pofiles = []
         for counter in xrange(count):
             pofile = self.factory.makePOFile(language_code='nl')
-            product = pofile.potemplate.productseries.product
-            product.translationgroup = self.translationgroup
+
+            if self.translationgroup:
+                product = pofile.potemplate.productseries.product
+                product.translationgroup = self.translationgroup
 
             if previously_worked_on:
                 potmsgset = self.factory.makePOTMsgSet(
@@ -61,6 +65,11 @@
 
         return pofiles
 
+    def _addUntranslatedMessages(self, pofile, untranslated_messages):
+        """Add to `pofile`'s count of untranslated messages."""
+        template = pofile.potemplate
+        removeSecurityProxy(template).messagecount += untranslated_messages
+
     def test_translation_groups(self):
         # translation_groups lists the translation groups a person is
         # in.
@@ -157,7 +166,7 @@
         self.assertEqual(
             set(expected_links), set(item['link'] for item in targets))
 
-    def test_top_projects_and_packages_caps_existing_involvement(self):
+    def test_top_p_n_p_to_review_caps_existing_involvement(self):
         # top_projects_and_packages will return at most 9 POFiles that
         # the person has already worked on.
         self._makeReviewer()
@@ -168,7 +177,7 @@
         self.assertEqual(9, len(targets))
         self.assertEqual(9, len(set(item['link'] for item in targets)))
 
-    def test_top_projects_and_packages_caps_suggestions(self):
+    def test_top_p_n_p_to_review_caps_suggestions(self):
         # top_projects_and_packages will suggest at most 10 POFiles that
         # the person has not worked on.
         self._makeReviewer()
@@ -179,7 +188,7 @@
         self.assertEqual(10, len(targets))
         self.assertEqual(10, len(set(item['link'] for item in targets)))
 
-    def test_top_projects_and_packages_caps_total(self):
+    def test_top_p_n_p_to_review_caps_total(self):
         # top_projects_and_packages will show at most 10 POFiles
         # overall.  The last one will be a suggestion.
         self._makeReviewer()
@@ -192,6 +201,141 @@
         self.assertEqual(10, len(targets))
         self.assertEqual(10, len(set(item['link'] for item in targets)))
 
+    def test_person_is_translator_false(self):
+        # By default, a user is not a translator.
+        self.assertFalse(self.view.person_is_translator)
+
+    def test_person_is_translator_true(self):
+        # Doing translation work turns a user into a translator.
+        self._makePOFiles(1, previously_worked_on=True)
+
+        self.assertTrue(self.view.person_is_translator)
+
+    def test_getTargetsForTranslation(self):
+        # If there's nothing to translate, _getTargetsForTranslation
+        # returns nothing.
+        self.assertEqual([], self.view._getTargetsForTranslation())
+
+        # If there's a translation that this person has worked on and
+        # is not a reviewer for, and it has untranslated strings, it
+        # shows up in _getTargetsForTranslation.
+        pofile = self._makePOFiles(1, previously_worked_on=True)[0]
+        self._addUntranslatedMessages(pofile, 1)
+        product = pofile.potemplate.productseries.product
+
+        descriptions = self.view._getTargetsForTranslation()
+
+        self.assertEqual(1, len(descriptions))
+        description = descriptions[0]
+        self.assertEqual(product, description['target'])
+        self.assertTrue(description['link'].startswith(canonical_url(pofile)))
+
+    def test_getTargetsForTranslation_max_fetch(self):
+        # The max_fetch parameter limits how many POFiles are considered
+        # by _getTargetsForTranslation.  This lets you get the target(s)
+        # with the most untranslated messages.
+        pofiles = self._makePOFiles(3, previously_worked_on=True)
+        urgent_pofile = pofiles[2]
+        medium_pofile = pofiles[1]
+        nonurgent_pofile = pofiles[0]
+        self._addUntranslatedMessages(urgent_pofile, 10)
+        self._addUntranslatedMessages(medium_pofile, 2)
+        self._addUntranslatedMessages(nonurgent_pofile, 1)
+
+        descriptions = self.view._getTargetsForTranslation(1)
+        self.assertEqual(1, len(descriptions))
+        self.assertEqual(
+            urgent_pofile.potemplate.productseries.product,
+            descriptions[0]['target'])
+
+        # Passing a negative max_fetch makes _getTargetsForTranslation
+        # pick translations with the fewest untranslated messages.
+        descriptions = self.view._getTargetsForTranslation(-1)
+        self.assertEqual(1, len(descriptions))
+        self.assertEqual(
+            nonurgent_pofile.potemplate.productseries.product,
+            descriptions[0]['target'])
+
+    def test_suggestTargetsForTranslation(self):
+        previous_contrib = self._makePOFiles(1, previously_worked_on=True)
+        pofile = self._makePOFiles(1, previously_worked_on=False)[0]
+        self._addUntranslatedMessages(pofile, 1)
+
+        descriptions = self.view._suggestTargetsForTranslation()
+
+        self.assertEqual(1, len(descriptions))
+        self.assertEqual(
+            pofile.potemplate.productseries.product,
+            descriptions[0]['target'])
+
+    def test_top_projects_and_packages_to_translate(self):
+        # top_projects_and_packages_to_translate lists targets that the
+        # user has worked on and could help translate, followed by
+        # randomly suggested ones that also need translation.
+        worked_on = self._makePOFiles(1, previously_worked_on=True)[0]
+        self._addUntranslatedMessages(worked_on, 1)
+        not_worked_on = self._makePOFiles(1, previously_worked_on=False)[0]
+        self._addUntranslatedMessages(not_worked_on, 1)
+
+        descriptions = self.view.top_projects_and_packages_to_translate
+
+        self.assertEqual(2, len(descriptions))
+        self.assertEqual(
+            worked_on.potemplate.productseries.product,
+            descriptions[0]['target'])
+        self.assertEqual(
+            not_worked_on.potemplate.productseries.product,
+            descriptions[1]['target'])
+
+    def test_top_p_n_p_to_translate_caps_existing_involvement(self):
+        # top_projects_and_packages_to_translate shows no more than 6
+        # targets that the user has already worked on.
+        pofiles = self._makePOFiles(7, previously_worked_on=True)
+        for pofile in pofiles:
+            self._addUntranslatedMessages(pofile, 1)
+
+        descriptions = self.view.top_projects_and_packages_to_translate
+
+        self.assertEqual(6, len(descriptions))
+
+    def test_top_p_n_p_to_translate_lists_most_and_least_translated(self):
+        # Of the maximum of 6 translations that the user has already
+        # worked on, the first 3 will be the ones with the most
+        # untranslated strings; the last 3 will have the fewest.
+        # We create a lot more POFiles because internally the property
+        # will fetch many POFiles.
+        pofiles = self._makePOFiles(50, previously_worked_on=True)
+        for number, pofile in enumerate(pofiles):
+            self._addUntranslatedMessages(pofile, number + 1)
+        products = [
+            pofile.potemplate.productseries.product for pofile in pofiles]
+
+        descriptions = self.view.top_projects_and_packages_to_translate
+
+        self.assertEqual(6, len(descriptions))
+        targets = [item['target'] for item in descriptions]
+
+        # We happen to know that no more than 15 POFiles are fetched for
+        # each of the two categories, so the top 3 targets must be taken
+        # from the last 15 pofiles and the next 3 must be taken from the
+        # first 15 pofiles.
+        self.assertTrue(set(targets[:3]).issubset(products[15:]))
+        self.assertTrue(set(targets[3:]).issubset(products[:15]))
+
+        # No target is mentioned more than once in the listing.
+        self.assertEqual(len(targets), len(set(targets)))
+
+    def test_top_p_n_p_to_translate_caps_total(self):
+        # The list never shows more than 10 entries.
+        for previously_worked_on in (True, False):
+            pofiles = self._makePOFiles(
+                11, previously_worked_on=previously_worked_on)
+            for pofile in pofiles:
+                self._addUntranslatedMessages(pofile, 1)
+
+        descriptions = self.view.top_projects_and_packages_to_translate
+        self.assertEqual(10, len(descriptions))
+
 
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/translations/doc/translationsperson.txt'
--- lib/lp/translations/doc/translationsperson.txt	2009-07-10 11:46:50 +0000
+++ lib/lp/translations/doc/translationsperson.txt	2009-08-29 06:37:30 +0000
@@ -1,6 +1,6 @@
 = TranslationsPerson =
 
-Adapting IPerson to an ITranslationsPerson yields a TranslationPerson
+Adapting IPerson to an ITranslationsPerson yields a TranslationsPerson
 object which provides translatable languages and translation history.
 
     >>> from zope.component import getUtility

=== modified file 'lib/lp/translations/interfaces/translationsperson.py'
--- lib/lp/translations/interfaces/translationsperson.py	2009-08-19 14:25:32 +0000
+++ lib/lp/translations/interfaces/translationsperson.py	2009-08-31 14:11:15 +0000
@@ -34,6 +34,14 @@
         title=_("Whether person agrees to relicense their translations"),
         readonly=False)
 
+    def getTranslationHistory(no_older_than=None):
+        """Query most recent `POFileTranslator` entries for this person.
+
+        :param no_older_than: Optional cutoff date.  If given, older
+            `POFileTranslator` entries are ignored.
+        :return: a Storm query result.
+        """
+
     def getReviewableTranslationFiles(no_older_than=None):
         """List `POFile`s this person should be able to review.
 
@@ -43,7 +51,7 @@
         :param no_older_than: Optional cutoff date.  Translations that
             this person hasn't contributed to since this date will be
             ignored.
-        :return: a sequence of `POFile`s ordered by age of oldest
+        :return: A query result of `POFile`s ordered by age of oldest
             unreviewed `TranslationMessage` (oldest first).
         """
 
@@ -56,3 +64,26 @@
 
         :param maximum: Maximum number of `POFile`s to return.
         """
+
+    def getTranslatableFiles(no_older_than=None, urgent_first=True):
+        """List `POFile`s this person should be able to help translate.
+
+        These are translations that this person is not a reviewer for,
+        but has worked on recently.
+
+        :param no_older_than: Optional cutoff date to define "recently."
+        :param urgent_first: If True, sort `POFile`s with the most
+            untranslated strings to the front.  If False, sort the other
+            way around.
+        :return: A query result of `POFile`s ordered by number of
+            untranslated messages.
+        """
+
+    def suggestTranslatableFiles(no_older_than=None):
+        """Suggest `POFile`s this person could help translate.
+
+        Similar to `getTranslatableFiles`, this method picks an
+        arbitrary series of `POFile`s that the user is not a reviewer
+        for and has not worked on recently, but which are in a language
+        the user works in.
+        """

=== modified file 'lib/lp/translations/model/translationsperson.py'
--- lib/lp/translations/model/translationsperson.py	2009-08-19 14:25:32 +0000
+++ lib/lp/translations/model/translationsperson.py	2009-08-31 14:11:15 +0000
@@ -6,7 +6,7 @@
     'TranslationsPerson',
     ]
 
-from storm.expr import And, Join, LeftJoin, Or
+from storm.expr import And, Coalesce, Desc, Join, LeftJoin, Or
 from storm.info import ClassAlias
 from storm.store import Store
 
@@ -19,7 +19,7 @@
 
 from lp.registry.interfaces.person import IPerson
 from lp.translations.interfaces.translationgroup import (
-    ITranslationGroupSet)
+    ITranslationGroupSet, TranslationPermission)
 from lp.translations.interfaces.translationsperson import (
     ITranslationsPerson)
 from lp.translations.interfaces.translator import ITranslatorSet
@@ -58,12 +58,21 @@
             Language.visible""" % sqlvalues(self.person),
             clauseTables=['PersonLanguage'], orderBy='englishname')
 
+    def getTranslationHistory(self, no_older_than=None):
+        """See `ITranslationsPerson`."""
+        conditions = (POFileTranslator.person == self.person)
+        if no_older_than is not None:
+            conditions = And(
+                conditions,
+                POFileTranslator.date_last_touched >= no_older_than)
+
+        entries = Store.of(self.person).find(POFileTranslator, conditions)
+        return entries.order_by(Desc(POFileTranslator.date_last_touched))
+
     @property
     def translation_history(self):
         """See `ITranslationsPerson`."""
-        return POFileTranslator.select(
-            'POFileTranslator.person = %s' % sqlvalues(self.person),
-            orderBy='-date_last_touched')
+        return self.getTranslationHistory()
 
     @property
     def translation_groups(self):
@@ -143,7 +152,89 @@
         query = source.find(POFile, conditions)
         return query.config(distinct=True).order_by(POFile.id)
 
-    def _composePOFileReviewerJoins(self):
+    def _queryTranslatableFiles(self, worked_on, no_older_than=None,
+                                languages=None):
+        """Get `POFile`s this person could help translate.
+
+        :param worked_on: If True, get `POFile`s that the person has
+            been working on recently (where "recently" is defined as
+            `no_older_than`).  If False, get ones that the person has
+            not been working on recently.
+        :param no_older_than: Oldest involvement to consider as
+            "recently been working on."  Whatever the person did with a
+            `POFile` before this date is ignored.
+        :param languages: Optional set of languages to restrict search to.
+        :return: An unsorted query yielding `POFile`s.
+        """
+        if self.person.isTeam():
+            return []
+
+        tables = self._composePOFileReviewerJoins(
+            expect_reviewer_status=False)
+
+        translator_join, translator_condition = (
+            self._composePOFileTranslatorJoin(worked_on, no_older_than))
+        tables.append(translator_join)
+
+        translated_count = (
+            POFile.currentcount + POFile.updatescount + POFile.rosettacount)
+
+        conditions = And(
+            translated_count < POTemplate.messagecount, translator_condition)
+
+        # The person must not be a reviewer for this translation (unless
+        # it's in the sense that any user gets review permissions
+        # for it).
+        permission = Coalesce(
+            Distribution.translationpermission,
+            Product.translationpermission,
+            Project.translationpermission)
+        Reviewership = ClassAlias(TeamParticipation, 'Reviewership')
+        # XXX JeroenVermeulen 2009-08-28 bug=420364: Storm's Coalesce()
+        # can't currently infer its return type from its inputs, leading
+        # to a "can't adapt" error.  Using the enum's .value works
+        # around the problem.
+        not_reviewer = Or(
+            permission == TranslationPermission.OPEN.value,
+            And(
+                permission == TranslationPermission.STRUCTURED.value,
+                Translator.id == None),
+            And(
+                permission == TranslationPermission.RESTRICTED.value,
+                Translator.id != None,
+                Reviewership.id == None))
+
+        conditions = And(conditions, not_reviewer)
+
+        if languages is not None:
+            conditions = And(conditions, POFile.languageID.is_in(languages))
+
+        source = Store.of(self.person).using(*tables)
+        return source.find(POFile, conditions)
+
+    def getTranslatableFiles(self, no_older_than=None, urgent_first=True):
+        """See `ITranslationsPerson`."""
+        results = self._queryTranslatableFiles(True, no_older_than)
+
+        translated_count = (
+            POFile.currentcount + POFile.updatescount + POFile.rosettacount)
+        ordering = translated_count - POTemplate.messagecount
+        if not urgent_first:
+            ordering = -ordering
+
+        return results.order_by(ordering)
+
+    def suggestTranslatableFiles(self, no_older_than=None):
+        """See `ITranslationsPerson`."""
+        # XXX JeroenVermeulen 2009-08-28: Ideally this would also check
+        # for a free license.  That's hard to do in SQL though.
+        languages = set([
+            language.id for language in self.translatable_languages])
+        results = self._queryTranslatableFiles(
+            False, no_older_than, languages=languages)
+        return results.order_by(['random()'])
+
+    def _composePOFileReviewerJoins(self, expect_reviewer_status=True):
         """Compose certain Storm joins for common `POFile` queries.
 
         Returns a list of Storm joins for a query on `POFile`.  The
@@ -163,22 +254,21 @@
             POTemplate.iscurrent == True))
 
         # This is a weird and complex diamond join.  Both DistroSeries
-        # and ProductSeries are left joins, but one of them will
+        # and ProductSeries are left joins, but one of them may
         # ultimately lead to a TranslationGroup.  In the case of
         # ProductSeries it may lead to up to two: one for the Product
         # and one for the Project.
         DistroSeriesJoin = LeftJoin(
             DistroSeries, DistroSeries.id == POTemplate.distroseriesID)
+
         # If there's a DistroSeries, it should be the distro's
         # translation focus.
-        # The check for translationgroup here is not necessary, but
-        # should give the query planner some extra selectivity to narrow
-        # down the query more aggressively.
-        DistroJoin = LeftJoin(Distribution, And(
+        distrojoin_conditions = And(
             Distribution.id == DistroSeries.distributionID,
             Distribution.official_rosetta == True,
-            Distribution.translationgroup != None,
-            Distribution.translation_focusID == DistroSeries.id))
+            Distribution.translation_focusID == DistroSeries.id)
+
+        DistroJoin = LeftJoin(Distribution, distrojoin_conditions)
 
         ProductSeriesJoin = LeftJoin(
             ProductSeries, ProductSeries.id == POTemplate.productseriesID)
@@ -188,23 +278,38 @@
 
         ProjectJoin = LeftJoin(Project, Project.id == Product.projectID)
 
-        # Restrict to translations this person is a reviewer for.
-        GroupJoin = Join(TranslationGroup, Or(
+        # Look up translation group.
+        groupjoin_conditions = Or(
             TranslationGroup.id == Product.translationgroupID,
             TranslationGroup.id == Distribution.translationgroupID,
-            TranslationGroup.id == Project.translationgroupID))
-        TranslatorJoin = Join(Translator, And(
+            TranslationGroup.id == Project.translationgroupID)
+        if expect_reviewer_status:
+            GroupJoin = Join(TranslationGroup, groupjoin_conditions)
+        else:
+            GroupJoin = LeftJoin(TranslationGroup, groupjoin_conditions)
+
+        # Look up translation team.
+        translatorjoin_conditions = And(
             Translator.translationgroupID == TranslationGroup.id,
-            Translator.languageID == POFile.languageID))
+            Translator.languageID == POFile.languageID)
+        if expect_reviewer_status:
+            TranslatorJoin = Join(Translator, translatorjoin_conditions)
+        else:
+            TranslatorJoin = LeftJoin(Translator, translatorjoin_conditions)
 
         # Check for translation-team membership.  Use alias for
         # TeamParticipation; the query may want to include other
         # instances of that table.  It's just a linking table so the
         # query won't be interested in its actual contents anyway.
         Reviewership = ClassAlias(TeamParticipation, 'Reviewership')
-        TranslationTeamJoin = Join(Reviewership, And(
+        reviewerjoin_condition = And(
             Reviewership.teamID == Translator.translatorID,
-            Reviewership.personID == self.person.id))
+            Reviewership.personID == self.person.id)
+        if expect_reviewer_status:
+            TranslationTeamJoin = Join(Reviewership, reviewerjoin_condition)
+        else:
+            TranslationTeamJoin = LeftJoin(
+                Reviewership, reviewerjoin_condition)
 
         return [
             POFile,

=== added file 'lib/lp/translations/stories/standalone/xx-translations-to-complete.txt'
--- lib/lp/translations/stories/standalone/xx-translations-to-complete.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/stories/standalone/xx-translations-to-complete.txt	2009-08-31 13:55:35 +0000
@@ -0,0 +1,64 @@
+Translations to Complete
+------------------------
+
+Jean Champollion is a translator.
+
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> login(ANONYMOUS)
+
+    >>> jean = factory.makePerson(
+    ...     name='jean', email='[e-mail]', password='test')
+
+Jean has been working on French translations for Foux in Launchpad.
+
+    >>> foux = factory.makeProduct(name='foux')
+    >>> trunk = foux.getSeries('trunk')
+    >>> template = factory.makePOTemplate(productseries=trunk)
+    >>> pofile = factory.makePOFile(potemplate=template, language_code='fr')
+    >>> pofile = removeSecurityProxy(pofile)
+
+    >>> potmsgset = factory.makePOTMsgSet(
+    ...     potemplate=template, singular='a', sequence=1)
+    >>> message = factory.makeTranslationMessage(
+    ...     potmsgset=potmsgset, pofile=pofile, translator=jean,
+    ...     translations=['un'])
+
+Foux needs more strings translated.
+
+    >>> removeSecurityProxy(pofile.potemplate).messagecount = 2
+
+    >>> logout()
+
+Jean visits his Translations dashboard.
+
+    >>> jean_browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> jean_home = 'http://translations.launchpad.dev/~jean'
+    >>> jean_browser.open(jean_home)
+
+The dashboard shows a listing of translations that need Jean's help.
+
+    >>> tag = find_tag_by_id(
+    ...     jean_browser.contents, 'translations-to-complete-table')
+    >>> print tag.renderContents()
+
+Only Jean sees his personal listing.
+
+    >>> user_browser.open(jean_home)
+    >>> tag = find_tag_by_id(
+    ...     user_browser.contents, 'translations-to-complete-table')
+    >>> print tag
+    None
+
+Pierre is not a translator.  Pierre does not get such a listing either.
+
+    >>> login(ANONYMOUS)
+    >>> pierre = factory.makePerson(
+    ...     name='pierre', email='[e-mail]', password='test')
+    >>> logout()
+
+    >>> pierre_browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> pierre_browser.open('http://translations.launchpad.dev/~pierre')
+    >>> tag = find_tag_by_id(
+    ...     pierre_browser.contents, 'translations-to-complete-table')
+    >>> print tag
+    None

=== modified file 'lib/lp/translations/stories/standalone/xx-translations-to-review.txt'
--- lib/lp/translations/stories/standalone/xx-translations-to-review.txt	2009-08-17 17:08:46 +0000
+++ lib/lp/translations/stories/standalone/xx-translations-to-review.txt	2009-08-31 13:04:32 +0000
@@ -1,5 +1,5 @@
-Stuff to Review
----------------
+Translations to Review
+----------------------
 
 When a translations reviewer visits their own homepage, it shows a list
 of translations that they could or should be reviewing.

=== added file 'lib/lp/translations/templates/person-translations-to-complete-table.pt'
--- lib/lp/translations/templates/person-translations-to-complete-table.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/templates/person-translations-to-complete-table.pt	2009-08-28 09:44:31 +0000
@@ -0,0 +1,34 @@
+<tal:root
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  omit-tag="">
+
+<table class="listing" id="translations-to-complete-table">
+  <tr tal:repeat="target_info view/top_projects_and_packages_to_translate">
+    <td>
+      <tal:product condition="target_info/is_product"
+		   replace="structure target_info/target/fmt:link">
+	alsa-utils
+      </tal:product>
+      <tal:package condition="not: target_info/is_product">
+	<a tal:attributes="href target_info/target/fmt:url">
+	  <img src="/@@/distribution" />
+	  <tal:packagename replace="target_info/target/name">
+	    alsa-utils
+	  </tal:packagename>
+	</a>
+      </tal:package>
+    </td>
+    <td>
+      needs
+      <a tal:attributes="href target_info/link">
+	<tal:stringcount replace="target_info/count_wording">
+	  1 string
+	</tal:stringcount>
+	translated
+      </a>
+    </td>
+  </tr>
+</table>
+</tal:root>

=== modified file 'lib/lp/translations/templates/person-translations.pt'
--- lib/lp/translations/templates/person-translations.pt	2009-08-19 16:48:17 +0000
+++ lib/lp/translations/templates/person-translations.pt	2009-08-28 08:23:04 +0000
@@ -37,6 +37,14 @@
         </span>
       </div>
     </tal:reviewer>
+
+    <tal:translator condition="view/person_is_translator">
+      <div id="translations-to-complete-section" style="max-width:800px">
+        <h2>Translations you can help complete</h2>
+        <metal:translations-to-complete
+          tal:replace="structure context/@@+translations-to-complete-table" />
+      </div>
+    </tal:translator>
   </tal:me>