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
1=== modified file 'lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt'
2--- lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt 2009-09-16 22:57:42 +0000
3+++ lib/canonical/launchpad/pagetests/basics/notfound-traversals.txt 2009-11-21 18:23:13 +0000
4@@ -401,7 +401,6 @@
5 >>> check("/~name16/+codesofconduct", auth=True)
6 >>> check("/~name16/+edithomepage", auth=True)
7 >>> check("/~name16/+review", auth=True)
8->>> check("/~name16/+portlet-emails")
9 >>> check("/~name16/+portlet-team-assignedbugs")
10 >>> check("/~name16/+specworkload")
11 >>> check("/~name16/+imports", host='translations.launchpad.dev')
12
13=== modified file 'lib/lp/registry/browser/configure.zcml'
14--- lib/lp/registry/browser/configure.zcml 2009-11-17 17:16:15 +0000
15+++ lib/lp/registry/browser/configure.zcml 2009-11-21 18:23:13 +0000
16@@ -873,9 +873,6 @@
17 name="+sshkeys"
18 attribute="showSSHKeys"/>
19 <browser:page
20- name="+portlet-emails"
21- template="../templates/person-portlet-emails.pt"/>
22- <browser:page
23 name="+portlet-contact-details"
24 template="../templates/person-portlet-contact-details.pt"/>
25 <browser:page
26
27=== modified file 'lib/lp/registry/templates/person-index.pt'
28--- lib/lp/registry/templates/person-index.pt 2009-10-16 00:47:43 +0000
29+++ lib/lp/registry/templates/person-index.pt 2009-11-21 18:23:13 +0000
30@@ -160,13 +160,14 @@
31 <tal:logged-in condition="request/lp:person">
32 <tal:person condition="not: view/context_is_probably_a_team">
33 <ul tal:condition="context/preferredemail">
34- <li class="mail">
35+ <li>
36+ <img src="/@@/private" alt=""
37+ tal:condition="view/email_address_visibility/are_allowed"/>
38+ <img src="/@@/mail" alt=""
39+ tal:condition="view/email_address_visibility/are_public"/>
40 <tal:email
41- replace="context/preferredemail/email">foo@bar.com</tal:email>
42- <a
43- tal:condition="context/required:launchpad.Edit"
44- tal:attributes="href overview_menu/editemailaddresses/url"
45- ><img tal:attributes="alt link/text" src="/@@/edit" /></a>
46+ replace="context/preferredemail/email">foo@bar.com</tal:email>
47+ <a tal:replace="structure overview_menu/editemailaddresses/fmt:icon" />
48 </li>
49 </ul>
50 <tal:editable tal:condition="context/required:launchpad.Edit">
51
52=== removed file 'lib/lp/registry/templates/person-portlet-emails.pt'
53--- lib/lp/registry/templates/person-portlet-emails.pt 2009-07-17 17:59:07 +0000
54+++ lib/lp/registry/templates/person-portlet-emails.pt 1970-01-01 00:00:00 +0000
55@@ -1,46 +0,0 @@
56-<tal:root
57- xmlns:tal="http://xml.zope.org/namespaces/tal"
58- xmlns:metal="http://xml.zope.org/namespaces/metal"
59- xmlns:i18n="http://xml.zope.org/namespaces/i18n"
60- omit-tag="">
61-
62-<tal:block condition="request/lp:person">
63-<tal:block condition="not: context/hide_email_addresses">
64- <tal:block condition="python:context.validatedemails or context.preferredemail">
65- <div class="portlet" id="portlet-emails">
66-
67- <h2>Confirmed e-mail addresses</h2>
68-
69- <div class="portletBody portletContent">
70-
71- <ul>
72- <li class="mail"
73- tal:condition="context/preferredemail"
74- tal:content="context/preferredemail/email">
75- foo@bar.com
76- </li>
77- <li class="mail"
78- tal:repeat="email context/validatedemails"
79- tal:content="email/email">
80- foo@bar.com
81- </li>
82- </ul>
83-
84- <ul tal:condition="context/required:launchpad.Edit">
85- <li class="edit">
86- <a tal:condition="not: context/isTeam" href="+editemails">
87- Manage Addresses
88- </a>
89- <a tal:condition="context/isTeam" href="+contactaddress">
90- Manage Addresses
91- </a>
92- </li>
93- </ul>
94-
95- </div>
96-
97- </div>
98- </tal:block>
99-</tal:block>
100-</tal:block>
101-</tal:root>