From 14206df38ccf2e91349847ff052ae1a2d5041e3d Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Wed, 14 Jun 2023 18:17:12 -0700 Subject: [PATCH] Fixed a bug impacting data filter configuration through the visual editor. Added integration test. Signed-off-by: AWSHurneyt --- .../integration/bucket_level_monitor_spec.js | 70 ++++++++++++++++--- .../expressions/WherePopover.js | 6 ++ .../expressions/utils/whereHelpers.js | 28 ++++---- .../MonitorTimeField/MonitorTimeField.js | 2 +- .../MonitorTimeField.test.js.snap | 1 + 5 files changed, 82 insertions(+), 25 deletions(-) diff --git a/cypress/integration/bucket_level_monitor_spec.js b/cypress/integration/bucket_level_monitor_spec.js index 297869084..1e49841bd 100644 --- a/cypress/integration/bucket_level_monitor_spec.js +++ b/cypress/integration/bucket_level_monitor_spec.js @@ -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 }); @@ -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 }); @@ -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 }); @@ -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 }); @@ -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 @@ -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); }); diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WherePopover.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WherePopover.js index cc54c5210..e038db14f 100644 --- a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WherePopover.js +++ b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/WherePopover.js @@ -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 }} @@ -136,6 +137,7 @@ export default class WherePopover extends Component { onChange: this.handleChangeWrapper, options: WHERE_BOOLEAN_FILTERS, isInvalid, + 'data-test-subj': `${fieldPath}fieldValue`, }} /> ); @@ -147,6 +149,7 @@ export default class WherePopover extends Component { placeholder: 'Enter a value', onChange: this.handleChangeWrapper, isInvalid, + 'data-test-subj': `${fieldPath}fieldValue`, }} /> ); @@ -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)} @@ -234,6 +238,7 @@ export default class WherePopover extends Component { onChange: this.handleFieldChange, isClearable: false, singleSelection: { asPlainText: true }, + 'data-test-subj': `${fieldPath}fieldName`, }} /> @@ -243,6 +248,7 @@ export default class WherePopover extends Component { inputProps={{ onChange: this.handleOperatorChange, options: fieldOperators, + 'data-test-subj': `${fieldPath}operator`, }} /> diff --git a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/whereHelpers.js b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/whereHelpers.js index 0c70fc474..17478ec0e 100644 --- a/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/whereHelpers.js +++ b/public/pages/CreateMonitor/components/MonitorExpressions/expressions/utils/whereHelpers.js @@ -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. @@ -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; diff --git a/public/pages/CreateMonitor/components/MonitorTimeField/MonitorTimeField.js b/public/pages/CreateMonitor/components/MonitorTimeField/MonitorTimeField.js index 7903cbdbf..ba89267db 100644 --- a/public/pages/CreateMonitor/components/MonitorTimeField/MonitorTimeField.js +++ b/public/pages/CreateMonitor/components/MonitorTimeField/MonitorTimeField.js @@ -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'; @@ -34,6 +33,7 @@ const MonitorTimeField = ({ dataTypes }) => { }, isClearable: false, singleSelection: { asPlainText: true }, + 'data-test-subj': 'timeFieldComboBox', }} /> ); diff --git a/public/pages/CreateMonitor/components/MonitorTimeField/__snapshots__/MonitorTimeField.test.js.snap b/public/pages/CreateMonitor/components/MonitorTimeField/__snapshots__/MonitorTimeField.test.js.snap index 2f4224418..bb7e8d93a 100644 --- a/public/pages/CreateMonitor/components/MonitorTimeField/__snapshots__/MonitorTimeField.test.js.snap +++ b/public/pages/CreateMonitor/components/MonitorTimeField/__snapshots__/MonitorTimeField.test.js.snap @@ -24,6 +24,7 @@ exports[`MonitorTimeField renders 1`] = ` aria-expanded="false" aria-haspopup="listbox" class="euiComboBox" + data-test-subj="timeFieldComboBox" name="timeField" role="combobox" >