From 64916cd7a9103a129f71d04266f01cbbd7bf13f2 Mon Sep 17 00:00:00 2001 From: Philipp Nowak Date: Mon, 2 Dec 2019 21:58:36 +0100 Subject: [PATCH] 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 | 4 +- .../panel/graph/specs/graph_ctrl.test.ts | 74 ++++++++++++++++++- public/app/plugins/panel/table/renderer.ts | 7 +- .../panel/table/specs/renderer.test.ts | 21 +++++- 4 files changed, 101 insertions(+), 5 deletions(-) diff --git a/public/app/plugins/panel/graph/module.ts b/public/app/plugins/panel/graph/module.ts index e12085ee0af..adafe10409d 100644 --- a/public/app/plugins/panel/graph/module.ts +++ b/public/app/plugins/panel/graph/module.ts @@ -327,7 +327,9 @@ class GraphCtrl extends MetricsPanelCtrl { exportCsv() { 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, { templateHtml: '', scope, diff --git a/public/app/plugins/panel/graph/specs/graph_ctrl.test.ts b/public/app/plugins/panel/graph/specs/graph_ctrl.test.ts index 94bfabffbfb..8eda844b9b4 100644 --- a/public/app/plugins/panel/graph/specs/graph_ctrl.test.ts +++ b/public/app/plugins/panel/graph/specs/graph_ctrl.test.ts @@ -1,5 +1,6 @@ import { GraphCtrl } from '../module'; import { dateTime } from '@grafana/data'; +import TimeSeries from 'app/core/time_series2'; jest.mock('../graph', () => ({})); @@ -17,7 +18,7 @@ describe('GraphCtrl', () => { }, }; - const scope = { + const scope: any = { $on: () => {}, }; @@ -106,4 +107,75 @@ describe('GraphCtrl', () => { 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'); + }); + }); }); diff --git a/public/app/plugins/panel/table/renderer.ts b/public/app/plugins/panel/table/renderer.ts index 6f64e7b92f8..50862420a74 100644 --- a/public/app/plugins/panel/table/renderer.ts +++ b/public/app/plugins/panel/table/renderer.ts @@ -344,17 +344,20 @@ export class TableRenderer { render_values() { const rows = []; + const visibleColumns = this.table.columns.filter(column => !column.hidden); for (let y = 0; y < this.table.rows.length; y++) { const row = this.table.rows[y]; const newRow = []; for (let i = 0; i < this.table.columns.length; i++) { - newRow.push(this.formatColumnValue(i, row[i])); + if (!this.table.columns[i].hidden) { + newRow.push(this.formatColumnValue(i, row[i])); + } } rows.push(newRow); } return { - columns: this.table.columns, + columns: visibleColumns, rows: rows, }; } diff --git a/public/app/plugins/panel/table/specs/renderer.test.ts b/public/app/plugins/panel/table/specs/renderer.test.ts index b351aac285e..1f7f8dc82e1 100644 --- a/public/app/plugins/panel/table/specs/renderer.test.ts +++ b/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 { getColorDefinitionByName } from '@grafana/data'; import { ScopedVars } from '@grafana/data'; +import { ColumnRender } from '../types'; describe('when rendering table', () => { const SemiDarkOrange = getColorDefinitionByName('semi-dark-orange'); @@ -23,9 +24,10 @@ describe('when rendering table', () => { { text: 'RangeMapping' }, { text: 'MappingColored' }, { text: 'RangeMappingColored' }, + { text: 'HiddenType' }, ]; 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 = { @@ -164,6 +166,10 @@ describe('when rendering table', () => { thresholds: [2, 5], 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'); expect(html).toBe('7.1'); }); + + 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); + }); }); });