From cfb9c83b41d700eb2681f82005840db74f41bca4 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com> Date: Mon, 1 Mar 2021 17:01:56 -0500 Subject: [PATCH] [Security Solution][Case][Bug] Removing empty collections when filtering on status (#92048) (#93096) * Removing empty collections when not filtering on status * Fixing add comment response Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- x-pack/plugins/case/server/services/index.ts | 21 +++++-- .../basic/tests/cases/find_cases.ts | 57 ++++++++++++++++++- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/case/server/services/index.ts b/x-pack/plugins/case/server/services/index.ts index f74e91ca10224..11ceb48d11e9f 100644 --- 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 { 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()); 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 index 7514044d376ca..6791c9d1c9e71 100644 --- 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 () => {