Skip to content

Commit

Permalink
[8.8] [Cases] Fix bug with registered attachmets memoization (#158441) (
Browse files Browse the repository at this point in the history
#158559)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Cases] Fix bug with registered attachmets memoization
(#158441)](#158441)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Christos
Nasikas","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-26T12:43:43Z","message":"[Cases]
Fix bug with registered attachmets memoization (#158441)\n\n##
Summary\r\n\r\nIn #156179 I fixed
a bug with the\r\nmemorization of the react components of registered
attachments in Cases.\r\nThis fix introduced another bug which I am
fixing in this PR.\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/158447\r\n\r\n###
Bug\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/782a552c-29bd-4bae-b66b-8620cbdeb5ff\r\n\r\n###
Fix\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/a4834ce2-73c8-48cb-9ec4-f480133eca38\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\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\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix a bug where ML embeddables, OsQuery, and IoCs
attachments in a case\r\nrender the wrong
view.","sha":"a2a0860f65013167a8e097d821eefa7fe81b8832","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:ResponseOps","Feature:Cases","v8.9.0","v8.8.1"],"number":158441,"url":"https://github.com/elastic/kibana/pull/158441","mergeCommit":{"message":"[Cases]
Fix bug with registered attachmets memoization (#158441)\n\n##
Summary\r\n\r\nIn #156179 I fixed
a bug with the\r\nmemorization of the react components of registered
attachments in Cases.\r\nThis fix introduced another bug which I am
fixing in this PR.\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/158447\r\n\r\n###
Bug\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/782a552c-29bd-4bae-b66b-8620cbdeb5ff\r\n\r\n###
Fix\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/a4834ce2-73c8-48cb-9ec4-f480133eca38\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\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\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix a bug where ML embeddables, OsQuery, and IoCs
attachments in a case\r\nrender the wrong
view.","sha":"a2a0860f65013167a8e097d821eefa7fe81b8832"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/158441","number":158441,"mergeCommit":{"message":"[Cases]
Fix bug with registered attachmets memoization (#158441)\n\n##
Summary\r\n\r\nIn #156179 I fixed
a bug with the\r\nmemorization of the react components of registered
attachments in Cases.\r\nThis fix introduced another bug which I am
fixing in this PR.\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/158447\r\n\r\n###
Bug\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/782a552c-29bd-4bae-b66b-8620cbdeb5ff\r\n\r\n###
Fix\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7871006/a4834ce2-73c8-48cb-9ec4-f480133eca38\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\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\r\n\r\n### For
maintainers\r\n\r\n- [x] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n##
Release notes\r\n\r\nFix a bug where ML embeddables, OsQuery, and IoCs
attachments in a case\r\nrender the wrong
view.","sha":"a2a0860f65013167a8e097d821eefa7fe81b8832"}},{"branch":"8.8","label":"v8.8.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
cnasikas authored May 26, 2023
1 parent 82c4eac commit 41428d0
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,27 @@ type BuilderArgs<C, R> = Pick<
/**
* Provides a render function for attachment type
*/
const getAttachmentRenderer = memoize(() => {
let AttachmentElement: React.ReactElement;
const getAttachmentRenderer = (cachingKey: string) =>
memoize(
() => {
let AttachmentElement: React.ReactElement;

const renderCallback = (attachmentViewObject: AttachmentViewObject, props: object) => {
if (!attachmentViewObject.children) return;
const renderCallback = (attachmentViewObject: AttachmentViewObject, props: object) => {
if (!attachmentViewObject.children) return;

if (!AttachmentElement) {
AttachmentElement = React.createElement(attachmentViewObject.children, props);
} else {
AttachmentElement = React.cloneElement(AttachmentElement, props);
}
if (!AttachmentElement) {
AttachmentElement = React.createElement(attachmentViewObject.children, props);
} else {
AttachmentElement = React.cloneElement(AttachmentElement, props);
}

return <Suspense fallback={<EuiLoadingSpinner />}>{AttachmentElement}</Suspense>;
};
return <Suspense fallback={<EuiLoadingSpinner />}>{AttachmentElement}</Suspense>;
};

return renderCallback;
});
return renderCallback;
},
() => cachingKey
);

export const createRegisteredAttachmentUserActionBuilder = <
C extends CommentResponse,
Expand Down Expand Up @@ -120,7 +124,7 @@ export const createRegisteredAttachmentUserActionBuilder = <

const attachmentViewObject = attachmentType.getAttachmentViewObject(props);

const renderer = getAttachmentRenderer();
const renderer = getAttachmentRenderer(userAction.id)();
const actions = attachmentViewObject.getActions?.(props) ?? [];
const [primaryActions, nonPrimaryActions] = partition(actions, 'isPrimary');
const visiblePrimaryActions = primaryActions.slice(0, 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
CommentType,
CaseResponse,
CommentRequest,
CommentRequestExternalReferenceType,
CommentRequestPersistableStateType,
} from '@kbn/cases-plugin/common/api';
import { expect } from 'expect';
import {
Expand Down Expand Up @@ -85,16 +87,10 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
describe('Attachment framework', () => {
describe('External reference attachments', () => {
let caseWithAttachment: CaseResponse;
const externalReferenceAttachment = getExternalReferenceAttachment();

before(async () => {
caseWithAttachment = await createAttachmentAndNavigate({
type: CommentType.externalReference,
externalReferenceId: 'my-id',
externalReferenceStorage: { type: ExternalReferenceStorageType.elasticSearchDoc },
externalReferenceAttachmentTypeId: '.test',
externalReferenceMetadata: null,
owner: 'cases',
});
caseWithAttachment = await createAttachmentAndNavigate(externalReferenceAttachment);
});

after(async () => {
Expand All @@ -109,77 +105,6 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
});

describe('Persistable state attachments', () => {
const getLensState = (dataViewId: string) => ({
title: '',
visualizationType: 'lnsXY',
type: 'lens',
references: [
{
type: 'index-pattern',
id: dataViewId,
name: 'indexpattern-datasource-layer-85863a23-73a0-4e11-9774-70f77b9a5898',
},
],
state: {
visualization: {
legend: { isVisible: true, position: 'right' },
valueLabels: 'hide',
fittingFunction: 'None',
axisTitlesVisibilitySettings: { x: true, yLeft: true, yRight: true },
tickLabelsVisibilitySettings: { x: true, yLeft: true, yRight: true },
labelsOrientation: { x: 0, yLeft: 0, yRight: 0 },
gridlinesVisibilitySettings: { x: true, yLeft: true, yRight: true },
preferredSeriesType: 'bar_stacked',
layers: [
{
layerId: '85863a23-73a0-4e11-9774-70f77b9a5898',
accessors: ['63810bd4-8481-4aab-822a-532d8513a8b1'],
position: 'top',
seriesType: 'bar_stacked',
showGridlines: false,
layerType: 'data',
xAccessor: 'ab807e89-c453-415b-8eb4-3986de52c923',
},
],
},
query: { query: '', language: 'kuery' },
filters: [],
datasourceStates: {
formBased: {
layers: {
'85863a23-73a0-4e11-9774-70f77b9a5898': {
columns: {
'ab807e89-c453-415b-8eb4-3986de52c923': {
label: '@timestamp',
dataType: 'date',
operationType: 'date_histogram',
sourceField: '@timestamp',
isBucketed: true,
scale: 'interval',
params: { interval: 'auto', includeEmptyRows: true, dropPartials: false },
},
'63810bd4-8481-4aab-822a-532d8513a8b1': {
label: 'Median of id',
dataType: 'number',
operationType: 'median',
sourceField: 'id',
isBucketed: false,
scale: 'ratio',
params: { emptyAsNull: true },
},
},
columnOrder: [
'ab807e89-c453-415b-8eb4-3986de52c923',
'63810bd4-8481-4aab-822a-532d8513a8b1',
],
incompleteColumns: {},
},
},
},
},
},
});

let caseWithAttachment: CaseResponse;
let dataViewId = '';

Expand All @@ -188,12 +113,8 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
const res = await createLogStashDataView(supertest);
dataViewId = res.data_view.id;

caseWithAttachment = await createAttachmentAndNavigate({
type: CommentType.persistableState,
persistableStateAttachmentTypeId: '.test',
persistableStateAttachmentState: getLensState(dataViewId),
owner: 'cases',
});
const persistableStateAttachment = getPersistableStateAttachment(dataViewId);
caseWithAttachment = await createAttachmentAndNavigate(persistableStateAttachment);
});

after(async () => {
Expand All @@ -206,7 +127,65 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
const attachmentId = caseWithAttachment?.comments?.[0].id;
await validateAttachment(CommentType.persistableState, attachmentId);
await retry.waitFor(
'actions accordion to exist',
'persistable state to exist',
async () => await find.existsByCssSelector('.lnsExpressionRenderer')
);
});
});

describe('Multiple attachments', () => {
let originalCase: CaseResponse;
let dataViewId = '';

before(async () => {
await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/logstash_functional');
const res = await createLogStashDataView(supertest);
dataViewId = res.data_view.id;

originalCase = await cases.api.createCase({
title: 'Registering multiple attachments',
});

const externalReferenceAttachment = getExternalReferenceAttachment();
const persistableStateAttachment = getPersistableStateAttachment(dataViewId);

await cases.api.createAttachment({
caseId: originalCase.id,
params: externalReferenceAttachment,
});

await cases.api.createAttachment({
caseId: originalCase.id,
params: persistableStateAttachment,
});

await cases.navigation.navigateToApp();
await cases.casesTable.waitForCasesToBeListed();
await cases.casesTable.goToFirstListedCase();
await header.waitUntilLoadingHasFinished();
});

after(async () => {
await cases.api.deleteAllCases();
await deleteLogStashDataView(supertest, dataViewId);
await esArchiver.unload('x-pack/test/functional/es_archives/logstash_functional');
});

it('renders multiple attachment types correctly', async () => {
const theCase = await getCase({
supertest,
caseId: originalCase.id,
includeComments: true,
});

const externalRefAttachmentId = theCase?.comments?.[0].id;
const persistableStateAttachmentId = theCase?.comments?.[1].id;
await validateAttachment(CommentType.externalReference, externalRefAttachmentId);
await validateAttachment(CommentType.persistableState, persistableStateAttachmentId);

await testSubjects.existOrFail('test-attachment-content');
await retry.waitFor(
'persistable state to exist',
async () => await find.existsByCssSelector('.lnsExpressionRenderer')
);
});
Expand Down Expand Up @@ -348,3 +327,90 @@ export default ({ getPageObject, getService }: FtrProviderContext) => {
});
});
};

const getLensState = (dataViewId: string) => ({
title: '',
visualizationType: 'lnsXY',
type: 'lens',
references: [
{
type: 'index-pattern',
id: dataViewId,
name: 'indexpattern-datasource-layer-85863a23-73a0-4e11-9774-70f77b9a5898',
},
],
state: {
visualization: {
legend: { isVisible: true, position: 'right' },
valueLabels: 'hide',
fittingFunction: 'None',
axisTitlesVisibilitySettings: { x: true, yLeft: true, yRight: true },
tickLabelsVisibilitySettings: { x: true, yLeft: true, yRight: true },
labelsOrientation: { x: 0, yLeft: 0, yRight: 0 },
gridlinesVisibilitySettings: { x: true, yLeft: true, yRight: true },
preferredSeriesType: 'bar_stacked',
layers: [
{
layerId: '85863a23-73a0-4e11-9774-70f77b9a5898',
accessors: ['63810bd4-8481-4aab-822a-532d8513a8b1'],
position: 'top',
seriesType: 'bar_stacked',
showGridlines: false,
layerType: 'data',
xAccessor: 'ab807e89-c453-415b-8eb4-3986de52c923',
},
],
},
query: { query: '', language: 'kuery' },
filters: [],
datasourceStates: {
formBased: {
layers: {
'85863a23-73a0-4e11-9774-70f77b9a5898': {
columns: {
'ab807e89-c453-415b-8eb4-3986de52c923': {
label: '@timestamp',
dataType: 'date',
operationType: 'date_histogram',
sourceField: '@timestamp',
isBucketed: true,
scale: 'interval',
params: { interval: 'auto', includeEmptyRows: true, dropPartials: false },
},
'63810bd4-8481-4aab-822a-532d8513a8b1': {
label: 'Median of id',
dataType: 'number',
operationType: 'median',
sourceField: 'id',
isBucketed: false,
scale: 'ratio',
params: { emptyAsNull: true },
},
},
columnOrder: [
'ab807e89-c453-415b-8eb4-3986de52c923',
'63810bd4-8481-4aab-822a-532d8513a8b1',
],
incompleteColumns: {},
},
},
},
},
},
});

const getExternalReferenceAttachment = (): CommentRequestExternalReferenceType => ({
type: CommentType.externalReference,
externalReferenceId: 'my-id',
externalReferenceStorage: { type: ExternalReferenceStorageType.elasticSearchDoc },
externalReferenceAttachmentTypeId: '.test',
externalReferenceMetadata: null,
owner: 'cases',
});

const getPersistableStateAttachment = (dataViewId: string): CommentRequestPersistableStateType => ({
type: CommentType.persistableState,
persistableStateAttachmentTypeId: '.test',
persistableStateAttachmentState: getLensState(dataViewId),
owner: 'cases',
});

0 comments on commit 41428d0

Please sign in to comment.