Launchpad itself

Merge lp://qastaging/~bac/launchpad/bug-429802 into lp://qastaging/launchpad

  1. bug-429802
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://qastaging/~bac/launchpad/bug-429802
Merge into: lp://qastaging/launchpad
Diff against target: 142 lines (+37/-13)
5 files modified
lib/lp/registry/doc/teammembership.txt (+10/-1)
lib/lp/registry/doc/vocabularies.txt (+16/-4)
lib/lp/registry/interfaces/person.py (+1/-1)
lib/lp/registry/model/person.py (+2/-0)
lib/lp/registry/vocabularies.py (+8/-7)
To merge this branch: bzr merge lp://qastaging/~bac/launchpad/bug-429802
Reviewer Review Type Date Requested Status
Aaron Bentley (community) code Approve
Review via email: mp+15449@code.qastaging.launchpad.net

Commit message

Do not include merged teams in getAdministratedTeams and ValidPersonOrTeamVocabulary.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :
Download full text (3.3 KiB)

= Summary =

Bug 429802 and bug 255798 address the issue of having merged teams included in lists
of valid teams. After a team is merged it is an artifact with no value and should
not be seen or heard from again.

== Proposed fix ==

Fix the query in getAdministratedTeams and the vocabulary to exclude teams where
Person.merged is not null.

== Pre-implementation notes ==

None but a brief discussion with Curtis.

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.registry -t teammembershipt.txt -t vocabularies.txt

== Demo and Q/A ==

As mark, go to https://launchpad.dev/people and click on the merge teams link. Merge
 hoary team into the guada team. Then go to
https://lauchpad.dev/~guadamen/+add-my-teams and ensure the hoary team is not listed.

For QA, follow the instructions in bug 429802 and ensure (by hovering) that none of
the teams shown have names that end in "-merge".

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/doc/teammembership.txt
  lib/lp/registry/vocabularies.py
  lib/lp/registry/doc/vocabularies.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/person.py

== Pyflakes Doctest notices ==

lib/lp/registry/doc/teammembership.txt
    135: 'Unauthorized' imported but unused

== Pylint notices ==

The following lint issues are crack.

lib/lp/registry/interfaces/person.py
    52: [F0401] Unable to import 'lazr.enum' (No module named enum)
    54: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    55: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    62: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    406: [E1002, PersonNameField._validate] Use super on an old style class
    1384: [C0322, IPersonEditRestricted.addMember] Operator not preceded by a space
    status=copy_field(ITeamMembership['status']),
    ^
    comment=Text(required=False))
    @export_write_operation()
    def addMember(person, reviewer, status=TeamMembershipStatus.APPROVED,
    comment=None, force_team_add=False,
    may_subscribe_to_list=True):
    1416: [C0322, IPersonEditRestricted.acceptInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def acceptInvitationToBeMemberOf(team, comment):
    1428: [C0322, IPersonEditRestricted.declineInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def declineInvitationToBeMemberOf(team, comment):
    1720: [C0322, IPersonSet.newTeam] Operator not preceded by a space
    defaultmembershipperiod='default_membership_period',
    ^
    defaultrenewalperiod='default_renewal_period')
    @operation_parameters(
    subscriptionpolicy=Choice(
    title=_('Subscription policy'), vocabulary=TeamSubscriptionPolicy,
    required=False, default=TeamSubscriptionPolicy.MODERATED))
    @export_factory_operation(
    ITeam, ['name', 'displayname', 'teamdescription',
    'defaultmembershipperiod', 'defaultrenewalperiod'])
    def newTeam(tea...

Read more...

Revision history for this message
Aaron Bentley (abentley) wrote :

Works for me.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/doc/teammembership.txt'
2--- lib/lp/registry/doc/teammembership.txt 2009-10-14 18:32:10 +0000
3+++ lib/lp/registry/doc/teammembership.txt 2009-12-04 19:24:14 +0000
4@@ -752,6 +752,15 @@
5 There is also the getAdministratedTeams() method that returns all the
6 teams for which the person/team has admin rights.
7
8+ >>> cprov_team = factory.makeTeam(owner=cprov, name="cprov-team")
9+ >>> [team.name for team in cprov.getAdministratedTeams()]
10+ [u'canonical-partner-dev', u'cprov-team', u'guadamen', u'launchpad-buildd-admins']
11+
12+If a team is merged it will not show up in the set of administered teams.
13+
14+ >>> login('foo.bar@canonical.com')
15+ >>> cprov_team.deactivateAllMembers("Merging", foobar)
16+ >>> personset.merge(cprov_team, guadamen_team)
17 >>> [team.name for team in cprov.getAdministratedTeams()]
18 [u'canonical-partner-dev', u'guadamen', u'launchpad-buildd-admins']
19
20@@ -767,6 +776,7 @@
21 person is an active member of as well as teams those teams are an active
22 member of.
23
24+ >>> login('celso.providelo@canonical.com')
25 >>> print '\n'.join(sorted(
26 ... team.name for team in salgado.teams_participated_in))
27 admins
28@@ -892,4 +902,3 @@
29 >>> bad_membership.sendAutoRenewalNotification()
30
31 >>> bad_membership.setStatus(TeamMembershipStatus.EXPIRED, bad_user)
32-
33
34=== modified file 'lib/lp/registry/doc/vocabularies.txt'
35--- lib/lp/registry/doc/vocabularies.txt 2009-11-21 18:17:59 +0000
36+++ lib/lp/registry/doc/vocabularies.txt 2009-12-04 19:24:14 +0000
37@@ -778,10 +778,22 @@
38 for 'team' should give us some of them. Notice that the
39 PRIVATE_MEMBERSHIP_TEAM 'myteam' is not included in the results.
40
41- >>> sorted(person.name for person in vocab.search('team'))
42- [u'hwdb-team', u'name18', u'name20', u'name21', u'no-team-memberships',
43- u'otherteam', u'simple-team', u'testing-spanish-team',
44- u'ubuntu-security', u'ubuntu-team', u'warty-gnome']
45+ >>> ephemeral = factory.makeTeam(owner=foo_bar, name='ephemeral-team')
46+ >>> sorted(person.name for person in vocab.search('team'))
47+ [u'ephemeral-team', u'hwdb-team', u'name18', u'name20', u'name21',
48+ u'no-team-memberships', u'otherteam', u'simple-team',
49+ u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
50+ u'warty-gnome']
51+
52+Valid teams do not include teams that have been merged.
53+
54+ >>> ephemeral.deactivateAllMembers("Merging", foo_bar)
55+ >>> person_set.merge(ephemeral, foo_bar)
56+ >>> sorted(person.name for person in vocab.search('team'))
57+ [u'hwdb-team', u'name18', u'name20', u'name21',
58+ u'no-team-memberships', u'otherteam', u'simple-team',
59+ u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
60+ u'warty-gnome']
61
62 Logging in as 'owner', who is a member of myteam shows that the token
63 lookup still omits myteam.
64
65=== modified file 'lib/lp/registry/interfaces/person.py'
66--- lib/lp/registry/interfaces/person.py 2009-09-08 12:30:13 +0000
67+++ lib/lp/registry/interfaces/person.py 2009-12-04 19:24:14 +0000
68@@ -1076,7 +1076,7 @@
69
70 This includes teams for which the person is the owner, a direct
71 member with admin privilege, or member of a team with such
72- privileges.
73+ privileges. It excludes teams which have been merged.
74 """
75
76 def getTeamAdminsEmailAddresses():
77
78=== modified file 'lib/lp/registry/model/person.py'
79--- lib/lp/registry/model/person.py 2009-11-15 01:05:49 +0000
80+++ lib/lp/registry/model/person.py 2009-12-04 19:24:14 +0000
81@@ -1374,6 +1374,7 @@
82 owner_of_teams = Person.select('''
83 Person.teamowner = TeamParticipation.team
84 AND TeamParticipation.person = %s
85+ AND Person.merged IS NULL
86 ''' % sqlvalues(self),
87 clauseTables=['TeamParticipation'])
88 admin_of_teams = Person.select('''
89@@ -1381,6 +1382,7 @@
90 AND TeamMembership.status = %(admin)s
91 AND TeamMembership.person = TeamParticipation.team
92 AND TeamParticipation.person = %(person)s
93+ AND Person.merged IS NULL
94 ''' % sqlvalues(person=self, admin=TeamMembershipStatus.ADMIN),
95 clauseTables=['TeamParticipation', 'TeamMembership'])
96 return admin_of_teams.union(
97
98=== modified file 'lib/lp/registry/vocabularies.py'
99--- lib/lp/registry/vocabularies.py 2009-11-21 18:17:59 +0000
100+++ lib/lp/registry/vocabularies.py 2009-12-04 19:24:14 +0000
101@@ -473,6 +473,7 @@
102 Or(Person.visibility == PersonVisibility.PUBLIC,
103 private_query,
104 ),
105+ Person.merged == None,
106 self.extra_clause
107 )
108 )
109@@ -630,10 +631,6 @@
110
111 displayname = 'Select a Team'
112
113- # XXX: BradCrittenden 2008-08-11 bug=255798: This method does not return
114- # only the valid teams as the name implies because it does not account for
115- # merged teams.
116-
117 # Because the base class does almost everything we need, we just need to
118 # restrict the search results to those Persons who have a non-NULL
119 # teamowner, i.e. a valid team.
120@@ -645,15 +642,19 @@
121 """Return the teams whose fti, IRC, or email address match :text:"""
122
123 private_query, private_tables = self._privateTeamQueryAndTables()
124- base_query = Or(
125- Person.visibility == PersonVisibility.PUBLIC,
126- private_query,
127+ base_query = And(
128+ Or(
129+ Person.visibility == PersonVisibility.PUBLIC,
130+ private_query,
131+ ),
132+ Person.merged == None
133 )
134
135 tables = [Person] + private_tables
136
137 if not text:
138 query = And(base_query,
139+ Person.merged == None,
140 self.extra_clause)
141 result = self.store.using(*tables).find(Person, query)
142 else: