fixed data source logic on page load for view/edit monitor; removed r…

…edundant code; updated tests Signed-off-by: Amardeepsingh Siglani <[email protected]>
opensearch-project · Aug 20, 2024 · fa07551 · fa07551
1 parent b64cc51
commit fa07551
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 56 deletions.
diff --git a/cypress/integration/composite_level_monitor_spec.js b/cypress/integration/composite_level_monitor_spec.js
@@ -75,7 +75,7 @@ describe('CompositeLevelMonitor', () => {
         .type('{backspace}')
         .type('Composite trigger');
 
-      cy.intercept('api/alerting/workflows').as('createMonitorRequest');
+      cy.intercept('api/alerting/workflows?*').as('createMonitorRequest');
       cy.intercept(`api/alerting/monitors?*`).as('getMonitorsRequest');
       cy.get('button').contains('Create').click({ force: true });
 
@@ -116,7 +116,7 @@ describe('CompositeLevelMonitor', () => {
             cy.visit(
               `${Cypress.env('opensearch_dashboards')}/app/${PLUGIN_NAME}#/monitors/${
                 createdMonitor._id
-              }?action=update-monitor&type=workflow`
+              }?action=update-monitor&type=workflow&dataSourceId=`
             );
           } else {
             cy.log('Failed to get created monitor ', SAMPLE_VISUAL_EDITOR_MONITOR);

diff --git a/public/pages/Dashboard/containers/Dashboard.js b/public/pages/Dashboard/containers/Dashboard.js
@@ -131,6 +131,7 @@ export default class Dashboard extends Component {
 
   getAlerts = _.debounce(
     (from, size, search, sortField, sortDirection, severityLevel, alertState, monitorIds) => {
+      const dataSourceId = getDataSourceId();
       const params = {
         from,
         size,
@@ -141,13 +142,12 @@ export default class Dashboard extends Component {
         alertState,
         monitorIds,
         monitorType: this.props.monitorType,
+        dataSourceId,
       };
 
       const queryParamsString = queryString.stringify(params);
-      location.search;
       const { httpClient, history, notifications, perAlertView } = this.props;
       history.replace({ ...this.props.location, search: queryParamsString });
-      const dataSourceId = getDataSourceId();
       const extendedParams = {
         ...(dataSourceId !== undefined && { dataSourceId }), // Only include dataSourceId if it exists
         ...params, // Other parameters

diff --git a/public/pages/Main/Main.js b/public/pages/Main/Main.js
@@ -22,8 +22,8 @@ import {
 } from '../../../public/services';
 import { MultiDataSourceContext } from '../../../public/utils/MultiDataSourceContext';
 import { parseQueryStringAndGetDataSource } from '../utils/helpers';
-import * as pluginManifest from "../../../opensearch_dashboards.json";
-import semver from "semver";
+import * as pluginManifest from '../../../opensearch_dashboards.json';
+import semver from 'semver';
 
 class Main extends Component {
   static contextType = CoreContext;
@@ -53,8 +53,10 @@ class Main extends Component {
   async updateBreadcrumbs() {
     if (this.props.dataSourceEnabled && this.props.location) {
       const search = this.props.location?.search;
-      const dataSourceId = search ? parseQueryStringAndGetDataSource(search) : parseQueryStringAndGetDataSource(this.props.location?.pathname);
-      if (dataSourceId) {
+      const dataSourceId = parseQueryStringAndGetDataSource(
+        search || this.props.location?.pathname
+      );
+      if (dataSourceId !== undefined) {
         setDataSource({ dataSourceId });
         this.setState({
           selectedDataSourceId: dataSourceId,
@@ -103,11 +105,13 @@ class Main extends Component {
   };
 
   dataSourceFilterFn = (dataSource) => {
-    const dataSourceVersion = dataSource?.attributes?.dataSourceVersion || "";
+    const dataSourceVersion = dataSource?.attributes?.dataSourceVersion || '';
     const installedPlugins = dataSource?.attributes?.installedPlugins || [];
     return (
       semver.satisfies(dataSourceVersion, pluginManifest.supportedOSDataSourceVersions) &&
-      pluginManifest.requiredOSDataSourcePlugins.every((plugin) => installedPlugins.includes(plugin))
+      pluginManifest.requiredOSDataSourcePlugins.every((plugin) =>
+        installedPlugins.includes(plugin)
+      )
     );
   };
 
@@ -236,7 +240,11 @@ class Main extends Component {
                                   setFlyout={this.setFlyout}
                                   notifications={core.notifications}
                                   landingDataSourceId={this.state.selectedDataSourceId}
-                                  defaultRoute={core.chrome?.navGroup?.getNavGroupEnabled() ? this.props.defaultRoute : undefined}
+                                  defaultRoute={
+                                    core.chrome?.navGroup?.getNavGroupEnabled()
+                                      ? this.props.defaultRoute
+                                      : undefined
+                                  }
                                 />
                               )}
                             />

diff --git a/public/pages/MonitorDetails/containers/MonitorDetails.js b/public/pages/MonitorDetails/containers/MonitorDetails.js
@@ -50,7 +50,7 @@ import FindingsDashboard from '../../Dashboard/containers/FindingsDashboard';
 import { TABLE_TAB_IDS } from '../../Dashboard/components/FindingsDashboard/findingsUtils';
 import { DeleteMonitorModal } from '../../../components/DeleteModal/DeleteMonitorModal';
 import { getLocalClusterName } from '../../CreateMonitor/components/CrossClusterConfigurations/utils/helpers';
-import { getDataSourceQueryObj } from '../../utils/helpers';
+import { getDataSourceQueryObj, parseQueryStringAndGetDataSource } from '../../utils/helpers';
 import { MultiDataSourceContext } from '../../../../public/utils/MultiDataSourceContext';
 import { getUseUpdatedUx, setDataSource } from '../../../services';
 import { PageHeader } from '../../../components/PageHeader/PageHeader';
@@ -72,9 +72,13 @@ export default class MonitorDetails extends Component {
       triggerToEdit: null,
       delegateMonitors: [],
       editMonitor: () => {
+        const dataSourceId = parseQueryStringAndGetDataSource(this.props.location?.search);
+        const monitorType = this.state.monitor.monitor_type;
         this.props.history.push({
           ...this.props.location,
-          search: `?action=${MONITOR_ACTIONS.UPDATE_MONITOR}`,
+          search: `?action=${MONITOR_ACTIONS.UPDATE_MONITOR}&monitorType=${monitorType}${
+            dataSourceId !== undefined ? `&dataSourceId=${dataSourceId}` : ''
+          }`,
         });
       },
       isJsonModalOpen: false,
@@ -275,26 +279,6 @@ export default class MonitorDetails extends Component {
       });
   };
 
-  onCreateTrigger = () => {
-    this.props.history.push({
-      ...this.props.location,
-      search: `?action=${TRIGGER_ACTIONS.CREATE_TRIGGER}`,
-    });
-  };
-
-  onCloseTrigger = () => {
-    this.props.history.push({ ...this.props.location, search: '' });
-    this.setState({ triggerToEdit: null });
-  };
-
-  onEditTrigger = (trigger) => {
-    this.setState({ triggerToEdit: trigger });
-    this.props.history.push({
-      ...this.props.location,
-      search: `?action=${TRIGGER_ACTIONS.UPDATE_TRIGGER}`,
-    });
-  };
-
   renderNoTriggersCallOut = () => {
     const { monitor, editMonitor } = this.state;
     const callout = (
@@ -577,8 +561,6 @@ export default class MonitorDetails extends Component {
           httpClient={httpClient}
           delegateMonitors={delegateMonitors}
           updateMonitor={this.updateMonitor}
-          onEditTrigger={this.onEditTrigger}
-          onCreateTrigger={this.onCreateTrigger}
         />
         <div className="eui-hideFor--xs eui-hideFor--s eui-hideFor--m">
           <EuiSpacer />

diff --git a/public/pages/MonitorDetails/containers/Triggers/Triggers.js b/public/pages/MonitorDetails/containers/Triggers/Triggers.js
@@ -78,7 +78,6 @@ export default class Triggers extends Component {
     };
 
     this.onDelete = this.onDelete.bind(this);
-    this.onEdit = this.onEdit.bind(this);
     this.onSelectionChange = this.onSelectionChange.bind(this);
     this.onTableChange = this.onTableChange.bind(this);
     this.monitorsById = {};
@@ -151,11 +150,6 @@ export default class Triggers extends Component {
     updateMonitor({ triggers: updatedTriggers });
   }
 
-  onEdit() {
-    const { monitor } = this.props;
-    this.props.onEditTrigger(monitor.triggers);
-  }
-
   onSelectionChange(selectedItems) {
     this.setState({ selectedItems });
   }
@@ -230,6 +224,4 @@ Triggers.propTypes = {
   monitor: PropTypes.object.isRequired,
   httpClient: PropTypes.object.isRequired,
   updateMonitor: PropTypes.func.isRequired,
-  onEditTrigger: PropTypes.func.isRequired,
-  onCreateTrigger: PropTypes.func.isRequired,
 };
diff --git a/public/pages/MonitorDetails/containers/Triggers/Triggers.test.js b/public/pages/MonitorDetails/containers/Triggers/Triggers.test.js
@@ -16,8 +16,6 @@ const props = {
   },
   delegateMonitors: [],
   updateMonitor: jest.fn(),
-  onEditTrigger: jest.fn(),
-  onCreateTrigger: jest.fn(),
 };
 
 jest.mock('uuid/v4', () => {
@@ -50,14 +48,6 @@ describe('Triggers', () => {
     expect(tableKey).not.toBe(diffTableKey);
   });
 
-  test('onEdit calls onEditTrigger', () => {
-    const onEdit = jest.spyOn(Triggers.prototype, 'onEdit');
-    const wrapper = getShallowWrapper();
-    wrapper.instance().onEdit();
-    expect(onEdit).toHaveBeenCalled();
-    expect(props.onEditTrigger).toHaveBeenCalled();
-  });
-
   test('onDelete calls updateMonitor with triggers to keep', () => {
     const onDelete = jest.spyOn(Triggers.prototype, 'onDelete');
     const monitor = {

diff --git a/public/pages/Monitors/containers/Monitors/Monitors.js b/public/pages/Monitors/containers/Monitors/Monitors.js
@@ -144,11 +144,11 @@ export default class Monitors extends Component {
   async getMonitors(from, size, search, sortField, sortDirection, state) {
     this.setState({ loadingMonitors: true });
     try {
-      const params = { from, size, search, sortField, sortDirection, state };
+      const dataSourceId = getDataSourceId();
+      const params = { from, size, search, sortField, sortDirection, state, dataSourceId };
       const queryParamsString = queryString.stringify(params);
       const { httpClient, history } = this.props;
       history.replace({ ...this.props.location, search: queryParamsString });
-      const dataSourceId = getDataSourceId();
       const extendedParams = {
         ...(dataSourceId !== undefined && { dataSourceId }), // Only include dataSourceId if it exists
         ...params, // Other parameters

diff --git a/public/pages/utils/helpers.js b/public/pages/utils/helpers.js
@@ -42,7 +42,7 @@ export function parseQueryStringAndGetDataSource(queryString) {
     const pair = param.split('=');
     params[pair[0]] = pair[1];
   }
-  return params['dataSourceId'];
+  return params.hasOwnProperty('dataSourceId') ? params['dataSourceId'] || '' : undefined;
 }
 
 export function constructUrlFromDataSource(url) {