Merge lp:~sinzui/launchpad/merge-memberships into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Merged at revision: 14904
Proposed branch: lp:~sinzui/launchpad/merge-memberships
Merge into: lp:launchpad
Diff against target: 71 lines (+36/-0)
2 files modified
lib/lp/registry/model/person.py (+18/-0)
lib/lp/registry/tests/test_personset.py (+18/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/merge-memberships
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+95703@code.launchpad.net

Description of the change

merge-person declines pending, invited team memberships.

    Launchpad bug: https://bugs.launchpad.net/bugs/58138
    Pre-implementation: no on

The UI shows merged users in the pending and invited membership state.
These users are invalid and pages oops trying to clear them from the queue.
Former memberships are also left in the DB that are shown, but links to
them 404.

Merge removed active memberships to avoid creating cyclic membership errors.
Merge needs to decline merged merged team's memberships so that historic
data does not interfere with the teams.

--------------------------------------------------------------------

RULES

    * Add a merge step that declines invited and proposed TeamMemberships.
    * There were about 85 bad memberships in production that can be
      cleanup with SQL and the help of a WebOps after this branch is
      deployed.

QA

    * Propose a team membership in another team
    * View the pending memberships to be certain you see the team listed.
    * Merge the team into a new team.
    * View the pending membership and verify that the team is not listed.

LINT

    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_personset.py

TEST

    ./bin/test -vvc -t TeamMembership lp.registry.tests.test_personset

IMPLEMENTATION

I added a new method to decline all proposed and invited memberships for
the merging team.
    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_personset.py

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

This branch looks good.

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/model/person.py'
--- lib/lp/registry/model/person.py 2012-03-01 16:33:58 +0000
+++ lib/lp/registry/model/person.py 2012-03-05 14:28:21 +0000
@@ -3935,6 +3935,23 @@
3935 'DELETE FROM TeamParticipation WHERE person = %s AND '3935 'DELETE FROM TeamParticipation WHERE person = %s AND '
3936 'team = %s' % sqlvalues(from_id, team_id))3936 'team = %s' % sqlvalues(from_id, team_id))
39373937
3938 def _mergeProposedInvitedTeamMembership(self, cur, from_id, to_id):
3939 # Memberships in an intermediate state are declined to avoid
3940 # cyclic membership errors and confusion about who the proposed
3941 # member is.
3942 TMS = TeamMembershipStatus
3943 update_template = ("""
3944 UPDATE TeamMembership
3945 SET status = %s
3946 WHERE
3947 person = %s
3948 AND status = %s
3949 """)
3950 cur.execute(update_template % sqlvalues(
3951 TMS.DECLINED, from_id, TMS.PROPOSED))
3952 cur.execute(update_template % sqlvalues(
3953 TMS.INVITATION_DECLINED, from_id, TMS.INVITED))
3954
3938 def _mergeKarmaCache(self, cur, from_id, to_id, from_karma):3955 def _mergeKarmaCache(self, cur, from_id, to_id, from_karma):
3939 # Merge the karma total cache so the user does not think the karma3956 # Merge the karma total cache so the user does not think the karma
3940 # was lost.3957 # was lost.
@@ -4218,6 +4235,7 @@
4218 src_tab, src_col, to_person.id, src_col, from_person.id))4235 src_tab, src_col, to_person.id, src_col, from_person.id))
42194236
4220 self._mergeTeamMembership(cur, from_id, to_id)4237 self._mergeTeamMembership(cur, from_id, to_id)
4238 self._mergeProposedInvitedTeamMembership(cur, from_id, to_id)
42214239
4222 # Flag the person as merged4240 # Flag the person as merged
4223 cur.execute('''4241 cur.execute('''
42244242
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2012-02-28 04:24:19 +0000
+++ lib/lp/registry/tests/test_personset.py 2012-03-05 14:28:21 +0000
@@ -32,6 +32,7 @@
32 IPersonSet,32 IPersonSet,
33 PersonCreationRationale,33 PersonCreationRationale,
34 TeamEmailAddressError,34 TeamEmailAddressError,
35 TeamMembershipStatus,
35 )36 )
36from lp.registry.interfaces.personnotification import IPersonNotificationSet37from lp.registry.interfaces.personnotification import IPersonNotificationSet
37from lp.registry.model.person import (38from lp.registry.model.person import (
@@ -574,6 +575,23 @@
574 self.assertEqual(from_person, job.from_person)575 self.assertEqual(from_person, job.from_person)
575 self.assertEqual(to_person, job.to_person)576 self.assertEqual(to_person, job.to_person)
576577
578 def test_mergeProposedInvitedTeamMembership(self):
579 # Proposed and invited memberships are declined.
580 TMS = TeamMembershipStatus
581 dupe_team = self.factory.makeTeam()
582 test_team = self.factory.makeTeam()
583 inviting_team = self.factory.makeTeam()
584 proposed_team = self.factory.makeTeam()
585 with celebrity_logged_in('admin'):
586 # Login as a user who can work with all these teams.
587 inviting_team.addMember(
588 dupe_team, inviting_team.teamowner)
589 proposed_team.addMember(
590 dupe_team, dupe_team.teamowner, status=TMS.PROPOSED)
591 self._do_merge(dupe_team, test_team, test_team.teamowner)
592 self.assertEqual(0, inviting_team.invited_member_count)
593 self.assertEqual(0, proposed_team.proposed_member_count)
594
577595
578class TestPersonSetCreateByOpenId(TestCaseWithFactory):596class TestPersonSetCreateByOpenId(TestCaseWithFactory):
579 layer = DatabaseFunctionalLayer597 layer = DatabaseFunctionalLayer