launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/kill-latest-people into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #928801 in Launchpad itself: "ProjectSet:+index timeout"
  https://bugs.launchpad.net/launchpad/+bug/928801

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/kill-latest-people/+merge/92384

Summary
=======
Products+index timed out because of a query determining which teams in "latest
teams" a signed user should be able to see. However: the page is about new
products, there's no good reason to show latest teams, it's just confusing UI.

Implementation
==============
We can solve a time out and bad UI by deleting things!

The most recently registed teams portlet is removed from the Products+index
template. The supporting code, PersonSet.latest_teams, is only used there, so
we can delete it and the implementation definition as well. Lastly, several
unit tests confirm that latest_teams shows the right thing--they were deleted
too.

Tests
=====
bin/test -vvcm lp.registry

QA
==
Open qastaging.launchpad.net/projects/+index, and confirm that you do not get
timeouts, and that the latest teams section is gone. Additionally, the most
recently registered projects portlet should be positioned to the left, as it
is now the only portlet.

Lint
====
This branch is lint free.
-- 
https://code.launchpad.net/~jcsackett/launchpad/kill-latest-people/+merge/92384
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/kill-latest-people into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2012-02-03 11:23:30 +0000
+++ lib/lp/registry/interfaces/person.py	2012-02-09 22:56:19 +0000
@@ -2305,9 +2305,6 @@
         address.
         """
 
-    def latest_teams(limit=5):
-        """Return the latest teams registered, up to the limit specified."""
-
     def mergeAsync(from_person, to_person, reviewer=None, delete=False):
         """Merge a person/team into another asynchronously.
 

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-02-03 11:23:30 +0000
+++ lib/lp/registry/model/person.py	2012-02-09 22:56:19 +0000
@@ -3533,18 +3533,6 @@
             (EmailAddress, Person),
             EmailAddress.email.lower().is_in(addresses), extra_query)
 
-    def latest_teams(self, limit=5):
-        """See `IPersonSet`."""
-        orderby = (Desc(Person.datecreated), Desc(Person.id))
-        result = IStore(Person).find(
-            Person,
-            And(
-                self._teamPrivacyQuery(),
-                TeamParticipation.team == Person.id,
-                Person.teamowner != None,
-                Person.merged == None))
-        return result.order_by(orderby).config(distinct=True)[:limit]
-
     def _merge_person_decoration(self, to_person, from_person, skip,
         decorator_table, person_pointer_column, additional_person_columns):
         """Merge a table that "decorates" Person.

=== modified file 'lib/lp/registry/templates/products-index.pt'
--- lib/lp/registry/templates/products-index.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/registry/templates/products-index.pt	2012-02-09 22:56:19 +0000
@@ -129,31 +129,6 @@
       href="/rosetta/+addquestion">ask the Translations staff</a>.
     </p>
 
-    <div class="left">
-
-      <div class="portlet">
-
-        <h2>Latest teams registered</h2>
-
-        <table>
-          <tbody>
-            <tr tal:repeat="team context/people/latest_teams">
-              <td><a tal:replace="structure team/fmt:link">A-Team</a>
-                  registered
-                  <span tal:attributes="title team/datecreated/fmt:datetime"
-                        tal:content="team/datecreated/fmt:displaydate">
-                    2007-09-17
-                  </span>
-              </td>
-            </tr>
-          </tbody>
-        </table>
-      </div>
-
-    </div>
-
-    <div class="right">
-
       <div class="portlet">
         <h2>Latest projects registered</h2>
         <table>
@@ -167,7 +142,6 @@
         </table>
         <p><a href="/projects/+all">&raquo; Show all projects</a></p>
       </div>
-    </div>
   </tal:no_search>
 </div>
 </body>

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2012-02-03 11:23:30 +0000
+++ lib/lp/registry/tests/test_person.py	2012-02-09 22:56:19 +0000
@@ -34,7 +34,6 @@
 from lp.registry.interfaces.person import (
     ImmutableVisibilityError,
     IPersonSet,
-    PersonVisibility,
     TeamSubscriptionPolicy,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket

=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py	2012-01-20 06:29:10 +0000
+++ lib/lp/registry/tests/test_personset.py	2012-02-09 22:56:19 +0000
@@ -145,43 +145,6 @@
                 person.preferredemail
         self.assertThat(recorder, HasQueryCount(LessThan(1)))
 
-    def test_latest_teams_public(self):
-        # Anyone can see the latest 5 teams if they are public.
-        teams = []
-        for num in xrange(1, 7):
-            teams.append(self.factory.makeTeam(name='team-%s' % num))
-        teams.reverse()
-        result = self.person_set.latest_teams()
-        self.assertEqual(teams[0:5], list(result))
-
-    def test_latest_teams_private(self):
-        # Private teams are only included in the latest teams if the
-        # user can view the team.
-        teams = []
-        for num in xrange(1, 7):
-            teams.append(self.factory.makeTeam(name='team-%s' % num))
-        owner = self.factory.makePerson()
-        teams.append(
-            self.factory.makeTeam(
-                name='private-team', owner=owner,
-                visibility=PersonVisibility.PRIVATE))
-        teams.reverse()
-        login_person(owner)
-        result = self.person_set.latest_teams()
-        self.assertEqual(teams[0:5], list(result))
-        login_person(self.factory.makePerson())
-        result = self.person_set.latest_teams()
-        self.assertEqual(teams[1:6], list(result))
-
-    def test_latest_teams_limit(self):
-        # The limit controls the number of latest teams returned.
-        teams = []
-        for num in xrange(1, 7):
-            teams.append(self.factory.makeTeam(name='team-%s' % num))
-        teams.reverse()
-        result = self.person_set.latest_teams(limit=3)
-        self.assertEqual(teams[0:3], list(result))
-
 
 class TestPersonSetMergeMailingListSubscriptions(TestCaseWithFactory):