From b42cc5417cf370d15c9004d2ccc9945068a13b04 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 23 May 2022 10:28:28 +0200 Subject: [PATCH 1/3] Add a key to userActionMarkdown to prevent stale state --- .../cases/public/components/user_actions/comment/user.tsx | 1 + .../cases/public/components/user_actions/description.tsx | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx index a4e6fe6cf2887..6c4c96a95bc46 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx @@ -65,6 +65,7 @@ export const createUserAttachmentUserActionBuilder = ({ }), children: ( (commentRefs.current[comment.id] = element)} id={comment.id} content={comment.comment} diff --git a/x-pack/plugins/cases/public/components/user_actions/description.tsx b/x-pack/plugins/cases/public/components/user_actions/description.tsx index 01b0e105ecd96..eae2bd3d1258e 100644 --- a/x-pack/plugins/cases/public/components/user_actions/description.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/description.tsx @@ -43,6 +43,7 @@ export const getDescriptionUserAction = ({ handleManageMarkdownEditId, handleManageQuote, }: GetDescriptionUserActionArgs): EuiCommentProps => { + const isEditable = manageMarkdownEditIds.includes(DESCRIPTION_ID); return { username: ( , children: ( (commentRefs.current[DESCRIPTION_ID] = element)} id={DESCRIPTION_ID} content={caseData.description} - isEditable={manageMarkdownEditIds.includes(DESCRIPTION_ID)} + isEditable={isEditable} onSaveContent={(content: string) => { onUpdateField({ key: DESCRIPTION_ID, value: content }); }} From b950b797b744f94081cb19e5c0b812c4b36c9f7c Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 23 May 2022 11:42:12 +0200 Subject: [PATCH 2/3] Add tests to illustrate the stale state problem --- .../user_actions/markdown_form.test.tsx | 112 +++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx b/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx index 19f60d7cb8c72..344267c8a8b33 100644 --- a/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx @@ -8,8 +8,9 @@ import React from 'react'; import { mount } from 'enzyme'; import { UserActionMarkdown } from './markdown_form'; -import { TestProviders } from '../../common/mock'; +import { AppMockRenderer, createAppMockRenderer, TestProviders } from '../../common/mock'; import { waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; const onChangeEditable = jest.fn(); const onSaveContent = jest.fn(); @@ -86,4 +87,113 @@ describe('UserActionMarkdown ', () => { expect(onChangeEditable).toHaveBeenCalledWith(defaultProps.id); }); }); + + describe('useForm stale state bug', () => { + let appMockRenderer: AppMockRenderer; + const oldContent = defaultProps.content; + const appendContent = ' appended content'; + const newContent = defaultProps.content + appendContent; + + beforeEach(() => { + appMockRenderer = createAppMockRenderer(); + }); + + it('creates a state state if a key is not passed to the component', async () => { + const TestComponent = () => { + const [isEditable, setIsEditable] = React.useState(true); + const [saveContent, setSaveContent] = React.useState(defaultProps.content); + return ( +
+ +
+ ); + }; + + const result = appMockRenderer.render(); + + expect(result.getByTestId('user-action-markdown-form')).toBeTruthy(); + + // append some content and save + userEvent.type(result.container.querySelector('textarea')!, appendContent); + userEvent.click(result.getByTestId('user-action-save-markdown')); + + // wait for the state to update + await waitFor(() => { + expect(onChangeEditable).toHaveBeenCalledWith(defaultProps.id); + }); + + // toggle to non-edit state + userEvent.click(result.getByTestId('test-button')); + expect(result.getByTestId('user-action-markdown')).toBeTruthy(); + + // toggle to edit state again + userEvent.click(result.getByTestId('test-button')); + + // the text area holds a stale value + // this is the wrong behaviour. The textarea holds the old content + expect(result.container.querySelector('textarea')!.value).toEqual(oldContent); + expect(result.container.querySelector('textarea')!.value).not.toEqual(newContent); + }); + + it("doesn't create a stale state if a key is passed to the component", async () => { + const TestComponent = () => { + const [isEditable, setIsEditable] = React.useState(true); + const [saveContent, setSaveContent] = React.useState(defaultProps.content); + return ( +
+ +
+ ); + }; + const result = appMockRenderer.render(); + expect(result.getByTestId('user-action-markdown-form')).toBeTruthy(); + + // append content and save + userEvent.type(result.container.querySelector('textarea')!, appendContent); + userEvent.click(result.getByTestId('user-action-save-markdown')); + + // wait for the state to update + await waitFor(() => { + expect(onChangeEditable).toHaveBeenCalledWith(defaultProps.id); + }); + + // toggle to non-edit state + userEvent.click(result.getByTestId('test-button')); + expect(result.getByTestId('user-action-markdown')).toBeTruthy(); + + // toggle to edit state again + userEvent.click(result.getByTestId('test-button')); + + // this is the correct behaviour. The textarea holds the new content + expect(result.container.querySelector('textarea')!.value).toEqual(newContent); + expect(result.container.querySelector('textarea')!.value).not.toEqual(oldContent); + }); + }); }); From 8f3f0ce7afd24c679fba7f3a6eee85c861b07376 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 23 May 2022 11:46:20 +0200 Subject: [PATCH 3/3] fix typo --- .../cases/public/components/user_actions/markdown_form.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx b/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx index 344267c8a8b33..ae242fc64aafa 100644 --- a/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/markdown_form.test.tsx @@ -98,7 +98,7 @@ describe('UserActionMarkdown ', () => { appMockRenderer = createAppMockRenderer(); }); - it('creates a state state if a key is not passed to the component', async () => { + it('creates a stale state if a key is not passed to the component', async () => { const TestComponent = () => { const [isEditable, setIsEditable] = React.useState(true); const [saveContent, setSaveContent] = React.useState(defaultProps.content);