Skip to content

Commit

Permalink
[Rollup] Metrics multiple selection (#42927)
Browse files Browse the repository at this point in the history
* remove duplicate columns declaration

* First iteration of select all menu working

* A lot of bugfixes and started on row-level selection using flatMap

* Wip on row selection

* Fix formatting

* More bug fixes and and first iteration of row selection and column selection working

* Minor refactors, cleaned up logging. Still has nasty UX bug on Popover menu

* Added translations and empty tests

* Tests are WiP

* Move checkboxes out of popover

* Implemented first iteration of select all functionality

* Move row select all into checkbox area

* Remove unused code and simplify select all if-else

* First iteration of tests

* fix tests and update name data-test-subj name

* Move checkboxes into popover with button

* - Fix tests
- Refactor test helpers for metric step
- Cleanup some JS style nits
- Make popover button disabled if all metric checkboxes are disabled

* Remove extra newline

* - Moved constants from metrics to own file so that it's more re-usable
- Removed unnecessary styling
- Fixed typos
- Updated comments
- i18n
  • Loading branch information
jloleysens authored Aug 15, 2019
1 parent dbb140d commit bd8984b
Show file tree
Hide file tree
Showing 7 changed files with 557 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const initTestBed = registerTestBed(JobCreate, { store: rollupJobsStore });

export const setup = (props) => {
const testBed = initTestBed(props);
const { component, form } = testBed;
const { component, form, table } = testBed;

// User actions
const clickNextStep = () => {
Expand Down Expand Up @@ -68,6 +68,30 @@ export const setup = (props) => {
}
};

// Helpers for the metrics step in job creation.
// Mostly placed here for now to make test file a bit smaller and specific
// to tests.
const getFieldListTableRows = () => {
const { rows } = table.getMetaData('rollupJobMetricsFieldList');
return rows;
};

const getFieldListTableRow = (row) => {
const rows = getFieldListTableRows();
return rows[row];
};

const getFieldChooserColumnForRow = (row) => {
const selectedRow = getFieldListTableRow(row);
const [,, fieldChooserColumn] = selectedRow.columns;
return fieldChooserColumn;
};

const getSelectAllInputForRow = (row) => {
const fieldChooser = getFieldChooserColumnForRow(row);
return fieldChooser.reactWrapper.find('input').first();
};

// Misc
const getEuiStepsHorizontalActive = () => component.find('.euiStepHorizontal-isSelected').text();

Expand All @@ -84,5 +108,11 @@ export const setup = (props) => {
...testBed.form,
fillFormFields,
},
metrics: {
getFieldListTableRows,
getFieldListTableRow,
getFieldChooserColumnForRow,
getSelectAllInputForRow,
}
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('Create Rollup Job, step 5: Metrics', () => {
let getEuiStepsHorizontalActive;
let goToStep;
let table;
let metrics;

beforeAll(() => {
({ server, httpRequestsMockHelpers } = setupEnvironment());
Expand All @@ -56,6 +57,7 @@ describe('Create Rollup Job, step 5: Metrics', () => {
getEuiStepsHorizontalActive,
goToStep,
table,
metrics,
} = setup());
});

Expand Down Expand Up @@ -175,6 +177,9 @@ describe('Create Rollup Job, step 5: Metrics', () => {
}
};

const numericTypeMetrics = ['avg', 'max', 'min', 'sum', 'value_count'];
const dateTypeMetrics = ['max', 'min', 'value_count'];

it('should have an empty field list', async () => {
await goToStep(5);

Expand All @@ -189,7 +194,6 @@ describe('Create Rollup Job, step 5: Metrics', () => {
});

it('should have "avg", "max", "min", "sum" & "value count" metrics for *numeric* fields', () => {
const numericTypeMetrics = ['avg', 'max', 'min', 'sum', 'value_count'];
addFieldToList('numeric');
numericTypeMetrics.forEach(type => {
try {
Expand All @@ -203,11 +207,10 @@ describe('Create Rollup Job, step 5: Metrics', () => {
const { rows: [firstRow] } = table.getMetaData('rollupJobMetricsFieldList');
const columnWithMetricsCheckboxes = 2;
const metricsCheckboxes = firstRow.columns[columnWithMetricsCheckboxes].reactWrapper.find('input');
expect(metricsCheckboxes.length).toBe(numericTypeMetrics.length);
expect(metricsCheckboxes.length).toBe(numericTypeMetrics.length + 1 /* add one for select all */);
});

it('should have "max", "min", & "value count" metrics for *date* fields', () => {
const dateTypeMetrics = ['max', 'min', 'value_count'];
addFieldToList('date');

dateTypeMetrics.forEach(type => {
Expand All @@ -222,7 +225,7 @@ describe('Create Rollup Job, step 5: Metrics', () => {
const { rows: [firstRow] } = table.getMetaData('rollupJobMetricsFieldList');
const columnWithMetricsCheckboxes = 2;
const metricsCheckboxes = firstRow.columns[columnWithMetricsCheckboxes].reactWrapper.find('input');
expect(metricsCheckboxes.length).toBe(dateTypeMetrics.length);
expect(metricsCheckboxes.length).toBe(dateTypeMetrics.length + 1 /* add one for select all */);
});

it('should not allow to go to the next step if at least one metric type is not selected', () => {
Expand All @@ -247,12 +250,162 @@ describe('Create Rollup Job, step 5: Metrics', () => {

const columnsFirstRow = fieldListRows[0].columns;
// The last column is the eui "actions" column
const deleteButton = columnsFirstRow[columnsFirstRow.length - 1].reactWrapper.find('button');
const deleteButton = columnsFirstRow[columnsFirstRow.length - 1].reactWrapper.find('button').last();
deleteButton.simulate('click');

({ rows: fieldListRows } = table.getMetaData('rollupJobMetricsFieldList'));
expect(fieldListRows[0].columns[0].value).toEqual('No metrics fields added');
});
});

describe('when using multi-selectors', () => {
let getSelectAllInputForRow;
let getFieldChooserColumnForRow;
let getFieldListTableRows;

beforeEach(async () => {
httpRequestsMockHelpers.setIndexPatternValidityResponse({ numericFields, dateFields });
await goToStep(5);
await addFieldToList('numeric');
await addFieldToList('date');
find('rollupJobSelectAllMetricsPopoverButton').simulate('click');
({ getSelectAllInputForRow, getFieldChooserColumnForRow, getFieldListTableRows } = metrics);
});

const expectAllFieldChooserInputs = (fieldChooserColumn, expected) => {
const inputs = fieldChooserColumn.reactWrapper.find('input');
inputs.forEach((input) => {
expect(input.props().checked).toBe(expected);
});
};

it('should select all of the fields in a row', async () => {
// The last column is the eui "actions" column
const selectAllCheckbox = getSelectAllInputForRow(0);
selectAllCheckbox.simulate('change', { checked: true });
const fieldChooserColumn = getFieldChooserColumnForRow(0);
expectAllFieldChooserInputs(fieldChooserColumn, true);
});

it('should deselect all of the fields in a row ', async () => {
const selectAllCheckbox = getSelectAllInputForRow(0);
selectAllCheckbox.simulate('change', { checked: true });

let fieldChooserColumn = getFieldChooserColumnForRow(0);
expectAllFieldChooserInputs(fieldChooserColumn, true);

selectAllCheckbox.simulate('change', { checked: false });
fieldChooserColumn = getFieldChooserColumnForRow(0);
expectAllFieldChooserInputs(fieldChooserColumn, false);
});

it('should select all of the metric types across rows (column-wise)', () => {
const selectAllAvgCheckbox = find('rollupJobMetricsSelectAllCheckbox-avg');
selectAllAvgCheckbox.first().simulate('change', { checked: true });

const rows = getFieldListTableRows();

rows
.filter((row) => {
const [, metricTypeCol ] = row.columns;
return metricTypeCol.value === 'numeric';
})
.forEach((row, idx) => {
const fieldChooser = getFieldChooserColumnForRow(idx);
fieldChooser.reactWrapper.find('input').forEach(input => {
const props = input.props();
if (props['data-test-subj'].endsWith('avg')) {
expect(props.checked).toBe(true);
} else {
expect(props.checked).toBe(false);
}
});
});
});

it('should deselect all of the metric types across rows (column-wise)', () => {
const selectAllAvgCheckbox = find('rollupJobMetricsSelectAllCheckbox-avg');

// Select first, which adds metrics column-wise, and then de-select column-wise to ensure
// that we correctly remove/undo our adding action.
selectAllAvgCheckbox.last().simulate('change', { checked: true });
selectAllAvgCheckbox.last().simulate('change', { checked: false });

const rows = getFieldListTableRows();

rows.forEach((row, idx) => {
const [, metricTypeCol ] = row.columns;
if (metricTypeCol.value !== 'numeric') {
return;
}
const fieldChooser = getFieldChooserColumnForRow(idx);
fieldChooser.reactWrapper.find('input').forEach(input => {
expect(input.props().checked).toBe(false);
});
});
});

it('should correctly select across rows and columns', async () => {
/**
* Tricky test case where we want to determine that the column-wise and row-wise
* selections are interacting correctly with each other.
*
* We will select avg (numeric) and max (numeric + date) to ensure that we are
* testing across metrics types too. The plan is:
*
* 1. Select all avg column-wise
* 2. Select all max column-wise
* 3. Select and deselect row-wise the first numeric metric row. Because some items will
* have been selected by the previous column-wise selection we want to test that row-wise
* select all followed by de-select can effectively undo the column-wise selections.
* 4. Expect the avg and max select all checkboxes to be unchecked
* 5. Select all on the last date metric row-wise
* 6. Select then deselect all max column-wise
* 7. Expect everything but all and max to be selected on the last date metric row
*
* Let's a go!
*/

// 1.
find('rollupJobMetricsSelectAllCheckbox-avg').first().simulate('change', { checked: true });
// 2.
find('rollupJobMetricsSelectAllCheckbox-max').first().simulate('change', { checked: true });

const selectAllCheckbox = getSelectAllInputForRow(0);

// 3.
// Select all (which should check all checkboxes)
selectAllCheckbox.simulate('change', { checked: true });
// Deselect all (which should deselect all checkboxes)
selectAllCheckbox.simulate('change', { checked: false });

// 4.
expect(find('rollupJobMetricsSelectAllCheckbox-avg').first().props().checked).toBe(false);
expect(find('rollupJobMetricsSelectAllCheckbox-max').first().props().checked).toBe(false);

let rows = getFieldListTableRows();
// 5.
getSelectAllInputForRow(rows.length - 1).simulate('change', { checked: true });

// 6.
find('rollupJobMetricsSelectAllCheckbox-max').first().simulate('change', { checked: true });
find('rollupJobMetricsSelectAllCheckbox-max').first().simulate('change', { checked: false });

rows = getFieldListTableRows();
const lastRowFieldChooserColumn = getFieldChooserColumnForRow(rows.length - 1);
// 7.
lastRowFieldChooserColumn.reactWrapper.find('input').forEach((input) => {
const props = input.props();
if (
props['data-test-subj'].endsWith('max') ||
props['data-test-subj'].endsWith('All')
) {
expect(props.checked).toBe(false);
} else {
expect(props.checked).toBe(true);
}
});
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@
export {
CRUD_APP_BASE_PATH,
} from './paths';

export {
METRICS_CONFIG
} from './metrics_config';
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';

export const METRICS_CONFIG = [
{
type: 'avg',
label: i18n.translate(
'xpack.rollupJobs.create.stepMetrics.checkboxAverageLabel',
{ defaultMessage: 'Average' },
),
},
{
type: 'max',
label: i18n.translate(
'xpack.rollupJobs.create.stepMetrics.checkboxMaxLabel',
{ defaultMessage: 'Maximum' },
),
},
{
type: 'min',
label: i18n.translate(
'xpack.rollupJobs.create.stepMetrics.checkboxMinLabel',
{ defaultMessage: 'Minimum' },
),
},
{
type: 'sum',
label: i18n.translate(
'xpack.rollupJobs.create.stepMetrics.checkboxSumLabel',
{ defaultMessage: 'Sum' },
),
},
{
type: 'value_count',
label: i18n.translate(
'xpack.rollupJobs.create.stepMetrics.checkboxValueCountLabel',
{ defaultMessage: 'Value count' },
),
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const FieldList = ({
type: 'icon',
color: 'danger',
onClick: (field) => onRemoveField(field),
}]
}],
});
} else {
extendedColumns = columns;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export class FieldChooser extends Component {
fields: PropTypes.array.isRequired,
selectedFields: PropTypes.array.isRequired,
onSelectField: PropTypes.func.isRequired,
columns: PropTypes.array.isRequired,
prompt: PropTypes.string,
dataTestSubj: PropTypes.string,
}
Expand Down Expand Up @@ -136,7 +135,6 @@ export class FieldChooser extends Component {
</EuiFlyout>
);
};

return (
<Fragment>
<EuiButton
Expand Down
Loading

0 comments on commit bd8984b

Please sign in to comment.