launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/translations-page-inactive-projects into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #591285 in Launchpad itself: "Translations user activity page includes deactivated projects"
  https://bugs.launchpad.net/launchpad/+bug/591285

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/translations-page-inactive-projects/+merge/61500

Ensure that recent translation activities are only for active projects.

= Implementation =

Modify the recent_activities property of the PersonTranslationView to exclude translation_history records with inactive projects.

= Tests =

Add new test_recent_translation_activity() test to TestPersonTranslationView.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/browser/person.py
  lib/lp/translations/browser/tests/test_persontranslationview.py

./lib/lp/translations/browser/tests/test_persontranslationview.py
     234: local variable 'pofiles_worked_on' is assigned to but never used
     235: local variable 'pofiles_not_worked_on' is assigned to but never used
     322: local variable 'previous_contrib' is assigned to but never used
     336: local variable 'previous_contrib' is assigned to but never used
-- 
https://code.launchpad.net/~wallyworld/launchpad/translations-page-inactive-projects/+merge/61500
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/translations-page-inactive-projects into lp:launchpad.

=== modified file 'lib/lp/translations/browser/person.py'
--- lib/lp/translations/browser/person.py	2011-02-02 15:43:31 +0000
+++ lib/lp/translations/browser/person.py	2011-05-19 02:53:43 +0000
@@ -200,9 +200,23 @@
 
     @cachedproperty
     def recent_activity(self):
-        """Recent translation activity by this person."""
-        entries = ITranslationsPerson(self.context).translation_history[:10]
-        return [ActivityDescriptor(self.context, entry) for entry in entries]
+        """Recent translation activity by this person.
+
+        If the translation activity is associated with a project, we ensure
+        that the project is active.
+        """
+        all_entries = ITranslationsPerson(self.context).translation_history
+
+        def is_active(entry):
+            potemplate = entry.pofile.potemplate
+            if potemplate is None:
+                return True
+            product = potemplate.product
+            return product is None or product.active
+
+        active_entries = [entry for entry in all_entries if is_active(entry)]
+        return [ActivityDescriptor(self.context, entry)
+            for entry in active_entries[:10]]
 
     @cachedproperty
     def latest_activity(self):

=== modified file 'lib/lp/translations/browser/tests/test_persontranslationview.py'
--- lib/lp/translations/browser/tests/test_persontranslationview.py	2011-01-24 15:51:18 +0000
+++ lib/lp/translations/browser/tests/test_persontranslationview.py	2011-05-19 02:53:43 +0000
@@ -4,6 +4,7 @@
 __metaclass__ = type
 
 from unittest import TestLoader
+import urllib
 
 from zope.security.proxy import removeSecurityProxy
 
@@ -63,7 +64,7 @@
 
             if previously_worked_on:
                 if languages is not None:
-                    sequence = counter+1
+                    sequence = counter + 1
                 else:
                     sequence = 1
                 potmsgset = self.factory.makePOTMsgSet(
@@ -178,6 +179,32 @@
         self.assertEqual(
             set(expected_links), set(item['link'] for item in targets))
 
+    def test_recent_translation_activity(self):
+        # the recent_activity property lists the most recent translation
+        # targets the person has worked on, for active projects only.
+        self._makeReviewer()
+
+        # make a translation record for an inactive project (will be excluded)
+        [pofile] = self._makePOFiles(1, previously_worked_on=True)
+        removeSecurityProxy(pofile).potemplate.product.active = False
+
+        # and make one which has not been worked on (will be excluded)
+        self._makePOFiles(1, previously_worked_on=False)
+
+        pofiles_worked_on = self._makePOFiles(11, previously_worked_on=True)
+
+        # the expected results
+        person_name = urllib.urlencode({'person': self.view.context.name})
+        expected_links = [
+            (pofile.potemplate.translationtarget.title,
+            canonical_url(pofile) + "/+filter?%s" % person_name)
+            for pofile in pofiles_worked_on[:10]]
+
+        recent_activity = self.view.recent_activity
+        self.assertEqual(
+            set(expected_links),
+            set([(item.title, item.url) for item in recent_activity]))
+
     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.