Fixed bucket monitor groupBy/aggregation display bug. (#827) (#831)

* Fixed a bug that was causing groupBy/aggregation fields from displaying in various areas of the UI. Related issues: 816, 817, 818. * Fixed trigger context object bug in issue 791. * Capitalized bucket column titles, and moved bucket columns to the end of the column array. * Added wait steps to reduce test flakiness. * Added wait step to reduce test flakiness. Adjusted test monitor trigger condition to always triggers on a healthy clusters. * Removed unused imports. * fixed bucket level monitor flaky cypress test --------- (cherry picked from commit 96dbe49) Signed-off-by: AWSHurneyt <hurneyt@amazon.com> Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Amardeepsingh Siglani <amardeep7194@gmail.com>
opensearch-project · Nov 22, 2023 · c0691e0 · c0691e0
1 parent 34e68b3
commit c0691e0
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 34 deletions.
diff --git 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
@@ -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
@@ -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/...s/CreateMonitor/containers/CreateMonitor/utils/__snapshots__/formikToMonitor.test.js.snap b/...s/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
@@ -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
@@ -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
@@ -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
@@ -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,