Merge lp:~cjwatson/launchpad/git-personmerge into lp:launchpad

Proposed by Colin Watson on 2015-02-23
Status: Merged
Approved by: Colin Watson on 2015-02-27
Approved revision: no longer in the source branch.
Merged at revision: 17367
Proposed branch: lp:~cjwatson/launchpad/git-personmerge
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/git-finish-sharing
Diff against target: 169 lines (+67/-8)
4 files modified
lib/lp/registry/browser/peoplemerge.py (+9/-1)
lib/lp/registry/browser/tests/test_peoplemerge.py (+14/-2)
lib/lp/registry/personmerge.py (+14/-4)
lib/lp/registry/tests/test_personmerge.py (+30/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-personmerge
Reviewer Review Type Date Requested Status
William Grant code 2015-02-23 Approve on 2015-02-26
Review via email: mp+250669@code.launchpad.net

Commit message

Handle Git repositories during person merging.

Description of the change

Handle Git repositories during person merging.

To post a comment you must log in.
William Grant (wgrant) :
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/browser/peoplemerge.py'
2--- lib/lp/registry/browser/peoplemerge.py 2013-05-02 00:40:14 +0000
3+++ lib/lp/registry/browser/peoplemerge.py 2015-02-26 17:37:28 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """People Merge related wiew classes."""
10@@ -25,6 +25,7 @@
11 )
12 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
13 from lp.code.interfaces.branchcollection import IAllBranches
14+from lp.code.interfaces.gitcollection import IAllGitRepositories
15 from lp.registry.interfaces.mailinglist import (
16 MailingListStatus,
17 PURGE_STATES,
18@@ -79,6 +80,13 @@
19 _("${name} owns private branches that must be "
20 "deleted or transferred to another owner first.",
21 mapping=dict(name=dupe_person.name)))
22+ all_repositories = getUtility(IAllGitRepositories)
23+ if not all_repositories.ownedBy(
24+ dupe_person).isPrivate().is_empty():
25+ self.addError(
26+ _("${name} owns private Git repositories that must be "
27+ "deleted or transferred to another owner first.",
28+ mapping=dict(name=dupe_person.name)))
29 if dupe_person.isMergePending():
30 self.addError(_("${name} is already queued for merging.",
31 mapping=dict(name=dupe_person.name)))
32
33=== modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py'
34--- lib/lp/registry/browser/tests/test_peoplemerge.py 2013-02-06 06:32:24 +0000
35+++ lib/lp/registry/browser/tests/test_peoplemerge.py 2015-02-26 17:37:28 +0000
36@@ -1,4 +1,4 @@
37-# Copyright 2010 Canonical Ltd. This software is licensed under the
38+# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
39 # GNU Affero General Public License version 3 (see the file LICENSE).
40 """Test the peoplemerge browser module."""
41
42@@ -286,7 +286,7 @@
43 view.errors)
44
45 def test_cannot_merge_person_with_private_branches(self):
46- # A team or user with a private branches cannot be merged.
47+ # A team or user with a private branch cannot be merged.
48 self.factory.makeBranch(
49 owner=self.dupe, information_type=InformationType.USERDATA)
50 login_celebrity('registry_experts')
51@@ -297,6 +297,18 @@
52 "transferred to another owner first."],
53 view.errors)
54
55+ def test_cannot_merge_person_with_private_git_repositories(self):
56+ # A team or user with a private Git repository cannot be merged.
57+ self.factory.makeGitRepository(
58+ owner=self.dupe, information_type=InformationType.USERDATA)
59+ login_celebrity('registry_experts')
60+ view = create_initialized_view(
61+ self.person_set, '+requestmerge', form=self.getForm())
62+ self.assertEqual(
63+ [u"dupe owns private Git repositories that must be deleted or "
64+ "transferred to another owner first."],
65+ view.errors)
66+
67 def test_cannot_merge_person_with_itself(self):
68 # A IPerson cannot be merged with itself.
69 login_person(self.target)
70
71=== modified file 'lib/lp/registry/personmerge.py'
72--- lib/lp/registry/personmerge.py 2015-02-10 00:46:18 +0000
73+++ lib/lp/registry/personmerge.py 2015-02-26 17:37:28 +0000
74@@ -1,4 +1,4 @@
75-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
76+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
77 # GNU Affero General Public License version 3 (see the file LICENSE).
78
79 """Person/team merger implementation."""
80@@ -14,6 +14,7 @@
81 IAllBranches,
82 IBranchCollection,
83 )
84+from lp.code.interfaces.gitcollection import IGitCollection
85 from lp.registry.interfaces.mailinglist import (
86 IMailingListSet,
87 MailingListStatus,
88@@ -152,6 +153,13 @@
89 %(from_id)s''', dict(to_id=to_id, from_id=from_id))
90
91
92+def _mergeGitRepositories(from_person, to_person):
93+ # This shouldn't use removeSecurityProxy.
94+ repositories = getUtility(IGitCollection).ownedBy(from_person)
95+ for repository in repositories.getRepositories():
96+ removeSecurityProxy(repository).setOwner(to_person, to_person)
97+
98+
99 def _mergeSourcePackageRecipes(from_person, to_person):
100 # This shouldn't use removeSecurityProxy.
101 recipes = from_person.recipes
102@@ -691,9 +699,6 @@
103 ('bugsummaryjournal', 'viewed_by'),
104 ('latestpersonsourcepackagereleasecache', 'creator'),
105 ('latestpersonsourcepackagereleasecache', 'maintainer'),
106- # This needs handling before we deploy the git code, but can be
107- # ignored for the purpose of deploying the database tables.
108- ('gitrepository', 'owner'),
109 ]
110
111 references = list(postgresql.listReferences(cur, 'person', 'id'))
112@@ -744,6 +749,11 @@
113 _mergeBranchMergeQueues(cur, from_id, to_id)
114 skip.append(('branchmergequeue', 'owner'))
115
116+ # Update the GitRepositories that will not conflict, and fudge the names
117+ # of ones that *do* conflict.
118+ _mergeGitRepositories(from_person, to_person)
119+ skip.append(('gitrepository', 'owner'))
120+
121 _mergeSourcePackageRecipes(from_person, to_person)
122 skip.append(('sourcepackagerecipe', 'owner'))
123
124
125=== modified file 'lib/lp/registry/tests/test_personmerge.py'
126--- lib/lp/registry/tests/test_personmerge.py 2014-06-18 18:29:13 +0000
127+++ lib/lp/registry/tests/test_personmerge.py 2015-02-26 17:37:28 +0000
128@@ -1,4 +1,4 @@
129-# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
130+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
131 # GNU Affero General Public License version 3 (see the file LICENSE).
132
133 """Tests for merge_people."""
134@@ -270,6 +270,35 @@
135 self.assertEqual(2, len(branches))
136 self.assertContentEqual([u'foo', u'foo-1'], branches)
137
138+ def test_merge_moves_git_repositories(self):
139+ # When person/teams are merged, Git repositories owned by the from
140+ # person are moved.
141+ person = self.factory.makePerson()
142+ repository = self.factory.makeGitRepository()
143+ duplicate = repository.owner
144+ self._do_premerge(repository.owner, person)
145+ login_person(person)
146+ duplicate, person = self._do_merge(duplicate, person)
147+ repositories = person.getGitRepositories()
148+ self.assertEqual(1, repositories.count())
149+
150+ def test_merge_with_duplicated_git_repositories(self):
151+ # If both the from and to people have Git repositories with the same
152+ # name, merging renames the duplicate from the from person's side.
153+ project = self.factory.makeProduct()
154+ from_repository = self.factory.makeGitRepository(
155+ target=project, name=u'foo')
156+ to_repository = self.factory.makeGitRepository(
157+ target=project, name=u'foo')
158+ mergee = to_repository.owner
159+ duplicate = from_repository.owner
160+ self._do_premerge(duplicate, mergee)
161+ login_person(mergee)
162+ duplicate, mergee = self._do_merge(duplicate, mergee)
163+ repositories = [r.name for r in mergee.getGitRepositories()]
164+ self.assertEqual(2, len(repositories))
165+ self.assertContentEqual([u'foo', u'foo-1'], repositories)
166+
167 def test_merge_moves_recipes(self):
168 # When person/teams are merged, recipes owned by the from person are
169 # moved.