From dae4b75720b33a92765b5be5d55bbe4243f1c3c3 Mon Sep 17 00:00:00 2001 From: Rajat Dabade Date: Wed, 25 Jun 2025 16:04:08 +0530 Subject: [PATCH] Ability to show quick action and add files to edit post (#8926) * Viewing Files in Edit mode in mobile with ability to delete and save * Added upload attachment to keyboard tracker view * using state instread of use ref * Minor * Added tests * intl extract * new function getFiles ById, batch file deletion and tests * Files fetching in edit options and tests * Removed DeviceEventEmitter and used React context * Added support to check minimum required version to show edit file attachments * resolve forward ref issue * Minor * memotized props for context and observe config with value * Import fixes * Ability to show quick action and add files to edit post * type safety for EditPostContext * Reverted back the post priority props * constant shift * Added test for QuickAction to show slashcomand * Added test for edit_post, upload_item and upload_remove * Added test for Edit_post_input and edit_post index * fix the height issue between attachment and keyboard due to safeArea * Minor: removed debugging border color * Addressed dev review comments * Import fixes * Address UX comments * Fixed props for UploadItem and remove effective Edit mode * handled save button disabled state when uploading attachments * handled newly added and retry file removal without alert message * Test updated * Added test for input_quick_action index.tsx * Added test for not in edit mode for upload_item index * Added test for upload_remove component when not in edit mode * added tests for file_upload_error hook * Added test for calling callback when in edit mode * Test for edit post input for server version check --- .../draft_handler/draft_handler.tsx | 35 +-- .../post_draft/draft_input/draft_input.tsx | 1 - .../input_quick_action/index.test.tsx | 55 ++++ .../input_quick_action/index.tsx | 6 +- .../quick_actions/quick_actions.test.tsx | 44 +++ .../quick_actions/quick_actions.tsx | 22 +- app/components/post_draft/uploads/index.tsx | 9 +- .../uploads/upload_item/index.test.tsx | 130 +++++++++ .../post_draft/uploads/upload_item/index.tsx | 44 ++- .../upload_item/upload_remove.test.tsx | 67 +++++ .../uploads/upload_item/upload_remove.tsx | 17 +- .../uploads/upload_item/upload_retry.tsx | 1 + app/constants/files.ts | 2 + app/context/edit_post/index.tsx | 16 +- app/hooks/file_upload_error.test.ts | 138 ++++++++++ app/hooks/file_upload_error.ts | 29 ++ .../draft_upload_manager/index.test.ts | 51 ++-- app/managers/draft_upload_manager/index.ts | 38 ++- app/screens/edit_post/edit_post.test.tsx | 233 +++++++++++++--- app/screens/edit_post/edit_post.tsx | 255 +++++++++++++++--- .../edit_post_input/edit_post_input.test.tsx | 30 +++ .../edit_post_input/edit_post_input.tsx | 47 +++- app/screens/edit_post/index.test.tsx | 76 ++++++ app/screens/edit_post/index.tsx | 11 +- types/components/upload_error_handlers.ts | 6 + 25 files changed, 1183 insertions(+), 180 deletions(-) create mode 100644 app/components/post_draft/quick_actions/input_quick_action/index.test.tsx create mode 100644 app/components/post_draft/quick_actions/quick_actions.test.tsx create mode 100644 app/components/post_draft/uploads/upload_item/index.test.tsx create mode 100644 app/components/post_draft/uploads/upload_item/upload_remove.test.tsx create mode 100644 app/hooks/file_upload_error.test.ts create mode 100644 app/hooks/file_upload_error.ts create mode 100644 app/screens/edit_post/index.test.tsx create mode 100644 types/components/upload_error_handlers.ts diff --git a/app/components/post_draft/draft_handler/draft_handler.tsx b/app/components/post_draft/draft_handler/draft_handler.tsx index 5de98330c..16c7ceb3f 100644 --- a/app/components/post_draft/draft_handler/draft_handler.tsx +++ b/app/components/post_draft/draft_handler/draft_handler.tsx @@ -1,16 +1,19 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useCallback, useEffect, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useRef} from 'react'; import {useIntl} from 'react-intl'; import {addFilesToDraft, removeDraft} from '@actions/local/draft'; import {useServerUrl} from '@context/server'; -import DraftUploadManager from '@managers/draft_upload_manager'; +import useFileUploadError from '@hooks/file_upload_error'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import {fileMaxWarning, fileSizeWarning, uploadDisabledWarning} from '@utils/file'; import SendHandler from '../send_handler'; +import type {ErrorHandlers} from '@typings/components/upload_error_handlers'; + type Props = { testID?: string; channelId: string; @@ -29,11 +32,6 @@ type Props = { } const emptyFileList: FileInfo[] = []; -const UPLOAD_ERROR_SHOW_INTERVAL = 5000; - -type ErrorHandlers = { - [clientId: string]: (() => void) | null; -} export default function DraftHandler(props: Props) { const { @@ -56,27 +54,14 @@ export default function DraftHandler(props: Props) { const serverUrl = useServerUrl(); const intl = useIntl(); - const [uploadError, setUploadError] = useState(null); - - const uploadErrorTimeout = useRef(); const uploadErrorHandlers = useRef({}); + const {uploadError, newUploadError} = useFileUploadError(); const clearDraft = useCallback(() => { removeDraft(serverUrl, channelId, rootId); updateValue(''); }, [serverUrl, channelId, rootId]); - const newUploadError = useCallback((error: React.ReactNode) => { - if (uploadErrorTimeout.current) { - clearTimeout(uploadErrorTimeout.current); - } - setUploadError(error); - - uploadErrorTimeout.current = setTimeout(() => { - setUploadError(null); - }, UPLOAD_ERROR_SHOW_INTERVAL); - }, []); - const addFiles = useCallback((newFiles: FileInfo[]) => { if (!newFiles.length) { return; @@ -103,8 +88,8 @@ export default function DraftHandler(props: Props) { addFilesToDraft(serverUrl, channelId, rootId, newFiles); for (const file of newFiles) { - DraftUploadManager.prepareUpload(serverUrl, file, channelId, rootId); - uploadErrorHandlers.current[file.clientId!] = DraftUploadManager.registerErrorHandler(file.clientId!, newUploadError); + DraftEditPostUploadManager.prepareUpload(serverUrl, file, channelId, rootId); + uploadErrorHandlers.current[file.clientId!] = DraftEditPostUploadManager.registerErrorHandler(file.clientId!, newUploadError); } newUploadError(null); @@ -115,7 +100,7 @@ export default function DraftHandler(props: Props) { useEffect(() => { let loadingFiles: FileInfo[] = []; if (files) { - loadingFiles = files.filter((v) => v.clientId && DraftUploadManager.isUploading(v.clientId)); + loadingFiles = files.filter((v) => v.clientId && DraftEditPostUploadManager.isUploading(v.clientId)); } for (const key of Object.keys(uploadErrorHandlers.current)) { @@ -127,7 +112,7 @@ export default function DraftHandler(props: Props) { for (const file of loadingFiles) { if (!uploadErrorHandlers.current[file.clientId!]) { - uploadErrorHandlers.current[file.clientId!] = DraftUploadManager.registerErrorHandler(file.clientId!, newUploadError); + uploadErrorHandlers.current[file.clientId!] = DraftEditPostUploadManager.registerErrorHandler(file.clientId!, newUploadError); } } }, [files]); diff --git a/app/components/post_draft/draft_input/draft_input.tsx b/app/components/post_draft/draft_input/draft_input.tsx index 0eb15c3e5..9235edfef 100644 --- a/app/components/post_draft/draft_input/draft_input.tsx +++ b/app/components/post_draft/draft_input/draft_input.tsx @@ -242,7 +242,6 @@ function DraftInput({ uploadFileError={uploadFileError} channelId={channelId} rootId={rootId} - isEditMode={false} /> { + it('should add If theres existing text and it doesnt end with a space, add a space before @', () => { + const updateValue = jest.fn(); + const testID = 'test-id'; + const inputType = 'at'; + const focus = jest.fn(); + + const {getByTestId} = renderWithIntlAndTheme( + ); + + const icon = getByTestId('test-id'); + fireEvent.press(icon); + + expect(updateValue).toHaveBeenCalledWith(expect.any(Function)); + const updateFunction = updateValue.mock.calls[0][0]; + expect(updateFunction('')).toBe('@'); + expect(focus).toHaveBeenCalled(); + }); + + it('should add space before @ if there is existing text and it doesnt end with a space', () => { + const updateValue = jest.fn(); + const testID = 'test-id'; + const inputType = 'at'; + const focus = jest.fn(); + + const {getByTestId} = renderWithIntlAndTheme( + ); + + const icon = getByTestId('test-id'); + fireEvent.press(icon); + + expect(updateValue).toHaveBeenCalledWith(expect.any(Function)); + const updateFunction = updateValue.mock.calls[0][0]; + expect(updateFunction('Hello')).toBe('Hello @'); + expect(focus).toHaveBeenCalled(); + }); + +}); diff --git a/app/components/post_draft/quick_actions/input_quick_action/index.tsx b/app/components/post_draft/quick_actions/input_quick_action/index.tsx index badd1e39e..1afb31614 100644 --- a/app/components/post_draft/quick_actions/input_quick_action/index.tsx +++ b/app/components/post_draft/quick_actions/input_quick_action/index.tsx @@ -41,12 +41,16 @@ export default function InputQuickAction({ const onPress = useCallback(() => { updateValue((v) => { if (inputType === 'at') { + // If there's existing text and it doesn't end with a space, add a space before @ + if (v.length > 0 && !v.endsWith(' ')) { + return `${v} @`; + } return `${v}@`; } return '/'; }); focus(); - }, [inputType]); + }, [inputType, updateValue, focus]); const actionTestID = disabled ? `${testID}.disabled` : diff --git a/app/components/post_draft/quick_actions/quick_actions.test.tsx b/app/components/post_draft/quick_actions/quick_actions.test.tsx new file mode 100644 index 000000000..0e403d406 --- /dev/null +++ b/app/components/post_draft/quick_actions/quick_actions.test.tsx @@ -0,0 +1,44 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {PostPriorityType} from '@constants/post'; +import {renderWithIntlAndTheme} from '@test/intl-test-helper'; + +import QuickActions from './quick_actions'; + +describe('Quick Actions', () => { + const baseProps: Parameters[0] = { + canUploadFiles: true, + fileCount: 0, + isPostPriorityEnabled: true, + canShowPostPriority: true, + canShowSlashCommands: true, + maxFileCount: 10, + value: '', + updateValue: jest.fn(), + addFiles: jest.fn(), + postPriority: { + priority: PostPriorityType.STANDARD, + }, + updatePostPriority: jest.fn(), + focus: jest.fn(), + }; + + it('Should render slash command if canShowSlashCommands is true', () => { + const props = { + ...baseProps, + canShowSlashCommands: true, + }; + const {queryByTestId} = renderWithIntlAndTheme(); + expect(queryByTestId('slash-input-action')).toBeDefined(); + }); + + it('Should not render slash command if canShowSlashCommands is false', () => { + const props = { + ...baseProps, + canShowSlashCommands: false, + }; + const {queryByTestId} = renderWithIntlAndTheme(); + expect(queryByTestId('slash-input-action')).toBeNull(); + }); +}); diff --git a/app/components/post_draft/quick_actions/quick_actions.tsx b/app/components/post_draft/quick_actions/quick_actions.tsx index a77cc645b..8e5a542d8 100644 --- a/app/components/post_draft/quick_actions/quick_actions.tsx +++ b/app/components/post_draft/quick_actions/quick_actions.tsx @@ -16,6 +16,7 @@ type Props = { fileCount: number; isPostPriorityEnabled: boolean; canShowPostPriority?: boolean; + canShowSlashCommands?: boolean; maxFileCount: number; // Draft Handler @@ -27,11 +28,13 @@ type Props = { focus: () => void; } +export const QUICK_ACTIONS_HEIGHT = 44; + const style = StyleSheet.create({ quickActionsContainer: { display: 'flex', flexDirection: 'row', - height: 44, + height: QUICK_ACTIONS_HEIGHT, }, }); @@ -41,6 +44,7 @@ export default function QuickActions({ value, fileCount, isPostPriorityEnabled, + canShowSlashCommands = true, canShowPostPriority, maxFileCount, updateValue, @@ -79,13 +83,15 @@ export default function QuickActions({ updateValue={updateValue} focus={focus} /> - + {canShowSlashCommands && ( + + )} { @@ -75,7 +74,6 @@ function Uploads({ uploadFileError, channelId, rootId, - isEditMode, }: Props) { const galleryIdentifier = `${channelId}-uploadedItems-${rootId}`; const theme = useTheme(); @@ -83,7 +81,7 @@ function Uploads({ const errorHeight = useSharedValue(ERROR_HEIGHT_MIN); const containerHeight = useSharedValue(files.length ? CONTAINER_HEIGHT_MAX : CONTAINER_HEIGHT_MIN); - const filesForGallery = useRef(files.filter((f) => !f.failed && !DraftUploadManager.isUploading(f.clientId!))); + const filesForGallery = useRef(files.filter((f) => !f.failed && !DraftEditPostUploadManager.isUploading(f.clientId!))); const hasFiles = files.length > 0; const errorAnimatedStyle = useAnimatedStyle(() => { @@ -103,7 +101,7 @@ function Uploads({ }), [files.length]); useEffect(() => { - filesForGallery.current = files.filter((f) => !f.failed && !DraftUploadManager.isUploading(f.clientId!)); + filesForGallery.current = files.filter((f) => !f.failed && !DraftEditPostUploadManager.isUploading(f.clientId!)); }, [files]); useEffect(() => { @@ -139,7 +137,6 @@ function Uploads({ key={file.clientId || file.id} openGallery={openGallery} rootId={rootId} - isEditMode={isEditMode} /> ); }); diff --git a/app/components/post_draft/uploads/upload_item/index.test.tsx b/app/components/post_draft/uploads/upload_item/index.test.tsx new file mode 100644 index 000000000..a243b4987 --- /dev/null +++ b/app/components/post_draft/uploads/upload_item/index.test.tsx @@ -0,0 +1,130 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {updateDraftFile} from '@actions/local/draft'; +import {EditPostProvider} from '@context/edit_post'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; +import {fireEvent, renderWithEverything} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import UploadItem from '.'; + +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('@actions/local/draft', () => ({ + updateDraftFile: jest.fn(), +})); + +jest.mock('@managers/draft_upload_manager', () => ({ + prepareUpload: jest.fn(), + isUploading: jest.fn(() => false), + registerProgressHandler: jest.fn(() => jest.fn()), + registerErrorHandler: jest.fn(() => jest.fn()), + cancel: jest.fn(), +})); + +jest.mock('@context/server', () => ({ + useServerUrl: () => 'serverUrl', +})); + +describe('UploadItem', () => { + const serverUrl = 'serverUrl'; + let database: Database; + + beforeEach(async () => { + jest.clearAllMocks(); + database = (await TestHelper.setupServerDatabase(serverUrl)).database; + }); + + const baseProps: Parameters[0] = { + channelId: 'channelId', + galleryIdentifier: 'galleryIdentifier', + index: 0, + file: { + id: 'fileId', + name: 'fileName', + extension: 'extension', + has_preview_image: false, + height: 0, + mime_type: 'mime_type', + size: 0, + width: 0, + bytesRead: 0, + clientId: 'clientId', + failed: false, + } as FileInfo, + openGallery: jest.fn(), + rootId: 'rootId' as string, + }; + + it('When file is failed, onclick of retry button, it should call prepareUpload with correct arguments in edit mode', () => { + const updateFileCallback = jest.fn(); + const failedFile = { + ...baseProps.file, + failed: true, + } as FileInfo; + + const props = { + ...baseProps, + file: failedFile, + }; + + const {getByTestId} = renderWithEverything( + + + , + {database}, + ); + + fireEvent.press(getByTestId('retry-button')); + + const expectedResetFile = {...failedFile, failed: false}; + + expect(updateFileCallback).toHaveBeenCalledWith(expectedResetFile); + expect(DraftEditPostUploadManager.prepareUpload).toHaveBeenCalledWith( + serverUrl, + expectedResetFile, + props.channelId, + props.rootId, + expectedResetFile.bytesRead, + true, + updateFileCallback, + ); + expect(updateDraftFile).not.toHaveBeenCalled(); + }); + + it('When file is failed, onclick of retry button, it should call prepareUpload with correct arguments in draft mode', () => { + const updateFileCallback = jest.fn(); + const failedFile = { + ...baseProps.file, + failed: true, + } as FileInfo; + + const props = { + ...baseProps, + file: failedFile, + }; + + const {getByTestId} = renderWithEverything( + + + , + {database}, + ); + + fireEvent.press(getByTestId('retry-button')); + + const expectedResetFile = {...failedFile, failed: false}; + + expect(updateDraftFile).toHaveBeenCalledWith(serverUrl, props.channelId, props.rootId, expectedResetFile); + expect(DraftEditPostUploadManager.prepareUpload).toHaveBeenCalledWith(serverUrl, expectedResetFile, props.channelId, props.rootId, expectedResetFile.bytesRead); + }); +}); diff --git a/app/components/post_draft/uploads/upload_item/index.tsx b/app/components/post_draft/uploads/upload_item/index.tsx index f5c3a3281..306cf7dc6 100644 --- a/app/components/post_draft/uploads/upload_item/index.tsx +++ b/app/components/post_draft/uploads/upload_item/index.tsx @@ -9,11 +9,12 @@ import {updateDraftFile} from '@actions/local/draft'; import FileIcon from '@components/files/file_icon'; import ImageFile from '@components/files/image_file'; import ProgressBar from '@components/progress_bar'; +import {useEditPost} from '@context/edit_post'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; import useDidUpdate from '@hooks/did_update'; import {useGalleryItem} from '@hooks/gallery'; -import DraftUploadManager from '@managers/draft_upload_manager'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import {isImage} from '@utils/file'; import {changeOpacity} from '@utils/theme'; @@ -27,7 +28,6 @@ type Props = { file: FileInfo; openGallery: (file: FileInfo) => void; rootId: string; - isEditMode: boolean; } const style = StyleSheet.create({ @@ -62,14 +62,15 @@ const style = StyleSheet.create({ export default function UploadItem({ channelId, galleryIdentifier, index, file, - rootId, openGallery, isEditMode, + rootId, openGallery, }: Props) { const theme = useTheme(); const serverUrl = useServerUrl(); - const removeCallback = useRef<(() => void)|null>(null); + const removeCallback = useRef<(() => void) | undefined>(undefined); const [progress, setProgress] = useState(0); + const {updateFileCallback, isEditMode} = useEditPost(); - const loading = DraftUploadManager.isUploading(file.clientId!); + const loading = DraftEditPostUploadManager.isUploading(file.clientId!); const handlePress = useCallback(() => { openGallery(file); @@ -77,21 +78,21 @@ export default function UploadItem({ useEffect(() => { if (file.clientId) { - removeCallback.current = DraftUploadManager.registerProgressHandler(file.clientId, setProgress); + removeCallback.current = DraftEditPostUploadManager.registerProgressHandler(file.clientId, setProgress); } return () => { removeCallback.current?.(); - removeCallback.current = null; + removeCallback.current = undefined; }; }, []); useDidUpdate(() => { if (loading && file.clientId) { - removeCallback.current = DraftUploadManager.registerProgressHandler(file.clientId, setProgress); + removeCallback.current = DraftEditPostUploadManager.registerProgressHandler(file.clientId, setProgress); } return () => { removeCallback.current?.(); - removeCallback.current = null; + removeCallback.current = undefined; }; }, [file.failed, file.id]); @@ -103,10 +104,26 @@ export default function UploadItem({ const newFile = {...file}; newFile.failed = false; - updateDraftFile(serverUrl, channelId, rootId, newFile); - DraftUploadManager.prepareUpload(serverUrl, newFile, channelId, rootId, newFile.bytesRead); - DraftUploadManager.registerProgressHandler(newFile.clientId!, setProgress); - }, [serverUrl, channelId, rootId, file]); + if (isEditMode && updateFileCallback) { + // In edit mode, use the context callback to update the file + updateFileCallback(newFile); + DraftEditPostUploadManager.prepareUpload( + serverUrl, + newFile, + channelId, + rootId, + newFile.bytesRead, + true, // isEditPost = true + updateFileCallback, + ); + } else { + // In draft mode, use the draft file system + updateDraftFile(serverUrl, channelId, rootId, newFile); + DraftEditPostUploadManager.prepareUpload(serverUrl, newFile, channelId, rootId, newFile.bytesRead); + } + + DraftEditPostUploadManager.registerProgressHandler(newFile.clientId!, setProgress); + }, [serverUrl, channelId, rootId, file, isEditMode, updateFileCallback]); const {styles, onGestureEvent, ref} = useGalleryItem(galleryIdentifier, index, handlePress); @@ -160,7 +177,6 @@ export default function UploadItem({ clientId={file.clientId!} channelId={channelId} rootId={rootId} - isEditMode={isEditMode} fileId={file.id!} /> diff --git a/app/components/post_draft/uploads/upload_item/upload_remove.test.tsx b/app/components/post_draft/uploads/upload_item/upload_remove.test.tsx new file mode 100644 index 000000000..088fc05af --- /dev/null +++ b/app/components/post_draft/uploads/upload_item/upload_remove.test.tsx @@ -0,0 +1,67 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {removeDraftFile} from '@actions/local/draft'; +import {fireEvent, renderWithEverything} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import UploadRemove from './upload_remove'; + +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('@context/edit_post', () => ({ + useEditPost: jest.fn(), +})); + +jest.mock('@actions/local/draft', () => ({ + removeDraftFile: jest.fn(), +})); + +const {useEditPost} = require('@context/edit_post'); + +describe('UploadRemove', () => { + const serverUrl = 'serverUrl'; + let database: Database; + + beforeEach(async () => { + database = (await TestHelper.setupServerDatabase(serverUrl)!).database; + }); + + it('should call onFileRemove when isEditMode is true', () => { + const onFileRemove = jest.fn(); + jest.mocked(useEditPost).mockReturnValue({ + onFileRemove, + isEditMode: true, + }); + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + fireEvent.press(getByTestId('remove-button-fileId')); + expect(onFileRemove).toHaveBeenCalledWith('fileId'); + }); + + it('should call removeDraftFile when isEditMode is false', () => { + jest.mocked(useEditPost).mockReturnValue({ + isEditMode: false, + }); + + const {getByTestId} = renderWithEverything( + , + {database, serverUrl}, + ); + fireEvent.press(getByTestId('remove-button-fileId')); + expect(removeDraftFile).toHaveBeenCalledWith(serverUrl, 'channelId', 'rootId', 'clientId'); + }); +}); diff --git a/app/components/post_draft/uploads/upload_item/upload_remove.tsx b/app/components/post_draft/uploads/upload_item/upload_remove.tsx index 9215b04ee..8ca139903 100644 --- a/app/components/post_draft/uploads/upload_item/upload_remove.tsx +++ b/app/components/post_draft/uploads/upload_item/upload_remove.tsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {useCallback} from 'react'; import {View, Platform} from 'react-native'; import {removeDraftFile} from '@actions/local/draft'; @@ -10,14 +10,13 @@ import TouchableWithFeedback from '@components/touchable_with_feedback'; import {useEditPost} from '@context/edit_post'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; -import DraftUploadManager from '@managers/draft_upload_manager'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import {makeStyleSheetFromTheme, changeOpacity} from '@utils/theme'; type Props = { channelId: string; rootId: string; clientId: string; - isEditMode: boolean; fileId: string; } @@ -49,28 +48,28 @@ export default function UploadRemove({ channelId, rootId, clientId, - isEditMode, fileId, }: Props) { const theme = useTheme(); const style = getStyleSheet(theme); const serverUrl = useServerUrl(); - const {onFileRemove} = useEditPost(); + const {onFileRemove, isEditMode} = useEditPost(); - const onPress = () => { + const onPress = useCallback(() => { if (isEditMode) { - onFileRemove?.(fileId); + onFileRemove?.(fileId || clientId); return; } - DraftUploadManager.cancel(clientId); + DraftEditPostUploadManager.cancel(clientId); removeDraftFile(serverUrl, channelId, rootId, clientId); - }; + }, [onFileRemove, isEditMode, fileId, clientId, serverUrl, channelId, rootId]); return ( void; + updateFileCallback?: (fileInfo: FileInfo) => void; + isEditMode: boolean; }; -const EditPostContext = createContext({}); +const EditPostContext = createContext({ + onFileRemove: undefined, + updateFileCallback: undefined, + isEditMode: false, +}); type EditPostProviderProps = { children: ReactNode; onFileRemove?: (fileId: string) => void; + updateFileCallback?: (fileInfo: FileInfo) => void; + isEditMode: boolean; }; -export const EditPostProvider = ({children, onFileRemove}: EditPostProviderProps) => { +export const EditPostProvider = ({children, onFileRemove, updateFileCallback, isEditMode}: EditPostProviderProps) => { const contextValue = useMemo(() => ({ onFileRemove, - }), [onFileRemove]); + updateFileCallback, + isEditMode, + }), [onFileRemove, updateFileCallback, isEditMode]); return ( diff --git a/app/hooks/file_upload_error.test.ts b/app/hooks/file_upload_error.test.ts new file mode 100644 index 000000000..8fba2d5b1 --- /dev/null +++ b/app/hooks/file_upload_error.test.ts @@ -0,0 +1,138 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {renderHook, act} from '@testing-library/react-hooks'; + +import {UPLOAD_ERROR_SHOW_INTERVAL} from '@constants/files'; + +import useFileUploadError from './file_upload_error'; + +jest.useFakeTimers(); + +describe('useFileUploadError', () => { + afterEach(() => { + jest.clearAllTimers(); + }); + + it('should initialize with no upload error', () => { + const {result} = renderHook(() => useFileUploadError()); + expect(result.current.uploadError).toBeNull(); + expect(typeof result.current.newUploadError).toBe('function'); + }); + + it('should set upload error when newUploadError is called', () => { + const {result} = renderHook(() => useFileUploadError()); + const errorMessage = 'Upload failed'; + act(() => { + result.current.newUploadError(errorMessage); + }); + expect(result.current.uploadError).toBe(errorMessage); + }); + + it('should clear upload error after timeout', () => { + const {result} = renderHook(() => useFileUploadError()); + const errorMessage = 'Upload failed'; + act(() => { + result.current.newUploadError(errorMessage); + }); + expect(result.current.uploadError).toBe(errorMessage); + act(() => { + jest.advanceTimersByTime(UPLOAD_ERROR_SHOW_INTERVAL); + }); + expect(result.current.uploadError).toBeNull(); + }); + + it('should handle different types of error messages', () => { + const {result} = renderHook(() => useFileUploadError()); + const complexErrorMessage = 'Complex error: File size too large (5MB), maximum allowed is 2MB'; + act(() => { + result.current.newUploadError(complexErrorMessage); + }); + expect(result.current.uploadError).toBe(complexErrorMessage); + }); + + it('should clear previous timeout when new error is set', () => { + const {result} = renderHook(() => useFileUploadError()); + const firstError = 'First error'; + const secondError = 'Second error'; + + // Set first error + act(() => { + result.current.newUploadError(firstError); + }); + expect(result.current.uploadError).toBe(firstError); + + // Set second error before first timeout completes + act(() => { + jest.advanceTimersByTime(2000); // Advance partway through first timeout + result.current.newUploadError(secondError); + }); + expect(result.current.uploadError).toBe(secondError); + + // Advance to where first timeout would have completed + act(() => { + jest.advanceTimersByTime(3000); // Total 5000ms from first error + }); + + // Error should still be second error (first timeout was cleared) + expect(result.current.uploadError).toBe(secondError); + + // Complete second timeout + act(() => { + jest.advanceTimersByTime(2000); // Complete 5000ms from second error + }); + + expect(result.current.uploadError).toBeNull(); + }); + + it('should not clear error before timeout completes', () => { + const {result} = renderHook(() => useFileUploadError()); + const errorMessage = 'Upload failed'; + act(() => { + result.current.newUploadError(errorMessage); + }); + + expect(result.current.uploadError).toBe(errorMessage); + + // Advance time but not to full timeout + act(() => { + jest.advanceTimersByTime(UPLOAD_ERROR_SHOW_INTERVAL - 1000); + }); + + // Error should still be present + expect(result.current.uploadError).toBe(errorMessage); + }); + + it('should handle multiple consecutive errors correctly', () => { + const {result} = renderHook(() => useFileUploadError()); + const error1 = 'Error 1'; + const error2 = 'Error 2'; + const error3 = 'Error 3'; + + // Set first error + act(() => { + result.current.newUploadError(error1); + }); + expect(result.current.uploadError).toBe(error1); + + // Quickly set second error + act(() => { + result.current.newUploadError(error2); + }); + expect(result.current.uploadError).toBe(error2); + + // Quickly set third error + act(() => { + result.current.newUploadError(error3); + }); + expect(result.current.uploadError).toBe(error3); + + // Only the last timeout should be active + act(() => { + jest.advanceTimersByTime(UPLOAD_ERROR_SHOW_INTERVAL); + }); + + expect(result.current.uploadError).toBeNull(); + }); +}); + diff --git a/app/hooks/file_upload_error.ts b/app/hooks/file_upload_error.ts new file mode 100644 index 000000000..ffefa2e55 --- /dev/null +++ b/app/hooks/file_upload_error.ts @@ -0,0 +1,29 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {useCallback, useRef, useState} from 'react'; + +import {UPLOAD_ERROR_SHOW_INTERVAL} from '@constants/files'; + +const useFileUploadError = () => { + const [uploadError, setUploadError] = useState(null); + const uploadErrorTimeout = useRef(); + + const newUploadError = useCallback((error: React.ReactNode) => { + if (uploadErrorTimeout.current) { + clearTimeout(uploadErrorTimeout.current); + } + setUploadError(error); + + uploadErrorTimeout.current = setTimeout(() => { + setUploadError(null); + }, UPLOAD_ERROR_SHOW_INTERVAL); + }, []); + + return { + uploadError, + newUploadError, + }; +}; + +export default useFileUploadError; diff --git a/app/managers/draft_upload_manager/index.test.ts b/app/managers/draft_upload_manager/index.test.ts index 3243b88c3..c7d1fcee0 100644 --- a/app/managers/draft_upload_manager/index.test.ts +++ b/app/managers/draft_upload_manager/index.test.ts @@ -14,7 +14,7 @@ import {exportedForTesting} from '.'; import type ServerDataOperator from '@database/operator/server_data_operator'; import type {ClientResponse, ProgressPromise} from '@mattermost/react-native-network-client'; -const {DraftUploadManagerSingleton} = exportedForTesting; +const {DraftEditPostUploadManagerSingleton} = exportedForTesting; const url = 'baseHandler.test.com'; const mockClient = TestHelper.createClient(); @@ -79,7 +79,7 @@ describe('draft upload manager', () => { }); it('File is uploaded and stored', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const fileClientId = 'clientId'; @@ -103,7 +103,7 @@ describe('draft upload manager', () => { }); it('Progress is not stored on progress, but stored on fail', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const fileClientId = 'clientId'; @@ -158,7 +158,7 @@ describe('draft upload manager', () => { const spyNow = jest.spyOn(Date, 'now'); spyNow.mockImplementation(() => now); AppState.currentState = 'active'; - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const progressFunc: {[fileUrl: string] : ((fractionCompleted: number, bytesRead?: number | null | undefined) => void)} = {}; const cancel = jest.fn(); @@ -255,7 +255,7 @@ describe('draft upload manager', () => { }); it('Error on complete: Received wrong response code', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const fileClientId = 'clientId'; @@ -280,7 +280,7 @@ describe('draft upload manager', () => { }); it('Error on complete: Received no data', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const clientId = 'clientId'; @@ -304,7 +304,7 @@ describe('draft upload manager', () => { }); it('Error on complete: Received no file info', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const clientId = 'clientId'; @@ -328,7 +328,7 @@ describe('draft upload manager', () => { }); it('Progress handler', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const clientId = 'clientId'; @@ -336,14 +336,14 @@ describe('draft upload manager', () => { const nullProgressHandler = jest.fn(); let cancelProgressHandler = manager.registerProgressHandler(clientId, nullProgressHandler); - expect(cancelProgressHandler).toBeNull(); + expect(cancelProgressHandler).toBeUndefined(); manager.prepareUpload(url, {clientId, localPath: 'path1'} as FileInfo, channelId, rootId, 0); expect(manager.isUploading(clientId)).toBe(true); const progressHandler = jest.fn(); cancelProgressHandler = manager.registerProgressHandler(clientId, progressHandler); - expect(cancelProgressHandler).not.toBeNull(); + expect(cancelProgressHandler).not.toBeUndefined(); let bytesRead = 200; uploadMocks.progressFunc!(0.1, bytesRead); @@ -369,7 +369,7 @@ describe('draft upload manager', () => { }); it('Error handler: normal error', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const clientId = 'clientId'; @@ -377,14 +377,14 @@ describe('draft upload manager', () => { const nullErrorHandler = jest.fn(); let cancelErrorHandler = manager.registerProgressHandler(clientId, nullErrorHandler); - expect(cancelErrorHandler).toBeNull(); + expect(cancelErrorHandler).toBeUndefined(); manager.prepareUpload(url, {clientId, localPath: 'path1'} as FileInfo, channelId, rootId, 0); expect(manager.isUploading(clientId)).toBe(true); const errorHandler = jest.fn(); cancelErrorHandler = manager.registerErrorHandler(clientId, errorHandler); - expect(cancelErrorHandler).not.toBeNull(); + expect(cancelErrorHandler).not.toBeUndefined(); uploadMocks.rejectPromise!({message: 'error'}); @@ -405,7 +405,7 @@ describe('draft upload manager', () => { }); it('Error handler: complete error', async () => { - const manager = new DraftUploadManagerSingleton(); + const manager = new DraftEditPostUploadManagerSingleton(); const uploadMocks = mockUpload(); const clientId = 'clientId'; @@ -413,14 +413,14 @@ describe('draft upload manager', () => { const nullErrorHandler = jest.fn(); let cancelErrorHandler = manager.registerProgressHandler(clientId, nullErrorHandler); - expect(cancelErrorHandler).toBeNull(); + expect(cancelErrorHandler).toBeUndefined(); manager.prepareUpload(url, {clientId, localPath: 'path1'} as FileInfo, channelId, rootId, 0); expect(manager.isUploading(clientId)).toBe(true); const errorHandler = jest.fn(); cancelErrorHandler = manager.registerErrorHandler(clientId, errorHandler); - expect(cancelErrorHandler).not.toBeNull(); + expect(cancelErrorHandler).not.toBeUndefined(); uploadMocks.resolvePromise!({ok: true, code: 500}); @@ -437,4 +437,23 @@ describe('draft upload manager', () => { expect(errorHandler).toHaveBeenCalledTimes(1); expect(nullErrorHandler).not.toHaveBeenCalled(); }); + + it('should call updateFileCallback when isEditPost is true', async () => { + const manager = new DraftEditPostUploadManagerSingleton(); + const uploadMocks = mockUpload(); + const updateFileCallback = jest.fn(); + const updateDraftFileSpy = jest.spyOn(require('@actions/local/draft'), 'updateDraftFile'); + const clientId = 'clientId'; + + manager.prepareUpload(url, {clientId, localPath: 'path1'} as FileInfo, channelId, rootId, 0, true, updateFileCallback); + expect(manager.isUploading(clientId)).toBe(true); + uploadMocks.rejectPromise!('Upload failed'); + await new Promise(process.nextTick); + + expect(updateFileCallback).toHaveBeenCalledWith({clientId, localPath: 'path1', failed: true}); + expect(updateDraftFileSpy).not.toHaveBeenCalled(); + expect(manager.isUploading(clientId)).toBe(false); + + updateDraftFileSpy.mockRestore(); + }); }); diff --git a/app/managers/draft_upload_manager/index.ts b/app/managers/draft_upload_manager/index.ts index 32b1b7df1..414a61e63 100644 --- a/app/managers/draft_upload_manager/index.ts +++ b/app/managers/draft_upload_manager/index.ts @@ -20,10 +20,12 @@ type FileHandler = { lastTimeStored: number; onError: Array<(msg: string) => void>; onProgress: Array<(p: number, b: number) => void>; + isEditPost?: boolean; + updateFileCallback?: (fileInfo: FileInfo) => void; }; } -class DraftUploadManagerSingleton { +class DraftEditPostUploadManagerSingleton { private handlers: FileHandler = {}; private previousAppState: AppStateStatus; @@ -38,6 +40,8 @@ class DraftUploadManagerSingleton { channelId: string, rootId: string, skipBytes = 0, + isEditPost = false, + updateFileCallback?: (fileInfo: FileInfo) => void, ) => { this.handlers[file.clientId!] = { fileInfo: file, @@ -47,6 +51,8 @@ class DraftUploadManagerSingleton { lastTimeStored: 0, onError: [], onProgress: [], + isEditPost, + updateFileCallback, }; const onProgress = (progress: number, bytesRead?: number | null | undefined) => { @@ -83,7 +89,7 @@ class DraftUploadManagerSingleton { public registerProgressHandler = (clientId: string, callback: (progress: number, bytes: number) => void) => { if (!this.handlers[clientId]) { - return null; + return undefined; } this.handlers[clientId].onProgress.push(callback); @@ -98,7 +104,7 @@ class DraftUploadManagerSingleton { public registerErrorHandler = (clientId: string, callback: (errMessage: string) => void) => { if (!this.handlers[clientId]) { - return null; + return undefined; } this.handlers[clientId].onError.push(callback); @@ -121,12 +127,12 @@ class DraftUploadManagerSingleton { h.onProgress.forEach((c) => c(progress, bytes)); if (AppState.currentState !== 'active' && h.lastTimeStored + PROGRESS_TIME_TO_STORE < Date.now()) { - updateDraftFile(h.serverUrl, h.channelId, h.rootId, this.handlers[clientId].fileInfo); + this.handleUpdateDraftFile(h, this.handlers[clientId].fileInfo, h.isEditPost || false); h.lastTimeStored = Date.now(); } }; - private handleComplete = (response: ClientResponse, clientId: string) => { + private handleComplete = async (response: ClientResponse, clientId: string) => { const h = this.handlers[clientId]; if (!h) { return; @@ -151,10 +157,10 @@ class DraftUploadManagerSingleton { fileInfo.clientId = h.fileInfo.clientId; fileInfo.localPath = h.fileInfo.localPath; - updateDraftFile(h.serverUrl, h.channelId, h.rootId, fileInfo); + await this.handleUpdateDraftFile(h, fileInfo, h.isEditPost || false); }; - private handleError = (errorMessage: string, clientId: string) => { + private handleError = async (errorMessage: string, clientId: string) => { const h = this.handlers[clientId]; if (!h) { return; @@ -166,7 +172,15 @@ class DraftUploadManagerSingleton { const fileInfo = {...h.fileInfo}; fileInfo.failed = true; - updateDraftFile(h.serverUrl, h.channelId, h.rootId, fileInfo); + await this.handleUpdateDraftFile(h, fileInfo, h.isEditPost || false); + }; + + private handleUpdateDraftFile = async (handler: FileHandler[string], fileInfo: FileInfo, isEditPost: boolean) => { + if (isEditPost && handler.updateFileCallback) { + handler.updateFileCallback(fileInfo); + } else { + await updateDraftFile(handler.serverUrl, handler.channelId, handler.rootId, fileInfo); + } }; private onAppStateChange = async (appState: AppStateStatus) => { @@ -180,15 +194,15 @@ class DraftUploadManagerSingleton { private storeProgress = async () => { for (const h of Object.values(this.handlers)) { // eslint-disable-next-line no-await-in-loop - await updateDraftFile(h.serverUrl, h.channelId, h.rootId, h.fileInfo); + await this.handleUpdateDraftFile(h, h.fileInfo, h.isEditPost || false); h.lastTimeStored = Date.now(); } }; } -const DraftUploadManager = new DraftUploadManagerSingleton(); -export default DraftUploadManager; +const DraftEditPostUploadManager = new DraftEditPostUploadManagerSingleton(); +export default DraftEditPostUploadManager; export const exportedForTesting = { - DraftUploadManagerSingleton, + DraftEditPostUploadManagerSingleton, }; diff --git a/app/screens/edit_post/edit_post.test.tsx b/app/screens/edit_post/edit_post.test.tsx index e4fb323b4..7d8d873b4 100644 --- a/app/screens/edit_post/edit_post.test.tsx +++ b/app/screens/edit_post/edit_post.test.tsx @@ -2,20 +2,89 @@ // See LICENSE.txt for license information. import React from 'react'; +import {Alert} from 'react-native'; -import {renderWithEverything} from '@test/intl-test-helper'; +import useFileUploadError from '@hooks/file_upload_error'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; +import {fireEvent, renderWithEverything} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; +import PickerUtil from '@utils/file/file_picker'; import EditPost from './edit_post'; import type {Database} from '@nozbe/watermelondb'; import type PostModel from '@typings/database/models/servers/post'; -const serverUrl = 'baseHandler.test.com'; +jest.mock('@hooks/file_upload_error'); +jest.mock('@utils/file/file_picker'); +jest.mock('@managers/draft_upload_manager', () => ({ + prepareUpload: jest.fn(), + isUploading: jest.fn(() => false), + registerProgressHandler: jest.fn(() => jest.fn()), + registerErrorHandler: jest.fn(() => jest.fn()), + cancel: jest.fn(), +})); + +const TEST_CONFIG = { + serverUrl: 'baseHandler.test.com', + maxPostSize: 1000, + maxFileCount: 10, + maxFileSize: 1000, +} as const; + +const TEST_FILES = { + existingFile1: { + id: 'file-1', + name: 'test-1', + extension: 'png', + has_preview_image: true, + height: 100, + mime_type: 'test', + size: 100, + user_id: '1', + width: 100, + }, + existingFile2: { + id: 'file-2', + name: 'test-2', + extension: 'pdf', + has_preview_image: false, + height: 100, + mime_type: 'test', + size: 100, + user_id: '1', + width: 100, + }, + newFile: { + clientId: 'file-3', + id: 'file-3', + name: 'test-3', + extension: 'txt', + mime_type: 'text/plain', + size: 999, + }, + smallFile: { + name: 'test.txt', + mime_type: 'text/plain', + }, + largeFile: { + name: 'test.txt', + mime_type: 'text/plain', + size: 1001, + }, +} as const satisfies Record>; + +const ERROR_MESSAGES = { + uploadsDisabled: 'File uploads from mobile are disabled.', + maxFilesReached: 'Uploads limited to 1 files maximum.', + fileTooLarge: 'Files must be less than 1000 B', + confirmDelete: 'Delete attachment', + confirmDeleteMessage: 'Are you sure you want to remove test-3?', +} as const; describe('Edit Post', () => { - let database: Database; + let mockNewUploadError: jest.Mock; const baseProps: Parameters[0] = { componentId: 'EditPost', @@ -29,42 +98,57 @@ describe('Edit Post', () => { rootId: '1', metadata: {}, } as PostModel, - maxPostSize: 1000, + maxPostSize: TEST_CONFIG.maxPostSize, hasFilesAttached: true, canDelete: true, - files: [ - { - id: 'file-1', - name: 'test-1', - extension: 'png', - has_preview_image: true, - height: 100, - mime_type: 'test', - size: 100, - user_id: '1', - width: 100, - }, - { - id: 'file-2', - name: 'test-2', - extension: 'pdf', - has_preview_image: false, - height: 100, - mime_type: 'test', - size: 100, - user_id: '1', - width: 100, - }, - ], + files: [TEST_FILES.existingFile1, TEST_FILES.existingFile2], + maxFileCount: TEST_CONFIG.maxFileCount, + maxFileSize: TEST_CONFIG.maxFileSize, + canUploadFiles: true, + }; + + const setupMocks = () => { + mockNewUploadError = jest.fn(); + jest.mocked(useFileUploadError).mockReturnValue({ + uploadError: null, + newUploadError: mockNewUploadError, + }); + }; + + const setupPickerMock = (file: Partial) => { + jest.mocked(PickerUtil).mockImplementation((intl, onUploadFiles) => ({ + attachFileFromFiles: jest.fn(() => { + onUploadFiles?.([file as ExtractedFileInfo]); + return Promise.resolve({error: undefined}); + }), + }) as unknown as PickerUtil); + }; + + const renderEditPost = (props = baseProps) => { + return renderWithEverything(, { + database, + serverUrl: TEST_CONFIG.serverUrl, + }); + }; + + const triggerFileUpload = (screen: ReturnType) => { + fireEvent.press(screen.getByTestId('edit_post.quick_actions.file_action')); + }; + + const triggerFileRemoval = (screen: ReturnType, fileId: string) => { + fireEvent.press(screen.getByTestId(`remove-button-${fileId}`)); }; beforeAll(async () => { - const server = await TestHelper.setupServerDatabase(serverUrl); + const server = await TestHelper.setupServerDatabase(TEST_CONFIG.serverUrl); database = server.database; const operator = server.operator; + operator.handleConfigs({ configs: [ {id: 'Version', value: '10.5.0'}, + {id: 'EnableFileAttachments', value: 'true'}, + {id: 'EnableMobileFileUpload', value: 'true'}, ], configsToDelete: [], prepareRecordsOnly: false, @@ -73,12 +157,93 @@ describe('Edit Post', () => { beforeEach(() => { jest.clearAllMocks(); + setupMocks(); }); - it('should render the attachments in edit mode', () => { - const {getByTestId} = renderWithEverything(, {database, serverUrl}); - expect(getByTestId('uploads')).toBeVisible(); - expect(getByTestId('file-1')).toBeVisible(); - expect(getByTestId('file-2')).toBeVisible(); + describe('Rendering', () => { + it('should render existing attachments in edit mode', () => { + const screen = renderEditPost(); + + expect(screen.getByTestId('uploads')).toBeVisible(); + expect(screen.getByTestId('file-1')).toBeVisible(); + expect(screen.getByTestId('file-2')).toBeVisible(); + }); + }); + + describe('File Upload Validation', () => { + it('should show error when file uploads are disabled', () => { + setupPickerMock(TEST_FILES.smallFile); + const props = {...baseProps, canUploadFiles: false}; + const screen = renderEditPost(props); + triggerFileUpload(screen); + + expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.uploadsDisabled); + }); + + it('should show error when maximum file count is reached', () => { + setupPickerMock(TEST_FILES.smallFile); + const props = {...baseProps, maxFileCount: 1}; + const screen = renderEditPost(props); + triggerFileUpload(screen); + + expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.maxFilesReached); + }); + + it('should show error when file size exceeds limit', () => { + setupPickerMock(TEST_FILES.largeFile); + const props = {...baseProps, maxFileSize: 1000}; + const screen = renderEditPost(props); + triggerFileUpload(screen); + + expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.fileTooLarge); + }); + }); + + describe('File Upload Integration', () => { + it('should integrate with DraftEditPostUploadManager for successful uploads', () => { + setupPickerMock(TEST_FILES.newFile); + const screen = renderEditPost(); + triggerFileUpload(screen); + + expect(DraftEditPostUploadManager.prepareUpload).toHaveBeenCalledWith( + TEST_CONFIG.serverUrl, + TEST_FILES.newFile, + baseProps.post.channelId, + baseProps.post.rootId, + 0, + true, + expect.any(Function), + ); + expect(DraftEditPostUploadManager.registerErrorHandler).toHaveBeenCalledWith( + TEST_FILES.newFile.clientId, + mockNewUploadError, + ); + expect(mockNewUploadError).toHaveBeenCalledWith(null); + }); + }); + + describe('File Removal', () => { + beforeEach(() => { + jest.mocked(DraftEditPostUploadManager.isUploading).mockReturnValue(true); + setupPickerMock(TEST_FILES.newFile); + }); + + it('should remove newly uploaded files without confirmation', () => { + const screen = renderEditPost(); + triggerFileUpload(screen); + triggerFileRemoval(screen, TEST_FILES.newFile.id); + expect(Alert.alert).not.toHaveBeenCalled(); + expect(DraftEditPostUploadManager.cancel).toHaveBeenCalledWith(TEST_FILES.newFile.clientId); + }); + + it('should show confirmation dialog for existing files', () => { + const screen = renderEditPost(); + triggerFileRemoval(screen, TEST_FILES.existingFile1.id); + expect(Alert.alert).toHaveBeenCalledWith( + ERROR_MESSAGES.confirmDelete, + 'Are you sure you want to remove test-1?', + expect.any(Array), + ); + }); }); }); diff --git a/app/screens/edit_post/edit_post.tsx b/app/screens/edit_post/edit_post.tsx index ee67669ed..1964630f3 100644 --- a/app/screens/edit_post/edit_post.tsx +++ b/app/screens/edit_post/edit_post.tsx @@ -3,11 +3,13 @@ import React, {useCallback, useEffect, useRef, useState} from 'react'; import {useIntl} from 'react-intl'; -import {Alert, Keyboard, type LayoutChangeEvent, Platform, SafeAreaView, View, StyleSheet} from 'react-native'; +import {Alert, Keyboard, type LayoutChangeEvent, Platform, View, StyleSheet} from 'react-native'; +import {SafeAreaView, type Edge} from 'react-native-safe-area-context'; import {deletePost, editPost} from '@actions/remote/post'; import Autocomplete from '@components/autocomplete'; import Loading from '@components/loading'; +import {QUICK_ACTIONS_HEIGHT} from '@components/post_draft/quick_actions/quick_actions'; import {EditPostProvider} from '@context/edit_post'; import {ExtraKeyboardProvider} from '@context/extra_keyboard'; import {useServerUrl} from '@context/server'; @@ -16,16 +18,20 @@ import useAndroidHardwareBackHandler from '@hooks/android_back_handler'; import {useAutocompleteDefaultAnimatedValues} from '@hooks/autocomplete'; import {useKeyboardOverlap} from '@hooks/device'; import useDidUpdate from '@hooks/did_update'; +import useFileUploadError from '@hooks/file_upload_error'; import {useInputPropagation} from '@hooks/input'; import useNavButtonPressed from '@hooks/navigation_button_pressed'; +import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import SecurityManager from '@managers/security_manager'; import PostError from '@screens/edit_post/post_error'; import {buildNavigationButton, dismissModal, setButtons} from '@screens/navigation'; +import {fileMaxWarning, fileSizeWarning, uploadDisabledWarning} from '@utils/file'; import {changeOpacity} from '@utils/theme'; import EditPostInput from './edit_post_input'; import type {PasteInputRef} from '@mattermost/react-native-paste-input'; +import type {ErrorHandlers} from '@typings/components/upload_error_handlers'; import type PostModel from '@typings/database/models/servers/post'; import type {AvailableScreens} from '@typings/screens/navigation'; @@ -50,6 +56,9 @@ const styles = StyleSheet.create({ const RIGHT_BUTTON = buildNavigationButton('edit-post', 'edit_post.save.button'); +// Exclude bottom edge from SafeAreaView to prevent gap between attachments and keyboard. +const safeAreaEdges: Edge[] = ['top', 'left', 'right']; + type EditPostProps = { componentId: AvailableScreens; closeButtonId: string; @@ -58,8 +67,23 @@ type EditPostProps = { hasFilesAttached: boolean; canDelete: boolean; files?: FileInfo[]; + maxFileCount: number; + maxFileSize: number; + canUploadFiles: boolean; } -const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttached, canDelete, files}: EditPostProps) => { + +const EditPost = ({ + componentId, + maxPostSize, + post, + closeButtonId, + hasFilesAttached, + canDelete, + files, + maxFileCount, + maxFileSize, + canUploadFiles, +}: EditPostProps) => { const editingMessage = post.messageSource || post.message; const [postMessage, setPostMessage] = useState(editingMessage); const [cursorPosition, setCursorPosition] = useState(editingMessage.length); @@ -71,6 +95,7 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach const [postFiles, setPostFiles] = useState(files || []); const mainView = useRef(null); + const uploadErrorHandlers = useRef({}); const postInputRef = useRef(undefined); const theme = useTheme(); @@ -78,6 +103,27 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach const serverUrl = useServerUrl(); const shouldDeleteOnSave = !postMessage && canDelete && !hasFilesAttached; + const {uploadError, newUploadError} = useFileUploadError(); + + const shouldEnableSaveButton = useCallback(() => { + const loadingFiles = postFiles.filter((v) => v.clientId && DraftEditPostUploadManager.isUploading(v.clientId)); + const hasUploadingFiles = loadingFiles.length > 0; + + const tooLong = postMessage.trim().length > maxPostSize; + + const messageChanged = editingMessage !== postMessage; + + const originalFiles = files || []; + const originalFileIds = originalFiles.map((f) => f.id).sort(); + const currentFileIds = postFiles.map((f) => f.id).filter((id) => id).sort(); + const filesChanged = JSON.stringify(originalFileIds) !== JSON.stringify(currentFileIds); + + // Enable save button if: + // 1. No files are uploading AND + // 2. No message length error AND + // 3. (Message changed OR files changed) + return !hasUploadingFiles && !tooLong && (messageChanged || filesChanged); + }, [postMessage, postFiles, editingMessage, maxPostSize, files]); useEffect(() => { toggleSaveButton(false); @@ -121,43 +167,164 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach }); }, [componentId, intl, theme]); - const handleFileRemoval = useCallback((id: string) => { - const filterFileById = (file: FileInfo) => { - return file.id !== id; + const updateFileInPostFiles = useCallback((updatedFile: FileInfo) => { + const hasSameClientId = (file: FileInfo) => { + return file.clientId === updatedFile.clientId; }; - Alert.alert( - intl.formatMessage({ - id: 'edit_post.delete_file.title', - defaultMessage: 'Delete attachment', - }), - intl.formatMessage({ - id: 'edit_post.delete_file.confirmation', - defaultMessage: 'Are you sure you want to remove {filename}?', - }, { - filename: postFiles?.find((file) => file.id === id)?.name || '', - }), - [ - { - text: intl.formatMessage({ - id: 'edit_post.delete_file.cancel', - defaultMessage: 'Cancel', - }), - style: 'cancel', - }, - { - text: intl.formatMessage({ - id: 'edit_post.delete_file.confirm', - defaultMessage: 'Delete', - }), - style: 'destructive', - onPress: () => { - setPostFiles((prevFiles) => prevFiles?.filter(filterFileById) || []); - toggleSaveButton(true); + + setPostFiles((prevFiles) => { + const fileIndex = prevFiles.findIndex(hasSameClientId); + if (fileIndex >= 0) { + const newFiles = [...prevFiles]; + newFiles[fileIndex] = updatedFile; + return newFiles; + } + return prevFiles; + }); + }, []); + + const addFiles = useCallback((newFiles: FileInfo[]) => { + if (!newFiles.length) { + return; + } + + if (!canUploadFiles) { + newUploadError(uploadDisabledWarning(intl)); + return; + } + + const currentFileCount = postFiles?.length || 0; + const availableCount = maxFileCount - currentFileCount; + if (newFiles.length > availableCount) { + newUploadError(fileMaxWarning(intl, maxFileCount)); + return; + } + + const largeFile = newFiles.find((file) => file.size > maxFileSize); + if (largeFile) { + newUploadError(fileSizeWarning(intl, maxFileSize)); + return; + } + + setPostFiles((prevFiles) => [...prevFiles, ...newFiles]); + + // Start uploads for new files using the upload manager + for (const file of newFiles) { + DraftEditPostUploadManager.prepareUpload( + serverUrl, + file, + post.channelId, + post.rootId, + 0, + true, // isEditPost = true + updateFileInPostFiles, + ); + uploadErrorHandlers.current[file.clientId!] = DraftEditPostUploadManager.registerErrorHandler(file.clientId!, newUploadError); + } + + newUploadError(null); + }, [canUploadFiles, postFiles?.length, maxFileCount, newUploadError, intl, maxFileSize, serverUrl, post.channelId, post.rootId, updateFileInPostFiles]); + + const handleFileRemoval = useCallback((id: string) => { + + const fileToRemove = postFiles?.find((file) => { + if (file.id && id) { + return file.id === id; + } + if (file.clientId && id) { + return file.clientId === id; + } + return false; + }); + + if (!fileToRemove) { + return; + } + + const shouldKeepFile = (file: FileInfo) => { + if (fileToRemove.id && file.id) { + return file.id !== fileToRemove.id; + } + if (fileToRemove.clientId && file.clientId) { + return file.clientId !== fileToRemove.clientId; + } + return file !== fileToRemove; + }; + + const removeFileAction = () => { + if (fileToRemove.clientId && DraftEditPostUploadManager.isUploading(fileToRemove.clientId)) { + DraftEditPostUploadManager.cancel(fileToRemove.clientId); + if (uploadErrorHandlers.current[fileToRemove.clientId]) { + uploadErrorHandlers.current[fileToRemove.clientId]?.(); + delete uploadErrorHandlers.current[fileToRemove.clientId]; + } + } + + // Remove the specific file by using a unique identifier combination + setPostFiles((prevFiles) => prevFiles?.filter(shouldKeepFile) || []); + }; + + const originalFiles = files || []; + const isNewlyUploadedFile = !originalFiles.some((originalFile) => { + return originalFile.id === fileToRemove.id; + }); + + if (isNewlyUploadedFile) { + removeFileAction(); + } else { + Alert.alert( + intl.formatMessage({ + id: 'edit_post.delete_file.title', + defaultMessage: 'Delete attachment', + }), + intl.formatMessage({ + id: 'edit_post.delete_file.confirmation', + defaultMessage: 'Are you sure you want to remove {filename}?', + }, { + filename: fileToRemove.name || '', + }), + [ + { + text: intl.formatMessage({ + id: 'edit_post.delete_file.cancel', + defaultMessage: 'Cancel', + }), + style: 'cancel', }, - }, - ], - ); - }, [intl, toggleSaveButton, postFiles]); + { + text: intl.formatMessage({ + id: 'edit_post.delete_file.confirm', + defaultMessage: 'Delete', + }), + style: 'destructive', + onPress: removeFileAction, + }, + ], + ); + } + }, [intl, postFiles, files]); + + useEffect(() => { + let loadingFiles: FileInfo[] = []; + if (postFiles) { + loadingFiles = postFiles.filter((v) => v.clientId && DraftEditPostUploadManager.isUploading(v.clientId)); + } + + toggleSaveButton(shouldEnableSaveButton()); + + for (const key of Object.keys(uploadErrorHandlers.current)) { + if (!loadingFiles.find((v) => v.clientId === key)) { + uploadErrorHandlers.current[key]?.(); + delete uploadErrorHandlers.current[key]; + } + } + + for (const file of loadingFiles) { + if (file.clientId && !uploadErrorHandlers.current[file.clientId]) { + uploadErrorHandlers.current[file.clientId] = DraftEditPostUploadManager.registerErrorHandler(file.clientId, newUploadError); + } + } + }, [postFiles, postMessage, newUploadError, shouldEnableSaveButton, toggleSaveButton]); const onChangeTextCommon = useCallback((message: string) => { const tooLong = message.trim().length > maxPostSize; @@ -169,8 +336,7 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach setErrorLine(line); setErrorExtra(extra); } - toggleSaveButton(editingMessage !== message && !tooLong); - }, [intl, maxPostSize, editingMessage, toggleSaveButton]); + }, [intl, maxPostSize]); const onAutocompleteChangeText = useCallback((message: string) => { setPostMessage(message); @@ -253,7 +419,7 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach useAndroidHardwareBackHandler(componentId, onClose); const overlap = useKeyboardOverlap(mainView, containerHeight); - const autocompletePosition = overlap + AUTOCOMPLETE_SEPARATION; + const autocompletePosition = overlap + AUTOCOMPLETE_SEPARATION + QUICK_ACTIONS_HEIGHT; const autocompleteAvailableSpace = containerHeight - autocompletePosition; const [animatedAutocompletePosition, animatedAutocompleteAvailableSpace] = useAutocompleteDefaultAnimatedValues(autocompletePosition, autocompleteAvailableSpace); @@ -270,10 +436,15 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach } return ( - + @@ -297,6 +468,8 @@ const EditPost = ({componentId, maxPostSize, post, closeButtonId, hasFilesAttach inputRef={postInputRef} post={post} postFiles={postFiles} + addFiles={addFiles} + uploadFileError={uploadError} /> diff --git a/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx b/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx index 60f98c512..80914b2c8 100644 --- a/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx +++ b/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx @@ -55,6 +55,8 @@ describe('EditPostInput', () => { inputRef: {current: undefined}, onTextSelectionChange: jest.fn(), onChangeText: jest.fn(), + addFiles: jest.fn(), + uploadFileError: undefined, }; beforeAll(async () => { @@ -84,4 +86,32 @@ describe('EditPostInput', () => { const {queryByTestId} = renderWithEverything(, {database, serverUrl}); expect(queryByTestId('uploads')).toBeNull(); }); + + it('should not render quick actions if the server version is less than 10.5.0', () => { + const props = {...baseProps, version: '10.4.0'}; + const {queryByTestId} = renderWithEverything(, {database, serverUrl}); + expect(queryByTestId('edit_post.quick_actions')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions.at_input_action')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions.file_action')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions.image_action')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions.camera_action')).toBeNull(); + }); + + it('should not render both uploads and quick actions when server version is unsupported', () => { + const props = {...baseProps, version: '9.8.0'}; + const {queryByTestId} = renderWithEverything(, {database, serverUrl}); + expect(queryByTestId('uploads')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions')).toBeNull(); + }); + + it('should render quick actions in edit mode', () => { + const {getByTestId, queryByTestId} = renderWithEverything(, {database, serverUrl}); + expect(getByTestId('edit_post.quick_actions')).toBeVisible(); + expect(getByTestId('edit_post.quick_actions.at_input_action')).toBeVisible(); + expect(getByTestId('edit_post.quick_actions.file_action')).toBeVisible(); + expect(getByTestId('edit_post.quick_actions.image_action')).toBeVisible(); + expect(getByTestId('edit_post.quick_actions.camera_action')).toBeVisible(); + expect(queryByTestId('edit_post.quick_actions.slash_input_action')).toBeNull(); + expect(queryByTestId('edit_post.quick_actions.post_priority_action')).toBeNull(); + }); }); diff --git a/app/screens/edit_post/edit_post_input/edit_post_input.tsx b/app/screens/edit_post/edit_post_input/edit_post_input.tsx index 2e7cda92e..d830d098c 100644 --- a/app/screens/edit_post/edit_post_input/edit_post_input.tsx +++ b/app/screens/edit_post/edit_post_input/edit_post_input.tsx @@ -7,6 +7,8 @@ import React, {useCallback, useMemo} from 'react'; import {useIntl} from 'react-intl'; import {type NativeSyntheticEvent, Platform, type TextInputSelectionChangeEventData, View} from 'react-native'; +import QuickActions from '@components/post_draft/quick_actions/'; +import {INITIAL_PRIORITY} from '@components/post_draft/send_handler/send_handler'; import Uploads from '@components/post_draft/uploads'; import {ExtraKeyboard, useExtraKeyboardContext} from '@context/extra_keyboard'; import {useTheme} from '@context/theme'; @@ -44,6 +46,8 @@ type PostInputProps = { onTextSelectionChange: (curPos: number) => void; onChangeText: (text: string) => void; inputRef: React.MutableRefObject; + addFiles: (file: FileInfo[]) => void; + uploadFileError: React.ReactNode; } const EditPostInput = ({ @@ -55,12 +59,26 @@ const EditPostInput = ({ postFiles, version, inputRef, + addFiles, + uploadFileError, }: PostInputProps) => { const intl = useIntl(); const theme = useTheme(); const styles = getStyleSheet(theme); const managedConfig = useManagedConfig(); const disableCopyAndPaste = managedConfig.copyAndPasteProtection === 'true'; + const focus = useCallback(() => { + inputRef.current?.focus(); + }, []); + + const updateValue = useCallback((valueOrUpdater: string | ((prevValue: string) => string)) => { + if (typeof valueOrUpdater === 'function') { + const newValue = valueOrUpdater(message); + onChangeText(newValue); + } else { + onChangeText(valueOrUpdater); + } + }, [message, onChangeText]); const keyboardContext = useExtraKeyboardContext(); @@ -110,14 +128,27 @@ const EditPostInput = ({ onBlur={onBlur} /> {isMinimumServerVersion(version, MAJOR_VERSION_TO_SHOW_ATTACHMENTS, MINOR_VERSION_TO_SHOW_ATTACHMENTS) && - + <> + + + } {Platform.select({ios: })} diff --git a/app/screens/edit_post/index.test.tsx b/app/screens/edit_post/index.test.tsx new file mode 100644 index 000000000..c597467a5 --- /dev/null +++ b/app/screens/edit_post/index.test.tsx @@ -0,0 +1,76 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {Screens} from '@constants'; +import {SYSTEM_IDENTIFIERS} from '@constants/database'; +import DatabaseManager from '@database/manager'; +import {renderWithEverything, waitFor} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import EditPost from './edit_post'; + +import EnhancedEditPost from './index'; + +import type ServerDataOperator from '@database/operator/server_data_operator'; +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('./edit_post', () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.mocked(EditPost).mockImplementation((props) => React.createElement('EditPost', {...props, testID: 'edit-post'})); + +describe('EditPost', () => { + const serverUrl = 'server-1'; + const teamId = 'team1'; + let database: Database; + let operator: ServerDataOperator; + + const baseProps: Parameters[0] = { + componentId: Screens.EDIT_POST, + closeButtonId: 'close-button', + post: TestHelper.fakePostModel({id: 'post-1'}), + canDelete: true, + files: [], + }; + + beforeEach(async () => { + await DatabaseManager.init([serverUrl]); + const serverDatabaseAndOperator = DatabaseManager.getServerDatabaseAndOperator(serverUrl); + database = serverDatabaseAndOperator.database; + operator = serverDatabaseAndOperator.operator; + + await operator.handleSystem({systems: [{id: SYSTEM_IDENTIFIERS.CURRENT_TEAM_ID, value: teamId}], prepareRecordsOnly: false}); + await operator.handleTeam({teams: [TestHelper.fakeTeam({id: teamId})], prepareRecordsOnly: false}); + await operator.handleConfigs({ + configs: [ + {id: 'Version', value: '10.6.0'}, + {id: 'MaxFileCount', value: '10'}, + {id: 'MaxFileSize', value: '1000'}, + {id: 'CanUploadFiles', value: 'true'}, + ], + configsToDelete: [], + prepareRecordsOnly: false, + }); + }); + + afterEach(async () => { + await DatabaseManager.destroyServerDatabase(serverUrl); + }); + + it('should pass props to the edit post component', async () => { + const {getByTestId} = renderWithEverything(, {database, serverUrl}); + + await waitFor(() => { + const editPost = getByTestId('edit-post'); + expect(editPost).toBeDefined(); + expect(editPost.props.maxFileCount).toBe(10); + expect(editPost.props.maxFileSize).toBe(1000); + expect(editPost.props.canUploadFiles).toBe(true); + expect(editPost.props.hasFilesAttached).toBe(false); + }); + }); +}); diff --git a/app/screens/edit_post/index.tsx b/app/screens/edit_post/index.tsx index 21d257bb0..554f1524e 100644 --- a/app/screens/edit_post/index.tsx +++ b/app/screens/edit_post/index.tsx @@ -5,9 +5,10 @@ import {withDatabase, withObservables} from '@nozbe/watermelondb/react'; import {of as of$} from 'rxjs'; import {switchMap} from 'rxjs/operators'; -import {MAX_MESSAGE_LENGTH_FALLBACK} from '@constants/post_draft'; +import {DEFAULT_SERVER_MAX_FILE_SIZE, MAX_MESSAGE_LENGTH_FALLBACK} from '@constants/post_draft'; import {observeFilesForPost} from '@queries/servers/file'; -import {observeConfigIntValue} from '@queries/servers/system'; +import {observeCanUploadFiles} from '@queries/servers/security'; +import {observeConfigIntValue, observeMaxFileCount} from '@queries/servers/system'; import EditPost from './edit_post'; @@ -16,12 +17,18 @@ import type PostModel from '@typings/database/models/servers/post'; const enhance = withObservables([], ({database, post}: WithDatabaseArgs & { post: PostModel}) => { const maxPostSize = observeConfigIntValue(database, 'MaxPostSize', MAX_MESSAGE_LENGTH_FALLBACK); + const canUploadFiles = observeCanUploadFiles(database); + const maxFileSize = observeConfigIntValue(database, 'MaxFileSize', DEFAULT_SERVER_MAX_FILE_SIZE); + const maxFileCount = observeMaxFileCount(database); const hasFilesAttached = observeFilesForPost(database, post.id).pipe(switchMap((files) => of$(files?.length > 0))); return { maxPostSize, hasFilesAttached, + canUploadFiles, + maxFileSize, + maxFileCount, }; }); diff --git a/types/components/upload_error_handlers.ts b/types/components/upload_error_handlers.ts new file mode 100644 index 000000000..29027cb74 --- /dev/null +++ b/types/components/upload_error_handlers.ts @@ -0,0 +1,6 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +export type ErrorHandlers = { + [clientId: string]: (() => void) | undefined; +}