Merge lp:~deryck/launchpad/hot-bugs-list-515232 into lp:launchpad

[Deryck Hodge]

Create a true hot bugs list for a bugs home page. Also, provide a link to the full list of hot bugs. (The original version of this was backed out for performance reasons.)

[Deryck Hodge (deryck)]

This branch converts a bugtask.py's hot_bugtasks method to a hot_bugs
method on a bug target. The effect of this change is that the bugs home
page for a project will no longer feature the most recently changed
bugs. The home page will now show the top 10 hottest bugs.

This also fixes bug 442170, bug 77701, and bug 515232.

== Implementation details ==

One version of this landed during week 3 last cycle, but the branch was
reverted last minute due to timeout issues. After much playing on
staging with queries (and after talking with Björn), I feel confident
setting a limit and not doing a secondary sort on -datecreated will fix
the timeouts we had.

I also took the opportunity to clean up some lint and add a link to the
rest of the hot bugs list. Because I added this link, I needed a way to
check that more than 10 hot bugs exist. I made the hot_bugs list a dict
with a flag for has_more_bugs, which allows doing this in one method,
too, avoiding having to look up the hot bugs list again.

== Tests ==

Test with:

./bin/test -cvvt xx-product-bugs-page.txt

== Demo and Q/A ==

To QA, visit any bugs home page (e.g.
https://bugs.launchpad.net/malone/) and confirm that the 10 bugs shown
are the top ten when you follow the link to the full hot bugs list.

= 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/bugs/browser/bugtarget.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/templates/bugtarget-bugs.pt
  lib/lp/bugs/stories/bugs/xx-product-bugs-page.txt

[Gavin Panella (allenap)]

Hi Deryck,

Woo! This is great :)

There are a few issues that need fixing up I think, but nothing that's
going to pose problems.

Gavin.

> === modified file 'lib/lp/bugs/browser/bugtarget.py'
> --- lib/lp/bugs/browser/bugtarget.py 2010-01-29 19:00:47 +0000
> +++ lib/lp/bugs/browser/bugtarget.py 2010-02-10 16:43:36 +0000
> @@ -40,6 +40,7 @@
> from canonical.config import config
> from lp.bugs.browser.bugtask import BugTaskSearchListingView
> from lp.bugs.interfaces.bug import IBug
> +from lp.bugs.interfaces.bugtask import BugTaskSearchParams
> from canonical.launchpad.browser.feeds import (
> BugFeedLink, BugTargetLatestBugsFeedLink, FeedsMixin,
> PersonLatestBugsFeedLink)
> @@ -55,6 +56,7 @@
> from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
> from canonical.launchpad.interfaces.temporaryblobstorage import (
> ITemporaryStorageManager)
> +from canonical.launchpad.searchbuilder import any
> from canonical.launchpad.webapp import urlappend
> from canonical.launchpad.webapp.breadcrumb import Breadcrumb
> from canonical.launchpad.webapp.interfaces import ILaunchBag, NotFoundError
> @@ -1264,6 +1266,28 @@
> else:
> return 'None specified'
>
> + @property
> + def hot_bugs(self):

It's not returning a straight list of hot bugs, so consider renaming
this to hot_bugtasks_info or something like that?

> + """Return a dict of the 10 hottest tasks and a has_more_bugs flag."""
> + has_more_bugs = False
> + params = BugTaskSearchParams(
> + orderby='-heat', omit_dupes=True,
> + user=self.user, status=any(*UNRESOLVED_BUGTASK_STATUSES))
> + # Use 4x as many tasks as bugs that are needed to improve performance.
> + bugtasks = self.context.searchTasks(params)[:40]
> + hot_bugs = []

This will contain bugtasks, so maybe rename it?

> + count = 0

len(hot_bugs) should contain the same number, and is authoritative, so
consider dropping count.

> + for task in bugtasks:
> + # Ensure we only represent a bug once in the list.
> + if task.bug not in [hot_task.bug for hot_task in hot_bugs]:

YOU'RE BURNING MY EYES!

:)

Actually, seeing as it's never going to create a temporary list of
more than 10 bugtasks, I guess it's not crazy. Still, to code
defensively, I think it would be better to accumulate the bugtask's
bugs in a set, and use that to test for membership.

> + if count < 10:
> + hot_bugs.append(task)
> + count += 1
> + else:
> + has_more_bugs = True
> + break
> + return {'has_more_bugs': has_more_bugs, 'bugtasks': hot_bugs}
> +
>
> class BugTargetBugTagsView(LaunchpadView):
> """Helper methods for rendering the bug tags portlet."""
>
> === modified file 'lib/lp/bugs/browser/bugtask.py'
> --- lib/lp/bugs/browser/bugtask.py 2010-01-26 14:22:41 +0000
> +++ lib/lp/bugs/browser/bugtask.py 2010-02-10 16:43:36 +0000
> @@ -2206,9 +2206,13 @@
> distrosourcepackage_context or sourcepackage_context):
> return ["id", "summary", "importance", "status",...

review: Needs Fixing

[Deryck Hodge (deryck)]

Hi, Gavin.

Thanks for the review! I knew I was doing some dumb things in there, but I was feeling a bit too close to it after so many iterations.

You had good recommendations all the way around, which I've implemented, except for a couple of them. I didn't change the test -- yes, it's fragile, but the whole point of the page test is to show the ordering of bugs by heat. Also, I needed at least status in the monster-long query to "show all bugs." I trimmed what I could, but it's still a bit long.

Otherwise, I think I took care of all your points.

Cheers,
deryck

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2010-02-10 23:14:56 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2010-02-12 12:39:21 +0000
@@ -40,6 +40,7 @@
 from canonical.config import config
 from lp.bugs.browser.bugtask import BugTaskSearchListingView
 from lp.bugs.interfaces.bug import IBug
+from lp.bugs.interfaces.bugtask import BugTaskSearchParams
 from canonical.launchpad.browser.feeds import (
     BugFeedLink, BugTargetLatestBugsFeedLink, FeedsMixin,
     PersonLatestBugsFeedLink)
@@ -55,6 +56,7 @@
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.temporaryblobstorage import (
     ITemporaryStorageManager)
+from canonical.launchpad.searchbuilder import any
 from canonical.launchpad.webapp import urlappend
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 from canonical.launchpad.webapp.interfaces import ILaunchBag, NotFoundError
@@ -1266,6 +1268,28 @@
         else:
             return 'None specified'
 
+    @cachedproperty
+    def hot_bugs_info(self):
+        """Return a dict of the 10 hottest tasks and a has_more_bugs flag."""
+        has_more_bugs = False
+        params = BugTaskSearchParams(
+            orderby='-heat', omit_dupes=True,
+            user=self.user, status=any(*UNRESOLVED_BUGTASK_STATUSES))
+        # Use 4x as many tasks as bugs that are needed to improve performance.
+        bugtasks = self.context.searchTasks(params)[:40]
+        hot_bugtasks = []
+        hot_bugs = []
+        for task in bugtasks:
+            # Use hot_bugs list to ensure a bug is only listed once.
+            if task.bug not in hot_bugs:
+                if len(hot_bugtasks) < 10:
+                    hot_bugtasks.append(task)
+                    hot_bugs.append(task.bug)
+                else:
+                    has_more_bugs = True
+                    break
+        return {'has_more_bugs': has_more_bugs, 'bugtasks': hot_bugtasks}
+
 
 class BugTargetBugTagsView(LaunchpadView):
     """Helper methods for rendering the bug tags portlet."""

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2010-01-26 14:22:41 +0000
+++ lib/lp/bugs/browser/bugtask.py	2010-02-12 12:39:21 +0000
@@ -2206,9 +2206,13 @@
             distrosourcepackage_context or sourcepackage_context):
             return ["id", "summary", "importance", "status", "heat"]
         elif distribution_context or distroseries_context:
-            return ["id", "summary", "packagename", "importance", "status", "heat"]
+            return [
+                "id", "summary", "packagename", "importance", "status",
+                "heat"]
         elif project_context:
-            return ["id", "summary", "productname", "importance", "status", "heat"]
+            return [
+                "id", "summary", "productname", "importance", "status",
+                "heat"]
         else:
             raise AssertionError(
                 "Unrecognized context; don't know which report "
@@ -2739,15 +2743,6 @@
         return IDistributionSourcePackage(self.context, None)
 
     @property
-    def hot_bugtasks(self):
-        """Return the 10 most recently updated bugtasks for this target."""
-        params = BugTaskSearchParams(
-            orderby="-date_last_updated", omit_dupes=True, user=self.user,
-            status=any(*UNRESOLVED_BUGTASK_STATUSES))
-        search = self.context.searchTasks(params)
-        return list(search[:10])
-
-    @property
     def addquestion_url(self):
         """Return the URL for the +addquestion view for the context."""
         if IQuestionTarget.providedBy(self.context):
@@ -2803,8 +2798,7 @@
             if bug_listing_item.review_action_widget is not None]
         self.widgets = formlib.form.Widgets(widgets_list, len(self.prefix)+1)
 
-    @action('Save changes', name='submit',
-            condition=canApproveNominations)
+    @action('Save changes', name='submit', condition=canApproveNominations)
     def submit_action(self, action, data):
         """Accept/Decline bug nominations."""
         accepted = declined = 0
@@ -3592,7 +3586,8 @@
         """Show the columns that summarise expirable bugs."""
         if (IDistribution.providedBy(self.context)
             or IDistroSeries.providedBy(self.context)):
-            return ['id', 'summary', 'packagename', 'date_last_updated', 'heat']
+            return [
+                'id', 'summary', 'packagename', 'date_last_updated', 'heat']
         else:
             return ['id', 'summary', 'date_last_updated', 'heat']
 

=== modified file 'lib/lp/bugs/stories/bugs/xx-product-bugs-page.txt'
--- lib/lp/bugs/stories/bugs/xx-product-bugs-page.txt	2010-01-26 14:22:41 +0000
+++ lib/lp/bugs/stories/bugs/xx-product-bugs-page.txt	2010-02-12 12:39:21 +0000
@@ -131,38 +131,81 @@
 
 == Hot Bugs ==
 
-A listing of the 10 'hottest' bugs (currently simply the bugs most
-recently touched) is displayed to allow a quick overview of the project.
+A listing of the 10 'hottest' bugs is displayed to allow a quick
+overview of the project.
+
+To demonstrate this, we create 10 bugs and adjust their heat values manually.
+
+    >>> from zope.component import getUtility
+    >>> from canonical.launchpad.ftests import login, logout
+    >>> from lp.registry.interfaces.product import IProductSet
+    >>> import transaction
+    >>> login('[e-mail]')
+    >>> firefox = getUtility(IProductSet).getByName("firefox")
+    >>> heat_values = [0, 400, 200, 600, 100, 50, 50, 50, 50, 50, 50, 50]
+    >>> for count in range(1, 11):
+    ...     summary = 'Summary for new bug %d' % count
+    ...     bug = factory.makeBug(title=summary, product=firefox)
+    ...     bug.setHeat(heat_values[count])
+    >>> transaction.commit()
+    >>> logout()
+
 For each bug we have the number, title, status, importance and the time
 since the last update.
 
     >>> anon_browser.open('http://bugs.launchpad.dev/firefox')
     >>> print extract_text(
     ...     find_tag_by_id(anon_browser.contents, 'hot-bugs'))
-        Summary            Status Importance Last changed
-    #5 Firefox install... New    Critical   on 2006-07-14
-    #4 Reflow problems... New    Medium     on 2006-07-14
-    #1 Firefox does no... New    Low        on 2006-05-19
-
-
-Fix released bugs are not shown. We demonstrate this by setting the bugtask
-for bug 4 to be "Fix released".
-
-    >>> from zope.component import getUtility
+        Summary                   Status   Importance    Last changed
+    #18 Summary for new bug 3    New      Undecided     ...
+    #16 Summary for new bug 1    New      Undecided     ...
+    #17 Summary for new bug 2    New      Undecided     ...
+    #19 Summary for new bug 4    New      Undecided     ...
+    #20 Summary for new bug 5    New      Undecided     ...
+    #21 Summary for new bug 6    New      Undecided     ...
+    #22 Summary for new bug 7    New      Undecided     ...
+    #23 Summary for new bug 8    New      Undecided     ...
+    #24 Summary for new bug 9    New      Undecided     ...
+    #25 Summary for new bug 10   New      Undecided     ...
+
+
+Fix Released bugs are not shown. We demonstrate this by setting the bugtask
+for bug 18 to be "Fix Released".
+
     >>> from lp.bugs.interfaces.bug import BugTaskStatus, IBugSet
     >>> from lp.registry.interfaces.person import IPersonSet
     >>> login('[e-mail]')
-    >>> bug_4 = getUtility(IBugSet).get(4)
+    >>> bug_18 = getUtility(IBugSet).get(18)
     >>> project_owner = getUtility(IPersonSet).getByName('name12')
-    >>> bug_4.bugtasks[0].transitionToStatus(
+    >>> bug_18.bugtasks[0].transitionToStatus(
     ...     BugTaskStatus.FIXRELEASED, project_owner)
     >>> logout()
 
-And then reloading the page. The Fix released bug, bug 4, is no longer shown.
+And then reloading the page. The Fix Released bug, bug 18, is no longer shown.
 
     >>> anon_browser.reload()
     >>> print extract_text(
     ...     find_tag_by_id(anon_browser.contents, 'hot-bugs'))
-        Summary            Status Importance Last changed
-    #5 Firefox install... New    Critical   on 2006-07-14
-    #1 Firefox does no... New    Low        on 2006-05-19
+        Summary                   Status   Importance    Last changed
+    #16 Summary for new bug 1    New      Undecided     ...
+    #17 Summary for new bug 2    New      Undecided     ...
+    #19 Summary for new bug 4    New      Undecided     ...
+    #20 Summary for new bug 5    New      Undecided     ...
+    #21 Summary for new bug 6    New      Undecided     ...
+    #22 Summary for new bug 7    New      Undecided     ...
+    #23 Summary for new bug 8    New      Undecided     ...
+    #24 Summary for new bug 9    New      Undecided     ...
+    #25 Summary for new bug 10   New      Undecided     ...
+
+There is a link to see all bugs by heat if the project has more
+than 10 hot bugs.
+
+    >>> find_tag_by_id(anon_browser.contents, 'more-hot-bugs') is None
+    False
+
+Jokosher does not have more than 10 bugs and does not have a link
+to more hot bugs.
+
+    >>> anon_browser.open('http://bugs.launchpad.dev/jokosher')
+    >>> find_tag_by_id(anon_browser.contents, 'more-hot-bugs') is None
+    True

=== modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
--- lib/lp/bugs/templates/bugtarget-bugs.pt	2010-01-26 14:22:41 +0000
+++ lib/lp/bugs/templates/bugtarget-bugs.pt	2010-02-12 12:39:21 +0000
@@ -10,8 +10,9 @@
   i18n:domain="malone"
 >
   <metal:block fill-slot="head_epilogue">
-    <script type="text/javascript" src="/+icing/FormatAndColor.js"></script>
-    <script type="text/javascript" src="/+icing/PlotKit_Packed.js"></script>
+    <style type="text/css">
+      p#more-hot-bugs {float:right; margin-top:7px;}
+    </style>
   </metal:block>
   <body>
     <tal:side metal:fill-slot="side" condition="view/uses_launchpad_bugtracker">
@@ -95,7 +96,7 @@
           </ul>
         </div>
 
-      <tal:has_hot_bugs condition="view/hot_bugtasks">
+      <tal:has_hot_bugs condition="view/hot_bugs_info/bugtasks">
         <div class="search-box">
           <metal:search
             use-macro="context/@@+bugtarget-macros-search/simple-search-form"
@@ -123,7 +124,7 @@
             </tr>
           </thead>
           <tbody>
-            <tr tal:repeat="bugtask view/hot_bugtasks">
+            <tr tal:repeat="bugtask view/hot_bugs_info/bugtasks">
               <td class="icon left">
                 <span tal:replace="structure bugtask/image:icon" />
               </td>
@@ -146,9 +147,13 @@
             </tr>
           </tbody>
         </table>
+        <p id="more-hot-bugs"
+           tal:condition="view/hot_bugs_info/has_more_bugs">
+          <a tal:attributes="href string:${context/fmt:url/+bugs}?orderby=-heat&field.status%3Alist=NEW&field.status%3Alist=INCOMPLETE_WITH_RESPONSE&field.status%3Alist=INCOMPLETE_WITHOUT_RESPONSE&field.status%3Alist=CONFIRMED&field.status%3Alist=TRIAGED&field.status%3Alist=INPROGRESS&field.status%3Alist=FIXCOMMITTED&field.omit_dupes=on">See all hot bugs</a>
+        </p>
       </tal:has_hot_bugs>
 
-      <tal:no_hot_bugs condition="not: view/hot_bugtasks">
+      <tal:no_hot_bugs condition="not: view/hot_bugs_info/bugtasks">
         <p id="no-bugs-filed"><strong>There are currently no bugs filed against
           <tal:project_title replace="context/title" />.</strong></p>