Launchpad itself

Merge lp:~ursinha/launchpad/bug-391569-add-last-changed-column into lp:launchpad

  1. bug-391569-add-last-changed-column
  2. Merge into devel
Proposed by Ursula Junque
Status: Merged
Approved by: Ursula Junque
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~ursinha/launchpad/bug-391569-add-last-changed-column
Merge into: lp:launchpad
Diff against target: 332 lines (+91/-25)
5 files modified
lib/lp/registry/model/productseries.py (+23/-7)
lib/lp/translations/interfaces/productserieslanguage.py (+7/-2)
lib/lp/translations/model/productserieslanguage.py (+10/-1)
lib/lp/translations/templates/productseries-translations-languages.pt (+20/-0)
lib/lp/translations/tests/test_productserieslanguage.py (+31/-15)
To merge this branch: bzr merge lp:~ursinha/launchpad/bug-391569-add-last-changed-column
Reviewer Review Type Date Requested Status
Graham Binns (community) Approve
Review via email: mp+14366@code.launchpad.net

Commit message

[r=gmb][ui=none][bug=391569] Adds a Last Changed column to +translations pages.

To post a comment you must log in.
Revision history for this message
Ursula Junque (ursinha) wrote :

= Summary =

https://bugs.edge.launchpad.net/rosetta/+bug/391569

This branch adds a "Last Changed" date in +translations pages, showing
the last date a translation in a given language was touched.

== Proposed fix ==

Added a last_changed_date property to the productserieslanguages interface, and changed
productserieslanguage to also return the last date changed among all POFiles
in a product, of a language.

== Pre-implementation notes ==

Discussed with Danilo.

== Tests ==

This should work:
  bin/test -vvt lp.translations.*

(but I did a full test run just to be sure :)

== Demo and Q/A ==

To check the changed pages, go to:
 - https://translations.launchpad.dev/evolution/trunk/+translations
 - https://translations.launchpad.dev/alsa-utils/trunk

= 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/model/productseries.py
  lib/lp/translations/interfaces/productserieslanguage.py
  lib/lp/translations/tests/test_productserieslanguage.py
  lib/lp/translations/templates/productseries-translations-languages.pt
  lib/lp/translations/model/productserieslanguage.py

== Pylint notices ==

lib/lp/translations/interfaces/productserieslanguage.py
    6: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)

Revision history for this message
Graham Binns (gmb) wrote :
Download full text (4.6 KiB)

Hi Ursula,

Nice branch! I've got a few comments but they're all stylistic nitpicks, so I'm happy to approve this with the changes I've requested.

> === modified file 'lib/lp/registry/model/productseries.py'
> --- lib/lp/registry/model/productseries.py 2009-10-16 15:00:55 +0000
> +++ lib/lp/registry/model/productseries.py 2009-11-03 15:00:31 +0000
> @@ -526,10 +528,21 @@
> POTemplate.iscurrent==True,
> Language.id!=english.id).group_by(Language)
>
> - for (language, imported, changed, new, unreviewed) in (
> - query.order_by(['Language.englishname'])):
> + # XXX: Ursinha 2009-11-02: The Max(POFile.date_changed) result
> + # here is a naive datetime. My guess is that it happens
> + # because UTC awareness is attibuted to the field in the POFile
> + # model class, and in this case the Max function deals directly
> + # with the value returned from the database without
> + # instantiating it.
> + # This seems to be irrelevant to what we're trying to achieve
> + # here, but making a note either way.
> +
> + for (language, imported, changed, new, unreviewed,
> + last_changed) in (
> + query.order_by(['Language.englishname'])):

This is a little hard to read. Maybe something like:

    ordered_results = query.order_by(['Language.englishname'])
    for (language, imported, changed, new, unreviewed,
        last_changed) in ordered_results:
        ...

Would be a little easier.

Also note that the indent on long loop definitions should still be four
spaces.

> psl = ProductSeriesLanguage(self, language)
> - psl.setCounts(total, imported, changed, new, unreviewed)
> + psl.setCounts(total, imported, changed, new, unreviewed,
> + last_changed)

When we wrap long method calls, we should wrap them like this:

    psl.setCounts(
        total, imported, changed, new, unreviewed, last_changed)

> results.append(psl)
>
> return results
>
> === modified file 'lib/lp/translations/tests/test_productserieslanguage.py'
> --- lib/lp/translations/tests/test_productserieslanguage.py 2009-09-12 16:07:05 +0000
> +++ lib/lp/translations/tests/test_productserieslanguage.py 2009-11-03 15:00:31 +0000
> @@ -46,6 +46,7 @@
> # Add a Serbian translation.
> serbian = getUtility(ILanguageSet).getLanguageByCode('sr')
> sr_pofile = self.factory.makePOFile(serbian.code, potemplate)
> +
> psls = list(self.productseries.productserieslanguages)
> self.assertEquals(len(psls), 1)
>
> @@ -111,7 +112,8 @@
> removeSecurityProxy(potemplate).messagecount = number_of_potmsgsets
> return potemplate
>
> - def setPOFileStatistics(self, pofile, imported, changed, new, unreviewed):
> + def setPOFileStatistics(self, pofile, imported, changed, new, unreviewed,
> + date_changed):

The wrapped lines of a method definition should start under the first
argument, thus:

    def setPOFileStatistics(self, pofile, imported, chang...

Read more...

review: Approve
Revision history for this message
Ursula Junque (ursinha) wrote :
Download full text (4.1 KiB)

> Hi Ursula,
>
> Nice branch! I've got a few comments but they're all stylistic nitpicks, so
> I'm happy to approve this with the changes I've requested.

All changes applied, incremental diff below. Thanks!

=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2009-11-03 04:55:45 +0000
+++ lib/lp/registry/model/productseries.py 2009-11-04 17:15:56 +0000
@@ -493,13 +493,15 @@
                 POTemplate.iscurrent==True,
                 Language.id!=english.id)

- for language, pofile in query.order_by(['Language.englishname']):
+ ordered_results = query.order_by(['Language.englishname'])
+
+ for language, pofile in ordered_results:
                 psl = ProductSeriesLanguage(self, language, pofile=pofile)
                 psl.setCounts(pofile.potemplate.messageCount(),
                               pofile.currentCount(),
                               pofile.updatesCount(),
                               pofile.rosettaCount(),
- pofile.unreviewedCount(),
+ pofile.unreviewedCount(),
                               pofile.date_changed)
                 results.append(psl)
         else:
@@ -537,12 +539,13 @@
             # This seems to be irrelevant to what we're trying to achieve
             # here, but making a note either way.

- for (language, imported, changed, new, unreviewed,
- last_changed) in (
- query.order_by(['Language.englishname'])):
+ ordered_results = query.order_by(['Language.englishname'])
+
+ for (language, imported, changed, new, unreviewed,
+ last_changed) in ordered_results:
                 psl = ProductSeriesLanguage(self, language)
- psl.setCounts(total, imported, changed, new, unreviewed,
- last_changed)
+ psl.setCounts(
+ total, imported, changed, new, unreviewed, last_changed)
                 results.append(psl)

         return results

=== modified file 'lib/lp/translations/tests/test_productserieslanguage.py'
--- lib/lp/translations/tests/test_productserieslanguage.py 2009-11-03 13:53:07 +0000
+++ lib/lp/translations/tests/test_productserieslanguage.py 2009-11-04 17:15:56 +0000
@@ -113,7 +113,7 @@
         return potemplate

     def setPOFileStatistics(self, pofile, imported, changed, new, unreviewed,
- ...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/productseries.py'
2--- lib/lp/registry/model/productseries.py 2009-10-16 15:00:55 +0000
3+++ lib/lp/registry/model/productseries.py 2009-11-10 18:11:11 +0000
4@@ -15,7 +15,7 @@
5
6 from sqlobject import (
7 ForeignKey, StringCol, SQLMultipleJoin, SQLObjectNotFound)
8-from storm.expr import Sum
9+from storm.expr import Sum, Max
10 from zope.component import getUtility
11 from zope.interface import implements
12 from storm.locals import And, Desc
13@@ -493,13 +493,16 @@
14 POTemplate.iscurrent==True,
15 Language.id!=english.id)
16
17- for language, pofile in query.order_by(['Language.englishname']):
18+ ordered_results = query.order_by(['Language.englishname'])
19+
20+ for language, pofile in ordered_results:
21 psl = ProductSeriesLanguage(self, language, pofile=pofile)
22 psl.setCounts(pofile.potemplate.messageCount(),
23 pofile.currentCount(),
24 pofile.updatesCount(),
25 pofile.rosettaCount(),
26- pofile.unreviewedCount())
27+ pofile.unreviewedCount(),
28+ pofile.date_changed)
29 results.append(psl)
30 else:
31 # If there is more than one template, do a single
32@@ -517,7 +520,8 @@
33 Sum(POFile.currentcount),
34 Sum(POFile.updatescount),
35 Sum(POFile.rosettacount),
36- Sum(POFile.unreviewed_count)),
37+ Sum(POFile.unreviewed_count),
38+ Max(POFile.date_changed)),
39 POFile.language==Language.id,
40 POFile.variant==None,
41 Language.visible==True,
42@@ -526,10 +530,22 @@
43 POTemplate.iscurrent==True,
44 Language.id!=english.id).group_by(Language)
45
46- for (language, imported, changed, new, unreviewed) in (
47- query.order_by(['Language.englishname'])):
48+ # XXX: Ursinha 2009-11-02: The Max(POFile.date_changed) result
49+ # here is a naive datetime. My guess is that it happens
50+ # because UTC awareness is attibuted to the field in the POFile
51+ # model class, and in this case the Max function deals directly
52+ # with the value returned from the database without
53+ # instantiating it.
54+ # This seems to be irrelevant to what we're trying to achieve
55+ # here, but making a note either way.
56+
57+ ordered_results = query.order_by(['Language.englishname'])
58+
59+ for (language, imported, changed, new, unreviewed,
60+ last_changed) in ordered_results:
61 psl = ProductSeriesLanguage(self, language)
62- psl.setCounts(total, imported, changed, new, unreviewed)
63+ psl.setCounts(
64+ total, imported, changed, new, unreviewed, last_changed)
65 results.append(psl)
66
67 return results
68
69=== modified file 'lib/lp/translations/interfaces/productserieslanguage.py'
70--- lib/lp/translations/interfaces/productserieslanguage.py 2009-09-11 09:29:51 +0000
71+++ lib/lp/translations/interfaces/productserieslanguage.py 2009-11-10 18:11:11 +0000
72@@ -7,7 +7,7 @@
73
74 from zope.interface import Attribute, Interface
75 from zope.schema import (
76- Choice, TextLine)
77+ Choice, Datetime, TextLine)
78
79 from canonical.launchpad import _
80 from lp.translations.interfaces.pofile import IPOFile
81@@ -45,6 +45,11 @@
82 "language. This includes only the real pofiles where translations "
83 "exist.")
84
85+
86+ last_changed_date = Datetime(
87+ title=_('When this file was last changed.'))
88+
89+
90 def getPOFilesFor(potemplates):
91 """Return `POFiles` for each of `potemplates`, in the same order.
92
93@@ -52,7 +57,7 @@
94 required language, a `DummyPOFile` is provided.
95 """
96
97- def setCounts(total, imported, changed, new, unreviewed):
98+ def setCounts(total, imported, changed, new, unreviewed, last_changed):
99 """Set aggregated message counts for ProductSeriesLanguage."""
100
101 def recalculateCounts(total, imported, changed, new, unreviewed):
102
103=== modified file 'lib/lp/translations/model/productserieslanguage.py'
104--- lib/lp/translations/model/productserieslanguage.py 2009-09-14 11:27:37 +0000
105+++ lib/lp/translations/model/productserieslanguage.py 2009-11-10 18:11:11 +0000
106@@ -36,12 +36,13 @@
107 self.variant = variant
108 self.pofile = pofile
109 self.id = 0
110+ self._last_changed_date = None
111
112 # Reset all cached counts.
113 self.setCounts()
114
115 def setCounts(self, total=None, imported=None, changed=None, new=None,
116- unreviewed=None):
117+ unreviewed=None, last_changed=None):
118 """See `IProductSeriesLanguage`."""
119 self._messagecount = total
120 # "currentcount" in RosettaStats conflicts our recent terminology
121@@ -51,6 +52,9 @@
122 self._updatescount = changed
123 self._rosettacount = new
124 self._unreviewed_count = unreviewed
125+ if last_changed is not None:
126+ self._last_changed_date = last_changed
127+
128
129 def _getMessageCount(self):
130 store = Store.of(self.language)
131@@ -112,6 +116,11 @@
132 return self._unreviewed_count
133
134 @property
135+ def last_changed_date(self):
136+ """See `IProductSeriesLanguage`."""
137+ return self._last_changed_date
138+
139+ @property
140 def pofiles(self):
141 """See `IProductSeriesLanguage`."""
142 store = Store.of(self.language)
143
144=== modified file 'lib/lp/translations/templates/productseries-translations-languages.pt'
145--- lib/lp/translations/templates/productseries-translations-languages.pt 2009-09-10 07:24:24 +0000
146+++ lib/lp/translations/templates/productseries-translations-languages.pt 2009-11-10 18:11:11 +0000
147@@ -11,6 +11,7 @@
148 <th>Untranslated</th>
149 <th>Needs review</th>
150 <th>Changed</th>
151+ <th>Last Changed</th>
152 </tr>
153 </thead>
154 <tbody>
155@@ -47,6 +48,25 @@
156 tal:content="language_stats/updatesCount">0</span>
157 <tal:value content="language_stats/updatesCount" />
158 </td>
159+ <td>
160+ <span class="sortkey"
161+ tal:condition="language_stats/last_changed_date"
162+ tal:content="language_stats/last_changed_date/fmt:datetime">
163+ time sort key
164+ </span>
165+ <span
166+ tal:condition="language_stats/last_changed_date"
167+ tal:attributes="
168+ title language_stats/last_changed_date/fmt:datetime"
169+ tal:content="
170+ language_stats/last_changed_date/fmt:approximatedate"
171+ >
172+ 2007-02-10
173+ </span>
174+ <tal:block condition="not: language_stats/last_changed_date">
175+ &mdash;
176+ </tal:block>
177+ </td>
178 </tr>
179 </tbody>
180 </table>
181
182=== modified file 'lib/lp/translations/tests/test_productserieslanguage.py'
183--- lib/lp/translations/tests/test_productserieslanguage.py 2009-09-12 16:07:05 +0000
184+++ lib/lp/translations/tests/test_productserieslanguage.py 2009-11-10 18:11:11 +0000
185@@ -46,6 +46,7 @@
186 # Add a Serbian translation.
187 serbian = getUtility(ILanguageSet).getLanguageByCode('sr')
188 sr_pofile = self.factory.makePOFile(serbian.code, potemplate)
189+
190 psls = list(self.productseries.productserieslanguages)
191 self.assertEquals(len(psls), 1)
192
193@@ -111,7 +112,8 @@
194 removeSecurityProxy(potemplate).messagecount = number_of_potmsgsets
195 return potemplate
196
197- def setPOFileStatistics(self, pofile, imported, changed, new, unreviewed):
198+ def setPOFileStatistics(self, pofile, imported, changed, new, unreviewed,
199+ date_changed):
200 # Instead of creating all relevant translation messages, we
201 # just fake cached statistics instead.
202 naked_pofile = removeSecurityProxy(pofile)
203@@ -119,6 +121,7 @@
204 naked_pofile.updatescount = changed
205 naked_pofile.rosettacount = new
206 naked_pofile.unreviewed_count = unreviewed
207+ naked_pofile.date_changed = date_changed
208 naked_pofile.sync()
209
210 def setUp(self):
211@@ -136,20 +139,22 @@
212 psl.currentCount(),
213 psl.rosettaCount(),
214 psl.updatesCount(),
215- psl.unreviewedCount()),
216- stats)
217+ psl.unreviewedCount(),
218+ psl.last_changed_date),
219+ stats)
220
221 def test_DummyProductSeriesLanguage(self):
222 # With no templates all counts are zero.
223 psl = self.psl_set.getDummy(self.productseries, self.language)
224 self.failUnless(verifyObject(IProductSeriesLanguage, psl))
225- self.assertPSLStatistics(psl, (0, 0, 0, 0, 0, 0))
226+ self.assertPSLStatistics(psl, (0, 0, 0, 0, 0, 0, None))
227
228 # Adding a single template with 10 messages makes the total
229 # count of messages go up to 10.
230 potemplate = self.createPOTemplateWithPOTMsgSets(10)
231 psl = self.psl_set.getDummy(self.productseries, self.language)
232- self.assertPSLStatistics(psl, (10, 0, 0, 0, 0, 0))
233+ self.assertPSLStatistics(
234+ psl, (10, 0, 0, 0, 0, 0, None))
235
236 def test_OneTemplate(self):
237 # With only one template, statistics match those of the POFile.
238@@ -158,7 +163,7 @@
239
240 # Set statistics to 4 imported, 3 new in rosetta (out of which 2
241 # are updates) and 5 with unreviewed suggestions.
242- self.setPOFileStatistics(pofile, 4, 2, 3, 5)
243+ self.setPOFileStatistics(pofile, 4, 2, 3, 5, pofile.date_changed)
244
245 # Getting PSL through PSLSet gives an uninitialized object.
246 psl = self.psl_set.getProductSeriesLanguage(
247@@ -173,7 +178,8 @@
248 pofile.currentCount(),
249 pofile.rosettaCount(),
250 pofile.updatesCount(),
251- pofile.unreviewedCount()))
252+ pofile.unreviewedCount(),
253+ pofile.date_changed))
254
255 def test_TwoTemplates(self):
256 # With two templates, statistics are added up.
257@@ -181,27 +187,34 @@
258 pofile1 = self.factory.makePOFile(self.language.code, potemplate1)
259 # Set statistics to 4 imported, 3 new in rosetta (out of which 2
260 # are updates) and 5 with unreviewed suggestions.
261- self.setPOFileStatistics(pofile1, 4, 2, 3, 5)
262+ self.setPOFileStatistics(pofile1, 4, 2, 3, 5, pofile1.date_changed)
263
264 potemplate2 = self.createPOTemplateWithPOTMsgSets(20)
265 pofile2 = self.factory.makePOFile(self.language.code, potemplate2)
266 # Set statistics to 1 imported, 1 new in rosetta (which is also the
267 # 1 update) and 1 with unreviewed suggestions.
268- self.setPOFileStatistics(pofile2, 1, 1, 1, 1)
269+ self.setPOFileStatistics(pofile2, 1, 1, 1, 1, pofile2.date_changed)
270
271 psl = self.productseries.productserieslanguages[0]
272
273+ # The psl.last_changed_date here is a naive datetime. So, for sake of
274+ # the tests, we should make pofile2 naive when checking if it matches
275+ # the last calculated changed date, that should be the same as
276+ # pofile2, created last.
277+
278 # Total is a sum of totals in both POTemplates (10+20).
279 # Translated is a sum of imported and rosetta translations,
280 # which adds up as (4+3)+(1+1).
281- self.assertPSLStatistics(psl, (30, 9, 5, 4, 3, 6))
282+ self.assertPSLStatistics(psl, (30, 9, 5, 4, 3, 6,
283+ pofile2.date_changed.replace(tzinfo=None)))
284 self.assertPSLStatistics(psl, (
285 pofile1.messageCount() + pofile2.messageCount(),
286 pofile1.translatedCount() + pofile2.translatedCount(),
287 pofile1.currentCount() + pofile2.currentCount(),
288 pofile1.rosettaCount() + pofile2.rosettaCount(),
289 pofile1.updatesCount() + pofile2.updatesCount(),
290- pofile1.unreviewedCount() + pofile2.unreviewedCount()))
291+ pofile1.unreviewedCount() + pofile2.unreviewedCount(),
292+ pofile2.date_changed.replace(tzinfo=None)))
293
294 def test_recalculateCounts(self):
295 # Test that recalculateCounts works correctly.
296@@ -210,21 +223,23 @@
297
298 # Set statistics to 1 imported, 3 new in rosetta (out of which 2
299 # are updates) and 4 with unreviewed suggestions.
300- self.setPOFileStatistics(pofile1, 1, 2, 3, 4)
301+ self.setPOFileStatistics(pofile1, 1, 2, 3, 4, pofile1.date_changed)
302
303 potemplate2 = self.createPOTemplateWithPOTMsgSets(20)
304 pofile2 = self.factory.makePOFile(self.language.code, potemplate2)
305 # Set statistics to 1 imported, 1 new in rosetta (which is also the
306 # 1 update) and 1 with unreviewed suggestions.
307- self.setPOFileStatistics(pofile2, 1, 1, 1, 1)
308+ self.setPOFileStatistics(pofile2, 1, 1, 1, 1, pofile2.date_changed)
309
310 psl = self.psl_set.getProductSeriesLanguage(self.productseries,
311 self.language)
312+ # recalculateCounts() doesn't recalculate the last changed date.
313 psl.recalculateCounts()
314 # Total is a sum of totals in both POTemplates (10+20).
315 # Translated is a sum of imported and rosetta translations,
316 # which adds up as (1+3)+(1+1).
317- self.assertPSLStatistics(psl, (30, 6, 2, 4, 3, 5))
318+ self.assertPSLStatistics(psl, (30, 6, 2, 4, 3, 5,
319+ None))
320
321 def test_recalculateCounts_no_pofiles(self):
322 # Test that recalculateCounts works correctly even when there
323@@ -235,7 +250,8 @@
324 self.language)
325 psl.recalculateCounts()
326 # And all the counts are zero.
327- self.assertPSLStatistics(psl, (3, 0, 0, 0, 0, 0))
328+ self.assertPSLStatistics(psl, (3, 0, 0, 0, 0, 0,
329+ None))
330
331
332 def test_suite():