Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Add a key to userActionMarkdown to prevent stale state #132681

Merged
merged 4 commits into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const createUserAttachmentUserActionBuilder = ({
}),
children: (
<UserActionMarkdown
key={isEdit ? comment.id : undefined}
ref={(element) => (commentRefs.current[comment.id] = element)}
id={comment.id}
content={comment.comment}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const getDescriptionUserAction = ({
handleManageMarkdownEditId,
handleManageQuote,
}: GetDescriptionUserActionArgs): EuiCommentProps => {
const isEditable = manageMarkdownEditIds.includes(DESCRIPTION_ID);
return {
username: (
<UserActionUsername
Expand All @@ -55,10 +56,11 @@ export const getDescriptionUserAction = ({
timestamp: <UserActionTimestamp createdAt={caseData.createdAt} />,
children: (
<UserActionMarkdown
key={isEditable ? DESCRIPTION_ID : undefined}
ref={(element) => (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 });
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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 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);
return (
<div>
<UserActionMarkdown
// note that this is not passing the key
{...defaultProps}
content={saveContent}
onSaveContent={setSaveContent}
isEditable={isEditable}
/>
<button
type="button"
data-test-subj="test-button"
onClick={() => {
setIsEditable(!isEditable);
}}
/>
</div>
);
};

const result = appMockRenderer.render(<TestComponent />);

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 (
<div>
<UserActionMarkdown
{...defaultProps}
content={saveContent}
isEditable={isEditable}
onSaveContent={setSaveContent}
// this is the important change. a key is passed to the component
key={isEditable ? 'key' : 'no-key'}
/>
<button
type="button"
data-test-subj="test-button"
onClick={() => {
setIsEditable(!isEditable);
}}
/>
</div>
);
};
const result = appMockRenderer.render(<TestComponent />);
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);
});
});
});