launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/merge-memberships into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #58138 in Launchpad itself: "Person merge code does not take into account inactive and proposed memberships"
  https://bugs.launchpad.net/launchpad/+bug/58138

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/merge-memberships/+merge/95703

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
-- 
https://code.launchpad.net/~sinzui/launchpad/merge-memberships/+merge/95703
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/merge-memberships into lp:launchpad.

=== 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-02 23:53:20 +0000
@@ -3935,6 +3935,23 @@
                     'DELETE FROM TeamParticipation WHERE person = %s AND '
                     'team = %s' % sqlvalues(from_id, team_id))
 
+    def _mergeProposedInvitedTeamMembership(self, cur, from_id, to_id):
+        # Memberships in an intermediate state are declined to avoid
+        # cyclic membership errors and confusion about who the proposed
+        # member is.
+        TMS = TeamMembershipStatus
+        update_template = ("""
+            UPDATE TeamMembership
+            SET status = %s
+            WHERE
+                person = %s
+                AND status = %s
+            """)
+        cur.execute(update_template % sqlvalues(
+            TMS.DECLINED, from_id, TMS.PROPOSED))
+        cur.execute(update_template % sqlvalues(
+            TMS.INVITATION_DECLINED, from_id, TMS.INVITED))
+
     def _mergeKarmaCache(self, cur, from_id, to_id, from_karma):
         # Merge the karma total cache so the user does not think the karma
         # was lost.
@@ -4218,6 +4235,7 @@
                 src_tab, src_col, to_person.id, src_col, from_person.id))
 
         self._mergeTeamMembership(cur, from_id, to_id)
+        self._mergeProposedInvitedTeamMembership(cur, from_id, to_id)
 
         # Flag the person as merged
         cur.execute('''

=== 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-02 23:53:20 +0000
@@ -32,6 +32,7 @@
     IPersonSet,
     PersonCreationRationale,
     TeamEmailAddressError,
+    TeamMembershipStatus,
     )
 from lp.registry.interfaces.personnotification import IPersonNotificationSet
 from lp.registry.model.person import (
@@ -574,6 +575,23 @@
         self.assertEqual(from_person, job.from_person)
         self.assertEqual(to_person, job.to_person)
 
+    def test_mergeProposedInvitedTeamMembership(self):
+        # Proposed and invited memberships are declined.
+        TMS = TeamMembershipStatus
+        dupe_team = self.factory.makeTeam()
+        test_team = self.factory.makeTeam()
+        inviting_team = self.factory.makeTeam()
+        proposed_team = self.factory.makeTeam()
+        with celebrity_logged_in('admin'):
+            # Login as a user who can work with all these teams.
+            inviting_team.addMember(
+                dupe_team, inviting_team.teamowner)
+            proposed_team.addMember(
+                dupe_team, dupe_team.teamowner, status=TMS.PROPOSED)
+            self._do_merge(dupe_team, test_team, test_team.teamowner)
+            self.assertEqual(0, inviting_team.invited_member_count)
+            self.assertEqual(0, proposed_team.proposed_member_count)
+
 
 class TestPersonSetCreateByOpenId(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer