On Aug 18, 2009, at 07:11 PM, Curtis Hovey wrote:

>This is very good. I know you had concerns about the menus, but I think they
>are close to solving the issues in this branch and in the other top-level
>collections. I think a subtle renaming of the marker interface will help you
>see what I see. I have a code fragment at the bottom of how I would name and
>restructure the menus so that you have something ready for your next branches.

Yep, it's been a frustrating branch; thanks for talking me off the ledge.  Thanks for the review and it's definitely encouraging to see how close this branch puts me to completing the other top level collections.

>...
>> === modified file 'lib/canonical/launchpad/icing/style-3-0.css'
>> --- lib/canonical/launchpad/icing/style-3-0.css	2009-08-17 11:22:39 +0000
>> +++ lib/canonical/launchpad/icing/style-3-0.css	2009-08-17 17:50:02 +0000
>> @@ -590,3 +590,10 @@
>>    word-wrap: break-word;
>>  }
>>  
>> +/* ==== Listing tables ==== */
>> +
>> +table.listing thead {
>> +  color: #9f2b33;
>
>This colour is the bug colour. We need something neutral #333; maybe.

I never made the correlation between the heading color and the app color.  I thought it was a general heading cue.  If not, then I think the default color setting is just fine.  You provided the hint for bolding the th in a comment on the bug and that works better, so I'll keep that and remove the color setting.

>> +  font-size: 1.125em;
>
>This font-size is not supported by YUI. We must specify all sizes
>using the percentages from the table in this file. try 116% or 123.1%.

116% looks good to me.

>> === modified file 'lib/canonical/launchpad/webapp/batching.py'
>> --- lib/canonical/launchpad/webapp/batching.py	2009-06-25 05:30:52 +0000
>> +++ lib/canonical/launchpad/webapp/batching.py	2009-08-14 18:41:41 +0000
>> @@ -62,6 +62,10 @@
>>      def max_batch_size(self):
>>          return config.launchpad.max_batch_size
>>  
>> +    @property
>> +    def multiple_pages(self):
>
>can you add a docstring? Is has_multiple_pages a better name for what it
>returns?

Docstring added.  'multiple_pages' was the original name for the property before refactoring, but I agree that 'has_multiple_pages' is a better name.  So changed.

>> === modified file 'lib/lp/code/browser/branchlisting.py'
>> --- lib/lp/code/browser/branchlisting.py	2009-08-13 04:08:38 +0000
>> +++ lib/lp/code/browser/branchlisting.py	2009-08-18 14:11:56 +0000
>> @@ -331,56 +331,13 @@
>>          branches = list(self.currentBatch())
>>          # XXX: TimPenhey 2009-04-08 bug=324546
>>          # Until there is an API to do this nicely, shove the launchpad.view
>> -        # permission into the request cache directly.  This is done because the
>> -        # security proxy sometimes causes extra requests to be made.
>> +        # permission into the request cache directly.  This is done because
>> +        # the security proxy sometimes causes extra requests to be made.
>>          request = self.view.request
>>          precache_permission_for_objects(request, 'launchpad.View', branches)
>>          return branches
>>  
>>      @cachedproperty
>> -    def branch_ids_with_bug_links(self):
>
>I am not sure why this big chunk of code was removed. I this diff lying?

It's not!  I mentioned this in my cover letter.  'make lint' complained about duplicate methods in this class and in fact the methods I removed were being shadowed by methods later in the same class, with the same name.  There's no way these methods could have been called (that I can think of anyway!) by any other code.  I think they are just dead code left after a refactoring, so I removed it.  I ran ec2 on revno 9127 of the branch and it passed, so if it doesn't break a test, it wasn't needed <wink>.

>> === added file 'lib/lp/registry/browser/menu.py'
>> --- lib/lp/registry/browser/menu.py	1970-01-01 00:00:00 +0000
>> +++ lib/lp/registry/browser/menu.py	2009-08-18 13:54:51 +0000
>> @@ -0,0 +1,40 @@
>> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Shared menus."""
>> +
>> +__metaclass__ = type
>> +__all__ = [
>> +    'TopLevelMenuMixin',
>> +    ]
>> +
>> +
>> +from canonical.launchpad.webapp.menu import Link
>> +
>> +
>> +class TopLevelMenuMixin:
>> +    """Menu shared by top level collection objects."""
>
>I think I would prefer this in __init__.py where other shared code
>is? Since these collections are in the root of the mainsite, I would
>look in __init__.py for them

Personally, I really don't like putting much if any code in __init__.py.  In a sense, that code is hidden (though not as bad as filling __init__.py with lots of from-import-*'s).  Other people may really like putting code in __init__.py, but my sensibilities here are shaped by early discussions about it when Python packages were being designed.  At the time, Guido even debated whether it should be legal to put anything in an __init__.py at all!  While it makes sense to allow it (and to put those names in the package's namespace), I think this is way overused in Launchpad.  Others might disagree.

>
>What other menus would go in this module to justify it?

I'm not sure anything else /has/ to go into this module to justify it.  I personally like keeping modules short and sweet.

>...
>
>> +    def meetings(self):
>> +        return Link('/sprints/', 'View meetings', icon='info')
>
>Seeing this really make me think we need to separate sprints from
>blueprints. I am certain this link belongs to the mainsite (which
>is registry)

mainsite seems reasonable.  Is there something you want me to change here?

>> +    def create_account(self):
>> +        text = 'Create an account'
>> +        return Link('/people/+login', text, icon='add')
>
>^ This state will not always be right. Use the enabled kwarg like
>we do other user variable links.
>
>    def create_account(self):
>        text = 'Create an account'
>        enabled = self.context.user is None:
>        return Link('/people/+login', text, icon='add', enabled=enabled)

Right. As you saw down below I enabled this link in a different way.  Yours is better so I've made the change.  I also took the opportunity to s/products/projects/ in TopLevelMenuMixin.  I'm just waiting for a swine+bird flu (club sandwich pandemic?) to lay me up in bed for a week so I can do a mass mechanical change of product->project and project->project group.

>> === modified file 'lib/lp/registry/browser/person.py'
>> --- lib/lp/registry/browser/person.py	2009-08-03 15:21:35 +0000
>> +++ lib/lp/registry/browser/person.py	2009-08-18 15:28:28 +0000
>
>...
>
>> @@ -1348,35 +1335,64 @@
>>          return self.proposed_memberships or self.invited_memberships
>>  
>>  
>> -class FOAFSearchView:
>> +class IPeopleSearchNavigationMenu(Interface):
>> +    """Marker interface for people search navigation menu."""
>
>I bet you will have to rename this in the next branch. Looking at the links
>below, this is the IRegistryCollectionNavigationMenu and you want all the
>collection search views to
>
>    implements(IRegistryCollectionNavigationMenu)
>
>So you have one marker interface, and one implementation below that
>is used in 5 collections. Your other branches will be much smaller.
>I think this is what you intend to do placed the like definitions in
>a shared place.

Well, now I have my justification for menu.py! :)

>
>> +class PeopleSearchNavigationMenu(NavigationMenu, TopLevelMenuMixin):
>> +    """Navigation menu for people search."""
>
>Most of the links are not for people search, they are for top-level
>registry collections.

Agreed!  Changes made.

>
>> +    usedfor = IPeopleSearchNavigationMenu
>> +    facet = 'overview'
>> +
>> +    links = ['products', 'distributions', 'people', 'meetings',
>> +             'register_team', 'register_project']
>
>I think we should show projectgroups too.

Agreed!  Added.

>
>> +    def initialize(self):
>> +        """See `MenuBase`."""
>> +        if self.context.user is None:
>> +            # Make a copy of the links so as not to permanently mutate the
>> +            # class attribute.
>> +            self.links = PeopleSearchNavigationMenu.links[:]
>> +            self.links.append('create_account')
>
>^ This is contrary to how we do this in other menus. We use the links
>enabled attribute to control that and we enabled or disable it based
>on state. I think you can remove this method.
>
>Other than register a project, I think the collections are a related
>paged menu. Actually, where are merging links:
>
>    Admin merge people
>    Admin merge teams
>    Merge accounts

After discussion on IRC between you, Martin, and myself, we decided to remove the inline text for the two admin functions and move them into the menu.  I've left the person-merge-request link inline though since this is something that normal people will use.

I also took the opportunity to disable the 'register' menu links for anonymous users.

>You can define a menu for the context object like this:
>
>{{{
>class PersonSetNavigationMenu(NavigationMenu, TopLevelMenuMixin):
>    """Navigation menu for people search."""
>
>    usedfor = IPerson
>    facet = 'overview'
>
>    links = [register_team', 'register_project', ... <merge links>]
>}}}
>
>And it will be available to every PersonSet page. Then use the action menu
>like
>
>    <tal:menu replace="structure context/@@+global-actions" />
>
>The related pages menu would be
>
>    <tal:menu replace="structure view/@@+related-pages" />

I see you have a more fleshed out example below...

>
>> +class PeopleSearchView(LaunchpadView):
>> +    """Search for people and teams on the /people page."""
>> +
>> +    implements(IPeopleSearchNavigationMenu)
>
>If we plan for a single collection menu used by all then this line
>would be:
>
>    implements(IRegistryCollectionNavigationMenu)
>
>...
>
>> === modified file 'lib/lp/registry/browser/product.py'
>> --- lib/lp/registry/browser/product.py	2009-08-12 13:20:10 +0000
>> +++ lib/lp/registry/browser/product.py	2009-08-18 13:54:51 +0000
>...
>> @@ -546,40 +547,17 @@
>>      enable_only = ['overview']
>>  
>>  
>> -class ProductSetContextMenu(ContextMenu):
>> +class ProductSetContextMenu(ContextMenu, TopLevelMenuMixin):
>>  
>>      usedfor = IProductSet
>>  
>>      links = ['products', 'distributions', 'people', 'meetings',
>> -             'all', 'register', 'register_team', 'review_licenses']
>
>I am no sure we need this any more. ContextMenus are 1.0. We are using
>Just change ContextMenu to NavigationMenu and the conversion is done I think.
>
>Oh, except that the links to the other collections belong in
>IRegistryCollectionNavigationMenu.

This is just a refactoring, so I'd rather leave this as a task for the next branch.  On the plus side, with all the changes here it should go very quickly!

>> === added file 'lib/lp/registry/browser/tests/people-views.txt'
>> --- lib/lp/registry/browser/tests/people-views.txt	1970-01-01 00:00:00 +0000
>> +++ lib/lp/registry/browser/tests/people-views.txt	2009-08-14 17:31:10 +0000
>> @@ -0,0 +1,70 @@
>> +==============================
>> +Searching for people and teams
>> +==============================
>> +
>> +The view behind the /people page provides searching for people and teams.  The
>> +view knows how many people and teams are currently registered in Launchpad.
>> +
>> +    >>> from lp.registry.interfaces.person import IPersonSet
>> +    >>> login('<email address hidden>')
>
>Let's use <email address hidden> from now on. Tim informed me of this. I think
>it makes the test clear.

It does!  I'll remember this for the next branch I review.

>...
>
>> +Batch navigator
>> +===============
>> +
>> +The view returns a batch navigator for searching through sets of teams,
>> +people, or both.  By default, both are searched for the given name.  There is
>> +one person and one team matching the 'test' string.
>> +
>> +    >>> from zope.security.proxy import removeSecurityProxy
>> +    >>> def print_batch(batch):
>> +    ...     for thing in batch.currentBatch():
>> +    ...         naked_thing = removeSecurityProxy(thing)
>> +    ...         print naked_thing
>
>> +    >>> form = dict(name='test')
>
>Would this test be more predictable and not require a function if it:
>
>    >>> person = factory.makePerson(name='qqq-person')
>    >>> team = factory.makePerson(name='qqq-team', teamowner=person)
>    >>> form = dict(name='qqq')

Not really.  It would reduce the dependency on sample data, but I think still require the use of the function.  You'll still want to iterate over all items in the batch, and I think you'll still need to unwrap the thing to print it.

>> === modified file 'lib/lp/registry/templates/people-index.pt'
>> --- lib/lp/registry/templates/people-index.pt	2009-07-17 17:59:07 +0000
>> +++ lib/lp/registry/templates/people-index.pt	2009-08-18 14:16:16 +0000
>...
>
>>      <div metal:fill-slot="main">
>> -      <tal:block define="bn view/searchPeopleBatchNavigator">
>> -        <div align="center">
>> +      There are currently <span tal:replace="view/number_of_people" /> people
>> +      and <span tal:replace="view/number_of_teams" /> teams registered in
>> +      Launchpad.
>
>The counts do not generate a span, so do not use it in the markup.
>consider <tal:count>

Sure.  It doesn't matter much since we're using tal:replace, but I made this change anyway.

>
>> +        <tal:block condition="not: batch">
>> +          <div id="application-summary">
>> +            <p>Launchpad creates
>> +            <a href="https://help.launchpad.net/YourAccount">profiles</a> for
>> +            people based on Launchpad usage, as well as information collected
>> +            from public sources such as bug trackers, mailing lists, public
>> +            key servers, and published application translations.  You can
>> +            <a href="+me">manage your account</a> in Launchpad, and you can
>> +            <a href="+newteam">create teams of people</a> for organizing
>> +            around common interests, discussions or permissions.</p>
>> +
>> +            <p>Launchpad may create
>> +            <a href="https://help.launchpad.net/YourAccount/Merging">duplicate
>> +            profiles</a> for you as it automatically collects information from
>> +            public sources.  If you find you have more than one profile on
>> +            Launchpad, you can <a href="+requestmerge">request a merge</a> of
>> +            the duplicate profiles.</p>
>> +
>> +            <p tal:condition="view/is_admin">As a Launchpad administrator, you
>> +            can also service
>> +            <a href="+adminpeoplemerge">person merge requests</a> and
>> +            <a href="+adminteammerge">team merge requests</a>.
>> +            </p>
>> +          </div>
>>          </tal:block>
>
>The links in this block should be using a menu to ensure the link is
>enabled and bound to the correct root site. This is an extreme case:
>
>    <a
>      tal:define="link context/menu:overview/new_team"
>      tal:condition="link/enabled"
>      tal:attributes="href link/url">create teams of people</a>
>
>This is why we prefer
>
>    <a tal:replace="structure context/menu:overview/new_team/fmt:link"/>
>
>But I do not think you want the icon and default text in the narrative.

No, definitely not.  It makes the page template fairly ugly but I've done
this.  I guess it's your lot in life to have to review it ;).

>
>...
>
>>            </tal:block>
>>          </tal:block>
>>        </tal:block>
>
>The end of this markup illustrated how hard this markup can be to read.
>Are their better names than tal:block, like tal:batch and tal:batch-nav?

Well it's TAL, so it's not meant to be human readable <wink>.  This is
inherited from the original template, but I've cleaned it up.

>
>You could end the main section with a related menu.
>
>    <tal:menu replace="structure view/@@+related-pages" />
>
>
>These look like the menus we want to build.
>
>{{{
>
>class PersonSetNavigationMenu(NavigationMenu):
>    """Navigation menu for PersonSet actions"""
>    usedfor = IPersonSet
>    links = [
>        register_team','adminpeoplemerge', 'adminteammerge', 'mergeaccounts']
>
>
>class IRegistryCollectionNavigationMenu(Interface):
>    """Marker interface for the registry collection navigation menu."""
>
>
>class RegistryCollectionNavigationMenu(NavigationMenu, TopLevelMenuMixin):
>    """Navigation menu for registry person search collection."""
>    usedfor = IRegistryCollectionNavigationMenu
>    facet = 'overview'
>    links = ['products', 'distributions', 'people', 'meetings']
> 
>
>class PeopleSearchView(LaunchpadView):
>    """Search for people and teams on the /people page."""
>    implements(IRegistryCollectionNavigationMenu)
>
>}}}

Nice.  This works but with a small ugliness.  Because `usedfor` in the action
menu must correspond to the context object's interface, it can't be completely
generalized.  It will force each top level collection view to create a
subclass containing only the `usedfor` attribute, set to the right interface.
Icky, but close enough for jazz.

Doing this does feel like a hack because it would be better to register both
menus on the view instead of one on the view and one on the context object.
But it's a hack that will live another day because I'm not going to worry
about it any more. :)

Incremental diff attached.

=== modified file 'lib/canonical/launchpad/doc/batch_navigation.txt'
--- lib/canonical/launchpad/doc/batch_navigation.txt	2009-08-14 18:41:41 +0000
+++ lib/canonical/launchpad/doc/batch_navigation.txt	2009-08-18 22:26:02 +0000
@@ -93,7 +93,7 @@
 
 The batch navigator tells us whether multiple pages will be used.
 
-    >>> batch_nav.multiple_pages
+    >>> batch_nav.has_multiple_pages
     True
 
 

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2009-08-18 18:22:11 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2009-08-18 22:26:02 +0000
@@ -593,7 +593,8 @@
 /* ==== Listing tables ==== */
 
 table.listing thead {
-  color: #9f2b33;
-  font-size: 1.125em;
+  font-size: 116%;
+}
+table.listing th {
   font-weight: bold;
 }

=== modified file 'lib/canonical/launchpad/webapp/batching.py'
--- lib/canonical/launchpad/webapp/batching.py	2009-08-14 18:41:41 +0000
+++ lib/canonical/launchpad/webapp/batching.py	2009-08-18 22:26:02 +0000
@@ -63,7 +63,13 @@
         return config.launchpad.max_batch_size
 
     @property
-    def multiple_pages(self):
+    def has_multiple_pages(self):
+        """Whether the total size is greater than the batch size.
+
+        When true, it means that the batch should be rendered on multiple
+        pages, and a navigation heading should be included above and below the
+        table.
+        """
         return self.batch.total() > self.batch.size
 
 
@@ -79,4 +85,3 @@
         if columns_to_show:
             for column_to_show in columns_to_show:
                 self.show_column[column_to_show] = True
-

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2009-08-18 14:11:56 +0000
+++ lib/lp/code/browser/branchlisting.py	2009-08-18 22:26:02 +0000
@@ -488,7 +488,7 @@
         # XXX: MichaelHudson 2007-10-18 bug=153894: This means there are two
         # ways of sorting a one-page branch listing, which is a confusing and
         # incoherent.
-        if self.multiple_pages:
+        if self.has_multiple_pages:
             return "listing"
         else:
             return "listing sortable"

=== modified file 'lib/lp/code/browser/branchmergeproposallisting.py'
--- lib/lp/code/browser/branchmergeproposallisting.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/browser/branchmergeproposallisting.py	2009-08-18 22:26:02 +0000
@@ -127,13 +127,9 @@
         proposals = self._proposals_for_current_batch
         return [self._createItem(proposal) for proposal in proposals]
 
-    @cachedproperty
-    def multiple_pages(self):
-        return self.batch.total() > self.batch.size
-
     @property
     def table_class(self):
-        if self.multiple_pages:
+        if self.has_multiple_pages:
             return "listing"
         else:
             return "listing sortable"

=== modified file 'lib/lp/code/templates/branch-listing.pt'
--- lib/lp/code/templates/branch-listing.pt	2009-08-03 05:29:15 +0000
+++ lib/lp/code/templates/branch-listing.pt	2009-08-18 22:26:02 +0000
@@ -196,7 +196,7 @@
 
 </script>
 
-  <tal:needs-batch condition="context/multiple_pages">
+  <tal:needs-batch condition="context/has_multiple_pages">
     <div id="branch-batch-links">
       <tal:navigation replace="structure context/@@+navigation-links-upper" />
     </div>
@@ -261,7 +261,7 @@
           <tal:badges replace="structure branch/badges:small"/>
         </td>
         <td>
-          <span tal:condition="not:context/multiple_pages"
+          <span tal:condition="not:context/has_multiple_pages"
                 tal:content="branch/lifecycle_status/sortkey"
                 class="sortkey">23</span>
           <span tal:content="branch/lifecycle_status/title"

=== modified file 'lib/lp/code/templates/branchmergeproposal-listing.pt'
--- lib/lp/code/templates/branchmergeproposal-listing.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/code/templates/branchmergeproposal-listing.pt	2009-08-18 22:26:02 +0000
@@ -3,7 +3,7 @@
   xmlns:metal="http://xml.zope.org/namespaces/metal"
 >
 
-  <tal:needs-batch condition="context/multiple_pages">
+  <tal:needs-batch condition="context/has_multiple_pages">
     <div id="batch-links">
       <tal:navigation replace="structure context/@@+navigation-links-upper" />
     </div>
@@ -46,7 +46,7 @@
 
         <td tal:condition="context/show_column/date_reviewed|nothing">
           <span class="sortkey"
-                tal:condition="not:context/multiple_pages"
+                tal:condition="not:context/has_multiple_pages"
                 tal:content="proposal/date_reviewed/fmt:datetime">
             2005-02-12 13:45 EST
           </span>

=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py	2009-08-15 13:48:33 +0000
+++ lib/lp/registry/browser/__init__.py	2009-08-18 22:26:02 +0000
@@ -127,7 +127,7 @@
 
 
 class RegistryEditFormView(LaunchpadEditFormView):
-    """"A base class that provides consistent edit form behaviour."""
+    """A base class that provides consistent edit form behaviour."""
     @property
     def page_title(self):
         """The page title."""

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-08-18 18:22:11 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-08-18 23:27:23 +0000
@@ -773,7 +773,6 @@
         <browser:menus
             module="lp.registry.browser.person"
             classes="
-                PeopleSearchNavigationMenu
                 PersonBugsMenu
                 PersonEditNavigationMenu
                 PersonFacets
@@ -786,6 +785,12 @@
                 TeamOverviewMenu
                 TeamOverviewNavigationMenu
                 TeamSpecsMenu
+                PersonSetNavigationMenu
+                "/>
+        <browser:menus
+            module="lp.registry.browser.menu"
+            classes="
+                RegistryCollectionNavigationMenu
                 "/>
         <browser:url
             for="lp.registry.interfaces.person.IPerson"

=== modified file 'lib/lp/registry/browser/menu.py'
--- lib/lp/registry/browser/menu.py	2009-08-18 13:54:51 +0000
+++ lib/lp/registry/browser/menu.py	2009-08-18 23:27:23 +0000
@@ -5,17 +5,24 @@
 
 __metaclass__ = type
 __all__ = [
+    'IRegistryCollectionNavigationMenu',
+    'RegistryCollectionActionMenuBase',
+    'RegistryCollectionNavigationMenu',
     'TopLevelMenuMixin',
     ]
 
 
-from canonical.launchpad.webapp.menu import Link
+from zope.component import getUtility
+from zope.interface import Interface
+
+from canonical.launchpad.webapp.menu import Link, NavigationMenu
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 
 
 class TopLevelMenuMixin:
     """Menu shared by top level collection objects."""
 
-    def products(self):
+    def projects(self):
         return Link('/projects/', 'View projects', icon='info')
 
     def distributions(self):
@@ -27,14 +34,72 @@
     def meetings(self):
         return Link('/sprints/', 'View meetings', icon='info')
 
+    def project_groups(self):
+        return Link('/projectgroups', 'View project groups', icon='info')
+
     def register_project(self):
         text = 'Register a project'
-        return Link('/projects/+new', text, icon='add')
+        enabled = self.user is not None
+        return Link('/projects/+new', text, icon='add', enabled=enabled)
 
     def register_team(self):
         text = 'Register a team'
-        return Link('/people/+newteam', text, icon='add')
+        enabled = self.user is not None
+        return Link('/people/+newteam', text, icon='add', enabled=enabled)
 
     def create_account(self):
         text = 'Create an account'
-        return Link('/people/+login', text, icon='add')
+        # Only enable this link for anonymous users.
+        enabled = self.user is None
+        return Link('/people/+login', text, icon='add', enabled=enabled)
+
+    def request_merge(self):
+        text = 'Request a merge'
+        enabled = self.user is not None
+        return Link('/people/+requestmerge', text, icon='edit',
+                    enabled=enabled)
+
+    def admin_merge_people(self):
+        text = 'Merge people'
+        enabled = (self.user is not None and
+                   self.user.inTeam(getUtility(ILaunchpadCelebrities).admin))
+        return Link('/people/+adminpeoplemerge', text, icon='edit',
+                    enabled=enabled)
+
+    def admin_merge_teams(self):
+        text = 'Merge teams'
+        enabled = (self.user is not None and
+                   self.user.inTeam(getUtility(ILaunchpadCelebrities).admin))
+        return Link('/people/+adminteammerge', text, icon='edit',
+                    enabled=enabled)
+
+
+class IRegistryCollectionNavigationMenu(Interface):
+    """Marker interface for top level registry collection navigation menu."""
+
+
+class RegistryCollectionNavigationMenu(NavigationMenu, TopLevelMenuMixin):
+    """Navigation menu for top level registry collections."""
+
+    usedfor = IRegistryCollectionNavigationMenu
+    facet = 'overview'
+
+    links = [
+        'projects',
+        'project_groups',
+        'distributions',
+        'people',
+        'meetings',
+        ]
+
+
+class RegistryCollectionActionMenuBase(NavigationMenu, TopLevelMenuMixin):
+    """Action menu for top level registry collections.
+
+    Because of the way menus work, you need to subclass this menu class and
+    set the `usedfor` attribute on the subclass.  `usedfor` should point to
+    the interface of the context object, so we can't do that for you.
+    """
+    facet = 'overview'
+    links = ['register_team', 'register_project', 'create_account',
+             'request_merge', 'admin_merge_people', 'admin_merge_teams']

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2009-08-18 15:28:28 +0000
+++ lib/lp/registry/browser/person.py	2009-08-18 23:27:23 +0000
@@ -199,7 +199,9 @@
 from canonical.launchpad.browser.branding import BrandingChangeView
 from lp.registry.browser.mailinglists import (
     enabled_with_active_mailing_list)
-from lp.registry.browser.menu import TopLevelMenuMixin
+from lp.registry.browser.menu import (
+    IRegistryCollectionNavigationMenu, RegistryCollectionActionMenuBase,
+    TopLevelMenuMixin)
 from lp.answers.browser.questiontarget import SearchQuestionsView
 
 from canonical.launchpad.fields import LocationField
@@ -633,7 +635,7 @@
 
     usedfor = IPersonSet
 
-    links = ['products', 'distributions', 'people', 'meetings',
+    links = ['projects', 'distributions', 'people', 'meetings',
              'register_team',
              'adminpeoplemerge', 'adminteammerge', 'mergeaccounts']
 
@@ -1335,32 +1337,15 @@
         return self.proposed_memberships or self.invited_memberships
 
 
-class IPeopleSearchNavigationMenu(Interface):
-    """Marker interface for people search navigation menu."""
-
-
-class PeopleSearchNavigationMenu(NavigationMenu, TopLevelMenuMixin):
-    """Navigation menu for people search."""
-
-    usedfor = IPeopleSearchNavigationMenu
-    facet = 'overview'
-
-    links = ['products', 'distributions', 'people', 'meetings',
-             'register_team', 'register_project']
-
-    def initialize(self):
-        """See `MenuBase`."""
-        if self.context.user is None:
-            # Make a copy of the links so as not to permanently mutate the
-            # class attribute.
-            self.links = PeopleSearchNavigationMenu.links[:]
-            self.links.append('create_account')
+class PersonSetNavigationMenu(RegistryCollectionActionMenuBase):
+    """Action menu for `PeopleSearchView`."""
+    usedfor = IPersonSet
 
 
 class PeopleSearchView(LaunchpadView):
     """Search for people and teams on the /people page."""
 
-    implements(IPeopleSearchNavigationMenu)
+    implements(IRegistryCollectionNavigationMenu)
 
     def __init__(self, context, request):
         super(PeopleSearchView, self).__init__(context, request)
@@ -1387,12 +1372,6 @@
             results = self.context.find(name)
         return BatchNavigator(results, self.request)
 
-    @property
-    def is_admin(self):
-        """Is the logged in user a Launchpad administrator?"""
-        return (self.user is not None and
-                self.user.inTeam(getUtility(ILaunchpadCelebrities).admin))
-
 
 class PersonAddView(LaunchpadFormView):
     """The page where users can create new Launchpad profiles."""

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2009-08-18 13:54:51 +0000
+++ lib/lp/registry/browser/product.py	2009-08-18 22:26:02 +0000
@@ -551,7 +551,7 @@
 
     usedfor = IProductSet
 
-    links = ['products', 'distributions', 'people', 'meetings',
+    links = ['projects', 'distributions', 'people', 'meetings',
              'all', 'register_project', 'register_team', 'review_licenses']
 
     def all(self):

=== modified file 'lib/lp/registry/browser/tests/people-views.txt'
--- lib/lp/registry/browser/tests/people-views.txt	2009-08-14 17:31:10 +0000
+++ lib/lp/registry/browser/tests/people-views.txt	2009-08-18 23:55:16 +0000
@@ -6,7 +6,7 @@
 view knows how many people and teams are currently registered in Launchpad.
 
     >>> from lp.registry.interfaces.person import IPersonSet
-    >>> login('<email address hidden>')
+    >>> login('<email address hidden>')
     >>> person_set = getUtility(IPersonSet)
 
     >>> view = create_initialized_view(person_set, '+index')
@@ -15,26 +15,6 @@
     >>> view.number_of_teams
     17
 
-The view also knows whether the logged in user, such as Foo Bar is a Launchpad
-administrator or not.
-
-    >>> view.is_admin
-    True
-
-But the anonymous user is not an administrator.
-
-    >>> logout()
-    >>> view = create_initialized_view(person_set, '+index')
-    >>> view.is_admin
-    False
-
-Sample Person is also not an administrator.
-
-    >>> login('<email address hidden>')
-    >>> view = create_initialized_view(person_set, '+index')
-    >>> view.is_admin
-    False
-
 
 Batch navigator
 ===============

=== modified file 'lib/lp/registry/templates/people-index.pt'
--- lib/lp/registry/templates/people-index.pt	2009-08-18 14:16:16 +0000
+++ lib/lp/registry/templates/people-index.pt	2009-08-18 23:52:57 +0000
@@ -14,13 +14,14 @@
 
     <tal:side metal:fill-slot="side">
       <tal:menu replace="structure view/@@+global-actions" />
+      <tal:menu replace="structure context/@@+related-pages" />
     </tal:side>
 
     <div metal:fill-slot="main">
-      There are currently <span tal:replace="view/number_of_people" /> people
-      and <span tal:replace="view/number_of_teams" /> teams registered in
-      Launchpad.
-      <tal:block define="batch view/searchPeopleBatchNavigator">
+      There are currently <tal:count tal:replace="view/number_of_people" />
+      people and <tal:count tal:replace="view/number_of_teams" /> teams
+      registered in Launchpad.
+      <tal:search define="batch view/searchPeopleBatchNavigator">
         <div>
           <form class="central" name="search" action="." method="GET">
             <table style="margin-bottom: 1em;">
@@ -108,35 +109,38 @@
         </div>
 
         <tal:block condition="not: batch">
-          <div id="application-summary">
+          <div id="application-summary"
+               tal:define="menu view/menu:overview">
             <p>Launchpad creates
             <a href="https://help.launchpad.net/YourAccount">profiles</a> for
             people based on Launchpad usage, as well as information collected
             from public sources such as bug trackers, mailing lists, public
-            key servers, and published application translations.  You can
-            <a href="+me">manage your account</a> in Launchpad, and you can
-            <a href="+newteam">create teams of people</a> for organizing
-            around common interests, discussions or permissions.</p>
+            key servers, and published application translations.
+            <tal:manage tal:condition="not: menu/create_account/enabled">You
+            can
+            <a href="/people/+me">manage your account</a> in Launchpad, and
+            you can
+            <a href="" tal:attributes="href menu/register_team/url">create
+            teams of people</a> for organizing
+            around common interests, discussions or
+            permissions.</tal:manage>  You can also
+            <a href="" tal:attributes="href menu/register_project/url">create
+            a new project</a> in Launchpad.</p>
 
             <p>Launchpad may create
             <a href="https://help.launchpad.net/YourAccount/Merging">duplicate
             profiles</a> for you as it automatically collects information from
             public sources.  If you find you have more than one profile on
-            Launchpad, you can <a href="+requestmerge">request a merge</a> of
-            the duplicate profiles.</p>
-
-            <p tal:condition="view/is_admin">As a Launchpad administrator, you
-            can also service
-            <a href="+adminpeoplemerge">person merge requests</a> and
-            <a href="+adminteammerge">team merge requests</a>.
-            </p>
+            Launchpad, you can
+            <a href="" tal:attributes="href menu/request_merge/url">request a
+            merge</a> of the duplicate profiles.</p>
           </div>
         </tal:block>
-        <tal:block condition="batch">
-          <tal:block condition="batch/multiple_pages">
+        <tal:batch tal:condition="batch">
+          <tal:multipage tal:condition="batch/has_multiple_pages">
             <tal:navigation
              replace="structure batch/@@+navigation-links-upper"/>
-          </tal:block>
+          </tal:multipage>
           <table class="listing">
             <thead>
               <tr>
@@ -155,12 +159,12 @@
               </tr>
             </tbody>
           </table>
-          <tal:block condition="batch/multiple_pages">
+          <tal:multipage condition="batch/has_multiple_pages">
             <tal:navigation
              replace="structure batch/@@+navigation-links-upper"/>
-          </tal:block>
-        </tal:block>
-      </tal:block>
+          </tal:multipage>
+        </tal:batch>
+      </tal:search>
     </div>
   </body>
 </html>