From 084c13fcd1752efb5c343e73506143684f7d7e48 Mon Sep 17 00:00:00 2001 From: AWSHurneyt Date: Tue, 21 Nov 2023 14:19:00 -0800 Subject: [PATCH] Fixed bucket monitor groupBy/aggregation display bug. (#827) * Fixed a bug that was causing groupBy/aggregation fields from displaying in various areas of the UI. Related issues: 816, 817, 818. Signed-off-by: AWSHurneyt * Fixed trigger context object bug in issue 791. Signed-off-by: AWSHurneyt * Capitalized bucket column titles, and moved bucket columns to the end of the column array. Signed-off-by: AWSHurneyt * Added wait steps to reduce test flakiness. Signed-off-by: AWSHurneyt * Added wait step to reduce test flakiness. Adjusted test monitor trigger condition to always triggers on a healthy clusters. Signed-off-by: AWSHurneyt * Removed unused imports. Signed-off-by: AWSHurneyt * fixed bucket level monitor flaky cypress test Signed-off-by: Amardeepsingh Siglani --------- Signed-off-by: AWSHurneyt Signed-off-by: Amardeepsingh Siglani Co-authored-by: Amardeepsingh Siglani Signed-off-by: AWSHurneyt --- .../alerts_dashboard_flyout_spec.js | 23 ++++++++++----- .../integration/bucket_level_monitor_spec.js | 17 +++++++---- .../integration/monitors_dashboard_spec.js | 7 ++++- .../formikToMonitor.test.js.snap | 8 ++--- .../CreateMonitor/utils/formikToMonitor.js | 2 +- .../ConfigureActions/ConfigureActions.js | 29 ++++++++++++++----- public/pages/Dashboard/utils/helpers.js | 4 +-- public/pages/Dashboard/utils/helpers.test.js | 12 ++++---- 8 files changed, 68 insertions(+), 34 deletions(-) diff --git a/cypress/integration/alerts_dashboard_flyout_spec.js b/cypress/integration/alerts_dashboard_flyout_spec.js index 478fdc2f3..7ad5089cb 100644 --- a/cypress/integration/alerts_dashboard_flyout_spec.js +++ b/cypress/integration/alerts_dashboard_flyout_spec.js @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React from 'react'; import { INDEX, PLUGIN_NAME } from '../support/constants'; import sampleAlertsFlyoutBucketMonitor from '../fixtures/sample_alerts_flyout_bucket_level_monitor.json'; import sampleAlertsFlyoutQueryMonitor from '../fixtures/sample_alerts_flyout_query_level_monitor.json'; @@ -23,12 +22,19 @@ describe('Alerts by trigger flyout', () => { // Load sample data cy.loadSampleEcommerceData(); + // Ensure monitors have been deleted + cy.visit(`${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/monitors`); + cy.contains('There are no existing monitors. Create a monitor to add triggers and actions.', { + timeout: TWENTY_SECONDS, + }); + // Create the test monitors cy.createMonitor(sampleAlertsFlyoutBucketMonitor); cy.createMonitor(sampleAlertsFlyoutQueryMonitor); // Visit Alerting OpenSearch Dashboards cy.visit(`${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/monitors`); + cy.reload(); // Confirm test monitors were created successfully cy.contains(BUCKET_MONITOR, { timeout: TWENTY_SECONDS }); @@ -41,6 +47,7 @@ describe('Alerts by trigger flyout', () => { beforeEach(() => { // Reloading the page to close any flyouts that were not closed by other tests that had failures. cy.visit(`${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/dashboard`); + cy.contains('Alerts by triggers', { timeout: TWENTY_SECONDS }); // Waiting 5 seconds for alerts to finish loading. // This short wait period alleviates flakiness observed during these tests. @@ -60,9 +67,10 @@ describe('Alerts by trigger flyout', () => { timeout: TWENTY_SECONDS, }).within(() => { // Confirm flyout header contains expected text. - cy.get( - `[data-test-subj="alertsDashboardFlyout_header_${BUCKET_TRIGGER}"]` - ).contains(`Alerts by ${BUCKET_TRIGGER}`, { timeout: TWENTY_SECONDS }); + cy.get(`[data-test-subj="alertsDashboardFlyout_header_${BUCKET_TRIGGER}"]`).contains( + `Alerts by ${BUCKET_TRIGGER}`, + { timeout: TWENTY_SECONDS } + ); // Confirm 'Trigger name' sections renders as expected. cy.get(`[data-test-subj="alertsDashboardFlyout_triggerName_${BUCKET_TRIGGER}"]`).as( @@ -154,9 +162,10 @@ describe('Alerts by trigger flyout', () => { timeout: TWENTY_SECONDS, }).within(() => { // Confirm flyout header contains expected text. - cy.get( - `[data-test-subj="alertsDashboardFlyout_header_${QUERY_TRIGGER}"]` - ).contains(`Alerts by ${QUERY_TRIGGER}`, { timeout: TWENTY_SECONDS }); + cy.get(`[data-test-subj="alertsDashboardFlyout_header_${QUERY_TRIGGER}"]`).contains( + `Alerts by ${QUERY_TRIGGER}`, + { timeout: TWENTY_SECONDS } + ); // Confirm 'Trigger name' sections renders as expected. cy.get(`[data-test-subj="alertsDashboardFlyout_triggerName_${QUERY_TRIGGER}"]`).as( diff --git a/cypress/integration/bucket_level_monitor_spec.js b/cypress/integration/bucket_level_monitor_spec.js index ed7440d96..0c1d507c1 100644 --- a/cypress/integration/bucket_level_monitor_spec.js +++ b/cypress/integration/bucket_level_monitor_spec.js @@ -225,23 +225,28 @@ describe('Bucket-Level Monitors', () => { cy.get('[data-test-subj="addMetricButton"]').click({ force: true }); cy.get('[data-test-subj="metrics.0.aggregationTypeSelect"]').select('count', { force: true }); + cy.wait(1000); - cy.get('[data-test-subj="metrics.0.ofFieldComboBox"]').type( - `${COUNT_METRIC_FIELD}{downArrow}{enter}` - ); + cy.get('[data-test-subj="metrics.0.ofFieldComboBox"] input') + .focus() + .type(`${COUNT_METRIC_FIELD}{downArrow}{enter}`); cy.get('button').contains('Save').click({ force: true }); + cy.wait(1000); // Add a second metric for the query cy.get('[data-test-subj="addMetricButton"]').click({ force: true }); cy.get('[data-test-subj="metrics.1.aggregationTypeSelect"]').select('avg', { force: true }); + cy.wait(1000); - cy.get('[data-test-subj="metrics.1.ofFieldComboBox"]').type( - `${AVERAGE_METRIC_FIELD}{downArrow}{enter}` - ); + cy.get('[data-test-subj="metrics.1.ofFieldComboBox"] input') + .focus() + .type(`${AVERAGE_METRIC_FIELD}{downArrow}{enter}`); + cy.wait(1000); cy.get('button').contains('Save').click({ force: true }); + cy.wait(1000); // Add data filters for the query const filters = [ diff --git a/cypress/integration/monitors_dashboard_spec.js b/cypress/integration/monitors_dashboard_spec.js index 44da4480d..ea7f4b183 100644 --- a/cypress/integration/monitors_dashboard_spec.js +++ b/cypress/integration/monitors_dashboard_spec.js @@ -34,7 +34,7 @@ const clusterHealthMonitor = { severity: '1', condition: { script: { - source: 'ctx.results[0].status != "green"', + source: 'ctx.results[0].status != "blue"', lang: 'painless', }, }, @@ -101,6 +101,11 @@ describe('Monitors dashboard page', () => { }); it('Displays expected number of alerts', () => { + // Wait for table to finish loading + cy.get('tbody > tr', { timeout: 20000 }).should(($tr) => + expect($tr).to.have.length.greaterThan(1) + ); + // Ensure the 'Monitor name' column is sorted in ascending order by sorting another column first cy.contains('Last notification time').click({ force: true }); cy.contains('Monitor name').click({ force: true }); diff --git a/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap b/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap index 530cf4593..4a180d30c 100644 --- a/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap +++ b/public/pages/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap @@ -198,8 +198,8 @@ Object { "aggregations": Array [], "bucketUnitOfTime": "h", "bucketValue": 1, - "cleanedGroupBy": Array [], "filters": Array [], + "groupBy": Array [], "searchType": "graph", "timeField": "", }, @@ -322,8 +322,8 @@ Object { "aggregations": Array [], "bucketUnitOfTime": "h", "bucketValue": 1, - "cleanedGroupBy": Array [], "filters": Array [], + "groupBy": Array [], "searchType": "graph", "timeField": "@timestamp", } @@ -334,8 +334,8 @@ Object { "aggregations": Array [], "bucketUnitOfTime": "h", "bucketValue": 1, - "cleanedGroupBy": Array [], "filters": Array [], + "groupBy": Array [], "searchType": "graph", "timeField": "@timestamp", } @@ -346,8 +346,8 @@ Object { "aggregations": Array [], "bucketUnitOfTime": "h", "bucketValue": 1, - "cleanedGroupBy": Array [], "filters": Array [], + "groupBy": Array [], "searchType": "graph", "timeField": "@timestamp", } diff --git a/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js b/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js index bb9d81a33..db7c6aedd 100644 --- a/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js +++ b/public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js @@ -196,7 +196,7 @@ export function formikToUiSearch(values) { searchType, timeField, aggregations, - cleanedGroupBy, + groupBy: cleanedGroupBy, bucketValue, bucketUnitOfTime, filters, diff --git a/public/pages/CreateTrigger/containers/ConfigureActions/ConfigureActions.js b/public/pages/CreateTrigger/containers/ConfigureActions/ConfigureActions.js index 3aaad7fac..8b2cc9701 100644 --- a/public/pages/CreateTrigger/containers/ConfigureActions/ConfigureActions.js +++ b/public/pages/CreateTrigger/containers/ConfigureActions/ConfigureActions.js @@ -5,7 +5,7 @@ import React from 'react'; import _ from 'lodash'; -import { EuiPanel, EuiText, EuiSpacer, EuiLoadingSpinner } from '@elastic/eui'; +import { EuiPanel, EuiText, EuiSpacer } from '@elastic/eui'; import Action from '../../components/Action'; import ActionEmptyPrompt from '../../components/ActionEmptyPrompt'; import AddActionButton from '../../components/AddActionButton'; @@ -22,12 +22,27 @@ import { formikToTrigger } from '../CreateTrigger/utils/formikToTrigger'; import { getChannelOptions, toChannelType } from '../../utils/helper'; import { getInitialActionValues } from '../../components/AddActionButton/utils'; -const createActionContext = (context, action) => ({ - ctx: { - ...context, - action, - }, -}); +const createActionContext = (context, action) => { + let trigger = context.trigger; + const triggerType = Object.keys(trigger)[0]; + if ( + Object.keys(trigger).length === 1 && + !_.isEmpty(triggerType) && + Object.values(TRIGGER_TYPE).includes(triggerType) + ) { + // If the trigger values is wrapped in the trigger type, unwrap it + trigger = trigger[triggerType]; + } else { + console.warn(`Unknown trigger type "${triggerType}".`, context); + } + return { + ctx: { + ...context, + trigger: { ...trigger }, + action, + }, + }; +}; export const checkForError = (response, error) => { for (const trigger_name in response.resp.trigger_results) { diff --git a/public/pages/Dashboard/utils/helpers.js b/public/pages/Dashboard/utils/helpers.js index 643ee3a41..292e757a8 100644 --- a/public/pages/Dashboard/utils/helpers.js +++ b/public/pages/Dashboard/utils/helpers.js @@ -75,9 +75,9 @@ export const renderEmptyValue = (value) => { export function insertGroupByColumn(groupBy = []) { let result = _.cloneDeep(bucketColumns); groupBy.map((fieldName) => - result.splice(0, 0, { + result.push({ field: `agg_alert_content.bucket.key.${fieldName}`, - name: fieldName, + name: _.capitalize(fieldName), render: renderEmptyValue, sortable: false, truncateText: false, diff --git a/public/pages/Dashboard/utils/helpers.test.js b/public/pages/Dashboard/utils/helpers.test.js index 08bdc2d7b..50e69081a 100644 --- a/public/pages/Dashboard/utils/helpers.test.js +++ b/public/pages/Dashboard/utils/helpers.test.js @@ -575,24 +575,24 @@ describe('Dashboard/utils/helpers', () => { test('with valid groupBy list', () => { const groupBy = ['keyword1', 'keyword2', 'keyword3']; const expectedOutput = _.cloneDeep(bucketColumns); - expectedOutput.unshift( + expectedOutput.push( { - field: 'agg_alert_content.bucket.key.keyword3', - name: 'keyword3', + field: 'agg_alert_content.bucket.key.keyword1', + name: 'Keyword1', render: renderEmptyValue, sortable: false, truncateText: false, }, { field: 'agg_alert_content.bucket.key.keyword2', - name: 'keyword2', + name: 'Keyword2', render: renderEmptyValue, sortable: false, truncateText: false, }, { - field: 'agg_alert_content.bucket.key.keyword1', - name: 'keyword1', + field: 'agg_alert_content.bucket.key.keyword3', + name: 'Keyword3', render: renderEmptyValue, sortable: false, truncateText: false,