Merge branch 'fix-drafts' into 'master'

Fix: sent messages sometimes coming back as drafts See merge request kchat/webapp!772
Infomaniak · Jun 4, 2024 · 6da2d26 · 6da2d26
2 parents 95d41b6 + 561508b
commit 6da2d26
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 33 deletions.
diff --git a/webapp/channels/src/actions/views/drafts.test.ts b/webapp/channels/src/actions/views/drafts.test.ts
@@ -12,7 +12,7 @@ import {StoragePrefixes} from 'utils/constants';
 
 import type {PostDraft} from 'types/store/draft';
 
-import {removeDraft, setGlobalDraftSource, addToUpdateDraftQueue} from './drafts';
+import {removeDraft, setGlobalDraftSource, updateDraft} from './drafts';
 
 jest.mock('mattermost-redux/client', () => {
     const original = jest.requireActual('mattermost-redux/client');
@@ -142,7 +142,7 @@ describe('draft actions', () => {
             jest.useFakeTimers();
             jest.setSystemTime(42);
 
-            await store.dispatch(addToUpdateDraftQueue(key, draft, '', false));
+            await store.dispatch(updateDraft(key, draft, '', false));
 
             const testStore = mockStore(initialState);
 
@@ -160,7 +160,7 @@ describe('draft actions', () => {
         });
 
         it('calls upsertDraft correctly', async () => {
-            await store.dispatch(addToUpdateDraftQueue(key, draft, '', true));
+            await store.dispatch(updateDraft(key, draft, '', true));
             expect(createDraftSpy).toHaveBeenCalled();
         });
     });

diff --git a/webapp/channels/src/actions/views/drafts.ts b/webapp/channels/src/actions/views/drafts.ts
@@ -1,7 +1,6 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import PQueue from 'p-queue';
 import {batchActions} from 'redux-batched-actions';
 
 import type {Draft as ServerDraft} from '@mattermost/types/drafts';
@@ -32,8 +31,6 @@ type Draft = {
     timestamp: Date;
 }
 
-const updateDraftQueue = new PQueue({concurrency: 1});
-
 /**
  * Gets drafts stored on the server and reconciles them with any locally stored drafts.
  * @param teamId Only drafts for the given teamId will be fetched.
@@ -115,13 +112,6 @@ export function removeDraft(key: string, channelId: string, rootId = ''): Action
     };
 }
 
-// Assert previous call ended before dispatching the action again to ensure latest draftId is used and prevent multiple draft creation on the same channel
-export const addToUpdateDraftQueue = (key: string, value: PostDraft|null, rootId = '', save = false, scheduleDelete = false): ActionFuncAsync<boolean, GlobalState> => {
-    return (dispatch) => {
-        return updateDraftQueue.add(() => dispatch(updateDraft(key, value, rootId, save, scheduleDelete)));
-    };
-};
-
 export function updateDraft(key: string, value: PostDraft|null, rootId = '', save = false, scheduleDelete = false): ActionFuncAsync<boolean, GlobalState> {
     return async (dispatch, getState) => {
         const state = getState();
@@ -164,21 +154,29 @@ export function updateDraft(key: string, value: PostDraft|null, rootId = '', sav
             const userId = getCurrentUserId(state);
 
             try {
-                // TODO: remove
-                if (value?.message === '' || value?.message.replace(/\s/g, '').length || (value && value?.fileInfos.length > 0)) {
-                    if (value?.message.replace(/\s/g, '').length) {
-                        const {id} = await upsertDraft(updatedValue, userId, rootId, scheduleDelete);
-                        dispatch(setGlobalDraft(key, {
-                            ...updatedValue,
-                            id,
-                        }, false));
-                    } else {
-                        //This case is when there is a file attached with no message
-                        await upsertDraft({...updatedValue, message: ''}, userId, rootId, scheduleDelete);
+                const isDraftMessageEmpty = (draft: PostDraft|null) =>
+                    (draft?.message.replace(/\s/g, '').length === 0);
+
+                /**
+                 * Test if a draft is empty, either:
+                 *  - the draft is null
+                 *  - both the draft message and files are empty
+                 */
+                const isDraftEmpty = (draft: PostDraft|null) =>
+                    (draft === null) ||
+                    (isDraftMessageEmpty(draft) && (draft?.fileInfos.length === 0));
+
+                const {id} = await upsertDraft(updatedValue as PostDraft, userId, rootId, scheduleDelete);
+
+                // Do not update id in case there is a file attached with no message [???]
+                if (!isDraftMessageEmpty(updatedValue)) {
+                    // Only update the draft id if it has not been cleared from the reducer
+                    // during the upsertDraft client call
+                    const draft = getGlobalItem(getState(), key, {});
+                    if (!isDraftEmpty(draft)) {
+                        dispatch(setGlobalDraft(key, {...draft, id}, false));
                     }
                 }
-
-                // await upsertDraft(updatedValue, userId, rootId, scheduleDelete);
             } catch (error) {
                 return {data: false, error};
             }

diff --git a/webapp/channels/src/components/advanced_create_post/index.ts b/webapp/channels/src/components/advanced_create_post/index.ts
@@ -37,7 +37,7 @@ import {runMessageWillBePostedHooks, runSlashCommandWillBePostedHooks} from 'act
 import {addReaction, createPost, setEditingPost, emitShortcutReactToLastPostFrom, submitReaction} from 'actions/post_actions';
 import {actionOnGlobalItemsWithPrefix} from 'actions/storage';
 import {scrollPostListToBottom} from 'actions/views/channel';
-import {addToUpdateDraftQueue, removeDraft, upsertScheduleDraft, setGlobalDraft} from 'actions/views/drafts';
+import {updateDraft, removeDraft, upsertScheduleDraft} from 'actions/views/drafts';
 import {searchAssociatedGroupsForReference} from 'actions/views/group';
 import {openModal} from 'actions/views/modals';
 import {selectPostFromRightHandSideSearchByPostId} from 'actions/views/rhs';
@@ -150,14 +150,14 @@ function setDraft(key: string, value: PostDraft | null, draftChannelId: string,
             updatedValue = {...value, channelId};
         }
         if (updatedValue) {
-            console.log('[advanced_create_post/idx ~ draft] setDraft.addToUpdateDraftQueue',
+            console.log('[advanced_create_post/idx ~ draft] setDraft.updateDraft',
                 `key: ${key}`,
                 `id: ${value?.id}`,
                 `updatedAt: ${value?.updateAt}`,
                 `createdAt: ${value?.createAt}`,
             );
 
-            return dispatch(addToUpdateDraftQueue(key, updatedValue, '', save));
+            return dispatch(updateDraft(key, updatedValue, '', save));
         }
 
         console.log('[advanced_create_post/idx ~ draft] setDraft.removeDraft',

diff --git a/webapp/channels/src/components/drafts/channel_draft/channel_draft.tsx b/webapp/channels/src/components/drafts/channel_draft/channel_draft.tsx
@@ -13,7 +13,7 @@ import type {DispatchFunc} from 'mattermost-redux/types/actions';
 
 import {createPost} from 'actions/post_actions';
 import {setGlobalItem} from 'actions/storage';
-import {removeDraft, upsertScheduleDraft, addToUpdateDraftQueue, setGlobalDraftSource} from 'actions/views/drafts';
+import {removeDraft, upsertScheduleDraft, updateDraft, setGlobalDraftSource} from 'actions/views/drafts';
 import {closeModal, openModal} from 'actions/views/modals';
 import {getGlobalItem} from 'selectors/storage';
 
@@ -135,7 +135,7 @@ function ChannelDraft({
         await dispatch(removeDraft(StoragePrefixes.DRAFT + newDraft.channelId, channel.id));
 
         // Update channel draft
-        const {error} = await dispatch(addToUpdateDraftQueue(StoragePrefixes.DRAFT + newDraft.channelId, value, '', true, true));
+        const {error} = await dispatch(updateDraft(StoragePrefixes.DRAFT + newDraft.channelId, value, '', true, true));
         if (error) {
             dispatch(setGlobalItem(`${StoragePrefixes.DRAFT}${newDraft.channelId}_${newDraft.id}`, value));
         }

diff --git a/webapp/channels/src/components/drafts/thread_draft/thread_draft.tsx b/webapp/channels/src/components/drafts/thread_draft/thread_draft.tsx
@@ -14,7 +14,7 @@ import {getPost} from 'mattermost-redux/actions/posts';
 
 import {setGlobalItem} from 'actions/storage';
 import {makeOnSubmit} from 'actions/views/create_comment';
-import {addToUpdateDraftQueue, removeDraft, setGlobalDraftSource, upsertScheduleDraft} from 'actions/views/drafts';
+import {updateDraft, removeDraft, setGlobalDraftSource, upsertScheduleDraft} from 'actions/views/drafts';
 import {closeModal, openModal} from 'actions/views/modals';
 import {selectPost} from 'actions/views/rhs';
 import {getGlobalItem} from 'selectors/storage';
@@ -144,7 +144,7 @@ function ThreadDraft({
         await dispatch(removeDraft(StoragePrefixes.COMMENT_DRAFT + newDraft.rootId, channel.id, newDraft.rootId));
 
         // Update thread draft
-        const {error} = await dispatch(addToUpdateDraftQueue(StoragePrefixes.COMMENT_DRAFT + newDraft.rootId, value, newDraft.rootId, true, true));
+        const {error} = await dispatch(updateDraft(StoragePrefixes.COMMENT_DRAFT + newDraft.rootId, value, newDraft.rootId, true, true));
         if (error) {
             dispatch(setGlobalItem(`${StoragePrefixes.COMMENT_DRAFT}${newDraft.rootId}_${newDraft.id}`, value));
         }