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. (#574) (#608)

* Reduced flakiness of query monitor cypress tests.

Signed-off-by: AWSHurneyt <[email protected]>

* Fixed a bug impacting data filter configuration through the visual editor. Added integration test.

Signed-off-by: AWSHurneyt <[email protected]>

* Removed some test refactoring that were intended to reduce flakiness as flakiness didn't improve.

Signed-off-by: AWSHurneyt <[email protected]>

---------

Signed-off-by: AWSHurneyt <[email protected]>
(cherry picked from commit 18bdd05)

Co-authored-by: AWSHurneyt <[email protected]>
  • Loading branch information
opensearch-trigger-bot[bot] and AWSHurneyt authored Jul 12, 2023
1 parent 9af360d commit b18b434
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 44 deletions.
38 changes: 34 additions & 4 deletions cypress/integration/bucket_level_monitor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,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 @@ -241,6 +243,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 @@ -351,7 +381,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
68 changes: 43 additions & 25 deletions cypress/integration/query_level_monitor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,58 +361,76 @@ describe('Query-Level Monitors', () => {
describe('schedule component displays as intended', () => {
before(() => {
cy.deleteAllMonitors();

// Create the test monitors
cy.createMonitor(sampleDaysIntervalQueryLevelMonitor);
cy.createMonitor(sampleCronExpressionQueryLevelMonitor);
});

beforeEach(() => {
cy.reload();
});

it('for an interval schedule', () => {
// Create the test monitor
cy.createMonitor(sampleDaysIntervalQueryLevelMonitor);

// Confirm we can see the created monitors in the list
cy.get(`input[type="search"]`).focus().type(SAMPLE_DAYS_INTERVAL_MONITOR);
cy.contains(SAMPLE_DAYS_INTERVAL_MONITOR, { timeout: 20000 });
cy.get(`input[type="search"]`)
.focus()
.type(SAMPLE_DAYS_INTERVAL_MONITOR + '{enter}');
cy.contains(SAMPLE_DAYS_INTERVAL_MONITOR);
cy.wait(1000);

// Select the existing monitor
cy.get(`[data-test-subj="${SAMPLE_DAYS_INTERVAL_MONITOR}"]`).click({ force: true });
cy.get(`[data-test-subj="${SAMPLE_DAYS_INTERVAL_MONITOR}"]`).click({
force: true,
});

// Wait for monitor details page to load
cy.contains('Overview');

// Click Edit button
cy.contains('Edit').click({ force: true });

// Wait for input to load and then check the Schedule component
cy.get('[data-test-subj="frequency_field"]', { timeout: 20000 }).contains('By interval');
cy.get('[data-test-subj="frequency_field"]').contains('By interval');

cy.get('[data-test-subj="interval_interval_field"]', { timeout: 20000 }).should(
'have.value',
7
);
cy.get('[data-test-subj="interval_interval_field"]', {
timeout: 20000,
}).should('have.value', 7);

cy.get('[data-test-subj="interval_unit_field"]', { timeout: 20000 }).contains('Days');
cy.get('[data-test-subj="interval_unit_field"]', {
timeout: 20000,
}).contains('Days');
});

it('for a cron expression schedule', () => {
// Create the test monitor
cy.createMonitor(sampleCronExpressionQueryLevelMonitor);

// Confirm we can see the created monitors in the list
cy.get(`input[type="search"]`).focus().type(SAMPLE_CRON_EXPRESSION_MONITOR);
cy.contains(SAMPLE_CRON_EXPRESSION_MONITOR, { timeout: 20000 });
cy.get(`input[type="search"]`)
.focus()
.type(SAMPLE_CRON_EXPRESSION_MONITOR + '{enter}');
cy.contains(SAMPLE_CRON_EXPRESSION_MONITOR);
cy.wait(1000);

// Select the existing monitor
cy.get(`[data-test-subj="${SAMPLE_CRON_EXPRESSION_MONITOR}"]`).click({ force: true });
cy.get(`[data-test-subj="${SAMPLE_CRON_EXPRESSION_MONITOR}"]`).click({
force: true,
});

// Wait for monitor details page to load
cy.contains('Overview');

// Click Edit button
cy.contains('Edit').click({ force: true });

// Wait for input to load and then check the Schedule component
cy.get('[data-test-subj="frequency_field"]', { timeout: 20000 }).contains(
'Custom cron expression'
);
cy.get('[data-test-subj="frequency_field"]').contains('Custom cron expression');

cy.get('[data-test-subj="customCron_cronExpression_field"]', { timeout: 20000 }).contains(
'30 11 * * 1-5'
);
cy.get('[data-test-subj="customCron_cronExpression_field"]', {
timeout: 20000,
}).contains('30 11 * * 1-5');

cy.get('[data-test-subj="timezoneComboBox"]', { timeout: 20000 }).contains('US/Pacific');
cy.get('[data-test-subj="timezoneComboBox"]', {
timeout: 20000,
}).contains('US/Pacific');
});
});

Expand Down
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
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
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
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 b18b434

Please sign in to comment.