From 4a2f7a5a59e7868b1a21222647d2632cb7d3edb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= <etienne@plot.ly>
Date: Thu, 28 Mar 2019 11:38:15 -0400
Subject: [PATCH] do not render hierarchy when partial sums > sector value

- add d3.hierachy error message
- add appropriate tests
---
 src/traces/sunburst/calc.js         |  7 +++++--
 test/jasmine/tests/sunburst_test.js | 25 +++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/traces/sunburst/calc.js b/src/traces/sunburst/calc.js
index bf2bf5eec8c..4c28d917386 100644
--- a/src/traces/sunburst/calc.js
+++ b/src/traces/sunburst/calc.js
@@ -130,10 +130,11 @@ exports.calc = function(gd, trace) {
             .id(function(d) { return d.id; })
             .parentId(function(d) { return d.pid; })(cd);
     } catch(e) {
-        return Lib.warn('Failed to build sunburst hierarchy.');
+        return Lib.warn('Failed to build sunburst hierarchy. Error: ' + e.message);
     }
 
     var hierarchy = d3Hierarchy.hierarchy(root);
+    var failed = false;
 
     if(hasVals) {
         switch(trace.branchvalues) {
@@ -149,7 +150,7 @@ exports.calc = function(gd, trace) {
                             return a + c.data.data.v;
                         }, 0);
                         if(v < partialSum) {
-                            d.value = partialSum;
+                            failed = true;
                             return Lib.warn([
                                 'Total value for node', d.data.data.id,
                                 'is smaller than the sum of its children.'
@@ -165,6 +166,8 @@ exports.calc = function(gd, trace) {
         hierarchy.count();
     }
 
+    if(failed) return;
+
     // TODO add way to sort by height also?
     hierarchy.sort(function(a, b) { return b.value - a.value; });
 
diff --git a/test/jasmine/tests/sunburst_test.js b/test/jasmine/tests/sunburst_test.js
index 3017b186717..995dd812df0 100644
--- a/test/jasmine/tests/sunburst_test.js
+++ b/test/jasmine/tests/sunburst_test.js
@@ -233,7 +233,7 @@ describe('Test sunburst calc:', function() {
         expect(Lib.warn).toHaveBeenCalledTimes(0);
     });
 
-    it('should warn when values under *branchvalues:total* do not add up', function() {
+    it('should warn when values under *branchvalues:total* do not add up and not show trace', function() {
         _calc({
             labels: ['Root', 'A', 'B', 'b'],
             parents: ['', 'Root', 'Root', 'B'],
@@ -241,12 +241,33 @@ describe('Test sunburst calc:', function() {
             branchvalues: 'total'
         });
 
-        expect(extractPt('value')).toEqual([3, 3, 1, 3]);
+        expect(gd.calcdata[0][0].hierarchy).toBe(undefined, 'no computed hierarchy');
 
         expect(Lib.warn).toHaveBeenCalledTimes(2);
         expect(Lib.warn.calls.allArgs()[0][0]).toBe('Total value for node Root is smaller than the sum of its children.');
         expect(Lib.warn.calls.allArgs()[1][0]).toBe('Total value for node B is smaller than the sum of its children.');
     });
+
+    it('should warn labels/parents lead to ambiguous hierarchy', function() {
+        _calc({
+            labels: ['Root', 'A', 'A', 'B'],
+            parents: ['', 'Root', 'Root', 'A']
+        });
+
+        expect(Lib.warn).toHaveBeenCalledTimes(1);
+        expect(Lib.warn).toHaveBeenCalledWith('Failed to build sunburst hierarchy. Error: ambiguous: A');
+    });
+
+    it('should warn ids/parents lead to ambiguous hierarchy', function() {
+        _calc({
+            labels: ['label 1', 'label 2', 'label 3', 'label 4'],
+            ids: ['a', 'b', 'b', 'c'],
+            parents: ['', 'a', 'a', 'b']
+        });
+
+        expect(Lib.warn).toHaveBeenCalledTimes(1);
+        expect(Lib.warn).toHaveBeenCalledWith('Failed to build sunburst hierarchy. Error: ambiguous: b');
+    });
 });
 
 describe('Test sunburst hover:', function() {