Merge lp:~jtv/launchpad/bug-358404 into lp:launchpad

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 358404 =

This bug just rose in relevance after an unknown number of Translations
import queue entries got approved for import into the wrong templates.
We found out about the problem within hours, and it would have been easy
to identify the exact queue entries that had the problem. But because
the "date_status_changed" property on the entries was not updated during
approval or import, and because the entries were old, the cleanup that
was supposed to have removed them 3 days after import deleted them
instantly instead.

Here's a branch to change that. We had already isolated the setting of
an entry's status into a setStatus method (mostly to simplify API work),
and that is the perfect place to update the date_status_changed as well.

One test was updated very slightly. The reason is relatively complex:
two of these dates were being compared, but the one that was supposed to
be further into the past is getting an extra "refresh" from this change.
In addition, the timestamp used is the beginning of the current database
transaction. Consequently the clock doesn't advance between actions in
the same transaction, and this contributed to the "older" date not being
further in the past than the "newer" one.

I fixed this with an extra commit, and took the opportunity to add a
check around just the setStatus() call.

Tests:
{{{
./bin/test -vv -t translationimportqueue
}}}

For Q/A, pick an entry on the import queue (/+imports) that is at least
3 days old, and is in a Needs Review state. Approve it. Wait for the
import to occur (watch it go to Imported, which means it should be
cleaned up after three days), and then wait for an hour or two more to
let the cleanup process run. Or if we're talking launchpad.dev, import
the file by running the rosetta-poimport.py cron job; and then run
rosetta-approve-imports.py to execute the cleanup.

The cleanup will clean up the entry 3 days after the import attempt,
not 3 days after the upload.

No lint.

Jeroen

[Eleanor Berger (intellectronica)]

review: Approve

=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
--- lib/lp/translations/doc/translationimportqueue.txt	2009-07-23 17:49:31 +0000
+++ lib/lp/translations/doc/translationimportqueue.txt	2009-09-28 09:49:14 +0000
@@ -130,8 +130,12 @@
     >>> login('[e-mail]')
     >>> entry.setStatus(RosettaImportStatus.IMPORTED)
 
-Need to update our saved date_status_changed to detect if it changes now.
-
+The status change updates date_status_changed as well.
+
+    >>> entry.date_status_changed > previous_date_status_changed
+    True
+
+    >>> transaction.commit()
     >>> previous_date_status_changed = entry.date_status_changed
 
 Do the new upload. It will be a published upload.
@@ -144,15 +148,15 @@
 
 And the new status is
 
-    >>> po_sr_entry.status.title
-    'Needs Review'
+    >>> print po_sr_entry.status.title
+    Needs Review
 
 The dateimported remains the same as it was already waiting to be imported.
 
     >>> po_sr_entry.dateimported > previous_dateimported
     True
 
-And the date_status_changed is newer
+However the date_status_changed is still updated.
 
     >>> po_sr_entry.date_status_changed > previous_date_status_changed
     True
 
=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py	2009-09-21 19:31:07 +0000
+++ lib/lp/translations/model/translationimportqueue.py	2009-09-28 09:49:14 +0000
@@ -256,9 +256,8 @@
 
     def setStatus(self, status):
         """See `ITranslationImportQueueEntry`."""
-        # XXX JeroenVermeulen 2009-04-09 bug=358404: This looks like a
-        # good place to set date_status_changed.
         self.status = status
+        self.date_status_changed = UTC_NOW
 
     def setErrorOutput(self, output):
         """See `ITranslationImportQueueEntry`."""