Launchpad itself

Merge lp:~bac/launchpad/bug-485237 into lp:launchpad

  1. bug-485237
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-485237
Merge into: lp:launchpad
Diff against target: 101 lines (+7/-56)
4 files modified
lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt (+0/-1)
lib/lp/registry/browser/configure.zcml (+0/-3)
lib/lp/registry/templates/person-index.pt (+7/-6)
lib/lp/registry/templates/person-portlet-emails.pt (+0/-46)
To merge this branch: bzr merge lp:~bac/launchpad/bug-485237
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+15095@code.launchpad.net

Commit message

Change a page template to use TALES formatter and prevent OOPS when viewing invalid person accounts.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 485237 reports an OOPS that occurs in the rare instance that an
unvalidated person with a preferred email address is viewed by an admin.

== Proposed fix ==

The template was using a broken, hand-crafted link to an image. It has
been replaced by using a proven TALES formatter.

In the process it was noticed that person-portlet-emails was unused and
has been removed.

== Pre-implementation notes ==

None.

== Implementation details ==

As above.

== Tests ==

No view tests existed for this corner case and none have been crafted.

== Demo and Q/A ==

As Mark go to http://launchpad.dev/~nsv. Click on 'Change details' and
enter an email address you control. If your system is set up to
properly send out Launchpad email you'll get a validation email. Click
on the link to confirm. That will take you back to the ~nsv page with
the exact scenario that previously broke. Verify you get no OOPS.

For QA have an admin visit the link provided in the bug.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt
  lib/lp/registry/templates/person-index.pt

Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Brad,

nice fix! And thanks for the additional clean up!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt'
--- lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt 2009-09-16 22:57:42 +0000
+++ lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt 2009-11-21 18:23:13 +0000
@@ -401,7 +401,6 @@
401>>> check("/~name16/+codesofconduct", auth=True)401>>> check("/~name16/+codesofconduct", auth=True)
402>>> check("/~name16/+edithomepage", auth=True)402>>> check("/~name16/+edithomepage", auth=True)
403>>> check("/~name16/+review", auth=True)403>>> check("/~name16/+review", auth=True)
404>>> check("/~name16/+portlet-emails")
405>>> check("/~name16/+portlet-team-assignedbugs")404>>> check("/~name16/+portlet-team-assignedbugs")
406>>> check("/~name16/+specworkload")405>>> check("/~name16/+specworkload")
407>>> check("/~name16/+imports", host='translations.launchpad.dev')406>>> check("/~name16/+imports", host='translations.launchpad.dev')
408407
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2009-11-17 17:16:15 +0000
+++ lib/lp/registry/browser/configure.zcml 2009-11-21 18:23:13 +0000
@@ -873,9 +873,6 @@
873 name="+sshkeys"873 name="+sshkeys"
874 attribute="showSSHKeys"/>874 attribute="showSSHKeys"/>
875 <browser:page875 <browser:page
876 name="+portlet-emails"
877 template="../templates/person-portlet-emails.pt"/>
878 <browser:page
879 name="+portlet-contact-details"876 name="+portlet-contact-details"
880 template="../templates/person-portlet-contact-details.pt"/>877 template="../templates/person-portlet-contact-details.pt"/>
881 <browser:page878 <browser:page
882879
=== modified file 'lib/lp/registry/templates/person-index.pt'
--- lib/lp/registry/templates/person-index.pt 2009-10-16 00:47:43 +0000
+++ lib/lp/registry/templates/person-index.pt 2009-11-21 18:23:13 +0000
@@ -160,13 +160,14 @@
160 <tal:logged-in condition="request/lp:person">160 <tal:logged-in condition="request/lp:person">
161 <tal:person condition="not: view/context_is_probably_a_team">161 <tal:person condition="not: view/context_is_probably_a_team">
162 <ul tal:condition="context/preferredemail">162 <ul tal:condition="context/preferredemail">
163 <li class="mail">163 <li>
164 <img src="/@@/private" alt=""
165 tal:condition="view/email_address_visibility/are_allowed"/>
166 <img src="/@@/mail" alt=""
167 tal:condition="view/email_address_visibility/are_public"/>
164 <tal:email168 <tal:email
165 replace="context/preferredemail/email">foo@bar.com</tal:email>169 replace="context/preferredemail/email">foo@bar.com</tal:email>
166 <a170 <a tal:replace="structure overview_menu/editemailaddresses/fmt:icon" />
167 tal:condition="context/required:launchpad.Edit"
168 tal:attributes="href overview_menu/editemailaddresses/url"
169 ><img tal:attributes="alt link/text" src="/@@/edit" /></a>
170 </li>171 </li>
171 </ul>172 </ul>
172 <tal:editable tal:condition="context/required:launchpad.Edit">173 <tal:editable tal:condition="context/required:launchpad.Edit">
173174
=== removed file 'lib/lp/registry/templates/person-portlet-emails.pt'
--- lib/lp/registry/templates/person-portlet-emails.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/person-portlet-emails.pt 1970-01-01 00:00:00 +0000
@@ -1,46 +0,0 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
5 omit-tag="">
6
7<tal:block condition="request/lp:person">
8<tal:block condition="not: context/hide_email_addresses">
9 <tal:block condition="python:context.validatedemails or context.preferredemail">
10 <div class="portlet" id="portlet-emails">
11
12 <h2>Confirmed e-mail addresses</h2>
13
14 <div class="portletBody portletContent">
15
16 <ul>
17 <li class="mail"
18 tal:condition="context/preferredemail"
19 tal:content="context/preferredemail/email">
20 foo@bar.com
21 </li>
22 <li class="mail"
23 tal:repeat="email context/validatedemails"
24 tal:content="email/email">
25 foo@bar.com
26 </li>
27 </ul>
28
29 <ul tal:condition="context/required:launchpad.Edit">
30 <li class="edit">
31 <a tal:condition="not: context/isTeam" href="+editemails">
32 Manage Addresses
33 </a>
34 <a tal:condition="context/isTeam" href="+contactaddress">
35 Manage Addresses
36 </a>
37 </li>
38 </ul>
39
40 </div>
41
42 </div>
43 </tal:block>
44</tal:block>
45</tal:block>
46</tal:root>