Skip to content

Commit

Permalink
Fix counter vizualization (#4385)
Browse files Browse the repository at this point in the history 
* crude unit tests for counter visualisation utils

* improve type safety with default param values for getCounterData()

* fix count rows never shows zero

* remove default values for getCounterData() params
  • Loading branch information
@dmudro @arikfr
dmudro authored and arikfr committed Dec 10, 2019
1 parent 7c05a73 commit 0385b6f
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 10 deletions.
16 changes: 6 additions & 10 deletions client/app/visualizations/counter/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,29 +72,25 @@ function formatTooltip(value, formatString) {

export function getCounterData(rows, options, visualizationName) {
const result = {};

const rowsCount = rows.length;
if (rowsCount > 0) {
const rowNumber = getRowNumber(options.rowNumber, rowsCount);
const targetRowNumber = getRowNumber(options.targetRowNumber, rowsCount);

if (rowsCount > 0 || options.countRow) {
const counterColName = options.counterColName;
const targetColName = options.targetColName;
const counterLabel = options.counterLabel;

if (counterLabel) {
result.counterLabel = counterLabel;
} else {
result.counterLabel = visualizationName;
}
result.counterLabel = options.counterLabel || visualizationName;

if (options.countRow) {
result.counterValue = rowsCount;
} else if (counterColName) {
const rowNumber = getRowNumber(options.rowNumber, rowsCount);
result.counterValue = rows[rowNumber][counterColName];
}

result.showTrend = false;

if (targetColName) {
const targetRowNumber = getRowNumber(options.targetRowNumber, rowsCount);
result.targetValue = rows[targetRowNumber][targetColName];

if (Number.isFinite(result.counterValue) && isFinite(result.targetValue)) {
Expand Down
192 changes: 192 additions & 0 deletions client/app/visualizations/counter/utils.test.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
import { getCounterData } from './utils';

let dummy;

describe('Visualizations -> Counter -> Utils', () => {
beforeEach(() => {
dummy = {
rows: [
{ city: 'New York City', population: 18604000 },
{ city: 'Shangai', population: 24484000 },
{ city: 'Tokyo', population: 38140000 },
],
options: {},
visualisationName: 'Visualisation Name',
result: {
counterLabel: 'Visualisation Name',
counterValue: '',
targetValue: null,
counterValueTooltip: '',
targetValueTooltip: '',
},
};
});

describe('getCounterData()', () => {
describe('"Count rows" option is disabled', () => {
test('No target and counter values return empty result', () => {
const result = getCounterData(dummy.rows, dummy.options, dummy.visualisationName);
expect(result).toEqual({
...dummy.result,
showTrend: false,
});
});

test('"Counter label" overrides vizualization name', () => {
const result = getCounterData(
dummy.rows,
{ counterLabel: 'Counter Label' },
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
counterLabel: 'Counter Label',
showTrend: false,
});
});

test('"Counter Value Column Name" must be set to a correct non empty value', () => {
const result = getCounterData(
dummy.rows,
{ rowNumber: 3 },
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
showTrend: false,
});

const result2 = getCounterData(
dummy.rows,
{ counterColName: 'missingColumn' },
dummy.visualisationName,
);
expect(result2).toEqual({
...dummy.result,
showTrend: false,
});
});

test('"Counter Value Column Name" uses correct column', () => {
const result = getCounterData(
dummy.rows,
{ counterColName: 'population' },
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
counterValue: '18,604,000.000',
counterValueTooltip: '18,604,000',
showTrend: false,
});
});

test('Counter and target values return correct result including trend', () => {
const result = getCounterData(
dummy.rows,
{
rowNumber: 1,
counterColName: 'population',
targetRowNumber: 2,
targetColName: 'population',
},
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
counterValue: '18,604,000.000',
counterValueTooltip: '18,604,000',
targetValue: '24484000',
targetValueTooltip: '24,484,000',
showTrend: true,
trendPositive: false,
});

const result2 = getCounterData(
dummy.rows,
{
rowNumber: 2,
counterColName: 'population',
targetRowNumber: 1,
targetColName: 'population',
},
dummy.visualisationName,
);
expect(result2).toEqual({
...dummy.result,
counterValue: '24,484,000.000',
counterValueTooltip: '24,484,000',
targetValue: '18604000',
targetValueTooltip: '18,604,000',
showTrend: true,
trendPositive: true,
});
});
});

describe('"Count rows" option is enabled', () => {
beforeEach(() => {
dummy.result = {
...dummy.result,
counterValue: '3.000',
counterValueTooltip: '3',
showTrend: false,
};
});

test('Rows are counted correctly', () => {
const result = getCounterData(
dummy.rows,
{ countRow: true },
dummy.visualisationName,
);
expect(result).toEqual(dummy.result);
});

test('Counter value is ignored', () => {
const result = getCounterData(
dummy.rows,
{
countRow: true,
rowNumber: 3,
counterColName: 'population',
},
dummy.visualisationName,
);
expect(result).toEqual(dummy.result);
});

test('Target value and trend are computed correctly', () => {
const result = getCounterData(
dummy.rows,
{
countRow: true,
targetRowNumber: 2,
targetColName: 'population',
},
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
targetValue: '24484000',
targetValueTooltip: '24,484,000',
showTrend: true,
trendPositive: false,
});
});

test('Empty rows return counter value 0', () => {
const result = getCounterData(
[],
{ countRow: true },
dummy.visualisationName,
);
expect(result).toEqual({
...dummy.result,
counterValue: '0.000',
counterValueTooltip: '0',
});
});
});
});
});

0 comments on commit 0385b6f

Please sign in to comment.