diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 9e06930afa7c..baf3daaa8e28 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -842,12 +842,43 @@ function addActions(reportID: string, notifyReportID: string, timezoneParam: Tim notifyNewAction(notifyReportID, lastAction?.actorAccountID, lastAction); } -/** Add an attachment and optional comment. */ -function addAttachment(reportID: string, notifyReportID: string, file: FileObject, timezoneParam: Timezone, text = '', shouldPlaySound?: boolean) { - if (shouldPlaySound) { +/** Add an attachment with an optional comment to a report */ +function addAttachmentWithComment( + reportID: string, + notifyReportID: string, + attachments: FileObject | FileObject[], + text = '', + timezone: Timezone = CONST.DEFAULT_TIME_ZONE, + shouldPlaySound = false, +) { + if (!reportID) { + return; + } + + const handlePlaySound = () => { + if (!shouldPlaySound) { + return; + } playSound(SOUNDS.DONE); + }; + + // Single attachment + if (!Array.isArray(attachments)) { + addActions(reportID, notifyReportID, timezone, text, attachments); + handlePlaySound(); + return; } - addActions(reportID, notifyReportID, timezoneParam, text, file); + + // Multiple attachments - first: combine text + first attachment as a single action + addActions(reportID, notifyReportID, timezone, text, attachments?.at(0)); + + // Remaining: attachment-only actions (no text duplication) + for (let i = 1; i < attachments?.length; i += 1) { + addActions(reportID, notifyReportID, timezone, '', attachments?.at(i)); + } + + // Play sound once + handlePlaySound(); } /** Add a single comment to a report */ @@ -6106,7 +6137,7 @@ function resolveConciergeCategoryOptions( export type {Video, GuidedSetupData, TaskForParameters, IntroSelected}; export { - addAttachment, + addAttachmentWithComment, addComment, addPolicyReport, broadcastUserIsLeavingRoom, diff --git a/src/pages/Share/ShareDetailsPage.tsx b/src/pages/Share/ShareDetailsPage.tsx index 5e7352be78b7..faa42e5705d5 100644 --- a/src/pages/Share/ShareDetailsPage.tsx +++ b/src/pages/Share/ShareDetailsPage.tsx @@ -19,7 +19,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import useThemeStyles from '@hooks/useThemeStyles'; -import {addAttachment, addComment, getCurrentUserAccountID, openReport} from '@libs/actions/Report'; +import {addAttachmentWithComment, addComment, getCurrentUserAccountID, openReport} from '@libs/actions/Report'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import {getFileName, readFileAsync} from '@libs/fileDownload/FileUtils'; import Navigation from '@libs/Navigation/Navigation'; @@ -128,7 +128,7 @@ function ShareDetailsPage({ ); } if (report.reportID) { - addAttachment(report.reportID, report.reportID, file, personalDetail.timezone ?? CONST.DEFAULT_TIME_ZONE, message); + addAttachmentWithComment(report.reportID, report.reportID, file, message, personalDetail.timezone); } const routeToNavigate = ROUTES.REPORT_WITH_ID.getRoute(reportOrAccountID); diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index 4074ba0413ff..429792640235 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -62,7 +62,7 @@ import ReportTypingIndicator from '@pages/home/report/ReportTypingIndicator'; import type {FileObject} from '@pages/media/AttachmentModalScreen/types'; import {hideEmojiPicker, isActive as isActiveEmojiPickerAction} from '@userActions/EmojiPickerAction'; import {initMoneyRequest, replaceReceipt, setMoneyRequestParticipantsFromReport, setMoneyRequestReceipt} from '@userActions/IOU'; -import {addAttachment as addAttachmentReportActions, setIsComposerFullSize} from '@userActions/Report'; +import {addAttachmentWithComment, setIsComposerFullSize} from '@userActions/Report'; import Timing from '@userActions/Timing'; import {buildOptimisticTransactionAndCreateDraft} from '@userActions/TransactionEdit'; import {isBlockedFromConcierge as isBlockedFromConciergeUserAction} from '@userActions/User'; @@ -326,22 +326,7 @@ function ReportActionCompose({ const newCommentTrimmed = newComment.trim(); if (attachmentFileRef.current) { - if (Array.isArray(attachmentFileRef.current)) { - // Handle multiple files - attachmentFileRef.current.forEach((file) => { - addAttachmentReportActions(transactionThreadReportID ?? reportID, reportID, file, personalDetail.timezone ?? CONST.DEFAULT_TIME_ZONE, newCommentTrimmed, true); - }); - } else { - // Handle single file - addAttachmentReportActions( - transactionThreadReportID ?? reportID, - reportID, - attachmentFileRef.current, - personalDetail.timezone ?? CONST.DEFAULT_TIME_ZONE, - newCommentTrimmed, - true, - ); - } + addAttachmentWithComment(transactionThreadReportID ?? reportID, reportID, attachmentFileRef.current, newCommentTrimmed, personalDetail.timezone, true); attachmentFileRef.current = null; } else { Performance.markStart(CONST.TIMING.SEND_MESSAGE, {message: newCommentTrimmed}); diff --git a/src/pages/settings/AboutPage/ShareLogList/index.native.tsx b/src/pages/settings/AboutPage/ShareLogList/index.native.tsx index d511e827f4a0..d132c72fa5ed 100644 --- a/src/pages/settings/AboutPage/ShareLogList/index.native.tsx +++ b/src/pages/settings/AboutPage/ShareLogList/index.native.tsx @@ -1,8 +1,7 @@ import React from 'react'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import Navigation from '@libs/Navigation/Navigation'; -import {addAttachment} from '@userActions/Report'; -import CONST from '@src/CONST'; +import {addAttachmentWithComment} from '@userActions/Report'; import ROUTES from '@src/ROUTES'; import BaseShareLogList from './BaseShareLogList'; import type {ShareLogListProps} from './types'; @@ -14,7 +13,7 @@ function ShareLogList({logSource}: ShareLogListProps) { return; } const src = `file://${logSource}`; - addAttachment(reportID, reportID, {name: filename, source: src, uri: src, type: 'text/plain'} as File, personalDetail.timezone ?? CONST.DEFAULT_TIME_ZONE); + addAttachmentWithComment(reportID, reportID, {name: filename, source: src, uri: src, type: 'text/plain'} as File, undefined, personalDetail.timezone); const routeToNavigate = ROUTES.REPORT_WITH_ID.getRoute(reportID); Navigation.navigate(routeToNavigate); diff --git a/src/pages/settings/AboutPage/ShareLogList/index.tsx b/src/pages/settings/AboutPage/ShareLogList/index.tsx index e45d4aeff905..9d127bc355aa 100644 --- a/src/pages/settings/AboutPage/ShareLogList/index.tsx +++ b/src/pages/settings/AboutPage/ShareLogList/index.tsx @@ -2,8 +2,7 @@ import React from 'react'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import {readFileAsync} from '@libs/fileDownload/FileUtils'; import Navigation from '@libs/Navigation/Navigation'; -import {addAttachment} from '@userActions/Report'; -import CONST from '@src/CONST'; +import {addAttachmentWithComment} from '@userActions/Report'; import ROUTES from '@src/ROUTES'; import BaseShareLogList from './BaseShareLogList'; import type {ShareLogListProps} from './types'; @@ -15,7 +14,7 @@ function ShareLogList({logSource}: ShareLogListProps) { logSource, filename, (file) => { - addAttachment(reportID, reportID, file, personalDetail?.timezone ?? CONST.DEFAULT_TIME_ZONE); + addAttachmentWithComment(reportID, reportID, file, undefined, personalDetail?.timezone); const routeToNavigate = ROUTES.REPORT_WITH_ID.getRoute(reportID); Navigation.navigate(routeToNavigate); }, diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 88b4d946bd0f..9accc97833ba 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -10,6 +10,7 @@ import {WRITE_COMMANDS} from '@libs/API/types'; import HttpUtils from '@libs/HttpUtils'; import {buildNextStep} from '@libs/NextStepUtils'; import {getOriginalMessage} from '@libs/ReportActionsUtils'; +import playSound, {SOUNDS} from '@libs/Sound'; import CONST from '@src/CONST'; import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; import * as PersistedRequests from '@src/libs/actions/PersistedRequests'; @@ -74,6 +75,12 @@ jest.mock('@hooks/useScreenWrapperTransitionStatus', () => ({ }), })); +jest.mock('@libs/Sound', () => ({ + __esModule: true, + default: jest.fn(), + SOUNDS: {DONE: 'DONE'}, +})); + const originalXHR = HttpUtils.xhr; OnyxUpdateManager(); describe('actions/Report', () => { @@ -1078,7 +1085,7 @@ describe('actions/Report', () => { await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); const file = new File([''], 'test.txt', {type: 'text/plain'}); - Report.addAttachment(REPORT_ID, REPORT_ID, file, CONST.DEFAULT_TIME_ZONE); + Report.addAttachmentWithComment(REPORT_ID, REPORT_ID, file); // Need the reportActionID to delete the comments const newComment = PersistedRequests.getAll().at(0); @@ -1147,7 +1154,7 @@ describe('actions/Report', () => { await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); - Report.addAttachment(REPORT_ID, REPORT_ID, file, CONST.DEFAULT_TIME_ZONE, 'Attachment with comment'); + Report.addAttachmentWithComment(REPORT_ID, REPORT_ID, file, 'Attachment with comment'); // Need the reportActionID to delete the comments const newComment = PersistedRequests.getAll().at(0); @@ -1205,6 +1212,103 @@ describe('actions/Report', () => { TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.DELETE_COMMENT, 0); }); + it('should post text + attachment as first action then attachment only for remaining attachments when adding multiple attachments with a comment', async () => { + global.fetch = TestHelper.getGlobalFetchMock(); + const playSoundMock = playSound as jest.MockedFunction; + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + await waitForBatchedUpdates(); + + const relevantPromise = new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.PERSISTED_REQUESTS, + callback: (persisted) => { + const relevant = (persisted ?? []).filter((r) => r?.command === WRITE_COMMANDS.ADD_ATTACHMENT || r?.command === WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT); + if (relevant.length >= 3) { + Onyx.disconnect(conn); + resolve(relevant); + } + }, + }); + }); + + const REPORT_ID = '1'; + const shouldPlaySound = true; + const fileA = new File(['a'], 'a.txt', {type: 'text/plain'}); + const fileB = new File(['b'], 'b.txt', {type: 'text/plain'}); + const fileC = new File(['c'], 'c.txt', {type: 'text/plain'}); + + Report.addAttachmentWithComment(REPORT_ID, REPORT_ID, [fileA, fileB, fileC], 'Hello world', CONST.DEFAULT_TIME_ZONE, shouldPlaySound); + const relevant = (await relevantPromise) as OnyxTypes.Request[]; + + expect(playSoundMock).toHaveBeenCalledTimes(1); + expect(playSoundMock).toHaveBeenCalledWith(SOUNDS.DONE); + expect(relevant.at(0)?.command).toBe(WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT); + expect(relevant.slice(1).every((r) => r.command === WRITE_COMMANDS.ADD_ATTACHMENT)).toBe(true); + }); + + it('should create attachment only actions when adding multiple attachments without a comment', async () => { + global.fetch = TestHelper.getGlobalFetchMock(); + const playSoundMock = playSound as jest.MockedFunction; + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + await waitForBatchedUpdates(); + + const relevantPromise = new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.PERSISTED_REQUESTS, + callback: (persisted) => { + const relevant = (persisted ?? []).filter((r) => r?.command === WRITE_COMMANDS.ADD_ATTACHMENT || r?.command === WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT); + if (relevant.length >= 2) { + Onyx.disconnect(conn); + resolve(relevant); + } + }, + }); + }); + + const REPORT_ID = '1'; + const shouldPlaySound = true; + const fileA = new File(['a'], 'a.txt', {type: 'text/plain'}); + const fileB = new File(['b'], 'b.txt', {type: 'text/plain'}); + + Report.addAttachmentWithComment(REPORT_ID, REPORT_ID, [fileA, fileB], undefined, CONST.DEFAULT_TIME_ZONE, shouldPlaySound); + const relevant = (await relevantPromise) as OnyxTypes.Request[]; + + expect(playSoundMock).toHaveBeenCalledTimes(1); + expect(playSoundMock).toHaveBeenCalledWith(SOUNDS.DONE); + expect(relevant.at(0)?.command).toBe(WRITE_COMMANDS.ADD_ATTACHMENT); + expect(relevant.slice(1).every((r) => r.command === WRITE_COMMANDS.ADD_ATTACHMENT)).toBe(true); + expect(relevant.some((r) => r.command === WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT)).toBe(false); + }); + + it('should create attachment only action & not play sound when adding attachment without a comment & shouldPlaySound not passed', async () => { + global.fetch = TestHelper.getGlobalFetchMock(); + const playSoundMock = playSound as jest.MockedFunction; + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + await waitForBatchedUpdates(); + + const relevantPromise = new Promise((resolve) => { + const conn = Onyx.connect({ + key: ONYXKEYS.PERSISTED_REQUESTS, + callback: (persisted) => { + const relevant = (persisted ?? []).filter((r) => r?.command === WRITE_COMMANDS.ADD_ATTACHMENT); + if (relevant.length >= 1) { + Onyx.disconnect(conn); + resolve(relevant); + } + }, + }); + }); + + const REPORT_ID = '1'; + const file = new File(['a'], 'a.txt', {type: 'text/plain'}); + + Report.addAttachmentWithComment(REPORT_ID, REPORT_ID, file); + const relevant = (await relevantPromise) as OnyxTypes.Request[]; + + expect(playSoundMock).toHaveBeenCalledTimes(0); + expect(relevant.at(0)?.command).toBe(WRITE_COMMANDS.ADD_ATTACHMENT); + }); + it('should not send DeleteComment request and remove any Reactions accordingly', async () => { global.fetch = TestHelper.getGlobalFetchMock(); // eslint-disable-next-line @typescript-eslint/no-unsafe-return