Skip to content

Commit

Permalink
[Cases] Add a key to userActionMarkdown to prevent stale state (elast…
Browse files Browse the repository at this point in the history
  • Loading branch information
academo authored and patrykkopycinski committed May 23, 2022
1 parent a059efa commit efb9930
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 2 deletions.
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);
});
});
});

0 comments on commit efb9930

Please sign in to comment.