[GitHub] mistercrunch commented on a change in pull request #3594: [Bugfix]: Fixing bug in label generation for multiple groupbys

[gi...@git.apache.org]

mistercrunch commented on a change in pull request #3594: [Bugfix]: Fixing bug in label generation for multiple groupbys
URL: https://github.com/apache/incubator-superset/pull/3594#discussion_r142723972
 
 

 ##########
 File path: superset/assets/visualizations/nvd3_vis.js
 ##########
 @@ -77,17 +77,16 @@ function getMaxLabelSize(container, axisClass) {
 /* eslint-disable camelcase */
 function formatLabel(column, verbose_map) {
   let label;
-  if (verbose_map) {
-    if (Array.isArray(column) && column.length) {
-      label = verbose_map[column[0]];
-      if (column.length > 1) {
-        label += `, ${column.slice(1).join(', ')}`;
-      }
-    } else {
-      label = verbose_map[column];
+  if (Array.isArray(column) && column.length) {
+    label = verbose_map[column[0]] || column[0];
 
 Review comment:
   Is it a safe assumption that metric is always in position 0? 
   
   `const label = column.map(s => verbose_map[s] || s).join(', ')` may be more elegant but it may "verbosify" dimension members instead of metrics (collisions are probably rare and not super problematic). This problem exist here too as in this current case column[0] could be a metric or a dimension member anyhow.
   
   Now I'm thinking verbosifying could be done in the backend where we know what is a metric or a dim member (this info is lost on the frontend). I know I said to verbosify it in the frontend earlier. What I wanted to avoid was using verbose names in dataframe column headers...
   
   In any case I'll merge this to fix the bug and we can revisit later if needed.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services