launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/less-hotness into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #915267 in Launchpad itself: "bug heat flames are expensive to calculate"
  https://bugs.launchpad.net/launchpad/+bug/915267

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/less-hotness/+merge/88456

The current implementation of bug heat ratios causes major performance problems due to the cached total updates. Stakeholders have OKed the replacement of the ratio with a raw number.

This branch adds a bugs.heat_ratio_display.disabled feature flag to hide the ratio and show a number.

http://people.canonical.com/~wgrant/launchpad/bug-915267/ has screenshots of the three changes.
-- 
https://code.launchpad.net/~wgrant/launchpad/less-hotness/+merge/88456
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/less-hotness into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2012-01-05 18:12:05 +0000
+++ lib/lp/bugs/browser/bugtask.py	2012-01-13 08:35:31 +0000
@@ -1082,11 +1082,16 @@
     @property
     def bug_heat_html(self):
         """HTML representation of the bug heat."""
-        if IDistributionSourcePackage.providedBy(self.context.target):
-            return bugtask_heat_html(
-                self.context, target=self.context.distribution)
+        if getFeatureFlag('bugs.heat_ratio_display.disabled'):
+            return (
+                '<span><a href="/+help-bugs/bug-heat.html" target="help" '
+                'class="sprite flame">%d</a></span>' % self.context.bug.heat)
         else:
-            return bugtask_heat_html(self.context)
+            if IDistributionSourcePackage.providedBy(self.context.target):
+                return bugtask_heat_html(
+                    self.context, target=self.context.distribution)
+            else:
+                return bugtask_heat_html(self.context)
 
     @property
     def privacy_notice_classes(self):
@@ -2212,7 +2217,20 @@
     @property
     def bug_heat_html(self):
         """Returns the bug heat flames HTML."""
-        return bugtask_heat_html(self.bugtask, target=self.target_context)
+        if getFeatureFlag('bugs.heat_ratio_display.disabled'):
+            if getFeatureFlag('bugs.dynamic_bug_listings.enabled'):
+                return (
+                    '<span class="sprite flame">%d</span>'
+                    % self.bugtask.bug.heat)
+            else:
+                return str(self.bugtask.bug.heat)
+        else:
+            return bugtask_heat_html(self.bugtask, target=self.target_context)
+
+    @property
+    def center_bug_heat(self):
+        """Returns whether the bug_heat_html should be centered."""
+        return not getFeatureFlag('bugs.heat_ratio_display.disabled')
 
     @property
     def model(self):

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2012-01-04 02:09:58 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2012-01-13 08:35:31 +0000
@@ -44,7 +44,6 @@
     )
 from lp.testing.pages import find_tag_by_id
 from lp.services.webapp import canonical_url
-from lp.services.webapp.authorization import clear_cache
 from lp.services.webapp.interfaces import (
     ILaunchBag,
     ILaunchpadRoot,
@@ -939,7 +938,7 @@
             'HTTP_REFERER': bugtask_url,
             }
         form = {
-            'field.actions.delete_bugtask': 'Delete'
+            'field.actions.delete_bugtask': 'Delete',
             }
         view = create_initialized_view(
             bugtask, name='+delete', server_url=server_url, form=form,
@@ -970,7 +969,7 @@
             'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest',
             }
         form = {
-            'field.actions.delete_bugtask': 'Delete'
+            'field.actions.delete_bugtask': 'Delete',
             }
         view = create_initialized_view(
             bug.default_bugtask, name='+delete', server_url=server_url,
@@ -1006,7 +1005,7 @@
             'HTTP_REFERER': default_bugtask_url,
             }
         form = {
-            'field.actions.delete_bugtask': 'Delete'
+            'field.actions.delete_bugtask': 'Delete',
             }
         view = create_initialized_view(
             bugtask, name='+delete', server_url=server_url, form=form,
@@ -2382,3 +2381,15 @@
                 2001, 1, 1, tzinfo=UTC)
             self.assertEqual(
                 'on 2001-01-01', item.model['last_updated'])
+
+    def test_model_numeric_heat(self):
+        """bug_heat_html contains just the number if the flag is enabled."""
+        with FeatureFixture({'bugs.heat_ratio_display.disabled': 'true'}):
+            with dynamic_listings():
+                owner, item = make_bug_task_listing_item(self.factory)
+                self.assertNotIn('/@@/bug-heat', item.bug_heat_html)
+                self.assertIn('sprite flame', item.bug_heat_html)
+                with person_logged_in(owner):
+                    model = item.model
+                    self.assertEqual(
+                        item.bug_heat_html, model['bug_heat_html'])

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-heat-on-bug-page.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-heat-on-bug-page.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-heat-on-bug-page.txt	2012-01-13 08:35:31 +0000
@@ -1,11 +1,23 @@
 = Bug heat on bug page =
 
-The bug heat appears on the bug index page as four flames.
+By default the bug heat appears on the bug index page as four flames.
 
+    >>> def print_heat():
+    ...     content = find_main_content(anon_browser.contents)
+    ...     print content.find('a', href='/+help-bugs/bug-heat.html')
     >>> anon_browser.open('http://bugs.launchpad.dev/firefox/+bug/1')
-    >>> print find_main_content(
-    ...     anon_browser.contents).find('img', src='/@@/bug-heat-0.png')
-    <img src="/@@/bug-heat-0.png" alt="0 out of 4 heat flames" title="Heat: 0" />
+    >>> print_heat()
+    <a href="/+help-bugs/bug-heat.html" target="help" class="icon"><img src="/@@/bug-heat-0.png" alt="0 out of 4 heat flames" title="Heat: 0" /></a>
+
+But the bugs.heat_ratio_display.disabled feature flag displays it as a number
+instead.
+
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> with FeatureFixture({'bugs.heat_ratio_display.disabled': 'true'}):
+    ...     anon_browser.open('http://bugs.launchpad.dev/firefox/+bug/1')
+    >>> print_heat()
+    <a href="/+help-bugs/bug-heat.html" target="help" class="sprite flame">0</a>
+
 
 Packages use their distribution as context when generating flame icons
 for a bug page, since a package of low heat value could be displayed

=== modified file 'lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt'
--- lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt	2011-10-24 14:00:23 +0000
+++ lib/lp/bugs/templates/bugs-listing-table-without-navlinks.pt	2012-01-13 08:35:31 +0000
@@ -71,7 +71,8 @@
             2007-11-25</td>
         <td tal:condition="context/show_column/heat|nothing"
             tal:content="structure bugtask/bug_heat_html"
-            style="text-align: center">
+            tal:attributes="style python: 'text-align: center' if
+              bugtask.center_bug_heat else ''">
             HEAT</td>
       </tr>
     </tbody>