Skip to content

Commit

Permalink
Fixed a bug impacting data filter configuration through the visual ed…
Browse files Browse the repository at this point in the history 
…itor. Added integration test.

Signed-off-by: AWSHurneyt <[email protected]>
  • Loading branch information
@AWSHurneyt
AWSHurneyt committed Jun 15, 2023
1 parent bef29d4 commit 14206df
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 25 deletions.
70 changes: 60 additions & 10 deletions cypress/integration/bucket_level_monitor_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const AVERAGE_METRIC_NAME = 'avg_products_base_price';
const TESTING_INDEX_A = 'bucket-level-monitor-test-index-a';
const TESTING_INDEX_B = 'bucket-level-monitor-test-index-b';

// 3 seconds
const INDICES_LOADING_WAIT = 3000;

const addTriggerToVisualEditorMonitor = (triggerName, triggerIndex, actionName, isEdit) => {
// Add a trigger
cy.contains('Add trigger').click({ force: true });
Expand Down Expand Up @@ -73,13 +76,15 @@ const addTriggerToVisualEditorMonitor = (triggerName, triggerIndex, actionName,
force: true,
});

cy.get(`[name="triggerDefinitions[${triggerIndex}].filters.0.fieldName"]`).type(
cy.get(`[data-test-subj="triggerDefinitions[${triggerIndex}].filters.0.fieldName"]`).type(
`${GROUP_BY_FIELD}{downarrow}{enter}`
);

cy.get(`[name="triggerDefinitions[${triggerIndex}].filters.0.operator"]`).select('includes');
cy.get(`[data-test-subj="triggerDefinitions[${triggerIndex}].filters.0.operator"]`).select(
'includes'
);

cy.get(`[name="triggerDefinitions[${triggerIndex}].filters.0.fieldValue"]`)
cy.get(`[data-test-subj="triggerDefinitions[${triggerIndex}].filters.0.fieldValue"]`)
.type('a*')
.trigger('blur', { force: true });

Expand Down Expand Up @@ -214,10 +219,22 @@ describe('Bucket-Level Monitors', () => {

// Wait for input to load and then type in the index name
// Pressing enter at the end to create combo box entry and trigger change events for time field below
cy.get('#index').type(`${INDEX.SAMPLE_DATA_ECOMMERCE}{enter}`, { force: true });
cy.get('[data-test-subj="indicesComboBox"]')
.click({ force: true })
.type(INDEX.SAMPLE_DATA_ECOMMERCE)
// Adding a short wait time here to reduce flakiness of async combobox
.wait(INDICES_LOADING_WAIT)
.type(`{enter}`)
.trigger('blur', { force: true });

// Select 'order_date' as the timeField for the data source index
cy.get('#timeField').type(`${TIME_FIELD}{downArrow}{enter}`, { force: true });
cy.get('[data-test-subj="timeFieldComboBox"]')
.click({ force: true })
.type(TIME_FIELD)
// Adding a short wait time here to reduce flakiness of async combobox
.wait(INDICES_LOADING_WAIT)
.type('{enter}')
.trigger('blur', { force: true });

// Add a metric for the query
cy.get('[data-test-subj="addMetricButton"]').click({ force: true });
Expand All @@ -241,6 +258,34 @@ describe('Bucket-Level Monitors', () => {

cy.get('button').contains('Save').click({ force: true });

// Add data filters for the query
const filters = [
// Number type
{ field: 'products.quantity', operator: 'is less than', value: 100 },
// Text type
{ field: 'manufacturer', operator: 'starts with', value: 'Amazon' },
// Keyword type
{ field: 'user', operator: 'contains', value: 'Jeff' },
];

filters.forEach((filter, index) => {
cy.get(`[data-test-subj="addFilterButton"]`).click({ force: true });

cy.get(`[data-test-subj="filters.${index}.fieldName"]`).type(
`${filter.field}{downarrow}{enter}`
);

cy.get(`[data-test-subj="filters.${index}.operator"]`).select(filter.operator);

cy.get(`[data-test-subj="filters.${index}.fieldValue"]`).type(`${filter.value}{enter}`);

// Close data filter popover
cy.contains('Data filter - optional').click({ force: true });
});

// Confirm filter limit text is correct
cy.contains('You can add up to 7 more data filters.', { timeout: 20000 });

// Add a group by field for the query
cy.get('[data-test-subj="addGroupByButton"]').click({ force: true });

Expand Down Expand Up @@ -321,11 +366,16 @@ describe('Bucket-Level Monitors', () => {
cy.contains('Edit').click({ force: true });

// Click on the Index field and type in multiple index names to replicate the bug
cy.get('#index')
cy.get('[data-test-subj="indicesComboBox"]')
.click({ force: true })
.type(`${TESTING_INDEX_A}{enter}${TESTING_INDEX_B}{enter}`, {
force: true,
})
.type(TESTING_INDEX_A)
// Adding a short wait time here to reduce flakiness of async combobox
.wait(INDICES_LOADING_WAIT)
.type(`{enter}`)
.type(TESTING_INDEX_B)
// Adding a short wait time here to reduce flakiness of async combobox
.wait(INDICES_LOADING_WAIT)
.type(`{enter}`)
.trigger('blur', { force: true });

// Confirm Index field only contains the expected text
Expand All @@ -351,7 +401,7 @@ describe('Bucket-Level Monitors', () => {
cy.deleteAllMonitors();

// Delete sample data
cy.deleteIndexByName(`${INDEX.SAMPLE_DATA_ECOMMERCE}`);
cy.deleteIndexByName(INDEX.SAMPLE_DATA_ECOMMERCE);
cy.deleteIndexByName(TESTING_INDEX_A);
cy.deleteIndexByName(TESTING_INDEX_B);
});
Expand Down
6 changes: 6 additions & 0 deletions public/pages/CreateMonitor/components/MonitorExpressions/expressions/WherePopover.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export default class WherePopover extends Component {
inputProps={{
placeholder: 'Enter a value',
onChange: this.handleChangeWrapper,
'data-test-subj': `${fieldPath}fieldValue`,
}}
formRow
rowProps={{ isInvalid, error: hasError }}
Expand All @@ -136,6 +137,7 @@ export default class WherePopover extends Component {
onChange: this.handleChangeWrapper,
options: WHERE_BOOLEAN_FILTERS,
isInvalid,
'data-test-subj': `${fieldPath}fieldValue`,
}}
/>
);
Expand All @@ -147,6 +149,7 @@ export default class WherePopover extends Component {
placeholder: 'Enter a value',
onChange: this.handleChangeWrapper,
isInvalid,
'data-test-subj': `${fieldPath}fieldValue`,
}}
/>
);
Expand Down Expand Up @@ -211,6 +214,7 @@ export default class WherePopover extends Component {
iconOnClickAriaLabel={'Remove filter'}
onClick={this.openPopover}
onClickAriaLabel={'Edit where filter'}
data-test-subj={`${whereFilterHeader}-${fieldPath}-badge`}
>
{displayText(values)}
</EuiBadge>
Expand All @@ -234,6 +238,7 @@ export default class WherePopover extends Component {
onChange: this.handleFieldChange,
isClearable: false,
singleSelection: { asPlainText: true },
'data-test-subj': `${fieldPath}fieldName`,
}}
/>
</EuiFlexItem>
Expand All @@ -243,6 +248,7 @@ export default class WherePopover extends Component {
inputProps={{
onChange: this.handleOperatorChange,
options: fieldOperators,
'data-test-subj': `${fieldPath}operator`,
}}
/>
</EuiFlexItem>
Expand Down
28 changes: 14 additions & 14 deletions public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/whereHelpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,23 @@ export const validateWhereFilter = (filter = FORMIK_INITIAL_WHERE_EXPRESSION_VAL
const fieldOperator = _.get(filter, 'operator', FORMIK_INITIAL_WHERE_EXPRESSION_VALUES.operator);
let filterIsValid = !_.isEmpty(fieldName.label);
switch (fieldOperator) {
case OPERATORS_MAP.IS:
case OPERATORS_MAP.IS_NOT:
case OPERATORS_MAP.IS_GREATER:
case OPERATORS_MAP.IS_GREATER_EQUAL:
case OPERATORS_MAP.IS_LESS:
case OPERATORS_MAP.IS_LESS_EQUAL:
case OPERATORS_MAP.STARTS_WITH:
case OPERATORS_MAP.ENDS_WITH:
case OPERATORS_MAP.CONTAINS:
case OPERATORS_MAP.NOT_CONTAINS:
case OPERATORS_MAP.IS.value:
case OPERATORS_MAP.IS_NOT.value:
case OPERATORS_MAP.IS_GREATER.value:
case OPERATORS_MAP.IS_GREATER_EQUAL.value:
case OPERATORS_MAP.IS_LESS.value:
case OPERATORS_MAP.IS_LESS_EQUAL.value:
case OPERATORS_MAP.STARTS_WITH.value:
case OPERATORS_MAP.ENDS_WITH.value:
case OPERATORS_MAP.CONTAINS.value:
case OPERATORS_MAP.DOES_NOT_CONTAINS.value:
// These operators store the query value in the 'fieldValue'
// attribute of FORMIK_INITIAL_WHERE_EXPRESSION_VALUES.
// Validate that value.
filterIsValid = filterIsValid && !_.isEmpty(filter.fieldValue?.toString());
break;
case OPERATORS_MAP.IN_RANGE:
case OPERATORS_MAP.NOT_IN_RANGE:
case OPERATORS_MAP.IN_RANGE.value:
case OPERATORS_MAP.NOT_IN_RANGE.value:
// These operators store the query values in the 'fieldRangeStart' and 'fieldRangeEnd'
// attributes of FORMIK_INITIAL_WHERE_EXPRESSION_VALUES.
// Both of those values need to be validated.
Expand All @@ -93,8 +93,8 @@ export const validateWhereFilter = (filter = FORMIK_INITIAL_WHERE_EXPRESSION_VAL
!validateRange(filter.fieldRangeStart, filter) &&
!validateRange(filter.fieldRangeEnd, filter);
break;
case OPERATORS_MAP.IS_NULL:
case OPERATORS_MAP.IS_NOT_NULL:
case OPERATORS_MAP.IS_NULL.value:
case OPERATORS_MAP.IS_NOT_NULL.value:
// These operators don't store a query value in the FORMIK_INITIAL_WHERE_EXPRESSION_VALUES.
// No further validation needed.
break;
Expand Down
2 changes: 1 addition & 1 deletion public/pages/CreateMonitor/components/MonitorTimeField/MonitorTimeField.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import React from 'react';
import PropTypes from 'prop-types';
import FormikSelect from '../../../../components/FormControls/FormikSelect/FormikSelect';
import { hasError, isInvalid } from '../../../../utils/validate';
import { validateTimeField } from './utils/validation';
import { FormikComboBox } from '../../../../components/FormControls';
Expand Down Expand Up @@ -34,6 +33,7 @@ const MonitorTimeField = ({ dataTypes }) => {
},
isClearable: false,
singleSelection: { asPlainText: true },
'data-test-subj': 'timeFieldComboBox',
}}
/>
);
Expand Down
1 change: 1 addition & 0 deletions ...ges/CreateMonitor/components/MonitorTimeField/__snapshots__/MonitorTimeField.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ exports[`MonitorTimeField renders 1`] = `
aria-expanded="false"
aria-haspopup="listbox"
class="euiComboBox"
data-test-subj="timeFieldComboBox"
name="timeField"
role="combobox"
>
Expand Down

0 comments on commit 14206df

Please sign in to comment.