launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/dont-show-obsolete-junk into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #582618 Branding not used for related projects on team page
  https://bugs.launchpad.net/bugs/582618
  #616845 Inactive projects appear on a team/person's "related projects" list.
  https://bugs.launchpad.net/bugs/616845


Summary

Fixes an issue with related projects show obsolete/inactive/invalid projects. Incidentally also fixes an issue with related projects not showing branding.


Proposed fix

If possible, change the model to not return inactive projects. Otherwise, change the view to iterate over the returned set and remove anything we don't want to show.


Pre-implementation notes

Spoke with Curtis Hovey (sinzui) about the problem and investigated the view and model with him.


Implementation details

The person model's getOwnedOrDrivenPillars now filters out inactive pillars where possible, and returns a DecoratedResultSet rather than a running a list comprehension before returning.

The template now users pillarname/pillar/fmt:link rather than a hand crafted url.


Demo and Q/A

Go to a team page (https://launchpad.dev/~ubuntu-team); you should see only active entities, and should see their branding.


lint
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/person.py
  lib/lp/registry/model/person.py
  lib/lp/registry/stories/person/xx-person-projects.txt
  lib/lp/registry/templates/person-portlet-related-projects.pt
  lib/lp/registry/tests/test_person.py

-- 
https://code.launchpad.net/~jcsackett/launchpad/dont-show-obsolete-junk/+merge/32585
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/dont-show-obsolete-junk into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2010-08-13 07:48:54 +0000
+++ lib/lp/registry/browser/person.py	2010-08-13 14:07:47 +0000
@@ -5168,7 +5168,7 @@
     @cachedproperty
     def related_projects_count(self):
         """The number of project owned or driven by this person."""
-        return len(self._related_projects())
+        return len(list(self._related_projects()))
 
     @cachedproperty
     def has_more_related_projects(self):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-08-13 08:26:15 +0000
+++ lib/lp/registry/model/person.py	2010-08-13 14:07:47 +0000
@@ -63,6 +63,8 @@
 
 from canonical.lazr.utils import get_current_browser_request, safe_hasattr
 
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet)
 from canonical.launchpad.database.account import Account, AccountPassword
 from canonical.launchpad.interfaces.account import AccountSuspendedError
 from lp.bugs.model.bugtarget import HasBugsBase
@@ -888,14 +890,16 @@
                 SELECT name, 3 as kind, displayname
                 FROM product
                 WHERE
-                    driver = %(person)s
-                    OR owner = %(person)s
+                    active = True AND
+                    (driver = %(person)s
+                    OR owner = %(person)s)
                 UNION
                 SELECT name, 2 as kind, displayname
                 FROM project
                 WHERE
-                    driver = %(person)s
-                    OR owner = %(person)s
+                    active = True AND
+                    (driver = %(person)s
+                    OR owner = %(person)s)
                 UNION
                 SELECT name, 1 as kind, displayname
                 FROM distribution
@@ -907,7 +911,12 @@
             """ % sqlvalues(person=self))
         results = IStore(self).using(origin).find(find_spec)
         results = results.order_by('kind', 'displayname')
-        return [pillar_name for pillar_name, kind, displayname in results]
+
+        def get_pillar_name(result):
+            pillar_name, kind, displayname = result
+            return pillar_name
+
+        return DecoratedResultSet(results, get_pillar_name)
 
     def getOwnedProjects(self, match_name=None):
         """See `IPerson`."""

=== modified file 'lib/lp/registry/stories/person/xx-person-projects.txt'
--- lib/lp/registry/stories/person/xx-person-projects.txt	2010-07-14 16:27:33 +0000
+++ lib/lp/registry/stories/person/xx-person-projects.txt	2010-08-13 14:07:47 +0000
@@ -9,8 +9,8 @@
     ...     anon_browser.contents, 'portlet-related-projects')
     >>> for tr in related_projects.findAll('tr'):
     ...     print extract_text(tr)
-    Ubuntu Linux
-    Ubuntu Test
+    Ubuntu
+    ubuntutest
     Tomcat
 
 The +related-software page displays a table with links to open bugs,

=== modified file 'lib/lp/registry/templates/person-portlet-related-projects.pt'
--- lib/lp/registry/templates/person-portlet-related-projects.pt	2009-08-25 02:01:55 +0000
+++ lib/lp/registry/templates/person-portlet-related-projects.pt	2010-08-13 14:07:47 +0000
@@ -25,9 +25,7 @@
     <tbody>
       <tr tal:repeat="pillarname pillarnames">
         <td>
-          <a tal:attributes="href string:/${pillarname/pillar/name};
-                             class pillarname/pillar/image:sprite_css"
-          tal:content="pillarname/pillar/title">Pillar title</a>
+          <a tal:replace="structure pillarname/pillar/fmt:link" />
         </td>
       </tr>
     </tbody>

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2010-08-13 07:48:54 +0000
+++ lib/lp/registry/tests/test_person.py	2010-08-13 14:07:47 +0000
@@ -1,8 +1,12 @@
 # Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import with_statement
+
+
 __metaclass__ = type
 
+
 from datetime import datetime
 import pytz
 import time
@@ -36,6 +40,7 @@
 from lp.blueprints.model.specification import Specification
 from lp.testing import login_person, logout, TestCase, TestCaseWithFactory
 from lp.testing.views import create_initialized_view
+from lp.testing import celebrity_logged_in
 from lp.registry.interfaces.person import PrivatePersonLinkageError
 from canonical.testing.layers import DatabaseFunctionalLayer, reconnect_stores
 
@@ -62,6 +67,22 @@
 
     layer = DatabaseFunctionalLayer
 
+    def test_getOwnedOrDrivenPillars(self):
+        user = self.factory.makePerson()
+        active_project = self.factory.makeProject(owner=user)
+        inactive_project = self.factory.makeProject(owner=user)
+        with celebrity_logged_in('admin'):
+            inactive_project.active = False
+        expected_pillars = [active_project.name]
+        received_pillars = [pillar.name for pillar in
+            user.getOwnedOrDrivenPillars()]
+        self.assertEqual(expected_pillars, received_pillars)
+
+
+class TestPersonStates(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
     def setUp(self):
         TestCaseWithFactory.setUp(self, 'foo.bar@xxxxxxxxxxxxx')
         person_set = getUtility(IPersonSet)