Skip to content

Commit

Permalink
MM-60541 Don't save an unmodified draft when changing channels (matte…
Browse files Browse the repository at this point in the history 
…rmost#28620)

* MM-60541 Don't save an unmodified draft when changing channels

* Address feedback
  • Loading branch information
@hmhealey
hmhealey authored Oct 11, 2024
1 parent bafe37d commit 3811dcc
Show file tree
Hide file tree
Showing 2 changed files with 232 additions and 11 deletions.
219 changes: 216 additions & 3 deletions webapp/channels/src/components/advanced_text_editor/advanced_text_editor.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,32 @@ import type {Channel} from '@mattermost/types/channels';

import Permissions from 'mattermost-redux/constants/permissions';

import {removeDraft, updateDraft} from 'actions/views/drafts';

import type {FileUpload} from 'components/file_upload/file_upload';
import type Textbox from 'components/textbox/textbox';

import mergeObjects from 'packages/mattermost-redux/test/merge_objects';
import {renderWithContext, userEvent, screen} from 'tests/react_testing_utils';
import {StoragePrefixes} from 'utils/constants';
import {TestHelper} from 'utils/test_helper';

import type {PostDraft} from 'types/store/draft';

import AdavancedTextEditor from './advanced_text_editor';
import AdvancedTextEditor from './advanced_text_editor';

jest.mock('actions/views/drafts', () => ({
...jest.requireActual('actions/views/drafts'),
updateDraft: jest.fn((...args) => ({type: 'MOCK_UPDATE_DRAFT', args})),
removeDraft: jest.fn((...args) => ({type: 'MOCK_REMOVE_DRAFT', args})),
}));

const mockedRemoveDraft = jest.mocked(removeDraft);
const mockedUpdateDraft = jest.mocked(updateDraft);

const currentUserId = 'current_user_id';
const channelId = 'current_channel_id';
const otherChannelId = 'other_channel_id';

const initialState = {
entities: {
Expand All @@ -39,15 +52,33 @@ const initialState = {
},
channels: {
channels: {
current_channel_id: TestHelper.getChannelMock({id: 'current_channel_id', team_id: 'current_team_id'}),
current_channel_id: TestHelper.getChannelMock({
id: 'current_channel_id',
team_id: 'current_team_id',
display_name: 'Test Channel',
}),
other_channel_id: TestHelper.getChannelMock({
id: 'other_channel_id',
team_id: 'current_team_id',
display_name: 'Other Channel',
}),
},
stats: {
current_channel_id: {
member_count: 1,
},
other_channel_id: {
member_count: 1,
},
},
roles: {
current_channel_id: new Set(['channel_roles']),
other_channel_id: new Set(['channel_roles']),
},
},
roles: {
roles: {
user_roles: {permissions: [Permissions.CREATE_POST]},
},
},
teams: {
Expand Down Expand Up @@ -147,7 +178,7 @@ describe('components/avanced_text_editor/advanced_text_editor', () => {
describe('keyDown behavior', () => {
it('ESC should blur the input', () => {
renderWithContext(
<AdavancedTextEditor
<AdvancedTextEditor
{...baseProps}
/>,
mergeObjects(initialState, {
Expand All @@ -165,4 +196,186 @@ describe('components/avanced_text_editor/advanced_text_editor', () => {
expect(textbox).not.toHaveFocus();
});
});

it('should set the textbox value to an existing draft on mount and when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
mergeObjects(initialState, {
storage: {
storage: {
[StoragePrefixes.DRAFT + channelId]: {
value: TestHelper.getPostDraftMock({
message: 'original draft',
}),
},
[StoragePrefixes.DRAFT + otherChannelId]: {
value: TestHelper.getPostDraftMock({
message: 'a different draft',
}),
},
},
},
}),
);

expect(screen.getByLabelText('write to test channel')).toHaveValue('original draft');

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);
expect(screen.getByLabelText('write to other channel')).toHaveValue('a different draft');
});

it('should save a new draft when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
initialState,
);

userEvent.type(screen.getByLabelText('write to test channel'), 'some text');

expect(mockedUpdateDraft).not.toHaveBeenCalled();

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);

expect(mockedUpdateDraft).toHaveBeenCalled();
expect(mockedUpdateDraft.mock.calls[0][1]).toMatchObject({
message: 'some text',
show: true,
});
});

it('MM-60541 should not save an unmodified draft when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
mergeObjects(initialState, {
storage: {
storage: {
[StoragePrefixes.DRAFT + channelId]: {
value: TestHelper.getPostDraftMock({
message: 'original draft',
}),
},
},
},
}),
);

expect(mockedUpdateDraft).not.toHaveBeenCalled();

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);

expect(mockedUpdateDraft).not.toHaveBeenCalled();
});

it('should save an updated draft when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
mergeObjects(initialState, {
storage: {
storage: {
[StoragePrefixes.DRAFT + channelId]: {
value: TestHelper.getPostDraftMock({
message: 'original draft',
}),
},
},
},
}),
);

userEvent.type(screen.getByLabelText('write to test channel'), ' plus some new text');

expect(mockedUpdateDraft).not.toHaveBeenCalled();

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);

expect(mockedUpdateDraft).toHaveBeenCalled();
expect(mockedUpdateDraft.mock.calls[0][1]).toMatchObject({
message: 'original draft plus some new text',
show: true,
});
});

it('should deleted a deleted draft when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
mergeObjects(initialState, {
storage: {
storage: {
[StoragePrefixes.DRAFT + channelId]: {
value: TestHelper.getPostDraftMock({
message: 'original draft',
}),
},
},
},
}),
);

userEvent.clear(screen.getByLabelText('write to test channel'));

expect(mockedRemoveDraft).not.toHaveBeenCalled();
expect(mockedUpdateDraft).not.toHaveBeenCalled();

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);

expect(mockedRemoveDraft).toHaveBeenCalled();
expect(mockedUpdateDraft).not.toHaveBeenCalled();
});

it('MM-60541 should not attempt to delete a non-existent draft when changing channels', () => {
const {rerender} = renderWithContext(
<AdvancedTextEditor
{...baseProps}
/>,
initialState,
);

expect(mockedRemoveDraft).not.toHaveBeenCalled();
expect(mockedUpdateDraft).not.toHaveBeenCalled();

rerender(
<AdvancedTextEditor
{...baseProps}
channelId={otherChannelId}
/>,
);

expect(mockedRemoveDraft).not.toHaveBeenCalled();
expect(mockedUpdateDraft).not.toHaveBeenCalled();
});
});
24 changes: 16 additions & 8 deletions webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ const AdvancedTextEditor = ({
const textboxRef = useRef<TextboxClass>(null);
const loggedInAriaLabelTimeout = useRef<NodeJS.Timeout>();
const saveDraftFrame = useRef<NodeJS.Timeout>();
const previousDraft = useRef(draftFromStore);
const draftRef = useRef(draftFromStore);
const storedDrafts = useRef<Record<string, PostDraft | undefined>>({});
const lastBlurAt = useRef(0);

Expand Down Expand Up @@ -427,16 +427,24 @@ const AdvancedTextEditor = ({
};
}, [handleDraftChange, draft]);

// Set the draft from store when changing post or channels, and store the previus one
// Keep track of the draft as a ref so that we can save it when changing channels
useEffect(() => {
setDraft(draftFromStore);
return () => handleDraftChange(previousDraft.current, {instant: true, show: true});
}, [channelId, postId]);
draftRef.current = draft;
}, [draft]);

// Keep track of the previous draft
// Set the draft from store when changing post or channels, and store the previous one
useEffect(() => {
previousDraft.current = draft;
}, [draft]);
// Store the draft that existed when we opened the channel to know if it should be saved
const draftOnOpen = draftFromStore;

setDraft(draftOnOpen);

return () => {
if (draftOnOpen !== draftRef.current) {
handleDraftChange(draftRef.current, {instant: true, show: true});
}
};
}, [channelId, postId]);

const disableSendButton = Boolean(readOnlyChannel || (!draft.message.trim().length && !draft.fileInfos.length)) || !isValidPersistentNotifications;
const sendButton = readOnlyChannel ? null : (
Expand Down

0 comments on commit 3811dcc

Please sign in to comment.