Merge lp:~makyo/juju-gui/consistent-health-graph into lp:juju-gui/experimental

[Madison Scott-Clary]

Service health graphs consistently rendered

d3 was sorting by the largest value, now sorts in the proper order: error - pending - running

https://codereview.appspot.com/6718048/

[Madison Scott-Clary (makyo)]

Reviewers: mp+130005_code.launchpad.net,

Message:
Please take a look.

Description:
Service health graphs consistently rendered

d3 was sorting by the largest value, now sorts in the proper order:
error - pending - running

https://code.launchpad.net/~makyo/juju-gui/consistent-health-graph/+merge/130005

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6718048/

Affected files:
   A [revision details]
   M app/views/environment.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/views/environment.js
=== modified file 'app/views/environment.js'
--- app/views/environment.js 2012-10-15 19:22:40 +0000
+++ app/views/environment.js 2012-10-16 23:01:10 +0000
@@ -775,7 +775,13 @@
                });

            var status_chart_layout = d3.layout.pie()
- .value(function(d) { return (d.value ? d.value : 1); });
+ .value(function(d) { return (d.value ? d.value : 1); })
+ .sort(function(a, b) {
+ // Ensure that the service health graphs will be renders in
+ // the correct order: error - pending - running.
+ var states = {error: 0, pending: 1, running: 2};
+ return states[a.name] - states[b.name];
+ });

            // Append to status charts to non-subordinate services
            var status_chart = node.append('g')

[Benjamin Saller (bcsaller)]

LGTM

https://codereview.appspot.com/6718048/diff/1/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6718048/diff/1/app/views/environment.js#newcode784
app/views/environment.js:784: });
This looks fine to me, can we however be sure those are the only three
states bubbling to this layer? I know that is supposed to be the case,
but should these ever come out of sync having a fallback value here
might make sense.

Probably fine for now though

https://codereview.appspot.com/6718048/

[Madison Scott-Clary (makyo)]

*** Submitted:

Service health graphs consistently rendered

d3 was sorting by the largest value, now sorts in the proper order:
error - pending - running

R=benjamin.saller
CC=
https://codereview.appspot.com/6718048

https://codereview.appspot.com/6718048/

[Madison Scott-Clary (makyo)]

*** Submitted:

Service health graphs consistently rendered

d3 was sorting by the largest value, now sorts in the proper order:
error - pending - running

R=
CC=
https://codereview.appspot.com/6718048

https://codereview.appspot.com/6718048/

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2012-10-15 19:22:40 +0000
+++ app/views/environment.js	2012-10-16 23:03:21 +0000
@@ -775,7 +775,13 @@
               });
 
           var status_chart_layout = d3.layout.pie()
-            .value(function(d) { return (d.value ? d.value : 1); });
+            .value(function(d) { return (d.value ? d.value : 1); })
+            .sort(function(a, b) {
+                // Ensure that the service health graphs will be renders in
+                // the correct order: error - pending - running.
+                var states = {error: 0, pending: 1, running: 2};
+                return states[a.name] - states[b.name];
+              });
 
           // Append to status charts to non-subordinate services
           var status_chart = node.append('g')