mirror of
https://github.com/elastic/kibana.git
synced 2025-04-25 02:09:32 -04:00
[Response Ops][Cases] Cases with empty string assignees throwing error (#209973)
## Summary Closes https://github.com/elastic/kibana/issues/209950 Testing steps in referenced issue ## Release note Fix error message in cases list if case assignee is an empty string
This commit is contained in:
parent
19fa1b828c
commit
0897d08786
11 changed files with 230 additions and 36 deletions
|
@ -90,35 +90,14 @@ const mockKibana = () => {
|
||||||
} as unknown as ReturnType<typeof useKibana>);
|
} as unknown as ReturnType<typeof useKibana>);
|
||||||
};
|
};
|
||||||
|
|
||||||
// FLAKY: https://github.com/elastic/kibana/issues/192739
|
// eslint-disable-next-line prefer-object-spread
|
||||||
describe.skip('AllCasesListGeneric', () => {
|
const originalGetComputedStyle = Object.assign({}, window.getComputedStyle);
|
||||||
const onRowClick = jest.fn();
|
|
||||||
const updateCaseProperty = jest.fn();
|
|
||||||
|
|
||||||
const emptyTag = getEmptyCellValue().props.children;
|
const restoreGetComputedStyle = () => {
|
||||||
|
Object.defineProperty(window, 'getComputedStyle', originalGetComputedStyle);
|
||||||
|
};
|
||||||
|
|
||||||
const defaultGetCases = {
|
const patchGetComputedStyle = () => {
|
||||||
...useGetCasesMockState,
|
|
||||||
};
|
|
||||||
|
|
||||||
const defaultColumnArgs = {
|
|
||||||
filterStatus: [CaseStatuses.open],
|
|
||||||
handleIsLoading: jest.fn(),
|
|
||||||
isLoadingCases: [],
|
|
||||||
isLoadingColumns: false,
|
|
||||||
isSelectorView: false,
|
|
||||||
userProfiles: new Map(),
|
|
||||||
currentUserProfile: undefined,
|
|
||||||
selectedColumns: [],
|
|
||||||
};
|
|
||||||
|
|
||||||
const removeMsFromDate = (value: string) => moment(value).format('YYYY-MM-DDTHH:mm:ss[Z]');
|
|
||||||
// eslint-disable-next-line prefer-object-spread
|
|
||||||
const originalGetComputedStyle = Object.assign({}, window.getComputedStyle);
|
|
||||||
|
|
||||||
let appMockRenderer: AppMockRenderer;
|
|
||||||
|
|
||||||
beforeAll(() => {
|
|
||||||
// The JSDOM implementation is too slow
|
// The JSDOM implementation is too slow
|
||||||
// Especially for dropdowns that try to position themselves
|
// Especially for dropdowns that try to position themselves
|
||||||
// perf issue - https://github.com/jsdom/jsdom/issues/3234
|
// perf issue - https://github.com/jsdom/jsdom/issues/3234
|
||||||
|
@ -147,14 +126,43 @@ describe.skip('AllCasesListGeneric', () => {
|
||||||
configurable: true,
|
configurable: true,
|
||||||
writable: true,
|
writable: true,
|
||||||
});
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
// FLAKY: https://github.com/elastic/kibana/issues/192739
|
||||||
|
describe.skip('AllCasesListGeneric', () => {
|
||||||
|
const onRowClick = jest.fn();
|
||||||
|
const updateCaseProperty = jest.fn();
|
||||||
|
|
||||||
|
const emptyTag = getEmptyCellValue().props.children;
|
||||||
|
|
||||||
|
const defaultGetCases = {
|
||||||
|
...useGetCasesMockState,
|
||||||
|
};
|
||||||
|
|
||||||
|
const defaultColumnArgs = {
|
||||||
|
filterStatus: [CaseStatuses.open],
|
||||||
|
handleIsLoading: jest.fn(),
|
||||||
|
isLoadingCases: [],
|
||||||
|
isLoadingColumns: false,
|
||||||
|
isSelectorView: false,
|
||||||
|
userProfiles: new Map(),
|
||||||
|
currentUserProfile: undefined,
|
||||||
|
selectedColumns: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
const removeMsFromDate = (value: string) => moment(value).format('YYYY-MM-DDTHH:mm:ss[Z]');
|
||||||
|
|
||||||
|
let appMockRenderer: AppMockRenderer;
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
patchGetComputedStyle();
|
||||||
mockKibana();
|
mockKibana();
|
||||||
const actionTypeRegistry = useKibanaMock().services.triggersActionsUi.actionTypeRegistry;
|
const actionTypeRegistry = useKibanaMock().services.triggersActionsUi.actionTypeRegistry;
|
||||||
registerConnectorsToMockActionRegistry(actionTypeRegistry, connectorsMock);
|
registerConnectorsToMockActionRegistry(actionTypeRegistry, connectorsMock);
|
||||||
});
|
});
|
||||||
|
|
||||||
afterAll(() => {
|
afterAll(() => {
|
||||||
Object.defineProperty(window, 'getComputedStyle', originalGetComputedStyle);
|
restoreGetComputedStyle();
|
||||||
});
|
});
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
|
|
|
@ -66,6 +66,15 @@ describe('User profiles API', () => {
|
||||||
expect(res).toEqual(userProfiles);
|
expect(res).toEqual(userProfiles);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should filter out empty user profiles', async () => {
|
||||||
|
const res = await bulkGetUserProfiles({
|
||||||
|
security,
|
||||||
|
uids: [...userProfilesIds, ''],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res).toEqual(userProfiles);
|
||||||
|
});
|
||||||
|
|
||||||
it('calls bulkGet correctly', async () => {
|
it('calls bulkGet correctly', async () => {
|
||||||
await bulkGetUserProfiles({
|
await bulkGetUserProfiles({
|
||||||
security,
|
security,
|
||||||
|
|
|
@ -8,6 +8,7 @@
|
||||||
import type { HttpStart } from '@kbn/core/public';
|
import type { HttpStart } from '@kbn/core/public';
|
||||||
import type { UserProfile } from '@kbn/security-plugin/common';
|
import type { UserProfile } from '@kbn/security-plugin/common';
|
||||||
import type { SecurityPluginStart } from '@kbn/security-plugin/public';
|
import type { SecurityPluginStart } from '@kbn/security-plugin/public';
|
||||||
|
import { isEmpty } from 'lodash';
|
||||||
import { INTERNAL_SUGGEST_USER_PROFILES_URL, DEFAULT_USER_SIZE } from '../../../common/constants';
|
import { INTERNAL_SUGGEST_USER_PROFILES_URL, DEFAULT_USER_SIZE } from '../../../common/constants';
|
||||||
|
|
||||||
export interface SuggestUserProfilesArgs {
|
export interface SuggestUserProfilesArgs {
|
||||||
|
@ -42,11 +43,12 @@ export const bulkGetUserProfiles = async ({
|
||||||
security,
|
security,
|
||||||
uids,
|
uids,
|
||||||
}: BulkGetUserProfilesArgs): Promise<UserProfile[]> => {
|
}: BulkGetUserProfilesArgs): Promise<UserProfile[]> => {
|
||||||
if (uids.length === 0) {
|
const cleanUids: string[] = uids.filter((uid) => !isEmpty(uid));
|
||||||
|
if (cleanUids.length === 0) {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|
||||||
return security.userProfiles.bulkGet({ uids: new Set(uids), dataPath: 'avatar' });
|
return security.userProfiles.bulkGet({ uids: new Set(cleanUids), dataPath: 'avatar' });
|
||||||
};
|
};
|
||||||
|
|
||||||
export interface GetCurrentUserProfileArgs {
|
export interface GetCurrentUserProfileArgs {
|
||||||
|
|
|
@ -399,6 +399,29 @@ describe('update', () => {
|
||||||
Operations.updateCase,
|
Operations.updateCase,
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should filter out empty user profiles', async () => {
|
||||||
|
const casesWithEmptyAssignee = {
|
||||||
|
cases: [
|
||||||
|
{
|
||||||
|
...cases.cases[0],
|
||||||
|
assignees: [{ uid: '' }, { uid: '2' }],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
await bulkUpdate(casesWithEmptyAssignee, clientArgs, casesClientMock);
|
||||||
|
expect(clientArgs.services.caseService.patchCases).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
cases: expect.arrayContaining([
|
||||||
|
expect.objectContaining({
|
||||||
|
updatedAttributes: expect.objectContaining({
|
||||||
|
assignees: [{ uid: '2' }],
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
]),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Category', () => {
|
describe('Category', () => {
|
||||||
|
|
|
@ -61,6 +61,7 @@ import type {
|
||||||
import { CasesPatchRequestRt } from '../../../common/types/api';
|
import { CasesPatchRequestRt } from '../../../common/types/api';
|
||||||
import { CasesRt, CaseStatuses, AttachmentType } from '../../../common/types/domain';
|
import { CasesRt, CaseStatuses, AttachmentType } from '../../../common/types/domain';
|
||||||
import { validateCustomFields } from './validators';
|
import { validateCustomFields } from './validators';
|
||||||
|
import { emptyCasesAssigneesSanitizer } from './sanitizers';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Throws an error if any of the requests attempt to update the owner of a case.
|
* Throws an error if any of the requests attempt to update the owner of a case.
|
||||||
|
@ -384,7 +385,8 @@ export const bulkUpdate = async (
|
||||||
} = clientArgs;
|
} = clientArgs;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const query = decodeWithExcessOrThrow(CasesPatchRequestRt)(cases);
|
const rawQuery = decodeWithExcessOrThrow(CasesPatchRequestRt)(cases);
|
||||||
|
const query = emptyCasesAssigneesSanitizer(rawQuery);
|
||||||
const caseIds = query.cases.map((q) => q.id);
|
const caseIds = query.cases.map((q) => q.id);
|
||||||
const myCases = await caseService.getCases({
|
const myCases = await caseService.getCases({
|
||||||
caseIds,
|
caseIds,
|
||||||
|
|
|
@ -161,6 +161,22 @@ describe('create', () => {
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should filter out empty assignees', async () => {
|
||||||
|
await create(
|
||||||
|
{ ...theCase, assignees: [{ uid: '' }, { uid: '1' }] },
|
||||||
|
clientArgs,
|
||||||
|
casesClientMock
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(clientArgs.services.caseService.createCase).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
attributes: expect.objectContaining({
|
||||||
|
assignees: [{ uid: '1' }],
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('Attributes', () => {
|
describe('Attributes', () => {
|
||||||
|
|
|
@ -21,6 +21,7 @@ import type { CasePostRequest } from '../../../common/types/api';
|
||||||
import { CasePostRequestRt } from '../../../common/types/api';
|
import { CasePostRequestRt } from '../../../common/types/api';
|
||||||
import {} from '../utils';
|
import {} from '../utils';
|
||||||
import { validateCustomFields } from './validators';
|
import { validateCustomFields } from './validators';
|
||||||
|
import { emptyCaseAssigneesSanitizer } from './sanitizers';
|
||||||
import { normalizeCreateCaseRequest } from './utils';
|
import { normalizeCreateCaseRequest } from './utils';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -40,7 +41,8 @@ export const create = async (
|
||||||
} = clientArgs;
|
} = clientArgs;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const query = decodeWithExcessOrThrow(CasePostRequestRt)(data);
|
const rawQuery = decodeWithExcessOrThrow(CasePostRequestRt)(data);
|
||||||
|
const query = emptyCaseAssigneesSanitizer(rawQuery);
|
||||||
const configurations = await casesClient.configure.get({ owner: data.owner });
|
const configurations = await casesClient.configure.get({ owner: data.owner });
|
||||||
const customFieldsConfiguration = configurations[0]?.customFields;
|
const customFieldsConfiguration = configurations[0]?.customFields;
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,34 @@
|
||||||
|
/*
|
||||||
|
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||||
|
* or more contributor license agreements. Licensed under the Elastic License
|
||||||
|
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||||
|
* 2.0.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { isEmpty } from 'lodash';
|
||||||
|
import type { CaseUserProfile } from '../../../common/types/domain/user/v1';
|
||||||
|
|
||||||
|
export const emptyCaseAssigneesSanitizer = <T extends { assignees?: CaseUserProfile[] }>(
|
||||||
|
theCase: T
|
||||||
|
): T => {
|
||||||
|
if (isEmpty(theCase.assignees)) {
|
||||||
|
return theCase;
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
...theCase,
|
||||||
|
assignees: theCase.assignees?.filter(({ uid }) => !isEmpty(uid)),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
export const emptyCasesAssigneesSanitizer = <T extends { assignees?: CaseUserProfile[] }>({
|
||||||
|
cases,
|
||||||
|
}: {
|
||||||
|
cases: T[];
|
||||||
|
}): { cases: T[] } => {
|
||||||
|
return {
|
||||||
|
cases: cases.map((theCase) => {
|
||||||
|
return emptyCaseAssigneesSanitizer(theCase);
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
};
|
|
@ -0,0 +1,54 @@
|
||||||
|
/*
|
||||||
|
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||||
|
* or more contributor license agreements. Licensed under the Elastic License
|
||||||
|
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||||
|
* 2.0.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import expect from '@kbn/expect';
|
||||||
|
|
||||||
|
import { postCaseReq, postCaseResp } from '../../../../common/lib/mock';
|
||||||
|
import {
|
||||||
|
deleteAllCaseItems,
|
||||||
|
createCase,
|
||||||
|
removeServerGeneratedPropertiesFromCase,
|
||||||
|
updateCase,
|
||||||
|
} from '../../../../common/lib/api';
|
||||||
|
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
|
||||||
|
|
||||||
|
export const defaultUser = { email: null, full_name: null, username: 'elastic' };
|
||||||
|
// eslint-disable-next-line import/no-default-export
|
||||||
|
export default ({ getService }: FtrProviderContext): void => {
|
||||||
|
const supertest = getService('supertest');
|
||||||
|
const es = getService('es');
|
||||||
|
|
||||||
|
describe('patch_case', () => {
|
||||||
|
afterEach(async () => {
|
||||||
|
await deleteAllCaseItems(es);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should filter out empty assignee.uid values', async () => {
|
||||||
|
const randomUid = '7f3e9d2a-1b8c-4c5f-9e6d-8f2a4b1d3c7e';
|
||||||
|
const postedCase = await createCase(supertest, postCaseReq);
|
||||||
|
const patchedCases = await updateCase({
|
||||||
|
supertest,
|
||||||
|
params: {
|
||||||
|
cases: [
|
||||||
|
{
|
||||||
|
id: postedCase.id,
|
||||||
|
version: postedCase.version,
|
||||||
|
assignees: [{ uid: '' }, { uid: randomUid }],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const data = removeServerGeneratedPropertiesFromCase(patchedCases[0]);
|
||||||
|
expect(data).to.eql({
|
||||||
|
...postCaseResp(),
|
||||||
|
assignees: [{ uid: randomUid }],
|
||||||
|
updated_by: defaultUser,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
};
|
|
@ -0,0 +1,42 @@
|
||||||
|
/*
|
||||||
|
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||||
|
* or more contributor license agreements. Licensed under the Elastic License
|
||||||
|
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||||
|
* 2.0.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import expect from '@kbn/expect';
|
||||||
|
|
||||||
|
import { getPostCaseRequest } from '../../../../common/lib/mock';
|
||||||
|
import {
|
||||||
|
deleteAllCaseItems,
|
||||||
|
createCase,
|
||||||
|
removeServerGeneratedPropertiesFromCase,
|
||||||
|
} from '../../../../common/lib/api';
|
||||||
|
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
|
||||||
|
|
||||||
|
// eslint-disable-next-line import/no-default-export
|
||||||
|
export default ({ getService }: FtrProviderContext): void => {
|
||||||
|
const supertest = getService('supertest');
|
||||||
|
const es = getService('es');
|
||||||
|
|
||||||
|
describe('post_case', () => {
|
||||||
|
afterEach(async () => {
|
||||||
|
await deleteAllCaseItems(es);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should filter out empty assignee.uid values', async () => {
|
||||||
|
const randomUid = '7f3e9d2a-1b8c-4c5f-9e6d-8f2a4b1d3c7e';
|
||||||
|
const createdCase = await createCase(
|
||||||
|
supertest,
|
||||||
|
getPostCaseRequest({
|
||||||
|
assignees: [{ uid: '' }, { uid: randomUid }],
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
const data = removeServerGeneratedPropertiesFromCase(createdCase);
|
||||||
|
|
||||||
|
expect(data.assignees).to.eql([{ uid: randomUid }]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
};
|
|
@ -31,6 +31,8 @@ export default ({ loadTestFile, getService }: FtrProviderContext): void => {
|
||||||
loadTestFile(require.resolve('./cases/user_actions/find_user_actions'));
|
loadTestFile(require.resolve('./cases/user_actions/find_user_actions'));
|
||||||
loadTestFile(require.resolve('./cases/assignees'));
|
loadTestFile(require.resolve('./cases/assignees'));
|
||||||
loadTestFile(require.resolve('./cases/find_cases'));
|
loadTestFile(require.resolve('./cases/find_cases'));
|
||||||
|
loadTestFile(require.resolve('./cases/post_case'));
|
||||||
|
loadTestFile(require.resolve('./cases/patch_case'));
|
||||||
loadTestFile(require.resolve('./configure'));
|
loadTestFile(require.resolve('./configure'));
|
||||||
loadTestFile(require.resolve('./attachments_framework/registered_persistable_state_trial'));
|
loadTestFile(require.resolve('./attachments_framework/registered_persistable_state_trial'));
|
||||||
// sub privileges are only available with a license above basic
|
// sub privileges are only available with a license above basic
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue