Bugfix/interval field can be empty (opensearch-project#379)

jovancvetkovic3006 · amsiglan · commit e2e0db42f541 · 2023-01-30T18:22:35.000-08:00
* [FEATURE] Detector must have at least one alert set opensearch-project#288 Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * [BUG] Create detector | Interval field can be empty opensearch-project#378 Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * fix PR failed tests Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * fix PR failed tests Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * unit tests Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * unit tests Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * testing github-action v5 Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * testing github-action v5 Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> * testing github-action v5 Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com> --------- Signed-off-by: Jovan Cvetkovic <jovanca.cvetkovic@gmail.com>
diff --git a/.github/workflows/cypress-workflow.yml b/.github/workflows/cypress-workflow.yml
@@ -111,10 +111,12 @@ jobs:
         with:
           path: ${{ matrix.cypress_cache_folder }}
           key: cypress-cache-v2-${{ runner.os }}-${{ hashFiles('**/package.json') }}
+          restore-keys: |
+            cypress-cache-v2-${{ runner.os }}-${{ hashFiles('**/package.json') }}
 
       # for now just chrome, use matrix to do all browsers later
       - name: Cypress tests
-        uses: cypress-io/github-action@v2
+        uses: cypress-io/github-action@v5
         with:
           working-directory: OpenSearch-Dashboards/plugins/security-analytics-dashboards-plugin
           command: yarn run cypress run
diff --git a/cypress.json b/cypress.json
@@ -1,7 +1,9 @@
 {
   "viewportHeight": 900,
   "viewportWidth": 1440,
-  "defaultCommandTimeout": 20000,
+  "defaultCommandTimeout": 30000,
+  "requestTimeout": 300000,
+  "responseTimeout": 300000,
   "baseUrl": "http://localhost:5601",
   "env": {
     "opensearch_url": "localhost:9200",
diff --git a/cypress/integration/1_detectors.spec.js b/cypress/integration/1_detectors.spec.js
@@ -38,24 +38,29 @@ describe('Detectors', () => {
   before(() => {
     cy.cleanUpTests();
     // Create test index
-    cy.createIndex(indexName, sample_index_settings);
-    cy.request('POST', '_plugins/_security_analytics/rules/_search?prePackaged=true', {
-      from: 0,
-      size: 5000,
-      query: {
-        nested: {
-          path: 'rule',
-          query: { bool: { must: [{ match: { 'rule.category': 'windows' } }] } },
-        },
-      },
-    });
+    cy.createIndex(indexName, sample_index_settings).then(() =>
+      cy
+        .request('POST', '_plugins/_security_analytics/rules/_search?prePackaged=true', {
+          from: 0,
+          size: 5000,
+          query: {
+            nested: {
+              path: 'rule',
+              query: { bool: { must: [{ match: { 'rule.category': 'windows' } }] } },
+            },
+          },
+        })
+        .should('have.property', 'status', 200)
+    );
 
     cy.contains(detectorName).should('not.exist');
   });
 
   beforeEach(() => {
+    cy.intercept('/detectors/_search').as('detectorsSearch');
     // Visit Detectors page
     cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/detectors`);
+    cy.wait('@detectorsSearch').should('have.property', 'state', 'Complete');
 
     // Check that correct page is showing
     cy.waitForPageLoad('detectors', {
diff --git a/cypress/integration/2_rules.spec.js b/cypress/integration/2_rules.spec.js
@@ -131,8 +131,10 @@ const checkRulesFlyout = () => {
 describe('Rules', () => {
   before(() => cy.cleanUpTests());
   beforeEach(() => {
+    cy.intercept('/rules/_search').as('rulesSearch');
     // Visit Rules page
     cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/rules`);
+    cy.wait('@rulesSearch').should('have.property', 'state', 'Complete');
 
     // Check that correct page is showing
     cy.waitForPageLoad('rules', {
@@ -194,12 +196,12 @@ describe('Rules', () => {
       force: true,
     });
 
+    cy.wait('@getRules');
+
     cy.waitForPageLoad('rules', {
       contains: 'Rules',
     });
 
-    cy.wait('@getRules');
-
     checkRulesFlyout();
   });
 
diff --git a/cypress/integration/3_alerts.spec.js b/cypress/integration/3_alerts.spec.js
@@ -84,7 +84,10 @@ describe('Alerts', () => {
 
   beforeEach(() => {
     // Visit Alerts table page
+    cy.intercept('/detectors/_search').as('detectorsSearch');
+    // Visit Detectors page
     cy.visit(`${OPENSEARCH_DASHBOARDS_URL}/alerts`);
+    cy.wait('@detectorsSearch').should('have.property', 'state', 'Complete');
 
     // Wait for page to load
     cy.waitForPageLoad('alerts', {
diff --git a/cypress/support/detectors.js b/cypress/support/detectors.js
@@ -68,5 +68,8 @@ Cypress.Commands.add('deleteAllDetectors', () => {
     method: 'DELETE',
     url: `${Cypress.env('opensearch')}/.opensearch-sap-detectors-config`,
     failOnStatusCode: false,
+  }).as('deleteAllDetectors');
+  cy.get('@deleteAllDetectors').should((response) => {
+    expect(response.status).to.be.oneOf([200, 404]);
   });
 });
diff --git a/cypress/support/indexes.js b/cypress/support/indexes.js
@@ -6,7 +6,11 @@
 const { NODE_API } = require('./constants');
 
 Cypress.Commands.add('createIndex', (index, settings = {}) => {
-  cy.request('PUT', `${Cypress.env('opensearch')}/${index}`, settings);
+  cy.request('PUT', `${Cypress.env('opensearch')}/${index}`, settings).should(
+    'have.property',
+    'status',
+    200
+  );
 });
 
 Cypress.Commands.add('createIndexTemplate', (name, template) => {
@@ -43,5 +47,9 @@ Cypress.Commands.add('deleteAllIndices', () => {
     method: 'DELETE',
     url: `${Cypress.env('opensearch')}/index*,sample*,opensearch_dashboards*,test*,cypress*`,
     failOnStatusCode: false,
+  }).as('deleteAllIndices');
+  cy.get('@deleteAllIndices').should((response) => {
+    // Both statuses are a pass, 200 means deleted successfully and 404 there was no index to delete
+    expect(response.status).to.be.oneOf([200, 404]);
   });
 });
diff --git a/cypress/support/rules.js b/cypress/support/rules.js
@@ -51,10 +51,14 @@ Cypress.Commands.add('deleteRule', (ruleName) => {
 });
 
 Cypress.Commands.add('deleteAllCustomRules', () => {
+  const url = `${Cypress.env('opensearch')}/.opensearch-sap-custom-rules-config`;
   cy.request({
     method: 'DELETE',
-    url: `${Cypress.env('opensearch')}/.opensearch-sap-custom-rules-config`,
+    url: url,
     failOnStatusCode: false,
     body: { query: { match_all: {} } },
+  }).as('deleteAllCustomRules');
+  cy.get('@deleteAllCustomRules').should((response) => {
+    expect(response.status).to.be.oneOf([200, 404]);
   });
 });
diff --git a/cypress/support/typings.js b/cypress/support/typings.js
@@ -9,7 +9,7 @@ Cypress.Commands.add(
     prevSubject: true,
   },
   (subject, text) => {
-    return cy.get(subject).ospType(text).realPress('Enter');
+    return cy.get(subject).clear().ospType(text).realPress('Enter');
   }
 );
 
diff --git a/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx b/public/pages/CreateDetector/components/DefineDetector/components/DetectorSchedule/Interval.tsx
@@ -20,14 +20,25 @@ export interface IntervalProps {
   onDetectorScheduleChange(schedule: PeriodSchedule): void;
 }
 
+export interface IntervalState {
+  isIntervalValid: boolean;
+}
+
 const unitOptions: EuiSelectOption[] = [
   { value: 'MINUTES', text: 'Minutes' },
   { value: 'HOURS', text: 'Hours' },
   { value: 'DAYS', text: 'Days' },
 ];
 
-export class Interval extends React.Component<IntervalProps> {
+export class Interval extends React.Component<IntervalProps, IntervalState> {
+  state = {
+    isIntervalValid: true,
+  };
+
   onTimeIntervalChange = (event: React.ChangeEvent<HTMLInputElement>) => {
+    this.setState({
+      isIntervalValid: !!event.target.value,
+    });
     this.props.onDetectorScheduleChange({
       period: {
         ...this.props.detector.schedule.period,
@@ -46,9 +57,14 @@ export class Interval extends React.Component<IntervalProps> {
   };
 
   render() {
+    const { isIntervalValid } = this.state;
     const { period } = this.props.detector.schedule;
     return (
-      <EuiFormRow label={<FormFieldHeader headerTitle={'Run every'} />}>
+      <EuiFormRow
+        label={<FormFieldHeader headerTitle={'Run every'} />}
+        isInvalid={!isIntervalValid}
+        error={'Enter schedule interval.'}
+      >
         <EuiFlexGroup>
           <EuiFlexItem>
             <EuiFieldNumber
@@ -57,6 +73,8 @@ export class Interval extends React.Component<IntervalProps> {
               value={period.interval}
               onChange={this.onTimeIntervalChange}
               data-test-subj={'detector-schedule-number-select'}
+              required={true}
+              isInvalid={!isIntervalValid}
             />
           </EuiFlexItem>
           <EuiFlexItem>
diff --git a/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx b/public/pages/CreateDetector/components/DefineDetector/containers/DefineDetector.tsx
@@ -43,7 +43,8 @@ export default class DefineDetector extends Component<DefineDetectorProps, Defin
     const isDataValid =
       !!detector.name &&
       !!detector.detector_type &&
-      detector.inputs[0].detector_input.indices.length >= MIN_NUM_DATA_SOURCES;
+      detector.inputs[0].detector_input.indices.length >= MIN_NUM_DATA_SOURCES &&
+      !!detector.schedule.period.interval;
     this.props.changeDetector(detector);
     this.props.updateDataValidState(DetectorCreationStep.DEFINE_DETECTOR, isDataValid);
   }

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ Cypress.Commands.add(`
`9`	`9`	`prevSubject: true,`
`10`	`10`	`},`
`11`	`11`	`(subject, text) => {`
`12`		`- return cy.get(subject).ospType(text).realPress('Enter');`
	`12`	`+ return cy.get(subject).clear().ospType(text).realPress('Enter');`
`13`	`13`	`}`
`14`	`14`	`);`
`15`	`15`