Merge lp:~bac/launchpad/bug-419930 into lp:launchpad
- bug-419930
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~bac/launchpad/bug-419930 |
Merge into: | lp:launchpad |
Diff against target: |
500 lines (+219/-105) 9 files modified
lib/canonical/launchpad/doc/tales.txt (+1/-2) lib/canonical/launchpad/interfaces/launchpad.py (+1/-1) lib/canonical/launchpad/pagetitles.py (+0/-4) lib/lp/bugs/doc/externalbugtracker-bug-imports.txt (+2/-2) lib/lp/registry/browser/peoplemerge.py (+48/-30) lib/lp/registry/interfaces/person.py (+8/-7) lib/lp/registry/stories/person/merge-people.txt (+63/-6) lib/lp/registry/templates/people-requestmerge-multiple.pt (+95/-52) lib/lp/soyuz/browser/sourcepackagerelease.py (+1/-1) |
To merge this branch: | bzr merge lp:~bac/launchpad/bug-419930 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Albisetti (community) | ui | Approve | |
Guilherme Salgado (community) | Approve | ||
Review via email: mp+17015@code.launchpad.net |
Commit message
Rework merge request pages
Description of the change
Brad Crittenden (bac) wrote : | # |
Guilherme Salgado (salgado) wrote : | # |
Hi Brad,
This looks pretty good. I have only a few comments/
request for a missing test.
review needsfixing
On Fri, 2010-01-08 at 13:36 +0000, Brad Crittenden wrote:
> Brad Crittenden has proposed merging lp:~bac/launchpad/bug-419930 into
> lp:launchpad/devel.
>
> Requested reviews:
> Canonical Launchpad Engineering (launchpad)
> Related bugs:
> #419930 non-public email address shown after merge request
> https:/
>
>
> = Summary =
>
> Users can choose to hide their email addresses and we should always
> honor that
> request. Currently anyone who attempts to merge a person with hidden
> email addresses
> is shown all addresses for that user.
>
> Also did a drive-by fix to the person picker vocabulary that was not
> catching an
> Unauthorized exception which prevented people with hidden emails from
> being selected
> in the picker.
Does that mean we can't merge people with hidden email addresses unless
we know their Launchpad ID or email address?
>
> == Proposed fix ==
>
> Rather than displaying the addresses and letting the user choose which
> ones to
> contact we should just show a count of addresses and send notification
> to them all.
>
> == Pre-implementation notes ==
>
> Quick call with Curtis as a sanity check of the approach.
>
> == Implementation details ==
>
> As above.
>
> == Tests ==
>
> bin/test -vvm lp.registry -t merge-people.txt
>
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -82,7 +82,7 @@
> def person_
> """Adapts IPerson to IPickerEntry."""
> extra = default_
> - if person.
> + if person.
I didn't see a test for this; care to add one?
> extra.description = person.
> return extra
>
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -359,8 +359,7 @@
>
>
> class RequestPeopleMe
> - """A view for the page where the user asks a merge and the dupe account
> - have more than one email address."""
> + """Merge request view when dupe account has multiple email addresses."""
>
> def __init__(self, context, request):
> self.context = context
> @@ -390,29 +389,38 @@
> login = getUtility(
> logintokenset = getUtility(
>
> - emails = self.request.
> - if emails is not None:
> - # We can have multiple email adressess selected, and in this case
> - # emails will be a list. Otherwise it will be a string and we need
> - # to make a list with that value to use in the for loop.
> - if not isinstance(emails, list):
> - em...
Brad Crittenden (bac) wrote : | # |
> Hi Brad,
>
> This looks pretty good. I have only a few comments/
> request for a missing test.
Thanks for the review.
> > Also did a drive-by fix to the person picker vocabulary that was not
> > catching an
> > Unauthorized exception which prevented people with hidden emails from
> > being selected
> > in the picker.
>
> Does that mean we can't merge people with hidden email addresses unless
> we know their Launchpad ID or email address?
Currently we cannot use the picker to select people with hidden email addresses. But you can merge them by typing in the ID into the field on the form.
> > === modified file 'lib/canonical/
> > --- lib/canonical/
> +0000
> > +++ lib/canonical/
> +0000
> > @@ -82,7 +82,7 @@
> > def person_
> > """Adapts IPerson to IPickerEntry."""
> > extra = default_
> > - if person.
> > + if person.
> person.
>
> I didn't see a test for this; care to add one?
I have discovered Edwin is working to redo the way these pickers are working and my drive-by change would soon be replaced by his work. Therefore I'm going to back out this portion of the change and just wait for his more thorough fix.
> > === modified file 'lib/lp/
> > + # We can have multiple email adressess selected, and in
> this
>
> Typo: adressess
This typo was pre-existing. I looked to see if it existed elsewhere and found and fixed six other occurrences.
> > === modified file 'lib/lp/
> > --- lib/lp/
> +0000
> > +++ lib/lp/
> +0000
> > @@ -172,3 +172,56 @@
> > #>>> browser.open('http://
> sent?dupe=55')
> > #>>> browser.url
> > #'http://
> > +
> > +If the duplicate account has multiple email addresses and has chosen
> > +to hide them the process is slightly different. We cannot display the
> > +hidden addresses so instead we just inform the user to check all of
> > +them (and hope they know which ones) and we send merge request
> > +messages to them all.
> > +
> > + >>> login('<email address hidden>')
> > + >>> dupe = factory.
> <email address hidden>",
> > + ... hide_email_
> > + >>> email = factory.
> > + >>> logout()
> > +
> > +Ensure there are no leftover emails.
> > +
> > + >>> len(stub.
> > + 0
> > +
> > +Open the merge page and merge our duplicate account with hidden email
> > +addresses.
> > +
> > + >>> browser.open('http://
> > + >>> browser.getControl(
> > + ... 'Duplicated Account').value = 'duplicate-person'
> > + >>> browser.getC...
Brad Crittenden (bac) wrote : | # |
Brad Crittenden (bac) wrote : | # |
Added screenshots to the bug 419930 for a UI review.
Guilherme Salgado (salgado) wrote : | # |
On Fri, 2010-01-08 at 21:37 +0000, Brad Crittenden wrote:
> > > === modified file 'lib/canonical/
> > > --- lib/canonical/
> > +0000
> > > +++ lib/canonical/
> > +0000
> > > @@ -82,7 +82,7 @@
> > > def person_
> > > """Adapts IPerson to IPickerEntry."""
> > > extra = default_
> > > - if person.
> > > + if person.
> > person.
> >
> > I didn't see a test for this; care to add one?
>
> I have discovered Edwin is working to redo the way these pickers are
> working and my drive-by change would soon be replaced by his work.
> Therefore I'm going to back out this portion of the change and just
> wait for his more thorough fix.
Ok, do you mind filing a bug and assigning it to him, just to make sure
it's not forgotten?
>
>
> > > === modified file 'lib/lp/
>
> > > + # We can have multiple email adressess selected, and in
> > this
> >
> > Typo: adressess
>
> This typo was pre-existing. I looked to see if it existed elsewhere and found and fixed six other occurrences.
>
Nice, thanks!
review approve
--
Guilherme Salgado <email address hidden>
Martin Albisetti (beuno) wrote : | # |
Thanks for the changes.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/doc/tales.txt' |
2 | --- lib/canonical/launchpad/doc/tales.txt 2010-01-12 09:12:59 +0000 |
3 | +++ lib/canonical/launchpad/doc/tales.txt 2010-01-15 01:40:24 +0000 |
4 | @@ -1111,7 +1111,7 @@ |
5 | >>> test_tales('foo/fmt:linkify-email', foo='nobody@example.com') |
6 | 'nobody@example.com' |
7 | |
8 | -Users who specifiy that their email adresses must be hidden also do not |
9 | +Users who specify that their email addresses must be hidden also do not |
10 | get linkified. test@canonical.com is hidden: |
11 | |
12 | >>> person_set = getUtility(IPersonSet) |
13 | @@ -1583,4 +1583,3 @@ |
14 | |
15 | >>> test_tales("team/fmt:unique_displayname", team=myteam) |
16 | u'<hidden>' |
17 | - |
18 | |
19 | === modified file 'lib/canonical/launchpad/interfaces/launchpad.py' |
20 | --- lib/canonical/launchpad/interfaces/launchpad.py 2010-01-08 16:16:50 +0000 |
21 | +++ lib/canonical/launchpad/interfaces/launchpad.py 2010-01-15 01:40:24 +0000 |
22 | @@ -612,7 +612,7 @@ |
23 | should be a short code that will appear in an |
24 | X-Launchpad-Message-Rationale header for automatic filtering. |
25 | |
26 | - :param person_or_email: An `IPerson` or email adress that is in the |
27 | + :param person_or_email: An `IPerson` or email address that is in the |
28 | recipients list. |
29 | |
30 | :raises UnknownRecipientError: if the person or email isn't in the |
31 | |
32 | === modified file 'lib/canonical/launchpad/pagetitles.py' |
33 | --- lib/canonical/launchpad/pagetitles.py 2009-11-23 03:10:04 +0000 |
34 | +++ lib/canonical/launchpad/pagetitles.py 2010-01-15 01:40:24 +0000 |
35 | @@ -526,10 +526,6 @@ |
36 | |
37 | people_mergerequest_sent = 'Merge request sent' |
38 | |
39 | -people_requestmerge = 'Merge Launchpad accounts' |
40 | - |
41 | -people_requestmerge_multiple = 'Merge Launchpad accounts' |
42 | - |
43 | person_answer_contact_for = ContextDisplayName( |
44 | 'Projects for which %s is an answer contact') |
45 | |
46 | |
47 | === modified file 'lib/lp/bugs/doc/externalbugtracker-bug-imports.txt' |
48 | --- lib/lp/bugs/doc/externalbugtracker-bug-imports.txt 2009-06-15 11:10:49 +0000 |
49 | +++ lib/lp/bugs/doc/externalbugtracker-bug-imports.txt 2010-01-15 01:40:24 +0000 |
50 | @@ -76,9 +76,9 @@ |
51 | account is marked as NEW, since we don't know whether it's valid. |
52 | |
53 | >>> from canonical.launchpad.interfaces import IEmailAddressSet |
54 | - >>> reporter_email_adresses = getUtility(IEmailAddressSet).getByPerson( |
55 | + >>> reporter_email_addresses = getUtility(IEmailAddressSet).getByPerson( |
56 | ... bug.owner) |
57 | - >>> for email_address in reporter_email_adresses: |
58 | + >>> for email_address in reporter_email_addresses: |
59 | ... print "%s: %s" % (email_address.email, email_address.status.name) |
60 | joe.bloggs@example.com: NEW |
61 | >>> print bug.owner.preferredemail |
62 | |
63 | === modified file 'lib/lp/registry/browser/peoplemerge.py' |
64 | --- lib/lp/registry/browser/peoplemerge.py 2010-01-05 15:44:09 +0000 |
65 | +++ lib/lp/registry/browser/peoplemerge.py 2010-01-15 01:40:24 +0000 |
66 | @@ -11,7 +11,8 @@ |
67 | 'DeleteTeamView', |
68 | 'FinishedPeopleMergeRequestView', |
69 | 'RequestPeopleMergeMultipleEmailsView', |
70 | - 'RequestPeopleMergeView'] |
71 | + 'RequestPeopleMergeView', |
72 | + ] |
73 | |
74 | |
75 | from zope.component import getUtility |
76 | @@ -44,6 +45,7 @@ |
77 | """ |
78 | |
79 | label = 'Merge Launchpad accounts' |
80 | + page_title = label |
81 | schema = IRequestPeopleMerge |
82 | |
83 | @property |
84 | @@ -359,8 +361,10 @@ |
85 | |
86 | |
87 | class RequestPeopleMergeMultipleEmailsView: |
88 | - """A view for the page where the user asks a merge and the dupe account |
89 | - have more than one email address.""" |
90 | + """Merge request view when dupe account has multiple email addresses.""" |
91 | + |
92 | + label = 'Merge Launchpad accounts' |
93 | + page_title = label |
94 | |
95 | def __init__(self, context, request): |
96 | self.context = context |
97 | @@ -368,6 +372,7 @@ |
98 | self.form_processed = False |
99 | self.dupe = None |
100 | self.notified_addresses = [] |
101 | + self.user = getUtility(ILaunchBag).user |
102 | |
103 | def processForm(self): |
104 | dupe = self.request.form.get('dupe') |
105 | @@ -386,33 +391,46 @@ |
106 | return |
107 | |
108 | self.form_processed = True |
109 | - user = getUtility(ILaunchBag).user |
110 | login = getUtility(ILaunchBag).login |
111 | logintokenset = getUtility(ILoginTokenSet) |
112 | |
113 | - emails = self.request.form.get("selected") |
114 | - if emails is not None: |
115 | - # We can have multiple email adressess selected, and in this case |
116 | - # emails will be a list. Otherwise it will be a string and we need |
117 | - # to make a list with that value to use in the for loop. |
118 | - if not isinstance(emails, list): |
119 | - emails = [emails] |
120 | - |
121 | - for email in emails: |
122 | - emailaddress = emailaddrset.getByEmail(email) |
123 | - assert emailaddress in self.dupeemails |
124 | - token = logintokenset.new( |
125 | - user, login, email, LoginTokenType.ACCOUNTMERGE) |
126 | - token.sendMergeRequestEmail() |
127 | - self.notified_addresses.append(email) |
128 | - |
129 | - # XXX: salgado, 2008-07-02: We need to somehow disclose the dupe person's |
130 | - # email addresses so that the logged in user knows where to look for the |
131 | - # message with instructions to finish the merge. Since people can choose |
132 | - # to have their email addresses hidden, we need to remove the security |
133 | - # proxy here to ensure they can be shown in this page. |
134 | - @property |
135 | - def naked_dupeemails(self): |
136 | - """Non-security-proxied email addresses of the dupe person.""" |
137 | - from zope.security.proxy import removeSecurityProxy |
138 | - return [removeSecurityProxy(email) for email in self.dupeemails] |
139 | + email_addresses = [] |
140 | + if self.email_hidden: |
141 | + # If the email addresses are hidden we must send a merge request |
142 | + # to each of them. But first we've got to remove the security |
143 | + # proxy so we can get to them. |
144 | + from zope.security.proxy import removeSecurityProxy |
145 | + email_addresses = [removeSecurityProxy(email).email |
146 | + for email in self.dupeemails] |
147 | + else: |
148 | + # Otherwise we send a merge request only to the ones the user |
149 | + # selected. |
150 | + emails = self.request.form.get("selected") |
151 | + if emails is not None: |
152 | + # We can have multiple email addresses selected, and in this |
153 | + # case emails will be a list. Otherwise it will be a string |
154 | + # and we need to make a list with that value to use in the for |
155 | + # loop. |
156 | + if not isinstance(emails, list): |
157 | + emails = [emails] |
158 | + |
159 | + for emailaddress in emails: |
160 | + email = emailaddrset.getByEmail(emailaddress) |
161 | + assert email in self.dupeemails |
162 | + email_addresses.append(emailaddress) |
163 | + |
164 | + for emailaddress in email_addresses: |
165 | + token = logintokenset.new( |
166 | + self.user, login, emailaddress, LoginTokenType.ACCOUNTMERGE) |
167 | + token.sendMergeRequestEmail() |
168 | + self.notified_addresses.append(emailaddress) |
169 | + |
170 | + @property |
171 | + def cancel_url(self): |
172 | + """Cancel URL.""" |
173 | + return canonical_url(self.user) |
174 | + |
175 | + @property |
176 | + def email_hidden(self): |
177 | + """Does the duplicate account hide email addresses?""" |
178 | + return self.dupe.hide_email_addresses |
179 | |
180 | === modified file 'lib/lp/registry/interfaces/person.py' |
181 | --- lib/lp/registry/interfaces/person.py 2010-01-13 23:18:29 +0000 |
182 | +++ lib/lp/registry/interfaces/person.py 2010-01-15 01:40:24 +0000 |
183 | @@ -96,6 +96,7 @@ |
184 | from canonical.launchpad.webapp.interfaces import NameLookupFailed |
185 | from canonical.launchpad.webapp.authorization import check_permission |
186 | |
187 | + |
188 | PRIVATE_TEAM_PREFIX = 'private-' |
189 | |
190 | |
191 | @@ -333,7 +334,7 @@ |
192 | MODERATED = DBItem(1, """ |
193 | Moderated Team |
194 | |
195 | - All subscriptions for this team are subject to approval by one of |
196 | + All subscriptions for this team are subject to approval by one of |
197 | the team's administrators. |
198 | """) |
199 | |
200 | @@ -366,17 +367,17 @@ |
201 | PRIVATE_MEMBERSHIP = DBItem(20, """ |
202 | Private Membership |
203 | |
204 | - Only Launchpad admins and team members can view the |
205 | - membership list for this team. The team is severely restricted in the |
206 | + Only Launchpad admins and team members can view the |
207 | + membership list for this team. The team is severely restricted in the |
208 | roles it can assume. |
209 | """) |
210 | |
211 | PRIVATE = DBItem(30, """ |
212 | Private |
213 | |
214 | - Only Launchpad admins and team members can view the membership list |
215 | - for this team or its name. The team roles are restricted to |
216 | - subscribing to bugs, being bug supervisor, owning code branches, and |
217 | + Only Launchpad admins and team members can view the membership list |
218 | + for this team or its name. The team roles are restricted to |
219 | + subscribing to bugs, being bug supervisor, owning code branches, and |
220 | having a PPA. |
221 | """) |
222 | |
223 | @@ -1850,7 +1851,7 @@ |
224 | While we don't have Full Text Indexes in the emailaddress table, we'll |
225 | be trying to match the text only against the beginning of an email |
226 | address. |
227 | - |
228 | + |
229 | If created_before or created_after are not None, they are used to |
230 | restrict the search to the dates provided. |
231 | """ |
232 | |
233 | === modified file 'lib/lp/registry/stories/person/merge-people.txt' |
234 | --- lib/lp/registry/stories/person/merge-people.txt 2009-07-31 22:34:24 +0000 |
235 | +++ lib/lp/registry/stories/person/merge-people.txt 2010-01-15 01:40:24 +0000 |
236 | @@ -63,8 +63,10 @@ |
237 | >>> browser.getControl( |
238 | ... 'Duplicated Account').value = 'foo' |
239 | >>> browser.getControl('Continue').click() |
240 | - >>> 'has more than one registered e-mail address' in browser.contents |
241 | - True |
242 | + >>> explanation = find_tag_by_id(browser.contents, 'explanation') |
243 | + >>> print extract_text(explanation) |
244 | + The account...has more than one registered e-mail address... |
245 | + |
246 | |
247 | Make sure we haven't got leftovers from a previous test |
248 | |
249 | @@ -76,9 +78,10 @@ |
250 | >>> email_select_control = browser.getControl(name='selected') |
251 | >>> for ctrl in email_select_control.controls: |
252 | ... ctrl.selected = True |
253 | - >>> browser.getControl('Submit').click() |
254 | - >>> 'Individual email messages were sent to' in browser.contents |
255 | - True |
256 | + >>> browser.getControl('Merge Accounts').click() |
257 | + >>> confirmation = find_tag_by_id(browser.contents, 'confirmation') |
258 | + >>> print extract_text(confirmation) |
259 | + Confirmation email messages were sent to:... |
260 | >>> 'foo@baz.com' in browser.contents |
261 | True |
262 | >>> 'bar.foo@canonical.com' in browser.contents |
263 | @@ -107,7 +110,7 @@ |
264 | True |
265 | |
266 | User confirms the merge request submitting the form, but the merge wasn't |
267 | -finished because the duplicate account still have a registered email adresses. |
268 | +finished because the duplicate account still have a registered email addresses. |
269 | |
270 | >>> browser.getControl('Confirm').click() |
271 | >>> 'has other registered e-mail addresses too' in browser.contents |
272 | @@ -172,3 +175,57 @@ |
273 | #>>> browser.open('http://launchpad.dev/people/+mergerequest-sent?dupe=55') |
274 | #>>> browser.url |
275 | #'http://launchpad.dev/~no-priv' |
276 | + |
277 | +If the duplicate account has multiple email addresses and has chosen |
278 | +to hide them the process is slightly different. We cannot display the |
279 | +hidden addresses so instead we just inform the user to check all of |
280 | +them (and hope they know which ones) and we send merge request |
281 | +messages to them all. |
282 | + |
283 | + >>> login('foo.bar@canonical.com') |
284 | + >>> dupe = factory.makePerson(name="duplicate-person", email="dupe1@example.com", |
285 | + ... hide_email_addresses=True) |
286 | + >>> email = factory.makeEmail(address="dupe2@example.com", person=dupe) |
287 | + >>> logout() |
288 | + |
289 | +Ensure there are no leftover emails. |
290 | + |
291 | + >>> len(stub.test_emails) |
292 | + 0 |
293 | + |
294 | +Open the merge page and merge our duplicate account with hidden email |
295 | +addresses. |
296 | + |
297 | + >>> browser.open('http://launchpad.dev/people/+requestmerge') |
298 | + >>> browser.getControl( |
299 | + ... 'Duplicated Account').value = 'duplicate-person' |
300 | + >>> browser.getControl('Continue').click() |
301 | + |
302 | + >>> explanation = find_tag_by_id(browser.contents, 'explanation') |
303 | + >>> print extract_text(explanation) |
304 | + The account...has 2 registered e-mail addresses... |
305 | + |
306 | + |
307 | +Since the addresses are hidden we cannot display them therefore there |
308 | +are no controls to select. |
309 | + |
310 | + >>> email_select_control = browser.getControl(name='selected') |
311 | + Traceback (most recent call last): |
312 | + ... |
313 | + LookupError: name 'selected' |
314 | + |
315 | + >>> browser.getControl('Merge Accounts').click() |
316 | + |
317 | + >>> len(stub.test_emails) |
318 | + 2 |
319 | + |
320 | + >>> confirmation = find_tag_by_id(browser.contents, 'confirmation') |
321 | + >>> print extract_text(confirmation) |
322 | + Confirmation email messages were sent to the 2 registered e-mail addresses... |
323 | + |
324 | + >>> maincontent = extract_text(find_main_content(browser.contents)) |
325 | + |
326 | + >>> 'dupe1@example.com' in maincontent |
327 | + False |
328 | + >>> 'dupe2@example.com' in maincontent |
329 | + False |
330 | |
331 | === modified file 'lib/lp/registry/templates/people-requestmerge-multiple.pt' |
332 | --- lib/lp/registry/templates/people-requestmerge-multiple.pt 2009-08-31 19:53:54 +0000 |
333 | +++ lib/lp/registry/templates/people-requestmerge-multiple.pt 2010-01-15 01:40:24 +0000 |
334 | @@ -14,59 +14,102 @@ |
335 | > |
336 | <body> |
337 | <div metal:fill-slot="main"> |
338 | - <h1 tal:condition="not: view/dupe">No account found to merge</h1> |
339 | + <p tal:condition="not: view/dupe">No account found to merge</p> |
340 | <tal:duplicate condition="view/dupe"> |
341 | <tal:block tal:condition="not: view/form_processed"> |
342 | - <h1>Merge accounts</h1> |
343 | - |
344 | - <p> |
345 | - The account <code tal:content="view/dupe/name">foo</code>, |
346 | - which you’re trying to merge, |
347 | - has more than one registered e-mail address. |
348 | - To complete the merge, you need to prove that you have access to |
349 | - all e-mail addresses registered for this account. |
350 | - To do this, select all of them and click <samp>Submit</samp>. |
351 | - If you don’t have access to one or more of these addresses, |
352 | - Launchpad will not be able to merge the account into your existing one, |
353 | - but any other addresses will be transferred to your account. |
354 | - </p> |
355 | - |
356 | - <form name="mergerequest" action="" method="POST"> |
357 | - <input type="hidden" name="dupe" tal:attributes="value request/dupe" /> |
358 | - <table> |
359 | - <tr tal:repeat="email view/naked_dupeemails"> |
360 | - <td> |
361 | - <input type="checkbox" name="selected" checked="checked" |
362 | - tal:attributes="value email/email" /> |
363 | - </td> |
364 | - <td tal:content="email/email"> |
365 | - </td> |
366 | - </tr> |
367 | - <tr> |
368 | - <td colspan="2" align="right"> |
369 | - <input type="submit" value="Submit" /> |
370 | - </td> |
371 | - </tr> |
372 | - </table> |
373 | - </form> |
374 | - </tal:block> |
375 | - |
376 | - <tal:block tal:condition="view/form_processed"> |
377 | - <h1>Confirmation mail sent</h1> |
378 | - |
379 | - <p>Individual email messages were sent to |
380 | - <tal:block repeat="email view/notified_addresses"> |
381 | - <strong tal:content="email" |
382 | - /><span tal:condition="not: repeat/email/end">, </span> |
383 | - </tal:block>. |
384 | - Please follow the instructions on each of those messages to complete |
385 | - the merge. |
386 | - </p> |
387 | - </tal:block> |
388 | - |
389 | - </tal:duplicate> |
390 | -</div> |
391 | - |
392 | -</body> |
393 | + |
394 | + <div style="margin-top: 1em;"> |
395 | + |
396 | + <tal:email_visible condition="not: view/email_hidden"> |
397 | + <p id="explanation"> |
398 | + The account <code tal:content="view/dupe/name">foo</code> |
399 | + has more than one registered e-mail address. |
400 | + You need to prove that you have access to |
401 | + all e-mail addresses registered for this account. |
402 | + Unselect any you cannot access. |
403 | + If you don’t have access to one or more of these addresses, |
404 | + Launchpad will not be able to merge the account but all confirmed |
405 | + addresses will be transferred to your account. |
406 | + </p> |
407 | + |
408 | + <form name="mergerequest" action="" method="POST"> |
409 | + <input type="hidden" name="dupe" tal:attributes="value request/dupe" /> |
410 | + <table> |
411 | + <tr tal:repeat="email view/dupeemails"> |
412 | + <td> |
413 | + <input type="checkbox" name="selected" checked="checked" |
414 | + tal:attributes="value email/email" /> |
415 | + <span tal:content="email/email" /> |
416 | + </td> |
417 | + </tr> |
418 | + <tr> |
419 | + <td> |
420 | + <input type="submit" value="Merge Accounts" /> |
421 | + or <a href tal:attributes="href view/cancel_url">Cancel</a> |
422 | + </td> |
423 | + </tr> |
424 | + </table> |
425 | + </form> |
426 | + </tal:email_visible> |
427 | + |
428 | + <tal:email_hidden condition="view/email_hidden"> |
429 | + <p id="explanation"> |
430 | + The account <code tal:content="view/dupe/name">foo</code> |
431 | + has <span tal:replace="view/dupeemails/count" /> registered |
432 | + e-mail addresses but they are hidden. |
433 | + You need to prove that you have access to |
434 | + all e-mail addresses registered for this account. |
435 | + </p> |
436 | + <p> |
437 | + To do so, click the button. An email will be sent to each |
438 | + address on file for the duplicate account. Follow the |
439 | + instructions for each email. |
440 | + If you don’t have access to some of these addresses, |
441 | + Launchpad will not be able to merge the account. |
442 | + The addresses you verify will be transferred to your account. |
443 | + </p> |
444 | + |
445 | + <form name="mergerequest" action="" method="POST"> |
446 | + <input type="hidden" name="dupe" tal:attributes="value request/dupe" /> |
447 | + <input type="submit" value="Merge Accounts" /> |
448 | + or <a href tal:attributes="href view/cancel_url">Cancel</a> |
449 | + </form> |
450 | + </tal:email_hidden> |
451 | + </div> |
452 | + </tal:block> |
453 | + |
454 | + <tal:block tal:condition="view/form_processed"> |
455 | + |
456 | + <tal:email_visible condition="not: view/email_hidden"> |
457 | + <p id="confirmation"> |
458 | + Confirmation email messages were sent to: |
459 | + </p> |
460 | + <tal:block repeat="email view/notified_addresses"> |
461 | + <p><strong tal:content="email"/></p> |
462 | + </tal:block> |
463 | + </tal:email_visible> |
464 | + |
465 | + <tal:email_hidden condition="view/email_hidden"> |
466 | + <p id="confirmation"> |
467 | + Confirmation email messages were sent to the |
468 | + <span tal:content="view/dupeemails/count" /> registered |
469 | + e-mail addresses for |
470 | + <code tal:content="view/dupe/name">foo</code>. |
471 | + </p> |
472 | + </tal:email_hidden> |
473 | + |
474 | + <p> |
475 | + Please follow the instructions on each of those messages to complete |
476 | + the merge. |
477 | + </p> |
478 | + <p> |
479 | + <a href tal:attributes="href view/cancel_url">Back to my account</a> |
480 | + </p> |
481 | + |
482 | + </tal:block> |
483 | + |
484 | + </tal:duplicate> |
485 | + </div> |
486 | + </body> |
487 | </html> |
488 | </tal:root> |
489 | |
490 | === modified file 'lib/lp/soyuz/browser/sourcepackagerelease.py' |
491 | --- lib/lp/soyuz/browser/sourcepackagerelease.py 2010-01-06 11:59:21 +0000 |
492 | +++ lib/lp/soyuz/browser/sourcepackagerelease.py 2010-01-15 01:40:24 +0000 |
493 | @@ -84,7 +84,7 @@ |
494 | |
495 | |
496 | def extract_email_addresses(text): |
497 | - '''Unique email adresses in the text.''' |
498 | + '''Unique email addresses in the text.''' |
499 | matches = re.finditer(FormattersAPI._re_email, text) |
500 | return list(set([match.group() for match in matches])) |
501 |
= Summary =
Users can choose to hide their email addresses and we should always honor that
request. Currently anyone who attempts to merge a person with hidden email addresses
is shown all addresses for that user.
Also did a drive-by fix to the person picker vocabulary that was not catching an
Unauthorized exception which prevented people with hidden emails from being selected
in the picker.
== Proposed fix ==
Rather than displaying the addresses and letting the user choose which ones to
contact we should just show a count of addresses and send notification to them all.
== Pre-implementation notes ==
Quick call with Curtis as a sanity check of the approach.
== Implementation details ==
As above.
== Tests ==
bin/test -vvm lp.registry -t merge-people.txt
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: registry/ browser/ peoplemerge. py registry/ stories/ person/ merge-people. txt registry/ interfaces/ person. py registry/ templates/ people- requestmerge- multiple. pt /launchpad/ browser/ vocabulary. py
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/canonical
== Pylint notices ==
Non-issues:
lib/lp/ registry/ browser/ peoplemerge. py iew.deactivate_ members_ and_merge_ action] Operator not 'deactivate_ members_ and_merge' ) members_ and_merge_ action( self, action, data):
250: [C0322, AdminTeamMergeV
preceded by a space
name=
^
def deactivate_
lib/lp/ registry/ interfaces/ person. py .snapshot' (No module named lifecycle) interface' (No module named restful) declarations' (No module named restful) fields' (No module named restful) ._validate] Use super on an old style class ricted. addMember] Operator not preceded by a space copy_field( ITeamMembership ['status' ]), Text(required= False)) write_operation () TeamMembershipS tatus.APPROVED, add=False, subscribe_ to_list= True): ricted. acceptInvitatio nToBeMemberOf] Operator not write_operation () nToBeMemberOf( team, comment): ricted. declineInvitati onToBeMemberOf] Operator not write_operation () onToBeMemberOf( team, comment): ershipperiod= 'default_ membership_ period' , walperiod= 'default_ renewal_ period' ) parameters( npolicy= Choice( _('Subscription policy'), vocabulary= TeamSubscriptio nPolicy, TeamSubscriptio nPolicy. MODERATED) ) factory_ oper...
52: [F0401] Unable to import 'lazr.enum' (No module named enum)
53: [F0401] Unable to import 'lazr.lifecycle
54: [F0401] Unable to import 'lazr.restful.
55: [F0401] Unable to import 'lazr.restful.
62: [F0401] Unable to import 'lazr.restful.
407: [E1002, PersonNameField
1405: [C0322, IPersonEditRest
status=
^
comment=
@export_
def addMember(person, reviewer, status=
comment=None, force_team_
may_
1444: [C0322, IPersonEditRest
preceded by a space
comment=Text())
^
@export_
def acceptInvitatio
1456: [C0322, IPersonEditRest
preceded by a space
comment=Text())
^
@export_
def declineInvitati
1754: [C0322, IPersonSet.newTeam] Operator not preceded by a space
defaultmemb
^
defaultrene
@operation_
subscriptio
title=
required=False, default=
@export_