launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14769
[Merge] lp:~sinzui/launchpad/person-owned-teams into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/person-owned-teams into lp:launchpad.
Commit message:
List teams a person owns.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #744499 in Launchpad itself: "no listing or search of owned teams"
https://bugs.launchpad.net/launchpad/+bug/744499
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/person-owned-teams/+merge/139091
Team owners are not necessarily team members. All code that lists teams
for people are based on membership. There is no way to discover which
teams a person owns. This is a problem for cases where a person leaves
an organisation -- the team owner does not show up when auditing,
and owners can add themselves back to the team to gain team privileges.
RULES
Pre-implementation: no one
* Add a method to IPerson to get the teams a person owns. The
method must take a user argument to filter out private teams the
observing user cannot see.
* Add a related teams page to the related packages/projects group
of pages. The page must batch the teams.
QA
* Visit https://qastaging.launchpad.net/~
* Verify there is a link to owned teams.
* View the link's page.
* Verify the page lists the teams.
* Run this script:
{{{
from launchpadlib.launchpad import Launchpad
lp = Launchpad.login_with(
'testing', service_root='https://api.qastaging.launchpad.net',
version='devel')
person = lp.people['sinzui']
teams = person.getOwnedTeams()
for team in teams:
print team.name
lp = Launchpad.login_anonymously(
'testing', service_root='https://api.qastaging.launchpad.net',
version='devel')
person = lp.people['sinzui']
teams = person.getOwnedTeams()
for team in teams:
print team.name
}}}
LINT
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_person.py
lib/lp/registry/interfaces/person.py
lib/lp/registry/model/person.py
lib/lp/registry/templates/person-owned-teams.pt
lib/lp/registry/templates/person-related-software-navlinks.pt
lib/lp/registry/tests/test_person.py
LoC
I have more than 10,000 lines of credit this week.
TEST
./bin/test -vc -t OwnedTeams lp.registry
IMPLEMENTATION
I added getOwnedTeams to IPerson that return the teams for a Person that
a user is permitted to see. I exported it to use the requesting user as
the user passed to the method.
lib/lp/registry/interfaces/person.py
lib/lp/registry/model/person.py
lib/lp/registry/tests/test_person.py
I added PersonOwnedTeamsView to the related-software group of pages. I
then added a link to the IPersonRelatedSoftwareMenu to complete the
integration.
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/person.py
lib/lp/registry/browser/tests/test_person.py
lib/lp/registry/templates/person-owned-teams.pt
lib/lp/registry/templates/person-related-software-navlinks.pt
--
https://code.launchpad.net/~sinzui/launchpad/person-owned-teams/+merge/139091
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/person-owned-teams into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2012-12-07 19:59:37 +0000
+++ lib/lp/registry/browser/configure.zcml 2012-12-10 20:52:23 +0000
@@ -909,6 +909,12 @@
template="../templates/person-related-projects.pt"/>
<browser:page
for="lp.registry.interfaces.person.IPerson"
+ permission="zope.Public"
+ class="lp.registry.browser.person.PersonOwnedTeamsView"
+ name="+owned-teams"
+ template="../templates/person-owned-teams.pt"/>
+ <browser:page
+ for="lp.registry.interfaces.person.IPerson"
class="lp.registry.browser.person.PersonRelatedSoftwareView"
permission="zope.Public"
name="+related-software-navlinks"
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-12-07 19:59:53 +0000
+++ lib/lp/registry/browser/person.py 2012-12-10 20:52:23 +0000
@@ -34,6 +34,7 @@
'PersonNavigation',
'PersonOAuthTokensView',
'PersonOverviewMenu',
+ 'PersonOwnedTeamsView',
'PersonRdfContentsView',
'PersonRdfView',
'PersonRelatedSoftwareView',
@@ -632,6 +633,11 @@
enabled = bool(self.person.getAffiliatedPillars(user))
return Link(target, text, enabled=enabled, icon='info')
+ def owned_teams(self):
+ target = '+owned-teams'
+ text = 'Owned teams'
+ return Link(target, text, icon='info')
+
def subscriptions(self):
target = '+subscriptions'
text = 'Direct subscriptions'
@@ -694,6 +700,7 @@
'activate_ppa',
'maintained',
'manage_vouchers',
+ 'owned_teams',
'synchronised',
'view_ppa_subscriptions',
'ppa',
@@ -837,7 +844,7 @@
usedfor = IPersonRelatedSoftwareMenu
facet = 'overview'
links = ('related_software_summary', 'maintained', 'uploaded', 'ppa',
- 'synchronised', 'projects')
+ 'synchronised', 'projects', 'owned_teams')
@property
def person(self):
@@ -3778,6 +3785,18 @@
return "Related projects"
+class PersonOwnedTeamsView(PersonRelatedSoftwareView):
+ """View for +owned-teams."""
+ page_title = "Owned teams"
+
+ def initialize(self):
+ """Set up the batch navigation."""
+ self.batchnav = BatchNavigator(
+ self.context.getOwnedTeams(self.user), self.request)
+ self.batchnav.setHeadings('team', 'teams')
+ self.batch = list(self.batchnav.currentBatch())
+
+
class PersonOAuthTokensView(LaunchpadView):
"""Where users can see/revoke their non-expired access tokens."""
=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py 2012-12-10 13:43:47 +0000
+++ lib/lp/registry/browser/tests/test_person.py 2012-12-10 20:52:23 +0000
@@ -79,6 +79,7 @@
from lp.testing.matchers import HasQueryCount
from lp.testing.pages import (
extract_text,
+ find_tag_by_id,
setupBrowserForUser,
)
from lp.testing.views import (
@@ -970,6 +971,45 @@
self.view.max_results_to_display)
+class PersonOwnedTeamsViewTestCase(TestCaseWithFactory):
+ """Test +owned-teams view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_properties(self):
+ # The batch is created when the view is initialized.
+ owner = self.factory.makePerson()
+ team = self.factory.makeTeam(owner=owner)
+ view = create_initialized_view(owner, '+owned-teams')
+ self.assertEqual('Owned teams', view.page_title)
+ self.assertEqual('team', view.batchnav._singular_heading)
+ self.assertEqual([team], view.batch)
+
+ def test_page_text_with_teams(self):
+ owner = self.factory.makePerson(name='snarf')
+ self.factory.makeTeam(owner=owner, name='pting')
+ with person_logged_in(owner):
+ view = create_initialized_view(
+ owner, '+owned-teams', principal=owner)
+ markup = view()
+ soup = find_tag_by_id(markup, 'maincontent')
+ participation_link = 'http://launchpad.dev/~snarf/+participation'
+ self.assertIsNotNone(soup.find('a', {'href': participation_link}))
+ self.assertIsNotNone(soup.find('table', {'id': 'owned-teams'}))
+ self.assertIsNotNone(soup.find('a', {'href': '/~pting'}))
+ self.assertIsNotNone(soup.find('table', {'class': 'upper-batch-nav'}))
+ self.assertIsNotNone(soup.find('table', {'class': 'lower-batch-nav'}))
+
+ def test_page_text_without_teams(self):
+ owner = self.factory.makePerson(name='pting')
+ with person_logged_in(owner):
+ view = create_initialized_view(
+ owner, '+owned-teams', principal=owner)
+ markup = view()
+ soup = find_tag_by_id(markup, 'maincontent')
+ self.assertIsNotNone(soup.find('p', {'id': 'no-teams'}))
+
+
class TestPersonSynchronisedPackagesView(TestCaseWithFactory):
"""Test the synchronised packages view."""
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2012-11-19 21:59:03 +0000
+++ lib/lp/registry/interfaces/person.py 2012-12-10 20:52:23 +0000
@@ -1301,6 +1301,17 @@
The person's membership may be direct or indirect.
"""
+ @call_with(user=REQUEST_USER)
+ @operation_returns_collection_of(Interface) # Really ITeam
+ @export_read_operation()
+ @operation_for_version("devel")
+ def getOwnedTeams(user=None):
+ """Return the teams that this person owns.
+
+ The iterator includes the teams that the user owns, but it not
+ a member of.
+ """
+
def getAdministratedTeams():
"""Return the teams that this person/team is an administrator of.
@@ -1308,7 +1319,6 @@
member with admin privilege, or member of a team with such
privileges. It excludes teams which have been merged.
"""
-
def getTeamAdminsEmailAddresses():
"""Return a set containing the email addresses of all administrators
of this team.
@@ -2547,6 +2557,8 @@
# 'lazr.webservice.exported')['return_type'].value_type.schema = IPerson
IPersonViewRestricted['getMembersByStatus'].queryTaggedValue(
LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = IPerson
+IPersonViewRestricted['getOwnedTeams'].queryTaggedValue(
+ LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = ITeam
# Fix schema of ITeamMembership fields. Has to be done here because of
# circular dependencies.
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-12-07 10:04:07 +0000
+++ lib/lp/registry/model/person.py 2012-12-10 20:52:23 +0000
@@ -1765,6 +1765,18 @@
tm.setExpirationDate(expires, reviewer)
tm.setStatus(status, reviewer, comment=comment)
+ def getOwnedTeams(self, user=None):
+ """See `IPerson`."""
+ query = And(
+ get_person_visibility_terms(user),
+ Person.teamowner == self.id,
+ Person.merged == None)
+ store = IStore(Person)
+ results = store.find(
+ Person, query).order_by(
+ Upper(Person.displayname), Upper(Person.name))
+ return results
+
@cachedproperty
def administrated_teams(self):
return list(self.getAdministratedTeams())
=== added file 'lib/lp/registry/templates/person-owned-teams.pt'
--- lib/lp/registry/templates/person-owned-teams.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/person-owned-teams.pt 2012-12-10 20:52:23 +0000
@@ -0,0 +1,72 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad"
+>
+
+<body>
+
+<div metal:fill-slot="heading">
+ <h1 tal:content="view/page_title"/>
+</div>
+
+<div metal:fill-slot="main">
+ <div class="top-portlet">
+ <tal:navlinks replace="structure context/@@+related-software-navlinks"/>
+ </div>
+
+
+ <div id="teams" class="top-portlet">
+ <p>
+ Team owners are not always team members. The
+ <a tal:attributes="href context/menu:overview/memberships/fmt:url">team
+ participation</a> page shows the teams that
+ <tal:name replace="context/displayname" /> is a member of.
+ </p>
+
+ <tal:navigation_top
+ replace="structure view/batchnav/@@+navigation-links-upper" />
+
+ <tal:maintained-packages define="teams view/batch">
+
+ <table id="owned-teams" class="listing"
+ tal:condition="teams">
+ <cols><col width="30%" /><col width="auto" /></cols>
+
+ <thead>
+ <tr>
+ <th>Name</th>
+ <th>Summary</th>
+ </tr>
+ </thead>
+
+ <tbody>
+ <tr tal:repeat="team teams">
+ <td>
+ <a tal:replace="structure team/fmt:link" />
+ </td>
+ <td>
+ <tal:summary replace="team/description/fmt:shorten/60"/>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+
+ <tal:navigation_bottom
+ replace="structure view/batchnav/@@+navigation-links-lower" />
+
+ <p id="no-teams"
+ tal:condition="not: teams">
+ <span tal:replace="context/displayname">Foo Bar</span>
+ doesn't own any teams.
+ </p>
+ </tal:maintained-packages>
+ </div>
+
+</div>
+
+</body>
+</html>
=== modified file 'lib/lp/registry/templates/person-related-software-navlinks.pt'
--- lib/lp/registry/templates/person-related-software-navlinks.pt 2011-09-23 07:49:54 +0000
+++ lib/lp/registry/templates/person-related-software-navlinks.pt 2012-12-10 20:52:23 +0000
@@ -29,6 +29,10 @@
tal:define="link view/menu:navigation/projects"
tal:condition="link/enabled"
tal:content="structure link/fmt:link" />
+ <li
+ tal:define="link view/menu:navigation/owned_teams"
+ tal:condition="link/enabled"
+ tal:content="structure link/fmt:link" />
</ul>
</tal:root>
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2012-11-26 08:33:03 +0000
+++ lib/lp/registry/tests/test_person.py 2012-12-10 20:52:23 +0000
@@ -70,6 +70,7 @@
from lp.soyuz.enums import ArchivePurpose
from lp.testing import (
celebrity_logged_in,
+ launchpadlib_for,
login,
login_person,
logout,
@@ -266,6 +267,49 @@
retrieved_members = sorted(list(self.a_team.all_members_prepopulated))
self.assertEqual(expected_members, retrieved_members)
+ def test_getOwnedTeams(self):
+ # The interator contains the teams that person owns, regardless of
+ # membership.
+ owner = self.a_team.teamowner
+ with person_logged_in(owner):
+ owner.leave(self.a_team)
+ results = list(owner.getOwnedTeams(self.user))
+ self.assertEqual([self.a_team], results)
+
+ def test_getOwnedTeams_visibility(self):
+ # The interator contains the teams that the user can see.
+ owner = self.a_team.teamowner
+ p_team = self.factory.makeTeam(
+ name='p', owner=owner, visibility=PersonVisibility.PRIVATE)
+ results = list(owner.getOwnedTeams(self.user))
+ self.assertEqual([self.a_team], results)
+ results = list(owner.getOwnedTeams(owner))
+ self.assertEqual([self.a_team, p_team], results)
+
+ def test_getOwnedTeams_webservice(self):
+ # The user in the interaction is used as the user arg.
+ owner = self.a_team.teamowner
+ self.factory.makeTeam(
+ name='p', owner=owner, visibility=PersonVisibility.PRIVATE)
+ owner_name = owner.name
+ lp = launchpadlib_for('test', person=self.user)
+ lp_owner = lp.people[owner_name]
+ results = lp_owner.getOwnedTeams()
+ self.assertEqual(['a'], [t.name for t in results])
+
+ def test_getOwnedTeams_webservice_anonymous(self):
+ # The user in the interaction is used as the user arg.
+ # Anonymous scripts also do not reveal private teams.
+ owner = self.a_team.teamowner
+ self.factory.makeTeam(
+ name='p', owner=owner, visibility=PersonVisibility.PRIVATE)
+ owner_name = owner.name
+ logout()
+ lp = launchpadlib_for('test', person=None)
+ lp_owner = lp.people[owner_name]
+ results = lp_owner.getOwnedTeams()
+ self.assertEqual(['a'], [t.name for t in results])
+
def test_administrated_teams(self):
# The property Person.administrated_teams is a cached copy of
# the result of Person.getAdministratedTeams().
