Merge lp:~jcsackett/charmworld/better-review-charting into lp:charmworld

[j.c.sackett]

Removes the min and max data from review and improves chart display.

This branch removes any display of min and max latency from the review queue.
The data is misleading to users, as they routinely mistake max for average,
causing communication errors between charm reviewers and charm authors.

Since min and max was added as a drive by addition, it's not necessary. The
easiest thing to do is just remove it from display. We continue to store it so
we have the data for analysis should we ever want it.

As a drive by, the d3 code to make the sparkline has been updated to make the
sparkline more legible.

[Brad Crittenden (bac)]

Thanks Jon. Looks good.

review: Approve (code)

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/static/sparklines.js'
--- charmworld/static/sparklines.js	2013-11-20 14:28:24 +0000
+++ charmworld/static/sparklines.js	2014-02-19 17:16:14 +0000
@@ -6,7 +6,7 @@
     var xlimit = data.length - 1;
     var ylimit = Math.max.apply(Math, data);
 
-    var x = d3.scale.linear().domain([0, xlimit]).range([0, 20]);
+    var x = d3.scale.linear().domain([0, xlimit]).range([0, 50]);
     var y = d3.scale.linear().domain([0, ylimit]).range([0, 10]);
 
     var line = d3.svg.line().x(function(d,i) { 

=== modified file 'charmworld/templates/review.pt'
--- charmworld/templates/review.pt	2013-11-20 14:29:00 +0000
+++ charmworld/templates/review.pt	2014-02-19 17:16:14 +0000
@@ -23,17 +23,9 @@
     <ul class="note">
       <li><strong>Items:</strong> ${len(queue)}</li>
       <li>
-        <strong>Max wait time:</strong> ${max}
-        <span class="sparkline" data-values="[${sparklines['maxes']}]"></span>
-      </li>
-      <li>
         <strong>Average wait time:</strong> ${mean}
         <span class="sparkline" data-values="[${sparklines['means']}]"></span>
       </li>
-      <li>
-        <strong>Min wait time:</strong> ${min}
-        <span class="sparkline" data-values="[${sparklines['mins']}]"></span>
-      </li>
     </ul>
     <p class="note">
       <strong>Reviewers</strong>: Please read the "Reviewer Tips and Criteria"

=== modified file 'charmworld/views/tests/test_tools.py'
--- charmworld/views/tests/test_tools.py	2013-11-08 22:03:03 +0000
+++ charmworld/views/tests/test_tools.py	2014-02-19 17:16:14 +0000
@@ -60,22 +60,16 @@
 
     def test_empty_sparklines(self):
         sparklines = get_sparklines([])
-        self.assertEqual('', sparklines['maxes'])
-        self.assertEqual('', sparklines['mins'])
         self.assertEqual('', sparklines['means'])
 
     def test_single_sparklines(self):
-        sparklines = get_sparklines([{'max': 1, 'min': 2, 'mean': 3}])
-        self.assertEqual('1', sparklines['maxes'])
-        self.assertEqual('2', sparklines['mins'])
+        sparklines = get_sparklines([{'mean': 3}])
         self.assertEqual('3', sparklines['means'])
 
     def test_sparklines(self):
         sparklines = get_sparklines([
-            {'max': 1, 'min': 1, 'mean': 1},
-            {'max': 1, 'min': 2, 'mean': 3}])
-        self.assertEqual('1,1', sparklines['maxes'])
-        self.assertEqual('1,2', sparklines['mins'])
+            {'mean': 1},
+            {'mean': 3}])
         self.assertEqual('1,3', sparklines['means'])
 
 

=== modified file 'charmworld/views/tools.py'
--- charmworld/views/tools.py	2013-11-08 19:27:59 +0000
+++ charmworld/views/tools.py	2014-02-19 17:16:14 +0000
@@ -6,7 +6,10 @@
 import pymongo
 from pyramid.view import view_config
 
-from charmworld import cached_view_config, utils
+from charmworld import (
+    cached_view_config,
+    utils,
+)
 from charmworld.models import (
     CharmSource,
     QADataSource,
@@ -91,18 +94,13 @@
 
 
 def get_sparklines(stats):
-    mins = []
-    maxes = []
     means = []
     for s in stats:
-        mins.append(str(s['min']))
         means.append(str(s['mean']))
-        maxes.append(str(s['max']))
 
     return {
-        'mins': ','.join(mins),
         'means': ','.join(means),
-        'maxes': ','.join(maxes)}
+    }
 
 
 @cached_view_config(
@@ -125,20 +123,15 @@
     sparklines = get_sparklines(stats)
     if stats:
         current_stats = stats[0]
-        max_latency = utils.prettify_timedelta(
-            datetime.timedelta(seconds=current_stats['max']))
-        min_latency = utils.prettify_timedelta(
-            datetime.timedelta(seconds=current_stats['min']))
         mean_latency = utils.prettify_timedelta(
             datetime.timedelta(seconds=current_stats['mean']))
     else:
-        max_latency = min_latency = mean_latency = 'N/A'
+        mean_latency = 'N/A'
 
     return {
         'queue': review_queue,
         'TYPES_WITH_DATES': TYPES_WITH_DATES,
         'age_format': utils.pretty_timedelta,
-        'max': max_latency,
         'mean': mean_latency,
-        'min': min_latency,
-        'sparklines': sparklines}
+        'sparklines': sparklines,
+    }