[Security Solution][Case][Bug] Removing empty collections when filtering on status (#92048) (#93096)

jonathan-buttner · kibanamachine · web-flow · commit cfb9c83b41d7 · 2021-03-01T17:01:56.000-05:00
* Removing empty collections when not filtering on status

* Fixing add comment response

Co-authored-by: Kibana Machine &lt;42973632+kibanamachine@users.noreply.github.com&gt;

Co-authored-by: Kibana Machine &lt;42973632+kibanamachine@users.noreply.github.com&gt;
diff --git a/x-pack/plugins/case/server/services/index.ts b/x-pack/plugins/case/server/services/index.ts
@@ -32,6 +32,7 @@ import {
   CaseType,
   CaseResponse,
   caseTypeField,
+  CasesFindRequest,
 } from '../../common/api';
 import { combineFilters, defaultSortField, groupTotalAlertsByID } from '../common';
 import { defaultPage, defaultPerPage } from '../routes/api';
@@ -194,6 +195,8 @@ interface CasesMapWithPageInfo {
   perPage: number;
 }
 
+type FindCaseOptions = CasesFindRequest & SavedObjectFindOptions;
+
 export interface CaseServiceSetup {
   deleteCase(args: GetCaseArgs): Promise<{}>;
   deleteComment(args: GetCommentArgs): Promise<{}>;
@@ -271,7 +274,7 @@ export class CaseService implements CaseServiceSetup {
     subCaseOptions,
   }: {
     client: SavedObjectsClientContract;
-    caseOptions: SavedObjectFindOptions;
+    caseOptions: FindCaseOptions;
     subCaseOptions?: SavedObjectFindOptions;
   }): Promise<CasesMapWithPageInfo> {
     const cases = await this.findCases({
@@ -291,10 +294,20 @@ export class CaseService implements CaseServiceSetup {
       const subCasesForCase = subCasesResp.subCasesMap.get(caseInfo.id);
 
       /**
-       * This will include empty collections unless the query explicitly requested type === CaseType.individual, in which
-       * case we'd not have any collections anyway.
+       * If this case is an individual add it to the return map
+       * If it is a collection and it has sub cases add it to the return map
+       * If it is a collection and it does not have sub cases, check and see if we're filtering on a status,
+       *  if we're filtering on a status then exclude the empty collection from the results
+       *  if we're not filtering on a status then include the empty collection (that way we can display all the collections
+       *  when the UI isn't doing any filtering)
        */
-      accMap.set(caseInfo.id, { case: caseInfo, subCases: subCasesForCase });
+      if (
+        caseInfo.attributes.type === CaseType.individual ||
+        subCasesForCase !== undefined ||
+        !caseOptions.status
+      ) {
+        accMap.set(caseInfo.id, { case: caseInfo, subCases: subCasesForCase });
+      }
       return accMap;
     }, new Map<string, Collection>());
 
diff --git a/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts b/x-pack/test/case_api_integration/basic/tests/cases/find_cases.ts
@@ -304,7 +304,9 @@ export default ({ getService }: FtrProviderContext): void => {
           .get(`${CASES_URL}/_find?sortOrder=asc&status=open`)
           .expect(200);
 
-        expect(body.total).to.eql(2);
+        // since we're filtering on status and the collection only has an in-progress case, it should only return the
+        // individual case that has the open status and no collections
+        expect(body.total).to.eql(1);
         expect(body.count_closed_cases).to.eql(1);
         expect(body.count_open_cases).to.eql(1);
         expect(body.count_in_progress_cases).to.eql(1);
@@ -353,7 +355,7 @@ export default ({ getService }: FtrProviderContext): void => {
         expect(body.count_in_progress_cases).to.eql(0);
       });
 
-      it('correctly counts stats including a collection without sub cases', async () => {
+      it('correctly counts stats including a collection without sub cases when not filtering on status', async () => {
         // delete the sub case on the collection so that it doesn't have any sub cases
         await supertest
           .delete(`${SUB_CASES_PATCH_DEL_URL}?ids=["${collection.newSubCaseInfo.subCases![0].id}"]`)
@@ -365,11 +367,62 @@ export default ({ getService }: FtrProviderContext): void => {
           .get(`${CASES_URL}/_find?sortOrder=asc`)
           .expect(200);
 
+        // it should include the collection without sub cases because we did not pass in a filter on status
+        expect(body.total).to.eql(3);
+        expect(body.count_closed_cases).to.eql(1);
+        expect(body.count_open_cases).to.eql(1);
+        expect(body.count_in_progress_cases).to.eql(0);
+      });
+
+      it('correctly counts stats including a collection without sub cases when filtering on tags', async () => {
+        // delete the sub case on the collection so that it doesn't have any sub cases
+        await supertest
+          .delete(`${SUB_CASES_PATCH_DEL_URL}?ids=["${collection.newSubCaseInfo.subCases![0].id}"]`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(204);
+
+        const { body }: { body: CasesFindResponse } = await supertest
+          .get(`${CASES_URL}/_find?sortOrder=asc&tags=defacement`)
+          .expect(200);
+
+        // it should include the collection without sub cases because we did not pass in a filter on status
         expect(body.total).to.eql(3);
         expect(body.count_closed_cases).to.eql(1);
         expect(body.count_open_cases).to.eql(1);
         expect(body.count_in_progress_cases).to.eql(0);
       });
+
+      it('does not return collections without sub cases matching the requested status', async () => {
+        const { body }: { body: CasesFindResponse } = await supertest
+          .get(`${CASES_URL}/_find?sortOrder=asc&status=closed`)
+          .expect(200);
+
+        // it should not include the collection that has a sub case as in-progress
+        expect(body.total).to.eql(1);
+        expect(body.count_closed_cases).to.eql(1);
+        expect(body.count_open_cases).to.eql(1);
+        expect(body.count_in_progress_cases).to.eql(1);
+      });
+
+      it('does not return empty collections when filtering on status', async () => {
+        // delete the sub case on the collection so that it doesn't have any sub cases
+        await supertest
+          .delete(`${SUB_CASES_PATCH_DEL_URL}?ids=["${collection.newSubCaseInfo.subCases![0].id}"]`)
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(204);
+
+        const { body }: { body: CasesFindResponse } = await supertest
+          .get(`${CASES_URL}/_find?sortOrder=asc&status=closed`)
+          .expect(200);
+
+        // it should not include the collection that has a sub case as in-progress
+        expect(body.total).to.eql(1);
+        expect(body.count_closed_cases).to.eql(1);
+        expect(body.count_open_cases).to.eql(1);
+        expect(body.count_in_progress_cases).to.eql(0);
+      });
     });
 
     it('unhappy path - 400s when bad query supplied', async () => {
