Launchpad itself

Merge lp:~bac/launchpad/bug-429455-team-pages into lp:launchpad

  1. bug-429455-team-pages
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-429455-team-pages
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~bac/launchpad/bug-429455-team-pages
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Canonical Launchpad Engineering Pending
Review via email: mp+11739@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 429455 requires the conversion of three team pages to UI 3.0:
team-map.pt
team-mugshots.pt
team-members.pt

== Proposed fix ==

Provide changes to fulfill the new UI requirements.

== Pre-implementation notes ==

N/A

== Implementation details ==

* team-map.pt
  * Convert to main_only.
  * Remove from pagetitles.py
  * Create label property
  * Remove h1
  * Remove the section that displays team members with no location
    data and invites anyone to set it for them - Bug 262193

* team-mugshots.pt
  * Convert to main_only.
  * Remove from pagetitles.py
  * Create label property
  * Remove h1

* team-members.pt

== Tests ==

bin/test -vvm lp.registry -t team-map.txt
     -t stories/mailinglists/moderation.txt

== Demo and Q/A ==

Screenshots are available at
http://people.canonical.com/~bac/bug-429455-team-pages

* As Mark, go to: https://launchpad.dev/~hwdb-team/+map
  * Note page title, label, and breadcrumbs.

* Go to https://launchpad.dev/~hwdb-team/+mugshots
  * Note page title, label, and breadcrumbs.
  * Without the side there is much more real estate for photos.

* Go to https://launchpad.dev/~hwdb-team/+members
  * Note page title, label, and breadcrumbs.

= 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/stories/foaf/xx-team-membership.txt
  lib/canonical/config/schema-lazr.conf
  lib/canonical/launchpad/pagetitles.py
  lib/canonical/launchpad/database/librarian.py
  lib/lp/registry/browser/team.py
  lib/lp/registry/templates/team-mugshots.pt
  lib/lp/registry/templates/team-members.pt
  lib/lp/registry/templates/team-map.pt
  lib/lp/registry/browser/person.py
  lib/lp/registry/stories/location/team-map.txt
  configs/development/launchpad-lazr.conf

== Pylint notices ==

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

--
Brad Crittenden
<email address hidden>

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

This looks pretty good. I have a few remarks that I think will improve the code before you land it.

> === modified file 'lib/canonical/launchpad/pagetitles.py'
> --- lib/canonical/launchpad/pagetitles.py 2009-09-14 15:16:12 +0000
> +++ lib/canonical/launchpad/pagetitles.py 2009-09-14 19:38:57 +0000
> @@ -1033,8 +1033,6 @@
> team_mailinglist_subscribers = ContextBrowsername(
> 'Mailing list subscribers for the %s team')
>
> -team_map = ContextBrowsername('Map of %s participants')
> -
> team_members = ContextBrowsername(smartquote('"%s" members'))
>
> team_mugshots = ContextBrowsername(smartquote('Mugshots in the "%s" team'))

Why didn't you remove these? IS it because base-layout still requires a
page_title. I think you can do this in the page views.

    page_title = label

So that you can remove the entries from pagetitles.py

> === modified file 'lib/lp/registry/browser/person.py'
> --- lib/lp/registry/browser/person.py 2009-09-14 14:10:10 +0000
> +++ lib/lp/registry/browser/person.py 2009-09-14 19:32:10 +0000

...

> @@ -4564,6 +4568,10 @@
> getUtility(IPersonSet).cacheBrandingForPeople(self.allmembers)
>
> @cachedproperty
> + def label(self):
> + return "Who's in this team?"

This does not need to be a cachedproperty.

    label = "Who's in this team?"

> === modified file 'lib/lp/registry/browser/team.py'
> --- lib/lp/registry/browser/team.py 2009-09-01 19:43:30 +0000
> +++ lib/lp/registry/browser/team.py 2009-09-14 19:32:10 +0000
> @@ -1019,6 +1019,10 @@
> self.request.needs_gmap2 = True
>
> @cachedproperty
> + def label(self):
> + return "Team member locations"

This does not need to be a cachedproperty.

    label = "Team member locations"

review: Approve (code)
Revision history for this message
Barry Warsaw (barry) wrote :

On Sep 14, 2009, at 5:35 PM, Curtis Hovey wrote:

> Why didn't you remove these? IS it because base-layout still
> requires a
> page_title. I think you can do this in the page views.
>
> page_title = label
>
> So that you can remove the entries from pagetitles.py

base-layout only requires page_title in some limited circumstances.

https://bugs.edge.launchpad.net/launchpad-foundations/+bug/429473/comments/1

Revision history for this message
Brad Crittenden (bac) wrote :

On Sep 14, 2009, at 17:35 , Curtis Hovey wrote:

> Review: Approve code
> This looks pretty good. I have a few remarks that I think will
> improve the code before you land it.
>
>> === modified file 'lib/canonical/launchpad/pagetitles.py'
>> --- lib/canonical/launchpad/pagetitles.py 2009-09-14 15:16:12 +0000
>> +++ lib/canonical/launchpad/pagetitles.py 2009-09-14 19:38:57 +0000
>> @@ -1033,8 +1033,6 @@
>> team_mailinglist_subscribers = ContextBrowsername(
>> 'Mailing list subscribers for the %s team')
>>
>> -team_map = ContextBrowsername('Map of %s participants')
>> -
>> team_members = ContextBrowsername(smartquote('"%s" members'))
>>
>> team_mugshots = ContextBrowsername(smartquote('Mugshots in the "%s"
>> team'))
>
> Why didn't you remove these? IS it because base-layout still
> requires a
> page_title. I think you can do this in the page views.

I just forgot. Thanks for catching them.

>
> page_title = label
>
> So that you can remove the entries from pagetitles.py

These views did not require a page_title.

>
>> === modified file 'lib/lp/registry/browser/person.py'
>> --- lib/lp/registry/browser/person.py 2009-09-14 14:10:10 +0000
>> +++ lib/lp/registry/browser/person.py 2009-09-14 19:32:10 +0000
>
> ...
>
>> @@ -4564,6 +4568,10 @@
>> getUtility(IPersonSet).cacheBrandingForPeople
>> (self.allmembers)
>>
>> @cachedproperty
>> + def label(self):
>> + return "Who's in this team?"
>
> This does not need to be a cachedproperty.
>
> label = "Who's in this team?"

Of course. Done.

>
>> === modified file 'lib/lp/registry/browser/team.py'
>> --- lib/lp/registry/browser/team.py 2009-09-01 19:43:30 +0000
>> +++ lib/lp/registry/browser/team.py 2009-09-14 19:32:10 +0000
>> @@ -1019,6 +1019,10 @@
>> self.request.needs_gmap2 = True
>>
>> @cachedproperty
>> + def label(self):
>> + return "Team member locations"
>
> This does not need to be a cachedproperty.
>
> label = "Team member locations"
>

Done.

Diff at http://pastebin.ubuntu.com/271285/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2009-08-19 12:28:32 +0000
3+++ configs/development/launchpad-lazr.conf 2009-09-14 19:32:10 +0000
4@@ -145,6 +145,7 @@
5 restricted_upload_port: 58095
6 restricted_download_port: 58085
7 restricted_download_url: http://launchpad.dev:58085/
8+use_https = False
9
10 [librarian_server]
11 root: /var/tmp/fatsam
12
13=== modified file 'lib/canonical/config/schema-lazr.conf'
14--- lib/canonical/config/schema-lazr.conf 2009-09-14 09:12:58 +0000
15+++ lib/canonical/config/schema-lazr.conf 2009-09-14 19:32:10 +0000
16@@ -1144,6 +1144,8 @@
17 # datatype: urlbase
18 restricted_download_url: http://restricted-librarian.launchpad.net/
19
20+use_https = True
21+
22 [librarian_gc]
23 # The database user which will be used by this process.
24 # datatype: string
25
26=== modified file 'lib/canonical/launchpad/database/librarian.py'
27--- lib/canonical/launchpad/database/librarian.py 2009-08-27 01:32:01 +0000
28+++ lib/canonical/launchpad/database/librarian.py 2009-09-14 19:32:10 +0000
29@@ -98,7 +98,7 @@
30
31 def getURL(self):
32 """See ILibraryFileAlias.getURL"""
33- if config.vhosts.use_https:
34+ if config.librarian.use_https:
35 return self.https_url
36 else:
37 return self.http_url
38
39=== modified file 'lib/canonical/launchpad/pagetitles.py'
40--- lib/canonical/launchpad/pagetitles.py 2009-09-14 15:16:12 +0000
41+++ lib/canonical/launchpad/pagetitles.py 2009-09-14 19:38:57 +0000
42@@ -1033,8 +1033,6 @@
43 team_mailinglist_subscribers = ContextBrowsername(
44 'Mailing list subscribers for the %s team')
45
46-team_map = ContextBrowsername('Map of %s participants')
47-
48 team_members = ContextBrowsername(smartquote('"%s" members'))
49
50 team_mugshots = ContextBrowsername(smartquote('Mugshots in the "%s" team'))
51
52=== modified file 'lib/lp/registry/browser/person.py'
53--- lib/lp/registry/browser/person.py 2009-09-14 14:10:10 +0000
54+++ lib/lp/registry/browser/person.py 2009-09-14 19:32:10 +0000
55@@ -1327,7 +1327,11 @@
56
57
58 class TeamMembershipView(LaunchpadView):
59- """The view behins ITeam/+members."""
60+ """The view behind ITeam/+members."""
61+
62+ @cachedproperty
63+ def label(self):
64+ return smartquote('Members of "%s"' % self.context.displayname)
65
66 @cachedproperty
67 def active_memberships(self):
68@@ -4564,6 +4568,10 @@
69 getUtility(IPersonSet).cacheBrandingForPeople(self.allmembers)
70
71 @cachedproperty
72+ def label(self):
73+ return "Who's in this team?"
74+
75+ @cachedproperty
76 def allmembers(self):
77 return list(self.context.allmembers)
78
79
80=== modified file 'lib/lp/registry/browser/team.py'
81--- lib/lp/registry/browser/team.py 2009-09-01 19:43:30 +0000
82+++ lib/lp/registry/browser/team.py 2009-09-14 19:32:10 +0000
83@@ -1019,6 +1019,10 @@
84 self.request.needs_gmap2 = True
85
86 @cachedproperty
87+ def label(self):
88+ return "Team member locations"
89+
90+ @cachedproperty
91 def mapped_participants(self):
92 """Participants with locations."""
93 return self.context.getMappedParticipants(limit=self.limit)
94
95=== modified file 'lib/lp/registry/stories/foaf/xx-team-membership.txt'
96--- lib/lp/registry/stories/foaf/xx-team-membership.txt 2009-01-03 19:32:48 +0000
97+++ lib/lp/registry/stories/foaf/xx-team-membership.txt 2009-09-14 20:37:47 +0000
98@@ -3,8 +3,6 @@
99 >>> browser.open('http://launchpad.dev/~ubuntu-team/+members')
100 >>> 'Ubuntu Team' in browser.contents
101 True
102- >>> 'Mark Shuttleworth' in browser.contents
103- True
104
105 Let's take a look at Colin's subscription page. Colin is an
106 administrator and his subscription never expires.
107@@ -240,4 +238,3 @@
108 >>> indirect = find_portlet(browser.contents, 'Indirect membership')
109 >>> print indirect
110 None
111-
112
113=== modified file 'lib/lp/registry/stories/location/team-map.txt'
114--- lib/lp/registry/stories/location/team-map.txt 2009-06-12 16:36:02 +0000
115+++ lib/lp/registry/stories/location/team-map.txt 2009-09-14 21:01:18 +0000
116@@ -15,16 +15,12 @@
117 >>> 'build/lp/mapping.js' in markup
118 True
119
120-You should also be able to see a map of the team, with links to the people
121-who need to have location data added to complete the team.
122+You should also be able to see a map of the team.
123
124 >>> maplink = nopriv_browser.getLink('View map and time zones')
125 >>> maplink.click()
126 >>> nopriv_browser.url
127 'http://launchpad.dev/~guadamen/+map'
128- >>> link = nopriv_browser.getLink('Edgar Bursic')
129- >>> link.url
130- 'http://launchpad.dev/~edgar/+editlocation?for_team=guadamen'
131
132 The map depends on a stream of XML-formatted data, giving the locations of
133 all members of the team. We show that this stream works for teams with, and
134@@ -60,24 +56,6 @@
135 </participants>
136 <BLANKLINE>
137
138-We have a nice way to add locations for all the members of a team. We look
139-at a page which shows all the members of the team on a map, together with
140-links to edit the location of folks who have not yet been mapped. When we
141-click on those links, we pass the name of the team, so when the location is
142-added, we come back to the team map, and from there we can click on the next
143-person.
144-
145- >>> admin_browser.open('http://launchpad.dev/~guadamen/+map')
146- >>> admin_browser.getLink('Edgar Bursic').click()
147- >>> admin_browser.url
148- 'http://launchpad.dev/~edgar/+editlocation?for_team=guadamen'
149- >>> admin_browser.getControl(name='field.location.latitude').value = '37.3'
150- >>> admin_browser.getControl(name='field.location.longitude').value = '2.45'
151- >>> admin_browser.getControl(name='field.location.time_zone').value = ['Europe/Madrid']
152- >>> admin_browser.getControl('Update').click()
153- >>> admin_browser.url
154- 'http://launchpad.dev/~guadamen/+map'
155-
156 It doesn't make sense to edit the location of the team itself, not even
157 if we are an admin, so a team's +editlocation page will simply redirect
158 to +map.
159@@ -92,13 +70,13 @@
160 The display name of all team participants will be escaped to prevent
161 XSS attacks on any callsite of +mapdata.
162
163- >>> admin_browser.open('http://launchpad.dev/~edgar/+edit')
164+ >>> admin_browser.open('http://launchpad.dev/~kamion/+edit')
165 >>> admin_browser.getControl('Display Name').value = (
166- ... "<script>alert('John \"nasty\"');</script>")
167+ ... "<script>alert('Colin \"nasty\"');</script>")
168 >>> admin_browser.getControl('Save Changes').click()
169
170 >>> anon_browser.open('http://launchpad.dev/~guadamen/+mapdata')
171 >>> print anon_browser.contents
172 <?xml version="1.0"...
173- ...displayname="&amp;lt;script&amp;gt;alert('John &amp;quot;nasty&amp;quot;');&amp;lt;/script&amp;gt;"
174+ ...displayname="&amp;lt;script&amp;gt;alert('Colin &amp;quot;nasty&amp;quot;');&amp;lt;/script&amp;gt;"
175 ...
176
177=== modified file 'lib/lp/registry/templates/team-map.pt'
178--- lib/lp/registry/templates/team-map.pt 2009-07-17 17:59:07 +0000
179+++ lib/lp/registry/templates/team-map.pt 2009-09-14 20:48:51 +0000
180@@ -3,10 +3,7 @@
181 xmlns:tal="http://xml.zope.org/namespaces/tal"
182 xmlns:metal="http://xml.zope.org/namespaces/metal"
183 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
184- xml:lang="en"
185- lang="en"
186- dir="ltr"
187- metal:use-macro="view/macro:page/onecolumn"
188+ metal:use-macro="view/macro:page/main_only"
189 i18n:domain="launchpad"
190 >
191
192@@ -14,28 +11,6 @@
193
194 <div metal:fill-slot="main">
195
196- <h1>Members' location</h1>
197-
198- <tal:unmapped condition="view/unmapped_participants_count">
199- <div style="margin: 4px;">
200- <h2>Complete the map!</h2>
201- Do you know the current location for these team members? Set their
202- locations!
203- </div>
204-
205- <div style="margin-bottom: 2em;">
206- <span style="float: left; width: 15em"
207- tal:repeat="person view/unmapped_participants">
208- <a tal:condition="person/name|nothing"
209- tal:attributes="
210- href string:${person/fmt:url}/+editlocation?for_team=${context/name}"
211- title="Set this person's location"
212- ><img src="/@@/edit" />
213- <tal:person replace="person/displayname" /></a>
214- </span>
215- </div>
216- </tal:unmapped>
217-
218 <div style="height:420px;">
219 <div tal:condition="not: view/has_mapped_participants">
220 None of the current participants in
221
222=== modified file 'lib/lp/registry/templates/team-members.pt'
223--- lib/lp/registry/templates/team-members.pt 2009-07-17 17:59:07 +0000
224+++ lib/lp/registry/templates/team-members.pt 2009-09-14 19:32:10 +0000
225@@ -3,27 +3,16 @@
226 xmlns:tal="http://xml.zope.org/namespaces/tal"
227 xmlns:metal="http://xml.zope.org/namespaces/metal"
228 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
229- xml:lang="en"
230- lang="en"
231- dir="ltr"
232- metal:use-macro="context/@@main_template/master"
233+ metal:use-macro="view/macro:page/main_only"
234 i18n:domain="launchpad"
235 >
236
237 <body>
238
239-<metal:portlets fill-slot="portlets">
240- <div tal:replace="structure context/@@+portlet-details" />
241-</metal:portlets>
242-
243 <div metal:fill-slot="main"
244 tal:define="user_can_edit_memberships context/required:launchpad.Edit;
245 active_member_count context/active_member_count">
246
247- <h1>Members of
248- &#8220;<span tal:replace="context/displayname" />&#8221;
249- </h1>
250-
251 <p>Active, pending and former members of this team.</p>
252
253 <ul>
254@@ -34,7 +23,7 @@
255
256 <p>
257 There are <span tal:replace="active_member_count">15</span>
258- direct members of the
259+ direct members of the
260 "<span tal:replace="context/displayname">Ubuntu Members</span>" team,
261 and <span tal:replace="context/all_member_count">23</span> people are
262 members in total, directly and indirectly through other team
263@@ -103,7 +92,7 @@
264
265 <li tal:condition="not: active_member_count">
266 <h2>
267- &#8220;<span tal:replace="context/displayname" />&#8221; has no
268+ &#8220;<span tal:replace="context/displayname" />&#8221; has no
269 members
270 </h2>
271
272
273=== modified file 'lib/lp/registry/templates/team-mugshots.pt'
274--- lib/lp/registry/templates/team-mugshots.pt 2009-07-17 17:59:07 +0000
275+++ lib/lp/registry/templates/team-mugshots.pt 2009-09-14 19:32:10 +0000
276@@ -3,32 +3,25 @@
277 xmlns:tal="http://xml.zope.org/namespaces/tal"
278 xmlns:metal="http://xml.zope.org/namespaces/metal"
279 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
280- xml:lang="en"
281- lang="en"
282- dir="ltr"
283- metal:use-macro="context/@@main_template/master"
284+ metal:use-macro="view/macro:page/main_only"
285 i18n:domain="launchpad"
286 >
287
288 <body>
289
290-<metal:heading fill-slot="pageheading">
291- <h1>Who's in this team?</h1>
292-</metal:heading>
293-
294 <div metal:fill-slot="main">
295
296-<tal:per_member repeat="member view/allmembers">
297- <a tal:condition="member/mugshot"
298- tal:attributes="
299- href member/fmt:url;
300- title member/displayname">
301- <img tal:attributes="src member/mugshot/http_url"/>
302- </a>
303- <span style="white-space: nowrap;" tal:condition="not: member/mugshot">
304- [<a tal:content="member/displayname"
305- tal:attributes="href member/fmt:url">Foo Bar</a>]</span>
306-</tal:per_member>
307+ <tal:per_member repeat="member view/allmembers">
308+ <a tal:condition="member/mugshot"
309+ tal:attributes="
310+ href member/fmt:url;
311+ title member/displayname">
312+ <img tal:attributes="src member/mugshot/http_url"/>
313+ </a>
314+ <span style="white-space: nowrap;" tal:condition="not: member/mugshot">
315+ [<a tal:content="member/displayname"
316+ tal:attributes="href member/fmt:url">Foo Bar</a>]</span>
317+ </tal:per_member>
318
319 </div>
320 </body>