Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-421975-identity-location-ui-3 into lp:launchpad

  1. bug-421975-identity-location-ui-3
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-421975-identity-location-ui-3
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-421975-identity-location-ui-3
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code an ui Approve
Barry Warsaw ui Pending
Review via email: mp+10989@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Summary
-------

Converted the following pages to UI 3.0.
    person-editircnicknames.pt
    person-editjabberids.pt
    person-editlocation.pt

Implementation details
----------------------

I converted two of the views to LaunchpadFormView so that I could
set the view.label, but I didn't separate the action into separate
validate and action methods, which would have been difficult.

Tests
-----

./bin/test -vv -t 'irc.txt|xx-person-edit-irc-ids.txt|jabber.txt|xx-person-edit-jabber-ids.txt|personlocation.txt|stories/location'

Demo and Q/A
------------

* Open http://launchpad.dev/~name16/+editircnicknames
* Open http://launchpad.dev/~name16/+editjabberids
* Open http://launchpad.dev/~name16/+editlocation

Lint
----

= Launchpad lint =

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

Linting changed files:
  lib/canonical/launchpad/pagetitles.py
  lib/lp/registry/browser/person.py
  lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt
  lib/lp/registry/templates/person-editircnicknames.pt
  lib/lp/registry/templates/person-editjabberids.pt
  lib/lp/registry/templates/person-editlocation.pt

== Pylint notices ==

lib/lp/registry/browser/person.py
    117: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    118: [F0401] Unable to import 'lazr.config' (No module named config)
    119: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

Revision history for this message
Curtis Hovey (sinzui) wrote :

> === modified file 'lib/lp/registry/browser/person.py'
> --- lib/lp/registry/browser/person.py 2009-08-31 22:11:14 +0000
> +++ lib/lp/registry/browser/person.py 2009-09-01 17:01:46 +0000

...

> +class PersonEditIRCNicknamesView(LaunchpadFormView):
> +
> + schema = Interface
> +
> + @property
> + def page_title(self):
> + return smartquote("%s's IRC nicknames" % self.context.displayname)
> +
> + label = page_title
> +
> + @property
> + def cancel_url(self):
> + return canonical_url(self.context)
> +
> + @action(_("Save Changes"), name="save")
> + def save(self, action, data):

Can you report a bug and add an XXX that this form and action should
use schema and form validation?

> +class PersonEditJabberIDsView(LaunchpadFormView):
> + schema = Interface
> +
> + @property
> + def page_title(self):
> + return smartquote("%s's Jabber IDs" % self.context.displayname)
> +
> + label = page_title
> +
> + @property
> + def cancel_url(self):
> + return canonical_url(self.context)
> +
> + @action(_("Save Changes"), name="save")
> + def save(self, action, data):
> """Process the Jabber ID form."""

Can you report a bug and add an XXX that this form and action should
use schema and form validation?

> === modified file 'lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt'
> --- lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-08-13 15:12:16 +0000
> +++ lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-09-01 17:01:46 +0000

This whole test is bad. I would prefer to see this as a view test. If I send you one will you consider replacing this test with it?

> === modified file 'lib/lp/registry/templates/person-editircnicknames.pt'
> --- lib/lp/registry/templates/person-editircnicknames.pt 2009-08-13 15:12:16 +0000
> +++ lib/lp/registry/templates/person-editircnicknames.pt 2009-09-01 15:51:26 +0000
> @@ -6,26 +6,18 @@
> xml:lang="en"
> lang="en"
> dir="ltr"

Remove the xml:lang, lang, and dir because base-layout provides these.

...

review: Approve (code an ui)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (10.5 KiB)

Hi Curtis,

Thanks for the review and the test. I've attached the incremental diff.

-Edwin

> > === modified file 'lib/lp/registry/browser/person.py'
> > --- lib/lp/registry/browser/person.py 2009-08-31 22:11:14 +0000
> > +++ lib/lp/registry/browser/person.py 2009-09-01 17:01:46 +0000
>
> ...
>
> > +class PersonEditIRCNicknamesView(LaunchpadFormView):
> > +
> > + schema = Interface
> > +
> > + @property
> > + def page_title(self):
> > + return smartquote("%s's IRC nicknames" % self.context.displayname)
> > +
> > + label = page_title
> > +
> > + @property
> > + def cancel_url(self):
> > + return canonical_url(self.context)
> > +
> > + @action(_("Save Changes"), name="save")
> > + def save(self, action, data):
>
> Can you report a bug and add an XXX that this form and action should
> use schema and form validation?

Done.

> > +class PersonEditJabberIDsView(LaunchpadFormView):
> > + schema = Interface
> > +
> > + @property
> > + def page_title(self):
> > + return smartquote("%s's Jabber IDs" % self.context.displayname)
> > +
> > + label = page_title
> > +
> > + @property
> > + def cancel_url(self):
> > + return canonical_url(self.context)
> > +
> > + @action(_("Save Changes"), name="save")
> > + def save(self, action, data):
> > """Process the Jabber ID form."""
>
> Can you report a bug and add an XXX that this form and action should
> use schema and form validation?

Done.

> > === modified file 'lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt'
> > --- lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-08-13
> 15:12:16 +0000
> > +++ lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-09-01
> 17:01:46 +0000
>
> This whole test is bad. I would prefer to see this as a view test. If I send
> you one will you consider replacing this test with it?
>
> > === modified file 'lib/lp/registry/templates/person-editircnicknames.pt'
> > --- lib/lp/registry/templates/person-editircnicknames.pt 2009-08-13
> 15:12:16 +0000
> > +++ lib/lp/registry/templates/person-editircnicknames.pt 2009-09-01
> 15:51:26 +0000
> > @@ -6,26 +6,18 @@
> > xml:lang="en"
> > lang="en"
> > dir="ltr"
>
> Remove the xml:lang, lang, and dir because base-layout provides these.
>
> ...

Done.

Incremental diff:
{{{
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2009-09-01 17:01:46 +0000
+++ lib/lp/registry/browser/person.py 2009-09-01 19:32:40 +0000
@@ -3185,6 +3185,8 @@
     @action(_("Save Changes"), name="save")
     def save(self, action, data):
         """Process the IRC nicknames form."""
+ # XXX: EdwinGrubbs 2009-09-01 bug=422784
+ # This view should use schema and form validation.
         form = self.request.form
         for ircnick in self.context.ircnicknames:
             # XXX: GuilhermeSalgado 2005-08-25:
@@ -3232,6 +3234,8 @@
     @action(_("Save Changes"), name="save")
     def save(self, action, data):
         """Process the Jabber ID form."""
+ # XXX: EdwinGrubbs 2009-09-01 bug=422784
+ # This view should use schema and form validation.
       ...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/pagetitles.py'
2--- lib/canonical/launchpad/pagetitles.py 2009-08-31 22:11:14 +0000
3+++ lib/canonical/launchpad/pagetitles.py 2009-09-01 15:51:26 +0000
4@@ -772,14 +772,8 @@
5
6 person_editemails = ContextDisplayName(smartquote("%s's e-mail addresses"))
7
8-person_editlocation = ContextDisplayName(smartquote("%s's usual location"))
9-
10 person_editpgpkeys = ContextDisplayName(smartquote("%s's OpenPGP keys"))
11
12-person_editircnicknames = ContextDisplayName(smartquote("%s's IRC nicknames"))
13-
14-person_editjabberids = ContextDisplayName(smartquote("%s's Jabber IDs"))
15-
16 person_editsshkeys = ContextDisplayName(smartquote("%s's SSH keys"))
17
18 person_editwikinames = ContextDisplayName(smartquote("%s's wiki names"))
19
20=== modified file 'lib/lp/registry/browser/person.py'
21--- lib/lp/registry/browser/person.py 2009-08-31 22:11:14 +0000
22+++ lib/lp/registry/browser/person.py 2009-09-01 17:01:46 +0000
23@@ -7,6 +7,8 @@
24
25 __metaclass__ = type
26
27+
28+
29 __all__ = [
30 'BeginTeamClaimView',
31 'BugSubscriberPackageBugsSearchListingView',
32@@ -3166,15 +3168,23 @@
33 return
34
35
36-class PersonEditIRCNicknamesView(LaunchpadView):
37-
38- def initialize(self):
39+class PersonEditIRCNicknamesView(LaunchpadFormView):
40+
41+ schema = Interface
42+
43+ @property
44+ def page_title(self):
45+ return smartquote("%s's IRC nicknames" % self.context.displayname)
46+
47+ label = page_title
48+
49+ @property
50+ def cancel_url(self):
51+ return canonical_url(self.context)
52+
53+ @action(_("Save Changes"), name="save")
54+ def save(self, action, data):
55 """Process the IRC nicknames form."""
56- self.error_message = None
57- if self.request.method != "POST":
58- # Nothing to do
59- return
60-
61 form = self.request.form
62 for ircnick in self.context.ircnicknames:
63 # XXX: GuilhermeSalgado 2005-08-25:
64@@ -3188,7 +3198,7 @@
65 nick = form.get('nick_%d' % ircnick.id)
66 network = form.get('network_%d' % ircnick.id)
67 if not (nick and network):
68- self.error_message = structured(
69+ self.request.response.addErrorNotification(
70 "Neither Nickname nor Network can be empty.")
71 return
72 ircnick.nickname = nick
73@@ -3202,20 +3212,26 @@
74 else:
75 self.newnick = nick
76 self.newnetwork = network
77- self.error_message = structured(
78+ self.request.response.addErrorNotification(
79 "Neither Nickname nor Network can be empty.")
80- return
81-
82-
83-class PersonEditJabberIDsView(LaunchpadView):
84-
85- def initialize(self):
86+
87+
88+class PersonEditJabberIDsView(LaunchpadFormView):
89+ schema = Interface
90+
91+ @property
92+ def page_title(self):
93+ return smartquote("%s's Jabber IDs" % self.context.displayname)
94+
95+ label = page_title
96+
97+ @property
98+ def cancel_url(self):
99+ return canonical_url(self.context)
100+
101+ @action(_("Save Changes"), name="save")
102+ def save(self, action, data):
103 """Process the Jabber ID form."""
104- self.error_message = None
105- if self.request.method != "POST":
106- # Nothing to do
107- return
108-
109 form = self.request.form
110 for jabber in self.context.jabberids:
111 if form.get('remove_%s' % jabber.jabberid):
112@@ -3223,7 +3239,7 @@
113 else:
114 jabberid = form.get('jabberid_%s' % jabber.jabberid)
115 if not jabberid:
116- self.error_message = structured(
117+ self.request.response.addErrorNotification(
118 "You cannot save an empty Jabber ID.")
119 return
120 jabber.jabberid = jabberid
121@@ -3235,16 +3251,15 @@
122 if existingjabber is None:
123 jabberset.new(self.context, jabberid)
124 elif existingjabber.person != self.context:
125- self.error_message = structured(
126- 'The Jabber ID %s is already registered by '
127- '<a href="%s">%s</a>.',
128- jabberid, canonical_url(existingjabber.person),
129- existingjabber.person.displayname)
130- return
131+ self.request.response.addErrorNotification(
132+ structured(
133+ 'The Jabber ID %s is already registered by '
134+ '<a href="%s">%s</a>.',
135+ jabberid, canonical_url(existingjabber.person),
136+ existingjabber.person.displayname))
137 else:
138- self.error_message = structured(
139- 'The Jabber ID %s already belongs to you.', jabberid)
140- return
141+ self.request.response.addErrorNotification(
142+ 'The Jabber ID %s already belongs to you.' % jabberid)
143
144
145 class PersonEditSSHKeysView(LaunchpadView):
146@@ -5002,6 +5017,13 @@
147 custom_widget('location', LocationWidget)
148
149 @property
150+ def page_title(self):
151+ return smartquote(
152+ "%s's location and timezone" % self.context.displayname)
153+
154+ label = page_title
155+
156+ @property
157 def field_names(self):
158 """See `LaunchpadFormView`.
159
160
161=== modified file 'lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt'
162--- lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-08-13 15:12:16 +0000
163+++ lib/lp/registry/stories/foaf/xx-person-edit-irc-ids.txt 2009-09-01 17:01:46 +0000
164@@ -6,7 +6,7 @@
165 ... Authorization: Basic Zm9vLmJhckBjYW5vbmljYWwuY29tOnRlc3Q=
166 ... Content-Type: application/x-www-form-urlencoded
167 ...
168- ... newnetwork=irc.freenode.net&newnick=&SAVE=Save+Changes""")
169+ ... newnetwork=irc.freenode.net&newnick=&field.actions.save=Save+Changes""")
170 HTTP/1.1 200 Ok
171 ...
172 ...Neither Nickname nor Network can be empty...
173@@ -20,7 +20,7 @@
174 ... Authorization: Basic Zm9vLmJhckBjYW5vbmljYWwuY29tOnRlc3Q=
175 ... Content-Type: application/x-www-form-urlencoded
176 ...
177- ... newnetwork=&newnick=mark&SAVE=Save+Changes""")
178+ ... newnetwork=&newnick=mark&field.actions.save=Save+Changes""")
179 HTTP/1.1 200 Ok
180 ...
181 ...Neither Nickname nor Network can be empty...
182@@ -33,7 +33,7 @@
183 ... Authorization: Basic Zm9vLmJhckBjYW5vbmljYWwuY29tOnRlc3Q=
184 ... Content-Type: application/x-www-form-urlencoded
185 ...
186- ... newnetwork=irc.freenode.net&newnick=mark&SAVE=Save+Changes""")
187+ ... newnetwork=irc.freenode.net&newnick=mark&field.actions.save=Save+Changes""")
188 HTTP/1.1 200 Ok
189 ...
190 ...IRC nicknames...
191@@ -49,6 +49,6 @@
192 ... Authorization: Basic Zm9vLmJhckBjYW5vbmljYWwuY29tOnRlc3Q=
193 ... Content-Type: application/x-www-form-urlencoded
194 ...
195- ... network_11=irc.freenode.net&nick_11=mark&remove_11=Remove&newnetwork=&newnick=&SAVE=Save+Changes""")
196+ ... network_11=irc.freenode.net&nick_11=mark&remove_11=Remove&newnetwork=&newnick=&field.actions.save=Save+Changes""")
197 HTTP/1.1 200 Ok
198 ...
199
200=== modified file 'lib/lp/registry/templates/person-editircnicknames.pt'
201--- lib/lp/registry/templates/person-editircnicknames.pt 2009-08-13 15:12:16 +0000
202+++ lib/lp/registry/templates/person-editircnicknames.pt 2009-09-01 15:51:26 +0000
203@@ -6,26 +6,18 @@
204 xml:lang="en"
205 lang="en"
206 dir="ltr"
207- metal:use-macro="context/@@main_template/master"
208+ metal:use-macro="view/macro:page/main_only"
209 i18n:domain="launchpad"
210 >
211- <body>
212- <metal:heading fill-slot="pageheading">
213- <h1>IRC nicknames</h1>
214- </metal:heading>
215-
216- <metal:leftportlets fill-slot="portlets_one">
217- </metal:leftportlets>
218-
219- <metal:rightportlets fill-slot="portlets_two">
220- </metal:rightportlets>
221-
222- <div metal:fill-slot="main">
223+<body>
224+
225+<div metal:fill-slot="main">
226+<div metal:use-macro="context/@@launchpad_form/form">
227+ <div metal:fill-slot="widgets">
228
229 <p tal:condition="view/error_message"
230 tal:content="structure view/error_message/escapedtext" class="error message" />
231
232- <form name="edit_irc" action="" method="POST">
233 <table>
234
235 <div tal:condition="context/ircnicknames">
236@@ -61,10 +53,6 @@
237 </div>
238
239 <tr>
240- <td colspan="2"><h2>New IRC nickname</h2></td>
241- </tr>
242-
243- <tr>
244 <td><label>Network:</label></td>
245 <td><label>Nickname:</label></td>
246 </tr>
247@@ -86,12 +74,9 @@
248 </tr>
249 </table>
250
251- <div class="formControls">
252- <input type="submit" value="Save Changes" name="SAVE" />
253- </div>
254-
255- </form>
256- </div>
257+ </div>
258+</div>
259+</div>
260
261 </body>
262 </html>
263
264=== modified file 'lib/lp/registry/templates/person-editjabberids.pt'
265--- lib/lp/registry/templates/person-editjabberids.pt 2009-07-17 17:59:07 +0000
266+++ lib/lp/registry/templates/person-editjabberids.pt 2009-09-01 16:24:26 +0000
267@@ -6,26 +6,14 @@
268 xml:lang="en"
269 lang="en"
270 dir="ltr"
271- metal:use-macro="context/@@main_template/master"
272+ metal:use-macro="view/macro:page/main_only"
273 i18n:domain="launchpad"
274 >
275- <body>
276- <metal:heading fill-slot="pageheading">
277- <h1>Jabber IDs</h1>
278- </metal:heading>
279-
280- <metal:leftportlets fill-slot="portlets_one">
281- </metal:leftportlets>
282-
283- <metal:rightportlets fill-slot="portlets_two">
284- </metal:rightportlets>
285-
286- <div metal:fill-slot="main">
287-
288- <p tal:condition="view/error_message"
289- tal:content="structure view/error_message/escapedtext" class="error message" />
290-
291- <form name="edit_jabber" action="" method="POST">
292+<body>
293+<div metal:fill-slot="main">
294+<div metal:use-macro="context/@@launchpad_form/form">
295+ <div metal:fill-slot="widgets">
296+
297 <table>
298
299 <div tal:condition="context/jabberids">
300@@ -57,7 +45,7 @@
301 <tr>
302 <td colspan="2"><h2>New Jabber ID</h2></td>
303 </tr>
304-
305+
306 <tr>
307 <td><label>Jabber ID:</label></td>
308 </tr>
309@@ -73,12 +61,9 @@
310 </tr>
311 </table>
312
313- <div class="formControls">
314- <input type="submit" value="Save Changes" name="SAVE" />
315- </div>
316-
317- </form>
318- </div>
319+ </div>
320+</div>
321+</div>
322
323 </body>
324 </html>
325
326=== modified file 'lib/lp/registry/templates/person-editlocation.pt'
327--- lib/lp/registry/templates/person-editlocation.pt 2009-07-17 17:59:07 +0000
328+++ lib/lp/registry/templates/person-editlocation.pt 2009-09-01 17:01:46 +0000
329@@ -6,15 +6,13 @@
330 xml:lang="en"
331 lang="en"
332 dir="ltr"
333- metal:use-macro="view/macro:page/onecolumn"
334+ metal:use-macro="view/macro:page/main_only"
335 i18n:domain="launchpad">
336
337 <body>
338
339 <div metal:fill-slot="main">
340
341- <h1>Location and time zone</h1>
342-
343 <div metal:use-macro="context/@@launchpad_form/form">
344 <div metal:fill-slot="extra_info">
345 <input type="hidden" name="for_team" value=""