Launchpad itself

Merge lp:~bac/launchpad/bug-429802 into lp: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:~bac/launchpad/bug-429802
Merge into: lp: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:~bac/launchpad/bug-429802
Reviewer Review Type Date Requested Status
Aaron Bentley (community) code Approve
Review via email: mp+15449@code.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
=== modified file 'lib/lp/registry/doc/teammembership.txt'
--- lib/lp/registry/doc/teammembership.txt 2009-10-14 18:32:10 +0000
+++ lib/lp/registry/doc/teammembership.txt 2009-12-04 19:24:14 +0000
@@ -752,6 +752,15 @@
752There is also the getAdministratedTeams() method that returns all the752There is also the getAdministratedTeams() method that returns all the
753teams for which the person/team has admin rights.753teams for which the person/team has admin rights.
754754
755 >>> cprov_team = factory.makeTeam(owner=cprov, name="cprov-team")
756 >>> [team.name for team in cprov.getAdministratedTeams()]
757 [u'canonical-partner-dev', u'cprov-team', u'guadamen', u'launchpad-buildd-admins']
758
759If a team is merged it will not show up in the set of administered teams.
760
761 >>> login('foo.bar@canonical.com')
762 >>> cprov_team.deactivateAllMembers("Merging", foobar)
763 >>> personset.merge(cprov_team, guadamen_team)
755 >>> [team.name for team in cprov.getAdministratedTeams()]764 >>> [team.name for team in cprov.getAdministratedTeams()]
756 [u'canonical-partner-dev', u'guadamen', u'launchpad-buildd-admins']765 [u'canonical-partner-dev', u'guadamen', u'launchpad-buildd-admins']
757766
@@ -767,6 +776,7 @@
767person is an active member of as well as teams those teams are an active776person is an active member of as well as teams those teams are an active
768member of.777member of.
769778
779 >>> login('celso.providelo@canonical.com')
770 >>> print '\n'.join(sorted(780 >>> print '\n'.join(sorted(
771 ... team.name for team in salgado.teams_participated_in))781 ... team.name for team in salgado.teams_participated_in))
772 admins782 admins
@@ -892,4 +902,3 @@
892 >>> bad_membership.sendAutoRenewalNotification()902 >>> bad_membership.sendAutoRenewalNotification()
893903
894 >>> bad_membership.setStatus(TeamMembershipStatus.EXPIRED, bad_user)904 >>> bad_membership.setStatus(TeamMembershipStatus.EXPIRED, bad_user)
895
896905
=== modified file 'lib/lp/registry/doc/vocabularies.txt'
--- lib/lp/registry/doc/vocabularies.txt 2009-11-21 18:17:59 +0000
+++ lib/lp/registry/doc/vocabularies.txt 2009-12-04 19:24:14 +0000
@@ -778,10 +778,22 @@
778for 'team' should give us some of them. Notice that the778for 'team' should give us some of them. Notice that the
779PRIVATE_MEMBERSHIP_TEAM 'myteam' is not included in the results.779PRIVATE_MEMBERSHIP_TEAM 'myteam' is not included in the results.
780780
781 >>> sorted(person.name for person in vocab.search('team'))781 >>> ephemeral = factory.makeTeam(owner=foo_bar, name='ephemeral-team')
782 [u'hwdb-team', u'name18', u'name20', u'name21', u'no-team-memberships',782 >>> sorted(person.name for person in vocab.search('team'))
783 u'otherteam', u'simple-team', u'testing-spanish-team',783 [u'ephemeral-team', u'hwdb-team', u'name18', u'name20', u'name21',
784 u'ubuntu-security', u'ubuntu-team', u'warty-gnome']784 u'no-team-memberships', u'otherteam', u'simple-team',
785 u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
786 u'warty-gnome']
787
788Valid teams do not include teams that have been merged.
789
790 >>> ephemeral.deactivateAllMembers("Merging", foo_bar)
791 >>> person_set.merge(ephemeral, foo_bar)
792 >>> sorted(person.name for person in vocab.search('team'))
793 [u'hwdb-team', u'name18', u'name20', u'name21',
794 u'no-team-memberships', u'otherteam', u'simple-team',
795 u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
796 u'warty-gnome']
785797
786Logging in as 'owner', who is a member of myteam shows that the token798Logging in as 'owner', who is a member of myteam shows that the token
787lookup still omits myteam.799lookup still omits myteam.
788800
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2009-09-08 12:30:13 +0000
+++ lib/lp/registry/interfaces/person.py 2009-12-04 19:24:14 +0000
@@ -1076,7 +1076,7 @@
10761076
1077 This includes teams for which the person is the owner, a direct1077 This includes teams for which the person is the owner, a direct
1078 member with admin privilege, or member of a team with such1078 member with admin privilege, or member of a team with such
1079 privileges.1079 privileges. It excludes teams which have been merged.
1080 """1080 """
10811081
1082 def getTeamAdminsEmailAddresses():1082 def getTeamAdminsEmailAddresses():
10831083
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2009-11-15 01:05:49 +0000
+++ lib/lp/registry/model/person.py 2009-12-04 19:24:14 +0000
@@ -1374,6 +1374,7 @@
1374 owner_of_teams = Person.select('''1374 owner_of_teams = Person.select('''
1375 Person.teamowner = TeamParticipation.team1375 Person.teamowner = TeamParticipation.team
1376 AND TeamParticipation.person = %s1376 AND TeamParticipation.person = %s
1377 AND Person.merged IS NULL
1377 ''' % sqlvalues(self),1378 ''' % sqlvalues(self),
1378 clauseTables=['TeamParticipation'])1379 clauseTables=['TeamParticipation'])
1379 admin_of_teams = Person.select('''1380 admin_of_teams = Person.select('''
@@ -1381,6 +1382,7 @@
1381 AND TeamMembership.status = %(admin)s1382 AND TeamMembership.status = %(admin)s
1382 AND TeamMembership.person = TeamParticipation.team1383 AND TeamMembership.person = TeamParticipation.team
1383 AND TeamParticipation.person = %(person)s1384 AND TeamParticipation.person = %(person)s
1385 AND Person.merged IS NULL
1384 ''' % sqlvalues(person=self, admin=TeamMembershipStatus.ADMIN),1386 ''' % sqlvalues(person=self, admin=TeamMembershipStatus.ADMIN),
1385 clauseTables=['TeamParticipation', 'TeamMembership'])1387 clauseTables=['TeamParticipation', 'TeamMembership'])
1386 return admin_of_teams.union(1388 return admin_of_teams.union(
13871389
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2009-11-21 18:17:59 +0000
+++ lib/lp/registry/vocabularies.py 2009-12-04 19:24:14 +0000
@@ -473,6 +473,7 @@
473 Or(Person.visibility == PersonVisibility.PUBLIC,473 Or(Person.visibility == PersonVisibility.PUBLIC,
474 private_query,474 private_query,
475 ),475 ),
476 Person.merged == None,
476 self.extra_clause477 self.extra_clause
477 )478 )
478 )479 )
@@ -630,10 +631,6 @@
630631
631 displayname = 'Select a Team'632 displayname = 'Select a Team'
632633
633 # XXX: BradCrittenden 2008-08-11 bug=255798: This method does not return
634 # only the valid teams as the name implies because it does not account for
635 # merged teams.
636
637 # Because the base class does almost everything we need, we just need to634 # Because the base class does almost everything we need, we just need to
638 # restrict the search results to those Persons who have a non-NULL635 # restrict the search results to those Persons who have a non-NULL
639 # teamowner, i.e. a valid team.636 # teamowner, i.e. a valid team.
@@ -645,15 +642,19 @@
645 """Return the teams whose fti, IRC, or email address match :text:"""642 """Return the teams whose fti, IRC, or email address match :text:"""
646643
647 private_query, private_tables = self._privateTeamQueryAndTables()644 private_query, private_tables = self._privateTeamQueryAndTables()
648 base_query = Or(645 base_query = And(
649 Person.visibility == PersonVisibility.PUBLIC,646 Or(
650 private_query,647 Person.visibility == PersonVisibility.PUBLIC,
648 private_query,
649 ),
650 Person.merged == None
651 )651 )
652652
653 tables = [Person] + private_tables653 tables = [Person] + private_tables
654654
655 if not text:655 if not text:
656 query = And(base_query,656 query = And(base_query,
657 Person.merged == None,
657 self.extra_clause)658 self.extra_clause)
658 result = self.store.using(*tables).find(Person, query)659 result = self.store.using(*tables).find(Person, query)
659 else:660 else: