launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-998992 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #998992 in Launchpad itself: ""Recently fixed" list on MaloneApplication:+index has nulls first"
  https://bugs.launchpad.net/launchpad/+bug/998992

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-998992/+merge/105617

Last week I removed a 'date_closed > foo' optimisation from most_recently_fixed_bugs, as it was no longer useful. But it turned out that this served another purpose on production: filtering out tasks where date_closed is unset. There are ~8000 Fix Released tasks on production without date_closed set, and those 8000 end up at the top of a list sorted by date_closed DESC.

This branch adds a date_closed IS NOT NULL to prevent them from showing up. We could potentially eventually fix the data too.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-998992/+merge/105617
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-998992 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2012-05-11 12:53:58 +0000
+++ lib/lp/bugs/browser/bug.py	2012-05-14 07:38:21 +0000
@@ -100,7 +100,10 @@
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.mail.mailwrapper import MailWrapper
 from lp.services.propertycache import cachedproperty
-from lp.services.searchbuilder import any
+from lp.services.searchbuilder import (
+    any,
+    not_equals,
+    )
 from lp.services.webapp import (
     canonical_url,
     ContextMenu,
@@ -422,7 +425,7 @@
         """Return the five most recently fixed bugs."""
         params = BugTaskSearchParams(
             self.user, status=BugTaskStatus.FIXRELEASED,
-            orderby='-date_closed')
+            date_closed=not_equals(None), orderby='-date_closed')
         return getUtility(IBugSet).getDistinctBugsForBugTasks(
             self.context.searchTasks(params), self.user, limit=5)
 

=== modified file 'lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt'
--- lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt	2012-05-14 07:38:21 +0000
@@ -4,9 +4,12 @@
 On the bugs front page there is a list of the most recently reported
 and recently fixed bugs, across all products and distributions.
 
-To demonstrate this, a few fixed bugs must be created.
+To demonstrate this, a few fixed bugs must be created. date_closed isn't
+always set in old real data, so we create a bug without it set to
+confirm it doesn't show up.
 
     >>> from zope.component import getUtility
+    >>> from zope.security.proxy import removeSecurityProxy
     >>> from lp.testing import login, logout
     >>> import transaction
     >>> from lp.services.webapp.interfaces import ILaunchBag
@@ -16,6 +19,7 @@
     ...     )
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> bigfixer = factory.makeProduct(name='bigfixer')
+    >>> bugs = []
     >>> for bug_x in range(1, 11):
     ...     summary = 'Summary for new bug %d' % bug_x
     ...     bug = factory.makeBug(title=summary, product=bigfixer)
@@ -23,6 +27,8 @@
     ...         BugTaskStatus.FIXRELEASED, getUtility(ILaunchBag).user)
     ...     bug.default_bugtask.transitionToImportance(
     ...         BugTaskImportance.HIGH, getUtility(ILaunchBag).user)
+    ...     bugs.append(bug)
+    >>> removeSecurityProxy(bugs[7].default_bugtask).date_closed = None
     >>> transaction.commit()
     >>> logout()
 
@@ -76,7 +82,8 @@
     >>> fixed_bugs = find_tag_by_id(anon_browser.contents, 'fixed-bugs')
 
 The list of recently fixed bugs contains up to the last 5 bugs fixed
-across Launchpad.
+across Launchpad. The 8th new bug is not listed because it has a null
+date_closed.
 
     >>> for li in fixed_bugs('li'):
     ...     print_bugs_links(li)
@@ -89,10 +96,6 @@
     /bugs/...
     /bigfixer
     Bigfixer:
-             Bug #...: Summary for new bug 8
-    /bugs/...
-    /bigfixer
-    Bigfixer:
              Bug #...: Summary for new bug 7
     /bugs/...
     /bigfixer
@@ -100,3 +103,7 @@
              Bug #...: Summary for new bug 6
     /bugs/...
     /bigfixer
+    Bigfixer:
+             Bug #...: Summary for new bug 5
+    /bugs/...
+    /bigfixer