620045 - single day "heat map" should be a separate view

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, defect, P1)

Description

[Jeff Hammel]

When you click on a calendar date from the main page, you get to a
single day page that is still ?display=HeatMap but the display is much
different from the general HeatMap page.  So this should be a separate
view that is really "bug count for period" by OS (though the X-axis should
be selectable).  The date range should also be configurable (not just
one day) in the usual way: bug 609986.

For the graphs, he x-axis labels should be aligned with the columns
and "All all" should be removed.  

The bug list should be a standard bug table: bug 620031.

The date shold be removed from the subtitle. 

The heat map should (probably) be removed entirely: 620017

Comment 1

[Mark Côté [:mcote]]

Note that we are also going to remove the calendar, so this will (probably) only be accessible from the menu (bug 619826).

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

we should keep the code in place, but not display a heatmap.  We need to revisit the whole coloring scheme.

Comment 3

[Mark Côté [:mcote]]

Attachment: Patch including date-range picker, separated "heat map" views, and other stuff

Here's the result of a day and a half or so of concentrated OF hacking.

This includes a bunch of stuff, including the date-range picker, the new Bug Count view (originally the one-day Heat Map), renamed views, better controls for CountSummary (now called TopBugs), and a few other things.

In addition to this bug, this patch fixes bug 620690, bug 620451, bug 620454, bug 620023 (unless we want to try other default date ranges), part of bug 620045 (still need configurable X axis), part of bug 620470 (still need to restrict groupings based on date range), and most of bug 620447 (have yet to experiment with Y axis labels).

The bugs that are partly fixed are still assigned to me; I'll get back to them next week.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

first off, it appears we are missing jquery.datepicker.js|css from the patch.  'hg add jquery.datepicker.js' would do the trick.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

I am unable to get this to work with adding in my own datepicker stuff.  I get errors about 'datepick is not a function'.  Needless to say, hacking on this is not getting me very far...I assume 5 mcote minutes will resolve this.

Comment 6

[Mark Côté [:mcote]]

Attachment: Patch including date-range picker, separated "heat map" views, and other stuff

Crud sorry.  Was missing two files, _attachments/scripts/jquery.datepicker.min.js and _attachments/style/jquery.datepicker.css.  Should be better now.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 499889 [details] [diff] [review]
Patch including date-range picker, separated "heat map" views, and other stuff


>diff --git a/_attachments/orange.html b/_attachments/orange.html
>--- a/_attachments/orange.html
>+++ b/_attachments/orange.html
>@@ -35,48 +37,91 @@
>+  function dateRangeLoader(pagename, args) {
>+    return function(startday, endday) {
>+      args.startday = startday;
>+      args.endday = endday;
>+      window.location.assign(buildUrl(pagename, args)); 
>+    };
>+  }

is there a need for value checking on startday/endday?


>diff --git a/_attachments/scripts/woo.bugs.js b/_attachments/scripts/woo.bugs.js
>--- a/_attachments/scripts/woo.bugs.js
>+++ b/_attachments/scripts/woo.bugs.js
>@@ -1,9 +1,18 @@
>-  function parseBugs(data) {
>+  function parseBugs(data, startdaystr, enddaystr) {
>+    var startdate = null;
>+    if (startdaystr) {
>+      startdate = getDate(startdaystr);
>+    }
>+    var enddate = null;
>+    if (enddaystr) {
>+      enddate = getDate(enddaystr);
>+    }
>+      

ugh, the names startdaystr and enddaystr just look wrong to me.  what about strStartDay?



>@@ -424,17 +470,17 @@
>                     var graphdate = getDate(idx).getTime();
>                     if (graphdate < lowdate) lowdate = graphdate;
>                     if (graphdate > highdate) highdate = graphdate;
>                     origarray.push([graphdate, Math.floor((original / pushes) * 100) / 100]);
>                     simarray.push([graphdate, Math.floor((simulated / pushes) * 100) / 100]);
>                     simfactor += Math.floor((simulated / pushes) * 100) / 100;
>                   }
>                   var simtext = "Simulated OF top " + args['numbugs'] + " fixed";
>-                  displayMetric(app, titleid, args.startday, args.endday);
>+                  displayMetric(app, titleid, args.startday, args.endday, dateRangeLoader("Simulator", args));

why do we pass args.startday and args.endday in here?  it seems that dateRangeLoader uses these as well.  I would like to simplify the arguments to this either now or in the future.


>diff --git a/_attachments/scripts/woo.utils.js b/_attachments/scripts/woo.utils.js
>--- a/_attachments/scripts/woo.utils.js
>+++ b/_attachments/scripts/woo.utils.js
>@@ -304,8 +311,29 @@ function listDetails(data, testindex) {
>+function normDateArgs(args, allTime) {
>+  if (!args.endday) {
>+    args.endday = getTboxDate();
>+  }
>+
>+  if (!args.startday) {
>+    if (!allTime) {
>+      args.startday = getTboxDate(getDate(args.endday), -28);
>+    } else {
>+      args.startday = '2010-08-01';  // "allTime" is actually when we started recording data
>+    }
>+  }
>+  
>+  return args;
>+}

any chance for these hardcoded pieces of data we can put them in a config file somewhere or a single include file?


Overall, this cleans up a lot of code and adds important functionality.  This is good for the code and moving towards a more usable website.  Great job on the calendar stuff!

Comment 8

[Mark Côté [:mcote]]

(In reply to comment #7)
> Comment on attachment 499889 [details] [diff] [review]
> Patch including date-range picker, separated "heat map" views, and other stuff
> 
> 
> >diff --git a/_attachments/orange.html b/_attachments/orange.html
> >--- a/_attachments/orange.html
> >+++ b/_attachments/orange.html
> >@@ -35,48 +37,91 @@
> >+  function dateRangeLoader(pagename, args) {
> >+    return function(startday, endday) {
> >+      args.startday = startday;
> >+      args.endday = endday;
> >+      window.location.assign(buildUrl(pagename, args)); 
> >+    };
> >+  }
> 
> is there a need for value checking on startday/endday?

I would say 'no' here because there's no real logic here.  This just substitutes in the given arguments and reloads the page; it's up to the other parts of the code to verify that these make sense (namely, the onSelect function of datepick()).

> >diff --git a/_attachments/scripts/woo.bugs.js b/_attachments/scripts/woo.bugs.js
> >--- a/_attachments/scripts/woo.bugs.js
> >+++ b/_attachments/scripts/woo.bugs.js
> >@@ -1,9 +1,18 @@
> >-  function parseBugs(data) {
> >+  function parseBugs(data, startdaystr, enddaystr) {
> >+    var startdate = null;
> >+    if (startdaystr) {
> >+      startdate = getDate(startdaystr);
> >+    }
> >+    var enddate = null;
> >+    if (enddaystr) {
> >+      enddate = getDate(enddaystr);
> >+    }
> >+      
> 
> ugh, the names startdaystr and enddaystr just look wrong to me.  what about
> strStartDay?

Heh sure thing.

> >@@ -424,17 +470,17 @@
> >                     var graphdate = getDate(idx).getTime();
> >                     if (graphdate < lowdate) lowdate = graphdate;
> >                     if (graphdate > highdate) highdate = graphdate;
> >                     origarray.push([graphdate, Math.floor((original / pushes) * 100) / 100]);
> >                     simarray.push([graphdate, Math.floor((simulated / pushes) * 100) / 100]);
> >                     simfactor += Math.floor((simulated / pushes) * 100) / 100;
> >                   }
> >                   var simtext = "Simulated OF top " + args['numbugs'] + " fixed";
> >-                  displayMetric(app, titleid, args.startday, args.endday);
> >+                  displayMetric(app, titleid, args.startday, args.endday, dateRangeLoader("Simulator", args));
> 
> why do we pass args.startday and args.endday in here?  it seems that
> dateRangeLoader uses these as well.  I would like to simplify the arguments to
> this either now or in the future.

Hm yeah, well the idea at first was that the 'calFunc' parameter of displayMetric and insertDatePicker would be completely generic and not necessarily tied to startday and endday.  However I'm now realizing that it's always dateRangeLoader() and probably will always be, so if you want I can change insertDatePicker() (which is called by displayMetric) to have the signature

insertDatePicker(selector, args)

and displayMetric() to

displayMetric(app, id, args)

and each will extract the necessary args (startday, endday, and display), and insertDatePicker() will call dateRangeLoader() itself.

Does that sound good?

> >diff --git a/_attachments/scripts/woo.utils.js b/_attachments/scripts/woo.utils.js
> >--- a/_attachments/scripts/woo.utils.js
> >+++ b/_attachments/scripts/woo.utils.js
> >@@ -304,8 +311,29 @@ function listDetails(data, testindex) {
> >+function normDateArgs(args, allTime) {
> >+  if (!args.endday) {
> >+    args.endday = getTboxDate();
> >+  }
> >+
> >+  if (!args.startday) {
> >+    if (!allTime) {
> >+      args.startday = getTboxDate(getDate(args.endday), -28);
> >+    } else {
> >+      args.startday = '2010-08-01';  // "allTime" is actually when we started recording data
> >+    }
> >+  }
> >+  
> >+  return args;
> >+}
> 
> any chance for these hardcoded pieces of data we can put them in a config file
> somewhere or a single include file?

Good idea.  Will do.  I'll create some functions in a config.js called "absoluteStartDay" and "relativeStartDay" or something like that.

Comment 9

[Mark Côté [:mcote]]

Pushed as http://hg.mozilla.org/automation/orangefactor/rev/79ee51bf830c.

Comment 10

[Mark Côté [:mcote]]

Oops, there's still the configurable X-axis to do here.

Comment 11

[Mark Côté [:mcote]]

Attachment: Select box for detail type to be used as x-axis

In addition to allowing the choice of x-axis, I did a few other cleanups:

- removed the clause in displayTestRun() when dr['start'] == dr['end'].  It didn't work properly, and the UI doesn't allow it.  There will still be problems if someone sets the values in the URL directly, but whatever--it's unsupported. :)

- the above means that showBarGraphDay() is now only called from displayBugCount(), so I removed unused stuff from the former.

- I renamed loadHeatMap() to orangeFactor(), which is (slightly) more accurate.  I put the commented-out heat-map stuff in displayHeatMap().

- I uncommented the 'WinXP':'WINNT 5.1' line from the platforms structure, since there are a lot of WINNT 5.1 results these days.  However the mapping in this structure should be revisited when fixing bug 633412.

Comment 12

[Mark Côté [:mcote]]

Sorry, the 3rd point should read "I put the commented-out heat-map stuff in displayOrangeFactor().", since displayOrangeFactor() was formerly displayHeatMap().

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 511835 [details] [diff] [review]
Select box for detail type to be used as x-axis

cool cleanup.  I like the refactoring of the test name and platform stuff.  Although this is cleaned up a lot, this code is still confusing as heck. 

I don't have any nits as this is mostly just cleaning up code that was sloppy to begin with.

r=me.

Comment 14

[Mark Côté [:mcote]]

Pushed as http://hg.mozilla.org/automation/orangefactor/rev/91d947212caf

Hopefully bug 620046 will reduce the confusion at least a bit.