Merge lp:~adiroiban/launchpad/bug-487970 into lp:launchpad

Proposed by Adi Roiban
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-487970
Merge into: lp:launchpad
Diff against target: 230 lines (+102/-33)
3 files modified
lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt (+56/-13)
lib/lp/translations/stories/productseries/xx-productseries-templates.txt (+27/-18)
lib/lp/translations/templates/object-templates.pt (+19/-2)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-487970
Reviewer Review Type Date Requested Status
Данило Шеган (community) code Approve
Review via email: mp+15250@code.launchpad.net

Commit message

Add the Last updated column in the +templates page for distroseries and productseries.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

-- Copied from the commit message

I'm still learning about bug submission process...

= Tests =

./bin/test --cdiff -ct ".*series-templates"

= Demo and Q/A =

You should access the +templates pages for both a producseries and a distroseries.
The table should also display the „Last update” column

= 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/apidoc/wadl-testrunner.xml
  lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt
  lib/lp/translations/stories/productseries/xx-productseries-templates.txt
  lib/lp/translations/templates/object-templates.pt

-- removed diff as it is below

Revision history for this message
Данило Шеган (danilo) wrote :
Download full text (7.1 KiB)

Hi Adi,

Thanks for working on this: it's a very welcome change! You've done a
great job here. I'll have some stylistic comments but don't let that
turn you off: it's just to better cope with development in a large team
such as Launchpad's.

I'd like this page (+templates) to become more of a maintainer-orienteed
starting point: a place they can go to look at stuff like disabled
templates and such. This branch is a step in the right direction.

 review approve code
 status approve

Let me know when you've fixed the minor points I note here so I can land
this for you (though, note that we are in "week 4", which means that our
development trees are closed for anything but "release critical"
landings).

У чет, 26. 11 2009. у 00:44 +0000, Adi Roiban пише:
> = Demo and Q/A =
>
> You should access the +templates pages for both a producseries and a distroseries.
> The table should also display the „Last update” column

As I mentioned earlier, it's useful to get direct links to pages which
you are modifying on launchpad.dev and on eg. edge.launchpad.net: while
I do know where to find them, others might need to spend more time to do
it.

> === modified file 'lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt'
> --- lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-10-30 10:09:17 +0000
> +++ lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-11-25 17:37:01 +0000
> @@ -18,6 +18,44 @@
>
> == The templates table ==
>
> +Anonymous users will see a link from distro series

I'd rather make this something like:

"Full template listing for a distribution series is reached by following
a link from the distribution series translations page."

The idea is to make narrative in tests more like statements and like an
actual user story. In this particular case, using anonymous users just
demonstrates that *everybody* can reach this page.

> + >>> anon_browser.open(
> + ... 'http://translations.launchpad.dev/ubuntu/hoary')
> + >>> anon_browser.getLink('full list of templates').click()
> +
> +The anon page shows a table with only source package, template name and last
> +update.

And this should probably be something like:

"Full listing of templates shows source package name, template name and
the date of last update for this distribution series."

> +
> + >>> table = find_tag_by_id(anon_browser.contents, 'templates_table')
> + >>> print extract_text(table)
> + Source package Template name Last update
> + evolution disabled-template 2007-01-05
> + evolution evolution-2.2 2005-05-06
> + evolution man 2006-08-14
> + mozilla pkgconf-mozilla 2005-05-06
> + pmount man 2006-08-14
> + pmount pmount 2005-05-06
> +
> +
> +Normal users will see a link from distro series

This is relatively unneeded. The only thing you want to demonstrate
with this test is that Download links are shown. You can cut down on
the narrative, and use something as simple as:

"Logged-in users can also choose to download all translations for each
of the templates."

followed b...

Read more...

review: Approve (code)
Revision history for this message
Adi Roiban (adiroiban) wrote :

I copied the diff according to the cover letter guidance https://dev.launchpad.net/DetailedCoverLetterTemplate

Since this was my first review I did not know the exact process.

I changed the pagetests narative.

I have also updated the pagetest style as Abel Deuring suggest this problem in another MP.

HTML style was updated... and got rid of that TAB

I need to increase the table size to accommodate the new column. The action column was wrapped when viewed by an admin.

All translation tests are OK.
Lint is clean.

Thanks for the review.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt'
2--- lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-10-30 10:09:17 +0000
3+++ lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt 2009-12-07 08:26:11 +0000
4@@ -1,11 +1,15 @@
5-= Templates view for DistroSeries =
6+
7+
8+Templates view for DistroSeries
9+===============================
10
11 The +templates view for DistroSeries gives an overview of the translation
12 templates in this series and provides easy access to the various subpages of
13 each template.
14
15
16-== Getting there ==
17+Getting there
18+-------------
19
20 To get to the listing of all templates, one needs to use the link
21 from the distribution series translations page.
22@@ -16,7 +20,45 @@
23 >>> print user_browser.url
24 http://translations.launchpad.dev/ubuntu/hoary/+templates
25
26-== The templates table ==
27+The templates table
28+-------------------
29+
30+Full template listing for a distribution series is reached by following
31+a link from the distribution series translations page.
32+
33+ >>> anon_browser.open(
34+ ... 'http://translations.launchpad.dev/ubuntu/hoary')
35+ >>> anon_browser.getLink('full list of templates').click()
36+
37+Full listing of templates shows source package name, template name and
38+the date of last update for this distribution series.
39+
40+ >>> table = find_tag_by_id(anon_browser.contents, 'templates_table')
41+ >>> print extract_text(table)
42+ Source package Template name Last update
43+ evolution disabled-template 2007-01-05
44+ evolution evolution-2.2 2005-05-06
45+ evolution man 2006-08-14
46+ mozilla pkgconf-mozilla 2005-05-06
47+ pmount man 2006-08-14
48+ pmount pmount 2005-05-06
49+
50+
51+Logged-in users will see a link from distro series
52+ >>> user_browser.open(
53+ ... 'http://translations.launchpad.dev/ubuntu/hoary')
54+ >>> user_browser.getLink('full list of templates').click()
55+
56+Logged-in users can also choose to download all translations for each
57+of the templates.
58+
59+ >>> table = find_tag_by_id(user_browser.contents, 'templates_table')
60+ >>> print extract_text(table)
61+ Source package Template name Last update Actions
62+ evolution disabled-template 2007-01-05 Download
63+ ...
64+ mozilla pkgconf-mozilla 2005-05-06 Download
65+ ...
66
67 Administrator can see all editing options.
68
69@@ -28,16 +70,17 @@
70
71 >>> table = find_tag_by_id(admin_browser.contents, 'templates_table')
72 >>> print extract_text(table)
73- Source package Template name Actions
74- evolution disabled-template Edit Upload Download Administer
75- evolution evolution-2.2 Edit Upload Download Administer
76- evolution man Edit Upload Download Administer
77- mozilla pkgconf-mozilla Edit Upload Download Administer
78- pmount man Edit Upload Download Administer
79- pmount pmount Edit Upload Download Administer
80-
81-
82-== Links to the templates ==
83+ Source package Template name Last update Actions
84+ evolution disabled-template 2007-01-05 Edit Upload Download Administer
85+ evolution evolution-2.2 2005-05-06 Edit Upload Download Administer
86+ evolution man 2006-08-14 Edit Upload Download Administer
87+ mozilla pkgconf-mozilla 2005-05-06 Edit Upload Download Administer
88+ pmount man 2006-08-14 Edit Upload Download Administer
89+ pmount pmount 2005-05-06 Edit Upload Download Administer
90+
91+
92+Links to the templates
93+----------------------
94
95 Clicking on a template name will take the user to that template's overview
96 page.
97
98=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-templates.txt'
99--- lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-10-30 10:09:17 +0000
100+++ lib/lp/translations/stories/productseries/xx-productseries-templates.txt 2009-12-07 08:26:11 +0000
101@@ -1,13 +1,18 @@
102-= Templates view for ProductSeries =
103+
104+
105+Templates view for ProductSeries
106+================================
107
108 The +templates view for ProductSeries gives an overview of the translation
109 templates in this series and provides easy access to the various subpages of
110 each template.
111
112-== Preparation ==
113-
114-To test the ordering of templates in the listing, we need another template
115-that is new but must appear at the top of the list.
116+
117+Preparation
118+-----------
119+
120+To test the ordering of templates in the listing, we need another
121+template that is new but must appear at the top of the list.
122
123 >>> login('foo.bar@canonical.com')
124 >>> from zope.component import getUtility
125@@ -18,7 +23,9 @@
126 ... name='at-the-top')
127 >>> logout()
128
129-== Getting there ==
130+
131+Getting there
132+-------------
133
134 To get to the listing of all templates, one needs to use the link
135 from the product series translations page.
136@@ -30,16 +37,17 @@
137 http://translations.launchpad.dev/evolution/trunk/+templates
138
139
140-== The templates table ==
141+The templates table
142+-------------------
143
144 The page shows a table of all templates and links to their subpages.
145
146 >>> table = find_tag_by_id(user_browser.contents, 'templates_table')
147 >>> print extract_text(table)
148- Template name Actions
149- at-the-top Download
150- evolution-2.2 Download
151- evolution-2.2-test Download
152+ Template name Last update Actions
153+ at-the-top ... Download
154+ evolution-2.2 2005-08-25 Download
155+ evolution-2.2-test 2006-12-13 Download
156
157 If an administrator views this page, links to the templates admin page are
158 shown, too.
159@@ -48,13 +56,14 @@
160 ... 'http://translations.launchpad.dev/evolution/trunk/+templates')
161 >>> table = find_tag_by_id(admin_browser.contents, 'templates_table')
162 >>> print extract_text(table)
163- Template name Actions
164- at-the-top Edit Upload Download Administer
165- evolution-2.2 Edit Upload Download Administer
166- evolution-2.2-test Edit Upload Download Administer
167-
168-
169-== Links to the templates ==
170+ Template name Last update Actions
171+ at-the-top ... Edit Upload Download Administer
172+ evolution-2.2 2005-08-25 Edit Upload Download Administer
173+ evolution-2.2-test 2006-12-13 Edit Upload Download Administer
174+
175+
176+Links to the templates
177+----------------------
178
179 Clicking on a template name will take the user to that template's overview
180 page.
181
182=== modified file 'lib/lp/translations/templates/object-templates.pt'
183--- lib/lp/translations/templates/object-templates.pt 2009-11-24 19:23:52 +0000
184+++ lib/lp/translations/templates/object-templates.pt 2009-12-07 08:26:11 +0000
185@@ -26,12 +26,12 @@
186 </style>
187 <style tal:condition="view/is_distroseries" type="text/css">
188 #templates_table {
189- width: 72em;
190+ width: 79em;
191 }
192 </style>
193 <style tal:condition="not:view/is_distroseries" type="text/css">
194 #templates_table {
195- width: 50em;
196+ width: 58em;
197 }
198 </style>
199 <script language="JavaScript" type="text/javascript">
200@@ -75,6 +75,7 @@
201 <th tal:condition="view/is_distroseries"
202 class="sourcepackage_column">Source package</th>
203 <th class="template_column">Template name</th>
204+ <th class="lastupdate_column">Last update</th>
205 <th class="actions_column"
206 tal:condition="context/required:launchpad.AnyPerson">
207 Actions</th>
208@@ -88,6 +89,22 @@
209 </td>
210 <td class="template_column"><a tal:attributes="href template/fmt:url"
211 tal:content="template/name">Template name</a></td>
212+ <td class="lastupdate_column">
213+ <span class="sortkey"
214+ tal:condition="template/date_last_updated"
215+ tal:content="template/date_last_updated/fmt:datetime">
216+ time sort key
217+ </span>
218+ <span class="lastupdate_column"
219+ tal:condition="template/date_last_updated"
220+ tal:attributes="
221+ title template/date_last_updated/fmt:datetime"
222+ tal:content="
223+ template/date_last_updated/fmt:approximatedate"
224+ >
225+ 2009-09-23
226+ </span>
227+ </td>
228 <td class="actions_column"
229 tal:condition="context/required:launchpad.AnyPerson">
230 <div class="template_links">