Merge lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad
- person-merge-job-4
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 12803 | ||||||||
Proposed branch: | lp:~sinzui/launchpad/person-merge-job-4 | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
782 lines (+154/-262) 16 files modified
lib/canonical/launchpad/browser/logintoken.py (+5/-4) lib/lp/registry/browser/peoplemerge.py (+18/-30) lib/lp/registry/browser/person.py (+8/-0) lib/lp/registry/browser/tests/peoplemerge-views.txt (+9/-21) lib/lp/registry/browser/tests/person-views.txt (+1/-0) lib/lp/registry/browser/tests/test_peoplemerge.py (+50/-26) lib/lp/registry/browser/tests/test_person_view.py (+18/-4) lib/lp/registry/browser/tests/test_team.py (+17/-0) lib/lp/registry/interfaces/person.py (+12/-7) lib/lp/registry/interfaces/persontransferjob.py (+2/-2) lib/lp/registry/model/person.py (+3/-4) lib/lp/registry/stories/person/merge-people.txt (+1/-35) lib/lp/registry/stories/person/xx-adminpeoplemerge.txt (+4/-14) lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt (+0/-45) lib/lp/registry/stories/team/xx-adminteammerge.txt (+4/-68) lib/lp/registry/tests/test_person.py (+2/-2) |
||||||||
To merge this branch: | bzr merge lp:~sinzui/launchpad/person-merge-job-4 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+57037@code.launchpad.net |
Commit message
Description of the change
Enable async person/team merges from the UI.
Launchpad bug:
https:/
https:/
Pre-
Test command: ./bin/test -vv \
-t test_is_
-t /xx-adminpeople
- Update call-sites to use mergeAsync() where appropriate.
- UI to show if a user is the subject of a scheduled merge request.
-------
RULES
* Add an info notification to the person/team index page that states
the team is involved in a merge.
* Ensure a merge cannot be requested is the user/team is involved
in a queued merge
* Change the view the call Person.
explaining that the merge is queued.
* ADDENDUM. Fix the ambiguous "Constraint not satisfied" message.
QA
* Using one of the unmergeable users, perform a merge.
* Verify (quickly) you see the notification that the user is being merged.
* Verify the user is merged after a few minutes.
* Using and undeletable teams, perform a delete.
* Verify (quickly) you see the notification that the team is being merged.
* Verify the team is merged after a few minutes.
LINT
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
IMPLEMENTATION
Fixed the signature of mergeAsync() and create() methods and updated the
documentation. Added reviewer to Merge() to be compatible with the
aforementioned methods.
lib/
lib/
Revised the rule for is_merge_pending because we care about merges in
the duplicate role. It is okay for a person or team to have several
dupes pending to merge. We expect this to be the case for team deletions
which are merges into ~registry.
lib/
lib/
I deleted the duplicate validate() from the direct subclass. I think it
was left over from a refactoring. I discovered that the common validate()
is used by forms that do not provide a target user since it is assumed to
be the requester. If fixed it to fix the tests I was writing.
Added a check to ensure the duplicate and target do not have a pending merge.
Updated the logintoken and peoplemerge views to use merge_async().
Changed the messages to explain the merge is queued.
I was startled by the "Constraint not satisfied" in a test, since that is
a UI problem. I found a bug was reported about it so I fixed the message.
lib/
lib/
lib/
lib/
I deleted corner cases from these story tests because there are testing in
the view tests. I updated the stories to verify the messages users see.
lib/
lib/
Added a notification to explain that the team or person is being merged.
lib/
lib/
lib/
Curtis Hovey (sinzui) wrote : | # |
There is a False test and a True test in lib/lp/
def test_no_
# is_merge_pending returns False when this person is not the "from"
# person of an active merge job.
person = self.factory.
def test_is_
# is_merge_pending returns True when this person is being merged with
# another person in an active merge job.
from_person = self.factory.
to_person = self.factory.
Benji York (benji) wrote : | # |
On Mon, Apr 11, 2011 at 5:00 PM, Curtis Hovey
<email address hidden> wrote:
> There is a False test and a True test in lib/lp/
Ah. I missed that. Thanks.
--
Benji York
Preview Diff
1 | === modified file 'lib/canonical/launchpad/browser/logintoken.py' | |||
2 | --- lib/canonical/launchpad/browser/logintoken.py 2011-02-16 17:31:11 +0000 | |||
3 | +++ lib/canonical/launchpad/browser/logintoken.py 2011-04-12 02:08:36 +0000 | |||
4 | @@ -605,10 +605,11 @@ | |||
5 | 605 | if emailset.getByPerson(self.dupe): | 605 | if emailset.getByPerson(self.dupe): |
6 | 606 | self.mergeCompleted = False | 606 | self.mergeCompleted = False |
7 | 607 | return | 607 | return |
12 | 608 | 608 | getUtility(IPersonSet).mergeAsync( | |
13 | 609 | # Call Stuart's magic function which will reassign all of the dupe | 609 | self.dupe, requester, reviewer=requester) |
14 | 610 | # account's stuff to the user account. | 610 | merge_message = _( |
15 | 611 | getUtility(IPersonSet).merge(self.dupe, requester) | 611 | 'A merge is queued and is expected to complete in a few minutes.') |
16 | 612 | self.request.response.addInfoNotification(merge_message) | ||
17 | 612 | self.mergeCompleted = True | 613 | self.mergeCompleted = True |
18 | 613 | 614 | ||
19 | 614 | 615 | ||
20 | 615 | 616 | ||
21 | === modified file 'lib/lp/registry/browser/peoplemerge.py' | |||
22 | --- lib/lp/registry/browser/peoplemerge.py 2011-03-25 23:42:33 +0000 | |||
23 | +++ lib/lp/registry/browser/peoplemerge.py 2011-04-12 02:08:36 +0000 | |||
24 | @@ -56,14 +56,14 @@ | |||
25 | 56 | def validate(self, data): | 56 | def validate(self, data): |
26 | 57 | """Check that user is not attempting to merge a person into itself.""" | 57 | """Check that user is not attempting to merge a person into itself.""" |
27 | 58 | dupe_person = data.get('dupe_person') | 58 | dupe_person = data.get('dupe_person') |
36 | 59 | target_person = data.get('target_person') | 59 | target_person = data.get('target_person') or self.user |
37 | 60 | 60 | if dupe_person is None: | |
38 | 61 | if dupe_person == target_person and dupe_person is not None: | 61 | self.setFieldError( |
39 | 62 | self.addError(_("You can't merge ${name} into itself.", | 62 | 'dupe_person', 'The duplicate is not a valid person or team.') |
40 | 63 | mapping=dict(name=dupe_person.name))) | 63 | else: |
41 | 64 | # We cannot merge if there is a PPA with published | 64 | if dupe_person == target_person: |
42 | 65 | # packages on the duplicate, unless that PPA is removed. | 65 | self.addError(_("You can't merge ${name} into itself.", |
43 | 66 | if dupe_person is not None: | 66 | mapping=dict(name=dupe_person.name))) |
44 | 67 | dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson( | 67 | dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson( |
45 | 68 | dupe_person, statuses=[ArchiveStatus.ACTIVE, | 68 | dupe_person, statuses=[ArchiveStatus.ACTIVE, |
46 | 69 | ArchiveStatus.DELETING]) | 69 | ArchiveStatus.DELETING]) |
47 | @@ -73,6 +73,12 @@ | |||
48 | 73 | "can be merged. It may take ten minutes to remove the " | 73 | "can be merged. It may take ten minutes to remove the " |
49 | 74 | "deleted PPA's files.", | 74 | "deleted PPA's files.", |
50 | 75 | mapping=dict(name=dupe_person.name))) | 75 | mapping=dict(name=dupe_person.name))) |
51 | 76 | if dupe_person.is_merge_pending: | ||
52 | 77 | self.addError(_("${name} is already queued for merging.", | ||
53 | 78 | mapping=dict(name=dupe_person.name))) | ||
54 | 79 | if target_person is not None and target_person.is_merge_pending: | ||
55 | 80 | self.addError(_("${name} is already queued for merging.", | ||
56 | 81 | mapping=dict(name=target_person.name))) | ||
57 | 76 | 82 | ||
58 | 77 | 83 | ||
59 | 78 | class AdminMergeBaseView(ValidatingMergeView): | 84 | class AdminMergeBaseView(ValidatingMergeView): |
60 | @@ -84,7 +90,8 @@ | |||
61 | 84 | # subclasses. | 90 | # subclasses. |
62 | 85 | should_confirm_email_reassignment = False | 91 | should_confirm_email_reassignment = False |
63 | 86 | should_confirm_member_deactivation = False | 92 | should_confirm_member_deactivation = False |
65 | 87 | merge_message = _('Merge completed successfully.') | 93 | merge_message = _( |
66 | 94 | 'A merge is queued and is expected to complete in a few minutes.') | ||
67 | 88 | 95 | ||
68 | 89 | dupe_person_emails = () | 96 | dupe_person_emails = () |
69 | 90 | dupe_person = None | 97 | dupe_person = None |
70 | @@ -98,25 +105,6 @@ | |||
71 | 98 | def success_url(self): | 105 | def success_url(self): |
72 | 99 | return canonical_url(self.target_person) | 106 | return canonical_url(self.target_person) |
73 | 100 | 107 | ||
74 | 101 | def validate(self, data): | ||
75 | 102 | """Check that user is not attempting to merge a person into itself.""" | ||
76 | 103 | dupe_person = data.get('dupe_person') | ||
77 | 104 | target_person = data.get('target_person') | ||
78 | 105 | if dupe_person == target_person and dupe_person is not None: | ||
79 | 106 | self.addError(_("You can't merge ${name} into itself.", | ||
80 | 107 | mapping=dict(name=dupe_person.name))) | ||
81 | 108 | # We cannot merge the teams if there is a PPA with published | ||
82 | 109 | # packages on the duplicate person, unless that PPA is removed. | ||
83 | 110 | dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson( | ||
84 | 111 | dupe_person, statuses=[ArchiveStatus.ACTIVE, | ||
85 | 112 | ArchiveStatus.DELETING]) | ||
86 | 113 | if dupe_person_ppas is not None: | ||
87 | 114 | self.addError(_( | ||
88 | 115 | "${name} has a PPA that must be deleted before it " | ||
89 | 116 | "can be merged. It may take ten minutes to remove the " | ||
90 | 117 | "deleted PPA's files.", | ||
91 | 118 | mapping=dict(name=dupe_person.name))) | ||
92 | 119 | |||
93 | 120 | def render(self): | 108 | def render(self): |
94 | 121 | # Subclasses may define other actions that they will render manually | 109 | # Subclasses may define other actions that they will render manually |
95 | 122 | # only in certain circunstances, so don't include them in the list of | 110 | # only in certain circunstances, so don't include them in the list of |
96 | @@ -151,7 +139,7 @@ | |||
97 | 151 | naked_email.personID = self.target_person.id | 139 | naked_email.personID = self.target_person.id |
98 | 152 | naked_email.accountID = self.target_person.accountID | 140 | naked_email.accountID = self.target_person.accountID |
99 | 153 | naked_email.status = EmailAddressStatus.NEW | 141 | naked_email.status = EmailAddressStatus.NEW |
101 | 154 | getUtility(IPersonSet).merge( | 142 | job = getUtility(IPersonSet).mergeAsync( |
102 | 155 | self.dupe_person, self.target_person, reviewer=self.user) | 143 | self.dupe_person, self.target_person, reviewer=self.user) |
103 | 156 | self.request.response.addInfoNotification(self.merge_message) | 144 | self.request.response.addInfoNotification(self.merge_message) |
104 | 157 | self.next_url = self.success_url | 145 | self.next_url = self.success_url |
105 | @@ -264,7 +252,7 @@ | |||
106 | 264 | 252 | ||
107 | 265 | page_title = 'Delete' | 253 | page_title = 'Delete' |
108 | 266 | field_names = ['dupe_person', 'target_person'] | 254 | field_names = ['dupe_person', 'target_person'] |
110 | 267 | merge_message = _('Team deleted.') | 255 | merge_message = _('The team is queued to be deleted.') |
111 | 268 | 256 | ||
112 | 269 | @property | 257 | @property |
113 | 270 | def label(self): | 258 | def label(self): |
114 | 271 | 259 | ||
115 | === modified file 'lib/lp/registry/browser/person.py' | |||
116 | --- lib/lp/registry/browser/person.py 2011-03-11 00:06:42 +0000 | |||
117 | +++ lib/lp/registry/browser/person.py 2011-04-12 02:08:36 +0000 | |||
118 | @@ -3391,6 +3391,14 @@ | |||
119 | 3391 | 3391 | ||
120 | 3392 | def initialize(self): | 3392 | def initialize(self): |
121 | 3393 | super(PersonIndexView, self).initialize() | 3393 | super(PersonIndexView, self).initialize() |
122 | 3394 | if self.context.is_merge_pending: | ||
123 | 3395 | if self.context.is_team: | ||
124 | 3396 | merge_action = 'merged or deleted' | ||
125 | 3397 | else: | ||
126 | 3398 | merge_action = 'merged' | ||
127 | 3399 | self.request.response.addInfoNotification( | ||
128 | 3400 | "%s is queued to be be %s in a few minutes." % ( | ||
129 | 3401 | self.context.displayname, merge_action)) | ||
130 | 3394 | if self.request.method == "POST": | 3402 | if self.request.method == "POST": |
131 | 3395 | self.processForm() | 3403 | self.processForm() |
132 | 3396 | 3404 | ||
133 | 3397 | 3405 | ||
134 | === modified file 'lib/lp/registry/browser/tests/peoplemerge-views.txt' | |||
135 | --- lib/lp/registry/browser/tests/peoplemerge-views.txt 2011-03-25 13:44:19 +0000 | |||
136 | +++ lib/lp/registry/browser/tests/peoplemerge-views.txt 2011-04-12 02:08:36 +0000 | |||
137 | @@ -31,13 +31,10 @@ | |||
138 | 31 | ... person_set, '+adminteammerge', form=form) | 31 | ... person_set, '+adminteammerge', form=form) |
139 | 32 | >>> len(view.errors) | 32 | >>> len(view.errors) |
140 | 33 | 0 | 33 | 0 |
148 | 34 | 34 | >>> for notification in view.request.response.notifications: | |
149 | 35 | The old team is now renamed but the target team remains. | 35 | ... print notification.message |
150 | 36 | 36 | A merge is queued and is expected to complete in a few minutes. | |
151 | 37 | >>> print person_set.getByName('name21') | 37 | |
145 | 38 | None | ||
146 | 39 | >>> print person_set.getByName('ubuntu-team').displayname | ||
147 | 40 | Ubuntu Team | ||
152 | 41 | 38 | ||
153 | 42 | Attempting to merge a non-existent team results in an error. | 39 | Attempting to merge a non-existent team results in an error. |
154 | 43 | 40 | ||
155 | @@ -76,12 +73,9 @@ | |||
156 | 76 | >>> view = create_initialized_view( | 73 | >>> view = create_initialized_view( |
157 | 77 | ... person_set, '+requestmerge', form=form) | 74 | ... person_set, '+requestmerge', form=form) |
158 | 78 | >>> len(view.errors) | 75 | >>> len(view.errors) |
163 | 79 | 1 | 76 | 2 |
160 | 80 | |||
161 | 81 | # XXX Salgado, 2009-07-08, bug=396410: There should be a meaningful | ||
162 | 82 | # error message here instead of this one. | ||
164 | 83 | >>> print view.getFieldError('dupe_person') | 77 | >>> print view.getFieldError('dupe_person') |
166 | 84 | Constraint not satisfied | 78 | The duplicate is not a valid person or team. |
167 | 85 | 79 | ||
168 | 86 | Admins, on the other hand, are allowed to merge people without a single email | 80 | Admins, on the other hand, are allowed to merge people without a single email |
169 | 87 | address. | 81 | address. |
170 | @@ -181,18 +175,15 @@ | |||
171 | 181 | 175 | ||
172 | 182 | >>> for notification in view.request.response.notifications: | 176 | >>> for notification in view.request.response.notifications: |
173 | 183 | ... print notification.message | 177 | ... print notification.message |
175 | 184 | Team deleted. | 178 | The team is queued to be deleted. |
176 | 185 | 179 | ||
177 | 186 | >>> print view.next_url | 180 | >>> print view.next_url |
178 | 187 | http://launchpad.dev/people | 181 | http://launchpad.dev/people |
179 | 188 | 182 | ||
180 | 189 | >>> print person_set.getByName('deletable') | ||
181 | 190 | None | ||
182 | 191 | |||
183 | 192 | The delete team view cannot be hacked to delete another team, or change | 183 | The delete team view cannot be hacked to delete another team, or change |
184 | 193 | the team that the merge operation is performed with. | 184 | the team that the merge operation is performed with. |
185 | 194 | 185 | ||
187 | 195 | >>> deletable_team = factory.makeTeam(owner=team_owner, name='deletable') | 186 | >>> deletable_team = factory.makeTeam(owner=team_owner, name='delete-me') |
188 | 196 | >>> transaction.commit() | 187 | >>> transaction.commit() |
189 | 197 | >>> form = { | 188 | >>> form = { |
190 | 198 | ... 'field.target_person': 'rosetta-admins', | 189 | ... 'field.target_person': 'rosetta-admins', |
191 | @@ -206,9 +197,6 @@ | |||
192 | 206 | >>> view.request.response.notifications | 197 | >>> view.request.response.notifications |
193 | 207 | [] | 198 | [] |
194 | 208 | 199 | ||
195 | 209 | >>> print deletable_team.name | ||
196 | 210 | deletable | ||
197 | 211 | |||
198 | 212 | A team with a mailing list cannot be deleted. | 200 | A team with a mailing list cannot be deleted. |
199 | 213 | 201 | ||
200 | 214 | >>> team, mailing_list = factory.makeTeamAndMailingList( | 202 | >>> team, mailing_list = factory.makeTeamAndMailingList( |
201 | @@ -246,4 +234,4 @@ | |||
202 | 246 | [] | 234 | [] |
203 | 247 | >>> for notification in view.request.response.notifications: | 235 | >>> for notification in view.request.response.notifications: |
204 | 248 | ... print notification.message | 236 | ... print notification.message |
206 | 249 | Team deleted. | 237 | The team is queued to be deleted. |
207 | 250 | 238 | ||
208 | === modified file 'lib/lp/registry/browser/tests/person-views.txt' | |||
209 | --- lib/lp/registry/browser/tests/person-views.txt 2010-12-06 23:18:20 +0000 | |||
210 | +++ lib/lp/registry/browser/tests/person-views.txt 2011-04-12 02:08:36 +0000 | |||
211 | @@ -121,6 +121,7 @@ | |||
212 | 121 | email addresses state is LOGIN_REQUIRED, there is no description, nor | 121 | email addresses state is LOGIN_REQUIRED, there is no description, nor |
213 | 122 | are there any email addresses. | 122 | are there any email addresses. |
214 | 123 | 123 | ||
215 | 124 | >>> login(ANONYMOUS) | ||
216 | 124 | >>> view = create_initialized_view(mark, '+index') | 125 | >>> view = create_initialized_view(mark, '+index') |
217 | 125 | >>> view.email_address_visibility.is_login_required | 126 | >>> view.email_address_visibility.is_login_required |
218 | 126 | True | 127 | True |
219 | 127 | 128 | ||
220 | === modified file 'lib/lp/registry/browser/tests/test_peoplemerge.py' | |||
221 | --- lib/lp/registry/browser/tests/test_peoplemerge.py 2011-03-25 13:44:19 +0000 | |||
222 | +++ lib/lp/registry/browser/tests/test_peoplemerge.py 2011-04-12 02:08:36 +0000 | |||
223 | @@ -6,15 +6,12 @@ | |||
224 | 6 | 6 | ||
225 | 7 | from zope.component import getUtility | 7 | from zope.component import getUtility |
226 | 8 | 8 | ||
227 | 9 | from canonical.launchpad.interfaces.emailaddress import ( | ||
228 | 10 | EmailAddressStatus, | ||
229 | 11 | IEmailAddressSet, | ||
230 | 12 | ) | ||
231 | 13 | from canonical.testing.layers import DatabaseFunctionalLayer | 9 | from canonical.testing.layers import DatabaseFunctionalLayer |
232 | 14 | from lp.registry.interfaces.person import ( | 10 | from lp.registry.interfaces.person import ( |
233 | 15 | IPersonSet, | 11 | IPersonSet, |
234 | 16 | TeamSubscriptionPolicy, | 12 | TeamSubscriptionPolicy, |
235 | 17 | ) | 13 | ) |
236 | 14 | from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource | ||
237 | 18 | from lp.testing import ( | 15 | from lp.testing import ( |
238 | 19 | login_celebrity, | 16 | login_celebrity, |
239 | 20 | login_person, | 17 | login_person, |
240 | @@ -27,33 +24,70 @@ | |||
241 | 27 | ) | 24 | ) |
242 | 28 | 25 | ||
243 | 29 | 26 | ||
245 | 30 | class TestRequestPeopleMergeView(TestCaseWithFactory): | 27 | class TestValidatingMergeView(TestCaseWithFactory): |
246 | 31 | 28 | ||
247 | 32 | layer = DatabaseFunctionalLayer | 29 | layer = DatabaseFunctionalLayer |
248 | 33 | 30 | ||
249 | 34 | def setUp(self): | 31 | def setUp(self): |
251 | 35 | super(TestRequestPeopleMergeView, self).setUp() | 32 | super(TestValidatingMergeView, self).setUp() |
252 | 36 | self.person_set = getUtility(IPersonSet) | 33 | self.person_set = getUtility(IPersonSet) |
253 | 37 | self.dupe = self.factory.makePerson(name='dupe') | 34 | self.dupe = self.factory.makePerson(name='dupe') |
255 | 38 | self.target = self.factory.makeTeam(name='target') | 35 | self.target = self.factory.makePerson(name='target') |
256 | 36 | |||
257 | 37 | def getForm(self, dupe_name=None): | ||
258 | 38 | if dupe_name is None: | ||
259 | 39 | dupe_name = self.dupe.name | ||
260 | 40 | return { | ||
261 | 41 | 'field.dupe_person': dupe_name, | ||
262 | 42 | 'field.target_person': self.target.name, | ||
263 | 43 | 'field.actions.continue': 'Continue', | ||
264 | 44 | } | ||
265 | 39 | 45 | ||
266 | 40 | def test_cannot_merge_person_with_ppas(self): | 46 | def test_cannot_merge_person_with_ppas(self): |
267 | 41 | # A team with a PPA cannot be merged. | 47 | # A team with a PPA cannot be merged. |
268 | 42 | login_celebrity('admin') | 48 | login_celebrity('admin') |
269 | 43 | archive = self.dupe.createPPA() | 49 | archive = self.dupe.createPPA() |
270 | 44 | login_celebrity('registry_experts') | 50 | login_celebrity('registry_experts') |
276 | 45 | form = { | 51 | view = create_initialized_view( |
277 | 46 | 'field.dupe_person': self.dupe.name, | 52 | self.person_set, '+requestmerge', form=self.getForm()) |
278 | 47 | 'field.target_person': self.target.name, | 53 | self.assertEqual( |
279 | 48 | 'field.actions.continue': 'Continue', | 54 | [u"dupe has a PPA that must be deleted before it can be " |
280 | 49 | } | 55 | "merged. It may take ten minutes to remove the deleted PPA's " |
281 | 56 | "files."], | ||
282 | 57 | view.errors) | ||
283 | 58 | |||
284 | 59 | def test_cannot_merge_person_with_itself(self): | ||
285 | 60 | # A IPerson cannot be merged with itself. | ||
286 | 61 | login_person(self.target) | ||
287 | 62 | form = self.getForm(dupe_name=self.target.name) | ||
288 | 50 | view = create_initialized_view( | 63 | view = create_initialized_view( |
289 | 51 | self.person_set, '+requestmerge', form=form) | 64 | self.person_set, '+requestmerge', form=form) |
290 | 52 | self.assertEqual( | 65 | self.assertEqual( |
295 | 53 | [u"dupe has a PPA that must be deleted before it can be " | 66 | ["You can't merge target into itself."], view.errors) |
296 | 54 | "merged. It may take ten minutes to remove the deleted PPA's " | 67 | |
297 | 55 | "files."], | 68 | def test_cannot_merge_dupe_person_with_an_existing_merge_job(self): |
298 | 56 | view.errors) | 69 | # A merge cannot be requested for an IPerson if it there is a job |
299 | 70 | # queued to merge it into another IPerson. | ||
300 | 71 | job_source = getUtility(IPersonMergeJobSource) | ||
301 | 72 | duplicate_job = job_source.create( | ||
302 | 73 | from_person=self.dupe, to_person=self.target) | ||
303 | 74 | login_person(self.target) | ||
304 | 75 | view = create_initialized_view( | ||
305 | 76 | self.person_set, '+requestmerge', form=self.getForm()) | ||
306 | 77 | self.assertEqual( | ||
307 | 78 | ["dupe is already queued for merging."], view.errors) | ||
308 | 79 | |||
309 | 80 | def test_cannot_merge_target_person_with_an_existing_merge_job(self): | ||
310 | 81 | # A merge cannot be requested for an IPerson if it there is a job | ||
311 | 82 | # queued to merge it into another IPerson. | ||
312 | 83 | job_source = getUtility(IPersonMergeJobSource) | ||
313 | 84 | duplicate_job = job_source.create( | ||
314 | 85 | from_person=self.target, to_person=self.dupe) | ||
315 | 86 | login_person(self.target) | ||
316 | 87 | view = create_initialized_view( | ||
317 | 88 | self.person_set, '+requestmerge', form=self.getForm()) | ||
318 | 89 | self.assertEqual( | ||
319 | 90 | ["target is already queued for merging."], view.errors) | ||
320 | 57 | 91 | ||
321 | 58 | 92 | ||
322 | 59 | class TestRequestPeopleMergeMultipleEmailsView(TestCaseWithFactory): | 93 | class TestRequestPeopleMergeMultipleEmailsView(TestCaseWithFactory): |
323 | @@ -135,16 +169,6 @@ | |||
324 | 135 | return create_initialized_view( | 169 | return create_initialized_view( |
325 | 136 | self.person_set, '+adminteammerge', form=form) | 170 | self.person_set, '+adminteammerge', form=form) |
326 | 137 | 171 | ||
327 | 138 | def test_merge_team_with_email_address(self): | ||
328 | 139 | # Team email addresses are not transferred. | ||
329 | 140 | self.factory.makeEmail( | ||
330 | 141 | "del@ex.dom", self.dupe_team, email_status=EmailAddressStatus.NEW) | ||
331 | 142 | view = self.getView() | ||
332 | 143 | self.assertEqual([], view.errors) | ||
333 | 144 | self.assertEqual(self.target_team, self.dupe_team.merged) | ||
334 | 145 | emails = getUtility(IEmailAddressSet).getByPerson(self.target_team) | ||
335 | 146 | self.assertEqual(0, emails.count()) | ||
336 | 147 | |||
337 | 148 | def test_cannot_merge_team_with_ppa(self): | 172 | def test_cannot_merge_team_with_ppa(self): |
338 | 149 | # A team with a PPA cannot be merged. | 173 | # A team with a PPA cannot be merged. |
339 | 150 | login_celebrity('admin') | 174 | login_celebrity('admin') |
340 | 151 | 175 | ||
341 | === modified file 'lib/lp/registry/browser/tests/test_person_view.py' | |||
342 | --- lib/lp/registry/browser/tests/test_person_view.py 2011-03-08 23:06:21 +0000 | |||
343 | +++ lib/lp/registry/browser/tests/test_person_view.py 2011-04-12 02:08:36 +0000 | |||
344 | @@ -6,10 +6,7 @@ | |||
345 | 6 | import transaction | 6 | import transaction |
346 | 7 | from storm.expr import LeftJoin | 7 | from storm.expr import LeftJoin |
347 | 8 | from storm.store import Store | 8 | from storm.store import Store |
352 | 9 | from testtools.matchers import ( | 9 | from testtools.matchers import LessThan |
349 | 10 | Equals, | ||
350 | 11 | LessThan, | ||
351 | 12 | ) | ||
353 | 13 | from zope.component import getUtility | 10 | from zope.component import getUtility |
354 | 14 | 11 | ||
355 | 15 | from canonical.config import config | 12 | from canonical.config import config |
356 | @@ -43,6 +40,7 @@ | |||
357 | 43 | PersonVisibility, | 40 | PersonVisibility, |
358 | 44 | IPersonSet, | 41 | IPersonSet, |
359 | 45 | ) | 42 | ) |
360 | 43 | from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource | ||
361 | 46 | from lp.registry.interfaces.teammembership import ( | 44 | from lp.registry.interfaces.teammembership import ( |
362 | 47 | ITeamMembershipSet, | 45 | ITeamMembershipSet, |
363 | 48 | TeamMembershipStatus, | 46 | TeamMembershipStatus, |
364 | @@ -69,6 +67,22 @@ | |||
365 | 69 | ) | 67 | ) |
366 | 70 | 68 | ||
367 | 71 | 69 | ||
368 | 70 | class TestPersonIndexView(TestCaseWithFactory): | ||
369 | 71 | |||
370 | 72 | layer = DatabaseFunctionalLayer | ||
371 | 73 | |||
372 | 74 | def test_is_merge_pending(self): | ||
373 | 75 | dupe_person = self.factory.makePerson(name='finch') | ||
374 | 76 | target_person = self.factory.makePerson() | ||
375 | 77 | job_source = getUtility(IPersonMergeJobSource) | ||
376 | 78 | job_source.create(from_person=dupe_person, to_person=target_person) | ||
377 | 79 | view = create_initialized_view(dupe_person, name="+index") | ||
378 | 80 | notifications = view.request.response.notifications | ||
379 | 81 | message = 'Finch is queued to be be merged in a few minutes.' | ||
380 | 82 | self.assertEqual(1, len(notifications)) | ||
381 | 83 | self.assertEqual(message, notifications[0].message) | ||
382 | 84 | |||
383 | 85 | |||
384 | 72 | class TestPersonViewKarma(TestCaseWithFactory): | 86 | class TestPersonViewKarma(TestCaseWithFactory): |
385 | 73 | 87 | ||
386 | 74 | layer = LaunchpadZopelessLayer | 88 | layer = LaunchpadZopelessLayer |
387 | 75 | 89 | ||
388 | === modified file 'lib/lp/registry/browser/tests/test_team.py' | |||
389 | --- lib/lp/registry/browser/tests/test_team.py 2010-12-03 00:03:15 +0000 | |||
390 | +++ lib/lp/registry/browser/tests/test_team.py 2011-04-12 02:08:36 +0000 | |||
391 | @@ -3,9 +3,12 @@ | |||
392 | 3 | 3 | ||
393 | 4 | __metaclass__ = type | 4 | __metaclass__ = type |
394 | 5 | 5 | ||
395 | 6 | from zope.component import getUtility | ||
396 | 7 | |||
397 | 6 | from canonical.launchpad.webapp.publisher import canonical_url | 8 | from canonical.launchpad.webapp.publisher import canonical_url |
398 | 7 | from canonical.testing.layers import DatabaseFunctionalLayer | 9 | from canonical.testing.layers import DatabaseFunctionalLayer |
399 | 8 | from lp.registry.browser.person import TeamOverviewMenu | 10 | from lp.registry.browser.person import TeamOverviewMenu |
400 | 11 | from lp.registry.interfaces.persontransferjob import IPersonMergeJobSource | ||
401 | 9 | from lp.testing import ( | 12 | from lp.testing import ( |
402 | 10 | login_person, | 13 | login_person, |
403 | 11 | TestCaseWithFactory, | 14 | TestCaseWithFactory, |
404 | @@ -149,3 +152,17 @@ | |||
405 | 149 | def test_add_member_step_title(self): | 152 | def test_add_member_step_title(self): |
406 | 150 | view = create_initialized_view(self.team, '+index') | 153 | view = create_initialized_view(self.team, '+index') |
407 | 151 | self.assertEqual('Search', view.add_member_step_title) | 154 | self.assertEqual('Search', view.add_member_step_title) |
408 | 155 | |||
409 | 156 | def test_is_merge_pending(self): | ||
410 | 157 | target_team = self.factory.makeTeam() | ||
411 | 158 | job_source = getUtility(IPersonMergeJobSource) | ||
412 | 159 | job_source.create( | ||
413 | 160 | from_person=self.team, to_person=target_team, | ||
414 | 161 | reviewer=target_team.teamowner) | ||
415 | 162 | view = create_initialized_view(self.team, name="+index") | ||
416 | 163 | notifications = view.request.response.notifications | ||
417 | 164 | message = ( | ||
418 | 165 | 'Test Team is queued to be be merged or deleted ' | ||
419 | 166 | 'in a few minutes.') | ||
420 | 167 | self.assertEqual(1, len(notifications)) | ||
421 | 168 | self.assertEqual(message, notifications[0].message) | ||
422 | 152 | 169 | ||
423 | === modified file 'lib/lp/registry/interfaces/person.py' | |||
424 | --- lib/lp/registry/interfaces/person.py 2011-03-23 16:28:51 +0000 | |||
425 | +++ lib/lp/registry/interfaces/person.py 2011-04-12 02:08:36 +0000 | |||
426 | @@ -1428,12 +1428,14 @@ | |||
427 | 1428 | "The number of real people who are members of this team.") | 1428 | "The number of real people who are members of this team.") |
428 | 1429 | # activemembers.value_type.schema will be set to IPerson once | 1429 | # activemembers.value_type.schema will be set to IPerson once |
429 | 1430 | # IPerson is defined. | 1430 | # IPerson is defined. |
431 | 1431 | activemembers = Attribute('List of direct members with ADMIN or APPROVED status') | 1431 | activemembers = Attribute( |
432 | 1432 | 'List of direct members with ADMIN or APPROVED status') | ||
433 | 1432 | # For the API we need eager loading | 1433 | # For the API we need eager loading |
434 | 1433 | api_activemembers = exported( | 1434 | api_activemembers = exported( |
435 | 1434 | doNotSnapshot( | 1435 | doNotSnapshot( |
436 | 1435 | CollectionField( | 1436 | CollectionField( |
438 | 1436 | title=_("List of direct members with ADMIN or APPROVED status"), | 1437 | title=_( |
439 | 1438 | "List of direct members with ADMIN or APPROVED status"), | ||
440 | 1437 | value_type=Reference(schema=Interface))), | 1439 | value_type=Reference(schema=Interface))), |
441 | 1438 | exported_as='members') | 1440 | exported_as='members') |
442 | 1439 | adminmembers = exported( | 1441 | adminmembers = exported( |
443 | @@ -2170,7 +2172,7 @@ | |||
444 | 2170 | def latest_teams(limit=5): | 2172 | def latest_teams(limit=5): |
445 | 2171 | """Return the latest teams registered, up to the limit specified.""" | 2173 | """Return the latest teams registered, up to the limit specified.""" |
446 | 2172 | 2174 | ||
448 | 2173 | def mergeAsync(from_person, to_person): | 2175 | def mergeAsync(from_person, to_person, reviewer=None): |
449 | 2174 | """Merge a person/team into another asynchronously. | 2176 | """Merge a person/team into another asynchronously. |
450 | 2175 | 2177 | ||
451 | 2176 | This schedules a call to `merge()` to happen outside of the current | 2178 | This schedules a call to `merge()` to happen outside of the current |
452 | @@ -2179,10 +2181,13 @@ | |||
453 | 2179 | guaranteed to succeed. If either user is in a pending person merge | 2181 | guaranteed to succeed. If either user is in a pending person merge |
454 | 2180 | job, None is returned. | 2182 | job, None is returned. |
455 | 2181 | 2183 | ||
456 | 2184 | :param from_person: An IPerson or ITeam that is a duplicate. | ||
457 | 2185 | :param to_person: An IPerson or ITeam that is a master. | ||
458 | 2186 | :param reviewer: An IPerson who approved the ITeam merger. | ||
459 | 2182 | :return: A `PersonMergeJob` or None. | 2187 | :return: A `PersonMergeJob` or None. |
460 | 2183 | """ | 2188 | """ |
461 | 2184 | 2189 | ||
463 | 2185 | def merge(from_person, to_person): | 2190 | def merge(from_person, to_person, reviewer=None): |
464 | 2186 | """Merge a person/team into another. | 2191 | """Merge a person/team into another. |
465 | 2187 | 2192 | ||
466 | 2188 | The old person/team (from_person) will be left as an atavism. | 2193 | The old person/team (from_person) will be left as an atavism. |
467 | @@ -2196,9 +2201,9 @@ | |||
468 | 2196 | passing deactivate_members=True. In that case the user who's | 2201 | passing deactivate_members=True. In that case the user who's |
469 | 2197 | performing the merge must be provided as well. | 2202 | performing the merge must be provided as well. |
470 | 2198 | 2203 | ||
474 | 2199 | We are not yet game to delete the `from_person` entry from the | 2204 | :param from_person: An IPerson or ITeam that is a duplicate. |
475 | 2200 | database yet. We will let it roll for a while and see what cruft | 2205 | :param to_person: An IPerson or ITeam that is a master. |
476 | 2201 | develops. -- StuartBishop 20050812 | 2206 | :param reviewer: An IPerson who approved the ITeam merger. |
477 | 2202 | """ | 2207 | """ |
478 | 2203 | 2208 | ||
479 | 2204 | def getValidPersons(persons): | 2209 | def getValidPersons(persons): |
480 | 2205 | 2210 | ||
481 | === modified file 'lib/lp/registry/interfaces/persontransferjob.py' | |||
482 | --- lib/lp/registry/interfaces/persontransferjob.py 2011-03-26 21:50:45 +0000 | |||
483 | +++ lib/lp/registry/interfaces/persontransferjob.py 2011-04-12 02:08:36 +0000 | |||
484 | @@ -102,7 +102,7 @@ | |||
485 | 102 | class IPersonMergeJobSource(IJobSource): | 102 | class IPersonMergeJobSource(IJobSource): |
486 | 103 | """An interface for acquiring IPersonMergeJobs.""" | 103 | """An interface for acquiring IPersonMergeJobs.""" |
487 | 104 | 104 | ||
489 | 105 | def create(from_person, to_person, review=None): | 105 | def create(from_person, to_person, reviewer=None): |
490 | 106 | """Create a new IPersonMergeJob. | 106 | """Create a new IPersonMergeJob. |
491 | 107 | 107 | ||
492 | 108 | None is returned if either the from_person or to_person are already | 108 | None is returned if either the from_person or to_person are already |
493 | @@ -111,7 +111,7 @@ | |||
494 | 111 | 111 | ||
495 | 112 | :param from_person: An IPerson or ITeam that is a duplicate. | 112 | :param from_person: An IPerson or ITeam that is a duplicate. |
496 | 113 | :param to_person: An IPerson or ITeam that is a master. | 113 | :param to_person: An IPerson or ITeam that is a master. |
498 | 114 | :param review: An IPerson who approved ITeam merger. | 114 | :param reviewer: An IPerson who approved ITeam merger. |
499 | 115 | """ | 115 | """ |
500 | 116 | 116 | ||
501 | 117 | def find(from_person=None, to_person=None, any_person=False): | 117 | def find(from_person=None, to_person=None, any_person=False): |
502 | 118 | 118 | ||
503 | === modified file 'lib/lp/registry/model/person.py' | |||
504 | --- lib/lp/registry/model/person.py 2011-04-06 17:08:45 +0000 | |||
505 | +++ lib/lp/registry/model/person.py 2011-04-12 02:08:36 +0000 | |||
506 | @@ -2076,8 +2076,7 @@ | |||
507 | 2076 | def is_merge_pending(self): | 2076 | def is_merge_pending(self): |
508 | 2077 | """See `IPublicPerson`.""" | 2077 | """See `IPublicPerson`.""" |
509 | 2078 | return not getUtility( | 2078 | return not getUtility( |
512 | 2079 | IPersonMergeJobSource).find( | 2079 | IPersonMergeJobSource).find(from_person=self).is_empty() |
511 | 2080 | from_person=self, to_person=self, any_person=True).is_empty() | ||
513 | 2081 | 2080 | ||
514 | 2082 | def visibilityConsistencyWarning(self, new_value): | 2081 | def visibilityConsistencyWarning(self, new_value): |
515 | 2083 | """Warning used when changing the team's visibility. | 2082 | """Warning used when changing the team's visibility. |
516 | @@ -3836,10 +3835,10 @@ | |||
517 | 3836 | naked_from_team.retractTeamMembership(team, reviewer) | 3835 | naked_from_team.retractTeamMembership(team, reviewer) |
518 | 3837 | IStore(from_team).flush() | 3836 | IStore(from_team).flush() |
519 | 3838 | 3837 | ||
521 | 3839 | def mergeAsync(self, from_person, to_person): | 3838 | def mergeAsync(self, from_person, to_person, reviewer=None): |
522 | 3840 | """See `IPersonSet`.""" | 3839 | """See `IPersonSet`.""" |
523 | 3841 | return getUtility(IPersonMergeJobSource).create( | 3840 | return getUtility(IPersonMergeJobSource).create( |
525 | 3842 | from_person=from_person, to_person=to_person) | 3841 | from_person=from_person, to_person=to_person, reviewer=reviewer) |
526 | 3843 | 3842 | ||
527 | 3844 | def merge(self, from_person, to_person, reviewer=None): | 3843 | def merge(self, from_person, to_person, reviewer=None): |
528 | 3845 | """See `IPersonSet`.""" | 3844 | """See `IPersonSet`.""" |
529 | 3846 | 3845 | ||
530 | === modified file 'lib/lp/registry/stories/person/merge-people.txt' | |||
531 | --- lib/lp/registry/stories/person/merge-people.txt 2010-01-12 00:19:20 +0000 | |||
532 | +++ lib/lp/registry/stories/person/merge-people.txt 2011-04-12 02:08:36 +0000 | |||
533 | @@ -12,12 +12,6 @@ | |||
534 | 12 | >>> foo = factory.makePerson(name='foo', email='foo@baz.com') | 12 | >>> foo = factory.makePerson(name='foo', email='foo@baz.com') |
535 | 13 | >>> logout() | 13 | >>> logout() |
536 | 14 | 14 | ||
537 | 15 | Workaround while https://launchpad.net/launchpad/+bug/39016 is not | ||
538 | 16 | fixed. | ||
539 | 17 | |||
540 | 18 | >>> from lp.services.mail import stub | ||
541 | 19 | >>> stub.test_emails[:] = [] | ||
542 | 20 | |||
543 | 21 | First we have to go to the +requestmerge page. | 15 | First we have to go to the +requestmerge page. |
544 | 22 | 16 | ||
545 | 23 | >>> browser.addHeader('Authorization', 'Basic no-priv@canonical.com:test') | 17 | >>> browser.addHeader('Authorization', 'Basic no-priv@canonical.com:test') |
546 | @@ -25,23 +19,6 @@ | |||
547 | 25 | >>> print browser.title | 19 | >>> print browser.title |
548 | 26 | Merge Launchpad accounts | 20 | Merge Launchpad accounts |
549 | 27 | 21 | ||
550 | 28 | If we try to merge a nonexistent account, we'll get an error page. | ||
551 | 29 | |||
552 | 30 | >>> browser.getControl('Duplicated Account').value = 'bar' | ||
553 | 31 | >>> browser.getControl('Continue').click() | ||
554 | 32 | >>> print "\n".join(get_feedback_messages(browser.contents)) | ||
555 | 33 | There is 1 error. | ||
556 | 34 | Invalid value | ||
557 | 35 | |||
558 | 36 | The same will happen if we try to merge a team. | ||
559 | 37 | |||
560 | 38 | >>> browser.getControl( | ||
561 | 39 | ... 'Duplicated Account').value = 'support@canonical.com' | ||
562 | 40 | >>> browser.getControl('Continue').click() | ||
563 | 41 | >>> print "\n".join(get_feedback_messages(browser.contents)) | ||
564 | 42 | There is 1 error. | ||
565 | 43 | Invalid value | ||
566 | 44 | |||
567 | 45 | In preparation for the actual merge (below), we add an extra email | 22 | In preparation for the actual merge (below), we add an extra email |
568 | 46 | address to the 'foo' account, to show how things work when the dupe | 23 | address to the 'foo' account, to show how things work when the dupe |
569 | 47 | account has more than one email address. | 24 | account has more than one email address. |
570 | @@ -70,6 +47,7 @@ | |||
571 | 70 | 47 | ||
572 | 71 | Make sure we haven't got leftovers from a previous test | 48 | Make sure we haven't got leftovers from a previous test |
573 | 72 | 49 | ||
574 | 50 | >>> from lp.services.mail import stub | ||
575 | 73 | >>> len(stub.test_emails) | 51 | >>> len(stub.test_emails) |
576 | 74 | 0 | 52 | 0 |
577 | 75 | 53 | ||
578 | @@ -164,18 +142,6 @@ | |||
579 | 164 | >>> assert not stub.test_emails | 142 | >>> assert not stub.test_emails |
580 | 165 | >>> token_url = get_token_url_from_email(raw_msg) | 143 | >>> token_url = get_token_url_from_email(raw_msg) |
581 | 166 | 144 | ||
582 | 167 | # XXX: Steve Alexander 2006-10-21: To prevent spurious failures in PQM. | ||
583 | 168 | #>>> browser.open(token_url) | ||
584 | 169 | #>>> browser.getControl('Confirm').click() | ||
585 | 170 | #>>> 'The accounts have been merged successfully' in browser.contents | ||
586 | 171 | #True | ||
587 | 172 | |||
588 | 173 | Revisiting the mergerequest-sent page now safely redirects the user: | ||
589 | 174 | |||
590 | 175 | #>>> browser.open('http://launchpad.dev/people/+mergerequest-sent?dupe=55') | ||
591 | 176 | #>>> browser.url | ||
592 | 177 | #'http://launchpad.dev/~no-priv' | ||
593 | 178 | |||
594 | 179 | If the duplicate account has multiple email addresses and has chosen | 145 | If the duplicate account has multiple email addresses and has chosen |
595 | 180 | to hide them the process is slightly different. We cannot display the | 146 | to hide them the process is slightly different. We cannot display the |
596 | 181 | hidden addresses so instead we just inform the user to check all of | 147 | hidden addresses so instead we just inform the user to check all of |
597 | 182 | 148 | ||
598 | === modified file 'lib/lp/registry/stories/person/xx-adminpeoplemerge.txt' | |||
599 | --- lib/lp/registry/stories/person/xx-adminpeoplemerge.txt 2010-10-10 15:39:28 +0000 | |||
600 | +++ lib/lp/registry/stories/person/xx-adminpeoplemerge.txt 2011-04-12 02:08:36 +0000 | |||
601 | @@ -1,4 +1,5 @@ | |||
603 | 1 | = Merging people or teams = | 1 | Merging people or teams |
604 | 2 | ======================= | ||
605 | 2 | 3 | ||
606 | 3 | Launchpad admins can merge any two people or teams, without the need of | 4 | Launchpad admins can merge any two people or teams, without the need of |
607 | 4 | any email address confirmation or something like that. There's one page | 5 | any email address confirmation or something like that. There's one page |
608 | @@ -35,17 +36,6 @@ | |||
609 | 35 | >>> admin_browser.url | 36 | >>> admin_browser.url |
610 | 36 | 'http://launchpad.dev/~salgado' | 37 | 'http://launchpad.dev/~salgado' |
611 | 37 | 38 | ||
625 | 38 | >>> from zope.component import getUtility | 39 | >>> print get_feedback_messages(admin_browser.contents)[0] |
626 | 39 | >>> from canonical.launchpad.ftests import ANONYMOUS, login, logout | 40 | A merge is queued and is expected to complete in a few minutes. |
614 | 40 | >>> from lp.registry.interfaces.person import IPersonSet | ||
615 | 41 | >>> login(ANONYMOUS) | ||
616 | 42 | >>> person_set = getUtility(IPersonSet) | ||
617 | 43 | >>> person_set.getByEmail('andrew.bennetts@ubuntulinux.com').name | ||
618 | 44 | u'salgado' | ||
619 | 45 | |||
620 | 46 | >>> spiv = person_set.getByName('spiv-merged', ignore_merged=False) | ||
621 | 47 | >>> spiv.merged.name | ||
622 | 48 | u'salgado' | ||
623 | 49 | |||
624 | 50 | >>> logout() | ||
627 | 51 | 41 | ||
628 | 52 | 42 | ||
629 | === removed file 'lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt' | |||
630 | --- lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt 2009-09-10 23:58:31 +0000 | |||
631 | +++ lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt 1970-01-01 00:00:00 +0000 | |||
632 | @@ -1,45 +0,0 @@ | |||
633 | 1 | = Merging people with hidden email addresses = | ||
634 | 2 | |||
635 | 3 | In order to merge FooBarDupe into FooBar we need to send an email to | ||
636 | 4 | FooBarDupe's email address with instructions for FooBar to complete the | ||
637 | 5 | merge. The merge should succeed even if FooBarDupe has chosen to hide his | ||
638 | 6 | email addresses. | ||
639 | 7 | |||
640 | 8 | # First we'll create profiles for FooBar and FooBarDupe, the latter with | ||
641 | 9 | # hidden email addresses. | ||
642 | 10 | >>> login(ANONYMOUS) | ||
643 | 11 | >>> foobar = factory.makePerson( | ||
644 | 12 | ... email='foobar@baz.com', name='foobar', password='test') | ||
645 | 13 | >>> foobar_dupe = factory.makePerson(name='foobar-dupe') | ||
646 | 14 | >>> login_person(foobar_dupe) | ||
647 | 15 | >>> foobar_dupe.hide_email_addresses = True | ||
648 | 16 | >>> logout() | ||
649 | 17 | |||
650 | 18 | FooBar will now request that FooBarDupe be merged into his account. | ||
651 | 19 | |||
652 | 20 | >>> browser = setupBrowser(auth="Basic foobar@baz.com:test") | ||
653 | 21 | >>> browser.open('http://launchpad.dev/people/+requestmerge') | ||
654 | 22 | >>> browser.getControl('Duplicated Account').value = 'foobar-dupe' | ||
655 | 23 | >>> browser.getControl('Continue').click() | ||
656 | 24 | >>> browser.url | ||
657 | 25 | 'http://launchpad.dev/people/+mergerequest-sent?dupe=... | ||
658 | 26 | >>> print extract_text(find_main_content(browser.contents)) | ||
659 | 27 | An email message was sent to... | ||
660 | 28 | Please follow the instructions on that message to complete the merge. | ||
661 | 29 | |||
662 | 30 | Following the link included in the email sent to FooBarDupe's email he'll | ||
663 | 31 | be able to complete the merge. | ||
664 | 32 | |||
665 | 33 | >>> from lp.services.mail import stub | ||
666 | 34 | >>> from canonical.launchpad.ftests.logintoken import ( | ||
667 | 35 | ... get_token_url_from_email) | ||
668 | 36 | >>> len(stub.test_emails) | ||
669 | 37 | 1 | ||
670 | 38 | >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop() | ||
671 | 39 | >>> token_url = get_token_url_from_email(raw_msg) | ||
672 | 40 | >>> browser.open(token_url) | ||
673 | 41 | >>> browser.url | ||
674 | 42 | 'http://launchpad.dev/token/.../+accountmerge' | ||
675 | 43 | >>> browser.getControl('Confirm').click() | ||
676 | 44 | >>> print "\n".join(get_feedback_messages(browser.contents)) | ||
677 | 45 | The accounts have been merged successfully... | ||
678 | 46 | 0 | ||
679 | === modified file 'lib/lp/registry/stories/team/xx-adminteammerge.txt' | |||
680 | --- lib/lp/registry/stories/team/xx-adminteammerge.txt 2010-10-18 22:24:59 +0000 | |||
681 | +++ lib/lp/registry/stories/team/xx-adminteammerge.txt 2011-04-12 02:08:36 +0000 | |||
682 | @@ -1,4 +1,5 @@ | |||
684 | 1 | = Merging Teams = | 1 | Merging Teams |
685 | 2 | ============= | ||
686 | 2 | 3 | ||
687 | 3 | There's a separate page for merging teams. We can't merge teams with | 4 | There's a separate page for merging teams. We can't merge teams with |
688 | 4 | active members, so the user will first have to confirm that the | 5 | active members, so the user will first have to confirm that the |
689 | @@ -32,70 +33,5 @@ | |||
690 | 32 | >>> registry_browser.url | 33 | >>> registry_browser.url |
691 | 33 | 'http://launchpad.dev/~guadamen' | 34 | 'http://launchpad.dev/~guadamen' |
692 | 34 | 35 | ||
760 | 35 | >>> from canonical.launchpad.ftests import ANONYMOUS, login, logout | 36 | >>> print get_feedback_messages(registry_browser.contents)[0] |
761 | 36 | >>> from zope.component import getUtility | 37 | A merge is queued and is expected to complete in a few minutes. |
695 | 37 | >>> from lp.registry.interfaces.mailinglist import IMailingListSet | ||
696 | 38 | >>> login(ANONYMOUS) | ||
697 | 39 | >>> new_team.merged.name | ||
698 | 40 | u'guadamen' | ||
699 | 41 | >>> logout() | ||
700 | 42 | |||
701 | 43 | |||
702 | 44 | == Non-display of merged teams == | ||
703 | 45 | |||
704 | 46 | Merged teams are not visible anymore. | ||
705 | 47 | |||
706 | 48 | >>> browser.open("http://launchpad.dev/~new-team-merged") | ||
707 | 49 | Traceback (most recent call last): | ||
708 | 50 | ... | ||
709 | 51 | NotFound:... | ||
710 | 52 | |||
711 | 53 | |||
712 | 54 | == Corner cases == | ||
713 | 55 | |||
714 | 56 | If the duplicated team is associated with a mailing list, it can't be | ||
715 | 57 | merged, though. | ||
716 | 58 | |||
717 | 59 | >>> login(ANONYMOUS) | ||
718 | 60 | >>> guadamen = getUtility(IPersonSet).getByName('guadamen') | ||
719 | 61 | >>> mailing_list = getUtility(IMailingListSet).new(guadamen) | ||
720 | 62 | >>> logout() | ||
721 | 63 | |||
722 | 64 | >>> registry_browser.open('http://launchpad.dev/people/+adminteammerge') | ||
723 | 65 | >>> registry_browser.getControl('Duplicated Team').value = 'guadamen' | ||
724 | 66 | >>> registry_browser.getControl('Target Team').value = 'ubuntu-team' | ||
725 | 67 | >>> registry_browser.getControl('Merge').click() | ||
726 | 68 | |||
727 | 69 | >>> registry_browser.url | ||
728 | 70 | 'http://launchpad.dev/people/+adminteammerge' | ||
729 | 71 | |||
730 | 72 | >>> print get_feedback_messages(registry_browser.contents) | ||
731 | 73 | [u'There is 1 error.', | ||
732 | 74 | u"guadamen is associated with a Launchpad mailing list; | ||
733 | 75 | we can't merge it."] | ||
734 | 76 | |||
735 | 77 | We also can't (for obvious reasons) merge any person/team into itself. | ||
736 | 78 | |||
737 | 79 | >>> registry_browser.getControl('Duplicated Team').value = 'shipit-admins' | ||
738 | 80 | >>> registry_browser.getControl('Target Team').value = 'shipit-admins' | ||
739 | 81 | >>> registry_browser.getControl('Merge').click() | ||
740 | 82 | |||
741 | 83 | >>> registry_browser.url | ||
742 | 84 | 'http://launchpad.dev/people/+adminteammerge' | ||
743 | 85 | |||
744 | 86 | >>> print get_feedback_messages(registry_browser.contents) | ||
745 | 87 | [u'There is 1 error.', u"You can't merge shipit-admins into itself."] | ||
746 | 88 | |||
747 | 89 | Nor can we merge a person into a team or a team into a person. | ||
748 | 90 | |||
749 | 91 | >>> registry_browser.getControl('Duplicated Team').value = 'ubuntu-team' | ||
750 | 92 | >>> registry_browser.getControl('Target Team').value = 'salgado' | ||
751 | 93 | >>> registry_browser.getControl('Merge').click() | ||
752 | 94 | |||
753 | 95 | >>> registry_browser.url | ||
754 | 96 | 'http://launchpad.dev/people/+adminteammerge' | ||
755 | 97 | |||
756 | 98 | # Yes, the error message is not of much help, but this UI is only for | ||
757 | 99 | # admins and they're supposed to know what they're doing. | ||
758 | 100 | >>> print get_feedback_messages(registry_browser.contents) | ||
759 | 101 | [u'There is 1 error.', u'Constraint not satisfied'] | ||
762 | 102 | 38 | ||
763 | === modified file 'lib/lp/registry/tests/test_person.py' | |||
764 | --- lib/lp/registry/tests/test_person.py 2011-04-05 12:10:01 +0000 | |||
765 | +++ lib/lp/registry/tests/test_person.py 2011-04-12 02:08:36 +0000 | |||
766 | @@ -289,14 +289,14 @@ | |||
767 | 289 | person = self.factory.makePerson() | 289 | person = self.factory.makePerson() |
768 | 290 | self.assertFalse(person.is_merge_pending) | 290 | self.assertFalse(person.is_merge_pending) |
769 | 291 | 291 | ||
771 | 292 | def test_merge_pending(self): | 292 | def test_is_merge_pending(self): |
772 | 293 | # is_merge_pending returns True when this person is being merged with | 293 | # is_merge_pending returns True when this person is being merged with |
773 | 294 | # another person in an active merge job. | 294 | # another person in an active merge job. |
774 | 295 | from_person = self.factory.makePerson() | 295 | from_person = self.factory.makePerson() |
775 | 296 | to_person = self.factory.makePerson() | 296 | to_person = self.factory.makePerson() |
776 | 297 | getUtility(IPersonSet).mergeAsync(from_person, to_person) | 297 | getUtility(IPersonSet).mergeAsync(from_person, to_person) |
777 | 298 | self.assertTrue(from_person.is_merge_pending) | 298 | self.assertTrue(from_person.is_merge_pending) |
779 | 299 | self.assertTrue(to_person.is_merge_pending) | 299 | self.assertFalse(to_person.is_merge_pending) |
780 | 300 | 300 | ||
781 | 301 | def test_mergeAsync_success(self): | 301 | def test_mergeAsync_success(self): |
782 | 302 | # mergeAsync returns a job with the from and to persons. | 302 | # mergeAsync returns a job with the from and to persons. |
This branch looks good.
One thing you might want to look at, I /think/ there is a hole in the
testing around the "is_merge_pending" property. I don't see that there
are any tests that the property is ever true.