[8.7] [ResponseOps] es query rule params not compatible between 8.6 a…

…nd 8.7 (#157710) (#157874) # Backport This will backport the following commits from `main` to `8.7`: - [[ResponseOps] es query rule params not compatible between 8.6 and 8.7 (#157710)](#157710)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Alexi Doak <[email protected]>"}},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/157710","number":157710,"mergeCommit":{"message":"[ResponseOps] es query rule params not compatible between 8.6 and 8.7 (#157710)\n\nResolves https://github.com/elastic/kibana/issues/156856\r\n\r\n## Summary\r\n\r\nAdds a default value to `aggType` and `groupBy` fields \r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"8ecd2d67a02f9de2a0e4f8778dcca3781dceaa90"}}]}] BACKPORT-->
elastic · May 16, 2023 · babee76 · babee76
1 parent 89a9926
commit babee76
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 6 deletions.
diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts
@@ -144,6 +144,28 @@ describe('ruleType Params validate()', () => {
     );
   });
 
+  it('uses default value "count" if aggType is undefined', async () => {
+    params.aggType = undefined;
+    expect(onValidate()).not.toThrow();
+  });
+
+  it('fails for invalid groupBy', async () => {
+    params.groupBy = 42;
+    expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
+      `"[groupBy]: expected value of type [string] but got [number]"`
+    );
+
+    params.groupBy = '-not-a-valid-groupBy-';
+    expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
+      `"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""`
+    );
+  });
+
+  it('uses default value "all" if groupBy is undefined', async () => {
+    params.groupBy = undefined;
+    expect(onValidate()).not.toThrow();
+  });
+
   it('fails for invalid aggField', async () => {
     params.aggField = 42;
     expect(onValidate()).toThrowErrorMatchingInlineSnapshot(

diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts
@@ -41,11 +41,11 @@ const EsQueryRuleParamsSchemaProperties = {
   threshold: schema.arrayOf(schema.number(), { minSize: 1, maxSize: 2 }),
   thresholdComparator: getComparatorSchemaType(validateComparator),
   // aggregation type
-  aggType: schema.string({ validate: validateAggType }),
+  aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }),
   // aggregation field
   aggField: schema.maybe(schema.string({ minLength: 1 })),
   // how to group
-  groupBy: schema.string({ validate: validateGroupBy }),
+  groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }),
   // field to group on (for groupBy: top)
   termField: schema.maybe(schema.string({ minLength: 1 })),
   // limit on number of groups returned

diff --git a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts
@@ -109,6 +109,28 @@ export function runTests(schema: ObjectType, defaultTypeParams: Record<string, u
       );
     });
 
+    it('uses default value "count" if aggType is undefined', async () => {
+      params.aggType = undefined;
+      expect(onValidate()).not.toThrow();
+    });
+
+    it('fails for invalid groupBy', async () => {
+      params.groupBy = 42;
+      expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
+        `"[groupBy]: expected value of type [string] but got [number]"`
+      );
+
+      params.groupBy = '-not-a-valid-groupBy-';
+      expect(onValidate()).toThrowErrorMatchingInlineSnapshot(
+        `"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""`
+      );
+    });
+
+    it('uses default value "all" if groupBy is undefined', async () => {
+      params.groupBy = undefined;
+      expect(onValidate()).not.toThrow();
+    });
+
     it('fails for invalid aggField', async () => {
       params.aggField = 42;
       expect(onValidate()).toThrowErrorMatchingInlineSnapshot(

diff --git a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts
@@ -22,11 +22,11 @@ export const CoreQueryParamsSchemaProperties = {
   // field in index used for date/time
   timeField: schema.string({ minLength: 1 }),
   // aggregation type
-  aggType: schema.string({ validate: validateAggType }),
+  aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }),
   // aggregation field
   aggField: schema.maybe(schema.string({ minLength: 1 })),
   // how to group
-  groupBy: schema.string({ validate: validateGroupBy }),
+  groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }),
   // field to group on (for groupBy: top)
   termField: schema.maybe(schema.string({ minLength: 1 })),
   // filter field

diff --git a/...ng_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts b/...ng_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts
@@ -979,6 +979,34 @@ export default function ruleTests({ getService }: FtrProviderContext) {
       });
     });
 
+    describe('aggType and groupBy', () => {
+      it('sets aggType: "count" and groupBy: "all" when they are undefined', async () => {
+        endDate = new Date().toISOString();
+
+        await createEsDocumentsInGroups(ES_GROUPS_TO_WRITE, endDate);
+
+        await createRule({
+          name: 'always fire',
+          esQuery: `{\n  \"query\":{\n    \"match_all\" : {}\n  }\n}`,
+          size: 100,
+          thresholdComparator: '>',
+          threshold: [0],
+          aggType: undefined,
+          groupBy: undefined,
+        });
+
+        const docs = await waitForDocs(2);
+
+        expect(docs[0]._source.hits.length).greaterThan(0);
+        const messagePattern =
+          /rule 'always fire' is active:\n\n- Value: \d+\n- Conditions Met: Number of matching documents is greater than 0 over 20s\n- Timestamp: \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z/;
+        expect(docs[0]._source.params.message).to.match(messagePattern);
+
+        expect(docs[1]._source.hits.length).to.be(0);
+        expect(docs[1]._source.params.message).to.match(/rule 'always fire' is recovered/);
+      });
+    });
+
     async function waitForDocs(count: number): Promise<any[]> {
       return await esTestIndexToolOutput.waitForDocs(
         ES_TEST_INDEX_SOURCE,
@@ -1080,8 +1108,8 @@ export default function ruleTests({ getService }: FtrProviderContext) {
             thresholdComparator: params.thresholdComparator,
             threshold: params.threshold,
             searchType: params.searchType,
-            aggType: params.aggType ?? 'count',
-            groupBy: params.groupBy ?? 'all',
+            aggType: params.aggType,
+            groupBy: params.groupBy,
             aggField: params.aggField,
             termField: params.termField,
             termSize: params.termSize,