This looks good. I see have a small concern about one menu and I think
some of the links need to be handled differently.
On Wed, 2009-08-19 at 00:03 +0000, Barry Warsaw wrote:
...
>> + 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?
>
No. This a remark. I recently realised how these were implement and I was offended. I think this is a great feature that is hard to find and use. Tim did not know this existed until I showed him yesterday.
> 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! :)
>
I think you are correct.
...
>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.
>
No. I do not see that. We created the one shared menu (RegistryCollectionNavigationMenu). All the views (that already exist) just need to add the implements.
I am not sure of the value of RegistryCollectionActionMenuBase. I do not think those links are usable between collections. Those links belong to the IPersonSet only. You might as well move those links to the one true implementing class PersonSetNavigationMenu
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. :)
>
Creating a base that has only one user is a waste, remove it.
OMG! We do not have permissions on IPersonSet, or the other collections. My
suggestion may not work. Oh, I see that 'launchpad.Admin' is on Interface
by default. This /should/ work.
> +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']
I do not think this is necessary. The action menu links I see will only
every be used by the IPersonSet Menu. PersonSetActionNavigationMenu should
do the mixin, and facet, and links.
+class PersonSetNavigationMenu(RegistryCollectionActionMenuBase):
+ """Action menu for `PeopleSearchView`."""
+ usedfor = IPersonSet
The name is ambiguous and as I said above, I do not see a need for the base.
This menu is not registered on the view. I think this is the menu that all
view of IPersonSet will use.
{{{
class PersonSetActionNavigationMenu(NavigationMenu, TopLevelMenuMixin):
"""Action menu for `IPersonSet`."""
facet = 'overview'
usedfor = IPersonSet
links = ['register_team', 'register_project', 'create_account', 'request_merge', 'admin_merge_people', 'admin_merge_teams']
Hi Barry.
This looks good. I see have a small concern about one menu and I think
some of the links need to be handled differently.
On Wed, 2009-08-19 at 00:03 +0000, Barry Warsaw wrote:
...
>> + 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?
>
No. This a remark. I recently realised how these were implement and I was offended. I think this is a great feature that is hard to find and use. Tim did not know this existed until I showed him yesterday.
...
>> === modified file 'lib/lp/ registry/ browser/ person. py' registry/ browser/ person. py 2009-08-03 15:21:35 +0000 registry/ browser/ person. py 2009-08-18 15:28:28 +0000
> >> --- lib/lp/
> >> +++ lib/lp/
>
...
> implements( IRegistryCollec tionNavigationM enu)
> >
> >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! :)
>
I think you are correct.
...
>These look like the menus we want to build. tionMenu( NavigationMenu) : team',' adminpeoplemerg e', 'adminteammerge', 'mergeaccounts'] tionNavigationM enu(Interface) : ionNavigationMe nu(NavigationMe nu, TopLevelMenuMixin): tionNavigationM enu w(LaunchpadView ): IRegistryCollec tionNavigationM enu) tionNavigationM enu). All the views (that already exist) just need to add the implements.
> >
> >{{{
> >
> >class PersonSetNaviga
> > """Navigation menu for PersonSet actions"""
> > usedfor = IPersonSet
> > links = [
> > register_
> >
> >
> >class IRegistryCollec
> > """Marker interface for the registry collection navigation menu."""
> >
> >
> >class RegistryCollect
> > """Navigation menu for registry person search collection."""
> > usedfor = IRegistryCollec
> > facet = 'overview'
> > links = ['products', 'distributions', 'people', 'meetings']
> >
> >
> >class PeopleSearchVie
> > """Search for people and teams on the /people page."""
> > implements(
> >
> >}}}
>
> 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.
>
No. I do not see that. We created the one shared menu (RegistryCollec
I am not sure of the value of RegistryCollect ionActionMenuBa se. I do not think those links are usable between collections. Those links belong to the IPersonSet only. You might as well move those links to the one true implementing class PersonSetNaviga tionMenu
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. :)
>
Creating a base that has only one user is a waste, remove it.
> === modified file 'lib/lp/ registry/ browser/ menu.py' registry/ browser/ menu.py 2009-08-18 13:54:51 +0000 registry/ browser/ menu.py 2009-08-18 23:27:23 +0000
> --- lib/lp/
> +++ lib/lp/
...
> @@ -27,14 +34,72 @@ groups( self): projectgroups' , 'View project groups', icon='info') project( self): projects/ +new', text, icon='add') projects/ +new', text, icon='add', enabled=enabled)
> def meetings(self):
> return Link('/sprints/', 'View meetings', icon='info')
>
> + def project_
> + return Link('/
> +
> def register_
> text = 'Register a project'
> - return Link('/
> + enabled = self.user is not None
> + return Link('/
I do not think we want to do this. An anonymous user should see this link.
he will be prompted to login/register as needed
> def register_ team(self) : people/ +newteam' , text, icon='add') people/ +newteam' , text, icon='add', enabled=enabled)
> text = 'Register a team'
> - return Link('/
> + enabled = self.user is not None
> + return Link('/
Sam as register_project.
> def create_ account( self): people/ +login' , text, icon='add') people/ +login' , text, icon='add', enabled=enabled) merge(self) : people/ +requestmerge' , text, icon='edit', people( self): inTeam( getUtility( ILaunchpadCeleb rities) .admin) ) people/ +adminpeoplemer ge', text, icon='edit', teams(self) : inTeam( getUtility( ILaunchpadCeleb rities) .admin) ) people/ +adminteammerge ', text, icon='edit',
> text = 'Create an account'
> - return Link('/
> + # Only enable this link for anonymous users.
> + enabled = self.user is None
> + return Link('/
> +
> + def request_
> + text = 'Request a merge'
> + enabled = self.user is not None
> + return Link('/
> + enabled=enabled)
> +
> + def admin_merge_
> + text = 'Merge people'
> + enabled = (self.user is not None and
> + self.user.
> + return Link('/
> + enabled=enabled)
> +
> + def admin_merge_
> + text = 'Merge teams'
> + enabled = (self.user is not None and
> + self.user.
> + return Link('/
> + enabled=enabled)
These 'admin' links are bad. This circumvents permissions. If you look at
other menus, the links use this decorator
@enabled_ with_permission ('launchpad. Admin') people( self):
def admin_merge_
and
@enabled_ with_permission ('launchpad. Admin') teams(self) :
def admin_merge_
This leads me to ask. Did we ever develop a permission for a logged in user?
/me looks in security.py.
Yes, this /should/ work.
@enabled_ with_permission ('launchpad. View') account( self):
def create_
@enabled_ with_permission ('launchpad. View') merge(self) :
def request_
OMG! We do not have permissions on IPersonSet, or the other collections. My
suggestion may not work. Oh, I see that 'launchpad.Admin' is on Interface
by default. This /should/ work.
> +class IRegistryCollec tionNavigationM enu(Interface) : ionNavigationMe nu(NavigationMe nu, TopLevelMenuMixin): tionNavigationM enu ionActionMenuBa se(NavigationMe nu, TopLevelMenuMixin): merge_people' , 'admin_ merge_teams' ]
> + """Marker interface for top level registry collection navigation menu."""
> +
> +
> +class RegistryCollect
> + """Navigation menu for top level registry collections."""
> +
> + usedfor = IRegistryCollec
> + facet = 'overview'
> +
> + links = [
> + 'projects',
> + 'project_groups',
> + 'distributions',
> + 'people',
> + 'meetings',
> + ]
> +
> +
> +class RegistryCollect
> + """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_
I do not think this is necessary. The action menu links I see will only NavigationMenu should
every be used by the IPersonSet Menu. PersonSetAction
do the mixin, and facet, and links.
> === modified file 'lib/lp/ registry/ browser/ person. py' registry/ browser/ person. py 2009-08-18 15:28:28 +0000 registry/ browser/ person. py 2009-08-18 23:27:23 +0000
> --- lib/lp/
> +++ lib/lp/
...
+class PersonSetNaviga tionMenu( RegistryCollect ionActionMenuBa se): ew`."""
+ """Action menu for `PeopleSearchVi
+ usedfor = IPersonSet
The name is ambiguous and as I said above, I do not see a need for the base.
This menu is not registered on the view. I think this is the menu that all
view of IPersonSet will use.
{{{
class PersonSetAction NavigationMenu( NavigationMenu, TopLevelMenuMixin):
' request_ merge', 'admin_ merge_people' , 'admin_ merge_teams' ]
"""Action menu for `IPersonSet`."""
facet = 'overview'
usedfor = IPersonSet
links = ['register_team', 'register_project', 'create_account',
}}}