TablePanel, GraphPanel: Exclude hidden columns from CSV (#19925)

* TablePanel: Don't include hidden columns in CSV export

Fixes #12076

* GraphPanel: Don't include hidden series in CSV export

Series are hidden if all values are zero/empty and the relevant graph
option is enabled as well. (e.g. "Hide series: With only nulls")

Fixes #12076

public/app/plugins/panel/graph/module.ts

@@ -327,7 +327,9 @@ class GraphCtrl extends MetricsPanelCtrl {
 
 
   exportCsv() {
   exportCsv() {
     const scope = this.$scope.$new(true);
     const scope = this.$scope.$new(true);
-    scope.seriesList = this.seriesList;
+    scope.seriesList = this.seriesList
+      .filter(series => !this.panel.legend.hideEmpty || !series.allIsNull)
+      .filter(series => !this.panel.legend.hideZero || !series.allIsZero);
     this.publishAppEvent(CoreEvents.showModal, {
     this.publishAppEvent(CoreEvents.showModal, {
       templateHtml: '<export-data-modal data="seriesList"></export-data-modal>',
       templateHtml: '<export-data-modal data="seriesList"></export-data-modal>',
       scope,
       scope,

public/app/plugins/panel/graph/specs/graph_ctrl.test.ts

 import { GraphCtrl } from '../module';
 import { GraphCtrl } from '../module';
 import { dateTime } from '@grafana/data';
 import { dateTime } from '@grafana/data';
+import TimeSeries from 'app/core/time_series2';
 
 
 jest.mock('../graph', () => ({}));
 jest.mock('../graph', () => ({}));
 
 
@@ -17,7 +18,7 @@ describe('GraphCtrl', () => {
     },
     },
   };
   };
 
 
-  const scope = {
+  const scope: any = {
     $on: () => {},
     $on: () => {},
   };
   };
 
 
@@ -106,4 +107,75 @@ describe('GraphCtrl', () => {
       expect(ctx.ctrl.dataWarning.title).toBe('No data');
       expect(ctx.ctrl.dataWarning.title).toBe('No data');
     });
     });
   });
   });
+
+  describe('when data is exported to CSV', () => {
+    const appEventMock = jest.fn();
+
+    beforeEach(() => {
+      appEventMock.mockReset();
+      scope.$root = { appEvent: appEventMock };
+      scope.$new = () => ({});
+      const data = [
+        {
+          target: 'test.normal',
+          datapoints: [[10, 1], [10, 2]],
+        },
+        {
+          target: 'test.nulls',
+          datapoints: [[null, 1], [null, 2]],
+        },
+        {
+          target: 'test.zeros',
+          datapoints: [[0, 1], [0, 2]],
+        },
+      ];
+      ctx.ctrl.onDataSnapshotLoad(data);
+      // allIsNull / allIsZero are set by getFlotPairs
+      ctx.ctrl.seriesList.forEach((series: TimeSeries) => series.getFlotPairs(''));
+    });
+
+    const thenExportYieldedNSeries = (n: number) => {
+      expect(appEventMock.mock.calls.length).toBe(1);
+      const eventPayload = appEventMock.mock.calls[0][1];
+      expect(eventPayload.scope.seriesList).toHaveLength(n);
+    };
+
+    const thenExportDidNotYieldSeriesName = (unexpectedName: string) => {
+      expect(appEventMock.mock.calls.length).toBe(1);
+      const eventPayload = appEventMock.mock.calls[0][1];
+      expect(
+        eventPayload.scope.seriesList.filter((series: TimeSeries) => series.label === unexpectedName)
+      ).toHaveLength(0);
+    };
+
+    it('should not ignore anything if not asked to', () => {
+      ctx.ctrl.exportCsv();
+      thenExportYieldedNSeries(3);
+    });
+
+    it('should ignore all-null series when asked to', () => {
+      ctx.ctrl.panel.legend.hideEmpty = true;
+      ctx.ctrl.exportCsv();
+      thenExportYieldedNSeries(2);
+      thenExportDidNotYieldSeriesName('test.nulls');
+    });
+
+    it('should ignore all-zero series when asked to', () => {
+      ctx.ctrl.panel.legend.hideZero = true;
+      ctx.ctrl.exportCsv();
+      // impl treats all-null series as all-zero as well
+      thenExportYieldedNSeries(1);
+      thenExportDidNotYieldSeriesName('test.zeros');
+      thenExportDidNotYieldSeriesName('test.empty');
+    });
+
+    it('should ignore both when asked to', () => {
+      ctx.ctrl.panel.legend.hideZero = true;
+      ctx.ctrl.panel.legend.hideEmpty = true;
+      ctx.ctrl.exportCsv();
+      thenExportYieldedNSeries(1);
+      thenExportDidNotYieldSeriesName('test.zeros');
+      thenExportDidNotYieldSeriesName('test.empty');
+    });
+  });
 });
 });

public/app/plugins/panel/table/renderer.ts

@@ -344,17 +344,20 @@ export class TableRenderer {
 
 
   render_values() {
   render_values() {
     const rows = [];
     const rows = [];
+    const visibleColumns = this.table.columns.filter(column => !column.hidden);
 
 
     for (let y = 0; y < this.table.rows.length; y++) {
     for (let y = 0; y < this.table.rows.length; y++) {
       const row = this.table.rows[y];
       const row = this.table.rows[y];
       const newRow = [];
       const newRow = [];
       for (let i = 0; i < this.table.columns.length; i++) {
       for (let i = 0; i < this.table.columns.length; i++) {
+        if (!this.table.columns[i].hidden) {
           newRow.push(this.formatColumnValue(i, row[i]));
           newRow.push(this.formatColumnValue(i, row[i]));
         }
         }
+      }
       rows.push(newRow);
       rows.push(newRow);
     }
     }
     return {
     return {
-      columns: this.table.columns,
+      columns: visibleColumns,
       rows: rows,
       rows: rows,
     };
     };
   }
   }

public/app/plugins/panel/table/specs/renderer.test.ts

@@ -3,6 +3,7 @@ import TableModel from 'app/core/table_model';
 import { TableRenderer } from '../renderer';
 import { TableRenderer } from '../renderer';
 import { getColorDefinitionByName } from '@grafana/data';
 import { getColorDefinitionByName } from '@grafana/data';
 import { ScopedVars } from '@grafana/data';
 import { ScopedVars } from '@grafana/data';
+import { ColumnRender } from '../types';
 
 
 describe('when rendering table', () => {
 describe('when rendering table', () => {
   const SemiDarkOrange = getColorDefinitionByName('semi-dark-orange');
   const SemiDarkOrange = getColorDefinitionByName('semi-dark-orange');
@@ -23,9 +24,10 @@ describe('when rendering table', () => {
       { text: 'RangeMapping' },
       { text: 'RangeMapping' },
       { text: 'MappingColored' },
       { text: 'MappingColored' },
       { text: 'RangeMappingColored' },
       { text: 'RangeMappingColored' },
+      { text: 'HiddenType' },
     ];
     ];
     table.rows = [
     table.rows = [
-      [1388556366666, 1230, 40, undefined, '', '', 'my.host.com', 'host1', ['value1', 'value2'], 1, 2, 1, 2],
+      [1388556366666, 1230, 40, undefined, '', '', 'my.host.com', 'host1', ['value1', 'value2'], 1, 2, 1, 2, 'ignored'],
     ];
     ];
 
 
     const panel = {
     const panel = {
@@ -164,6 +166,10 @@ describe('when rendering table', () => {
           thresholds: [2, 5],
           thresholds: [2, 5],
           colors: ['#00ff00', SemiDarkOrange.name, 'rgb(1,0,0)'],
           colors: ['#00ff00', SemiDarkOrange.name, 'rgb(1,0,0)'],
         },
         },
+        {
+          pattern: 'HiddenType',
+          type: 'hidden',
+        },
       ],
       ],
     };
     };
 
 
@@ -385,6 +391,19 @@ describe('when rendering table', () => {
       const html = renderer.renderCell(12, 0, '7.1');
       const html = renderer.renderCell(12, 0, '7.1');
       expect(html).toBe('<td style="color:rgb(1,0,0)">7.1</td>');
       expect(html).toBe('<td style="color:rgb(1,0,0)">7.1</td>');
     });
     });
+
+    it('hidden columns should not be rendered', () => {
+      const html = renderer.renderCell(13, 0, 'ignored');
+      expect(html).toBe('');
+    });
+
+    it('render_values should ignore hidden columns', () => {
+      renderer.render(0); // this computes the hidden markers on the columns
+      const { columns, rows } = renderer.render_values();
+      expect(rows).toHaveLength(1);
+      expect(columns).toHaveLength(table.columns.length - 1);
+      expect(columns.filter((col: ColumnRender) => col.hidden)).toHaveLength(0);
+    });
   });
   });
 });
 });