Merge lp:~danilo/launchpad/bug-414832 into lp:launchpad

[Данило Шеган]

[Данило Шеган (danilo)]

= Bug #414832 =

I've used a silly little clause that reduces duplicate rows in SQL when
doing symmetric joins of the same table twice. However, Jeroen rightly
noted how this is not the right thing to do here, so here's a fix.

Note that this code has not rolled out to production yet, so I can
thank Jeroen for catching this so early even before the QA.

== Proposed fix ==

Remove "other.id >= this.id" clause from SQL.

== Tests ==

bin/test -vvt getPOFilesTouchedSince_bug_414832

(I didn't want to bother to figure out a better name for the test,
since the test is basically what it is; it's very dependent on the
previous broken implementation).

== Demo and Q/A ==

Test

  cronscripts/rosetta-pofile-stats-daily.py

on staging.

= 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/translations/tests/test_pofile.py
  lib/lp/translations/model/pofile.py

[Henning Eggers (henninge)]

With test rename as discussed.

review: Approve

=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2009-08-07 17:58:19 +0000
+++ lib/lp/translations/model/pofile.py	2009-08-17 15:20:16 +0000
@@ -1584,8 +1584,7 @@
         # and its ProductSeries and DistroSeries, if they are defined.
         OtherPOT = ClassAlias(POTemplate)
         OtherPOTJoin = Join(
-            OtherPOT, And(OtherPOT.name == MatchingPOT.name,
-                          OtherPOT.id >= MatchingPOT.id))
+            OtherPOT, And(OtherPOT.name == MatchingPOT.name))
         OtherProductSeries = ClassAlias(ProductSeries)
         OtherProductSeriesJoin = LeftJoin(
             OtherProductSeries,

=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py	2009-08-07 17:58:19 +0000
+++ lib/lp/translations/tests/test_pofile.py	2009-08-17 15:20:16 +0000
@@ -1194,6 +1194,32 @@
         pofiles = self.pofileset.getPOFilesTouchedSince(yesterday)
         self.assertContentEqual([pofile1, pofile2], pofiles)
 
+    def test_POFileSet_getPOFilesTouchedSince_bug_414832(self):
+        # Make sure that all relevant POFiles are returned,
+        # even the sharing ones with smaller IDs.
+        product = self.factory.makeProduct()
+        product.official_rosetta = True
+        series1 = self.factory.makeProductSeries(product=product,
+                                                 name='one')
+        series2 = self.factory.makeProductSeries(product=product,
+                                                 name='two')
+        potemplate1 = self.factory.makePOTemplate(name='shared',
+                                                  productseries=series1)
+        pofile1 = self.factory.makePOFile('sr', potemplate=potemplate1)
+        potemplate2 = self.factory.makePOTemplate(name='shared',
+                                                  productseries=series2)
+        pofile2 = potemplate2.getPOFileByLang('sr')
+        now = datetime.now(pytz.UTC)
+        yesterday = now - timedelta(1)
+        week_ago = now - timedelta(7)
+        pofile1.date_changed = week_ago
+
+        # Let's make sure the condition from the bug holds,
+        # since pofile2 is created implicitely with the makePOTemplate call.
+        self.assertTrue(pofile1.id < pofile2.id)
+        pofiles = self.pofileset.getPOFilesTouchedSince(yesterday)
+        self.assertContentEqual([pofile1, pofile2], pofiles)
+
     def test_POFileSet_getPOFilesTouchedSince_shared_in_distribution(self):
         # Make sure actual touched POFiles and POFiles that are sharing
         # with them in the same distribution/sourcepackage are all returned