From 00c81de74981ff189e3fe76b3be415c802e2ff56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 27 Feb 2024 17:54:47 +0100 Subject: [PATCH 01/26] remove hasDraft from Report model --- .../LHNOptionsList/OptionRowLHNData.tsx | 13 +------------ src/libs/actions/Policy.ts | 9 ++------- src/libs/actions/Report.ts | 6 ------ .../ComposerWithSuggestions.js | 16 ---------------- src/pages/home/sidebar/SidebarLinksData.js | 1 - src/types/onyx/Report.ts | 1 - 6 files changed, 3 insertions(+), 43 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index a18d5a8ec1ec..7f7e2b4232ca 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -1,8 +1,7 @@ import {deepEqual} from 'fast-equals'; -import React, {useEffect, useMemo, useRef} from 'react'; +import React, {useMemo, useRef} from 'react'; import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; -import * as Report from '@userActions/Report'; import CONST from '@src/CONST'; import type {OptionData} from '@src/libs/ReportUtils'; import OptionRowLHN from './OptionRowLHN'; @@ -30,8 +29,6 @@ function OptionRowLHNData({ canUseViolations, ...propsToForward }: OptionRowLHNDataProps) { - const reportID = propsToForward.reportID; - const optionItemRef = useRef(); const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(fullReport, transactionViolations, parentReportAction ?? null); @@ -71,14 +68,6 @@ function OptionRowLHNData({ receiptTransactions, ]); - useEffect(() => { - if (!optionItem || !!optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) { - return; - } - Report.setReportWithDraft(reportID, true); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - return ( { - const {reportID, stateNum, statusNum, hasDraft, oldPolicyName} = report ?? {}; + const {reportID, stateNum, statusNum, oldPolicyName} = report ?? {}; failureData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { stateNum, statusNum, - hasDraft, oldPolicyName, }, }); @@ -547,7 +544,6 @@ function removeMembers(accountIDs: number[], policyID: string) { statusNum: CONST.REPORT.STATUS_NUM.CLOSED, stateNum: CONST.REPORT.STATE_NUM.APPROVED, oldPolicyName: policy.name, - hasDraft: false, }, }); }); @@ -602,14 +598,13 @@ function removeMembers(accountIDs: number[], policyID: string) { ...announceRoomMembers.onyxFailureData, ]; - filteredWorkspaceChats.forEach(({reportID, stateNum, statusNum, hasDraft, oldPolicyName = null}) => { + filteredWorkspaceChats.forEach(({reportID, stateNum, statusNum, oldPolicyName = null}) => { failureData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { stateNum, statusNum, - hasDraft, oldPolicyName, }, }); diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index f29f8a4fbaab..0bf6074909a0 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -1051,11 +1051,6 @@ function saveReportCommentNumberOfLines(reportID: string, numberOfLines: number) Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT_NUMBER_OF_LINES}${reportID}`, numberOfLines); } -/** Immediate indication whether the report has a draft comment. */ -function setReportWithDraft(reportID: string, hasDraft: boolean): Promise { - return Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {hasDraft}); -} - /** Broadcasts whether or not a user is typing on a report over the report's private pusher channel. */ function broadcastUserIsTyping(reportID: string) { const privateReportChannelName = getReportChannelName(reportID); @@ -2923,7 +2918,6 @@ export { saveReportActionDraftNumberOfLines, deleteReportComment, navigateToConciergeChat, - setReportWithDraft, addPolicyReport, deleteReport, navigateToConciergeChatAndDeleteReport, diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js index 026df340040e..b702a66eab99 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js @@ -284,16 +284,6 @@ function ComposerWithSuggestions({ }); } - // Indicate that draft has been created. - if (commentRef.current.length === 0 && newCommentConverted.length !== 0) { - Report.setReportWithDraft(reportID, true); - } - - // The draft has been deleted. - if (newCommentConverted.length === 0) { - Report.setReportWithDraft(reportID, false); - } - commentRef.current = newCommentConverted; if (shouldDebounceSaveComment) { debouncedSaveReportComment(reportID, newCommentConverted); @@ -544,12 +534,6 @@ function ComposerWithSuggestions({ // Scrolls the composer to the bottom and sets the selection to the end, so that longer drafts are easier to edit updateMultilineInputRange(textInputRef.current, shouldAutoFocus); - if (value.length === 0) { - return; - } - - Report.setReportWithDraft(reportID, true); - // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index c4cc0713c596..621294b80044 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -223,7 +223,6 @@ const chatReportSelector = (report) => report && { reportID: report.reportID, participantAccountIDs: report.participantAccountIDs, - hasDraft: report.hasDraft, isPinned: report.isPinned, isHidden: report.isHidden, notificationPreference: report.notificationPreference, diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index bb86d2cf4ae4..a98b814292b2 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -121,7 +121,6 @@ type Report = OnyxCommon.OnyxValueWithOfflineFeedback< parentReportID?: string; parentReportActionID?: string; isOptimisticReport?: boolean; - hasDraft?: boolean; managerID?: number; lastVisibleActionLastModified?: string; displayName?: string; From d17731419574aeea8490ffc627236c17386f0470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 27 Feb 2024 17:57:51 +0100 Subject: [PATCH 02/26] remove reportWithoutHasDraftSelector --- src/libs/OnyxSelectors/reportWithoutHasDraftSelector.ts | 9 --------- src/pages/home/HeaderView.js | 2 -- src/pages/home/ReportScreen.js | 2 -- src/pages/home/report/ReportActionItemCreated.tsx | 2 -- 4 files changed, 15 deletions(-) delete mode 100644 src/libs/OnyxSelectors/reportWithoutHasDraftSelector.ts diff --git a/src/libs/OnyxSelectors/reportWithoutHasDraftSelector.ts b/src/libs/OnyxSelectors/reportWithoutHasDraftSelector.ts deleted file mode 100644 index 9c7e6402d69b..000000000000 --- a/src/libs/OnyxSelectors/reportWithoutHasDraftSelector.ts +++ /dev/null @@ -1,9 +0,0 @@ -import type {OnyxValue} from '@src/ONYXKEYS'; - -export default function reportWithoutHasDraftSelector(report: OnyxValue<'report_'>) { - if (!report) { - return report; - } - const {hasDraft, ...reportWithoutHasDraft} = report; - return reportWithoutHasDraft; -} diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index faa70bb0633a..c812845ea43a 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -26,7 +26,6 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import {getGroupChatName} from '@libs/GroupChatUtils'; import * as HeaderUtils from '@libs/HeaderUtils'; import Navigation from '@libs/Navigation/Navigation'; -import reportWithoutHasDraftSelector from '@libs/OnyxSelectors/reportWithoutHasDraftSelector'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -401,7 +400,6 @@ export default memo( }, parentReport: { key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report.parentReportID || report.reportID}`, - selector: reportWithoutHasDraftSelector, }, session: { key: ONYXKEYS.SESSION, diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 7fba188dcedd..ce723f6a2f79 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -25,7 +25,6 @@ import Timing from '@libs/actions/Timing'; import compose from '@libs/compose'; import Navigation from '@libs/Navigation/Navigation'; import clearReportNotifications from '@libs/Notification/clearReportNotifications'; -import reportWithoutHasDraftSelector from '@libs/OnyxSelectors/reportWithoutHasDraftSelector'; import Performance from '@libs/Performance'; import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; @@ -615,7 +614,6 @@ export default compose( report: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, allowStaleData: true, - selector: reportWithoutHasDraftSelector, }, reportMetadata: { key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, diff --git a/src/pages/home/report/ReportActionItemCreated.tsx b/src/pages/home/report/ReportActionItemCreated.tsx index 95578c10e816..786f422dbc3b 100644 --- a/src/pages/home/report/ReportActionItemCreated.tsx +++ b/src/pages/home/report/ReportActionItemCreated.tsx @@ -10,7 +10,6 @@ import useLocalize from '@hooks/useLocalize'; import useStyleUtils from '@hooks/useStyleUtils'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; -import reportWithoutHasDraftSelector from '@libs/OnyxSelectors/reportWithoutHasDraftSelector'; import * as ReportUtils from '@libs/ReportUtils'; import {navigateToConciergeChatAndDeleteReport} from '@userActions/Report'; import CONST from '@src/CONST'; @@ -97,7 +96,6 @@ ReportActionItemCreated.displayName = 'ReportActionItemCreated'; export default withOnyx({ report: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - selector: reportWithoutHasDraftSelector, }, policy: { From eeec9ba7bb8a1afc58f36500ecf14cb2acd876d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 27 Feb 2024 18:18:29 +0100 Subject: [PATCH 03/26] use draft comment to calculate hasDraftComment --- src/components/LHNOptionsList/OptionRowLHNData.tsx | 1 + src/libs/SidebarUtils.ts | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index 7f7e2b4232ca..3cfc8f0e642d 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -43,6 +43,7 @@ function OptionRowLHNData({ policy, parentReportAction, hasViolations: !!hasViolations, + hasDraftComment: !!comment, }); if (deepEqual(item, optionItemRef.current)) { return optionItemRef.current; diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 35cf52a5ff99..d4d9de7a64d1 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -175,6 +175,7 @@ function getOptionData({ policy, parentReportAction, hasViolations, + hasDraftComment, }: { report: OnyxEntry; reportActions: OnyxEntry; @@ -183,6 +184,7 @@ function getOptionData({ policy: OnyxEntry | undefined; parentReportAction: OnyxEntry | undefined; hasViolations: boolean; + hasDraftComment: boolean; }): ReportUtils.OptionData | undefined { // When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for // this method to be called after the Onyx data has been cleared out. In that case, it's fine to do @@ -246,7 +248,7 @@ function getOptionData({ // setting it Unread so we add additional condition here to avoid empty chat LHN from being bold. result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID; result.isUnreadWithMention = ReportUtils.isUnreadWithMention(report); - result.hasDraftComment = report.hasDraft; + result.hasDraftComment = hasDraftComment; result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); From 7b8a757c4a7499153f4b7746a211b8d9c450b036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Wed, 28 Feb 2024 09:02:19 +0100 Subject: [PATCH 04/26] fix shouldREportBeInOptionList --- src/libs/ReportUtils.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 558225202a35..b0a3a7b95419 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -49,6 +49,7 @@ import type {EmptyObject} from '@src/types/utils/EmptyObject'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type IconAsset from '@src/types/utils/IconAsset'; import * as CollectionUtils from './CollectionUtils'; +import getDraftComment from './ComposerUtils/getDraftComment'; import * as CurrencyUtils from './CurrencyUtils'; import DateUtils from './DateUtils'; import isReportMessageAttachment from './isReportMessageAttachment'; @@ -3935,9 +3936,12 @@ function shouldReportBeInOptionList({ return true; } + // Retrieve the draft comment for the report and convert it to a boolean + const hasDraft = !!(getDraftComment(report.reportID) ?? '').trim(); + // Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - if (report.hasDraft || requiresAttentionFromCurrentUser(report)) { + if (hasDraft || requiresAttentionFromCurrentUser(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); From 9df353b03a7210b6db5440606fcb01b20c7d588f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Wed, 28 Feb 2024 09:03:44 +0100 Subject: [PATCH 05/26] trim the comment for hasDraftComment calculations --- src/components/LHNOptionsList/OptionRowLHNData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index 3cfc8f0e642d..45322551a209 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -43,7 +43,7 @@ function OptionRowLHNData({ policy, parentReportAction, hasViolations: !!hasViolations, - hasDraftComment: !!comment, + hasDraftComment: !!comment.trim(), }); if (deepEqual(item, optionItemRef.current)) { return optionItemRef.current; From 8876c089e618d1c99e09b2ca26483e96d58d0e43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 4 Mar 2024 09:30:01 +0100 Subject: [PATCH 06/26] create draft message store --- .../getDraftComment.ts => DraftCommentStore.ts} | 14 ++++++++++++-- src/libs/OptionsListUtils.ts | 3 ++- src/libs/ReportUtils.ts | 4 ++-- src/libs/SidebarUtils.ts | 5 +++-- .../ComposerWithSuggestions.js | 2 +- .../ReportActionCompose/ReportActionCompose.js | 2 +- 6 files changed, 21 insertions(+), 9 deletions(-) rename src/libs/{ComposerUtils/getDraftComment.ts => DraftCommentStore.ts} (60%) diff --git a/src/libs/ComposerUtils/getDraftComment.ts b/src/libs/DraftCommentStore.ts similarity index 60% rename from src/libs/ComposerUtils/getDraftComment.ts rename to src/libs/DraftCommentStore.ts index 7f11825004a1..1c3469db582a 100644 --- a/src/libs/ComposerUtils/getDraftComment.ts +++ b/src/libs/DraftCommentStore.ts @@ -18,8 +18,18 @@ Onyx.connect({ /** * Returns a draft comment from the onyx collection. * Note: You should use the HOCs/hooks to get onyx data, instead of using this directly. - * A valid use case to use this is if the value is only needed once for an initial value. + * A valid use-case of this function is outside React components, like in utility functions. */ -export default function getDraftComment(reportID: string): OnyxEntry { +function getDraftComment(reportID: string): OnyxEntry { return draftCommentMap[reportID]; } + +/** + * Returns true if the report has a valid draft comment. + * A valid draft comment is a non-empty string. + */ +function hasValidDraftComment(reportID: string): boolean { + return !!getDraftComment(reportID)?.trim(); +} + +export {getDraftComment, hasValidDraftComment}; diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 3d11795f5452..fd1409d9e359 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -35,6 +35,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import times from '@src/utils/times'; import Timing from './actions/Timing'; import * as CollectionUtils from './CollectionUtils'; +import {hasValidDraftComment} from './DraftCommentStore'; import * as ErrorUtils from './ErrorUtils'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; @@ -660,7 +661,7 @@ function createOption( result.ownerAccountID = report.ownerAccountID; result.reportID = report.reportID; result.isUnread = ReportUtils.isUnread(report); - result.hasDraftComment = report.hasDraft; + result.hasDraftComment = hasValidDraftComment(report.reportID); result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index b0a3a7b95419..b89c70bebcd2 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -49,9 +49,9 @@ import type {EmptyObject} from '@src/types/utils/EmptyObject'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type IconAsset from '@src/types/utils/IconAsset'; import * as CollectionUtils from './CollectionUtils'; -import getDraftComment from './ComposerUtils/getDraftComment'; import * as CurrencyUtils from './CurrencyUtils'; import DateUtils from './DateUtils'; +import {hasValidDraftComment} from './DraftCommentStore'; import isReportMessageAttachment from './isReportMessageAttachment'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; @@ -3937,7 +3937,7 @@ function shouldReportBeInOptionList({ } // Retrieve the draft comment for the report and convert it to a boolean - const hasDraft = !!(getDraftComment(report.reportID) ?? '').trim(); + const hasDraft = hasValidDraftComment(report.reportID); // Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index d4d9de7a64d1..419b1e26c256 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -13,6 +13,7 @@ import type {ReportActions} from '@src/types/onyx/ReportAction'; import type ReportAction from '@src/types/onyx/ReportAction'; import type DeepValueOf from '@src/types/utils/DeepValueOf'; import * as CollectionUtils from './CollectionUtils'; +import {hasValidDraftComment} from './DraftCommentStore'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; import * as Localize from './Localize'; @@ -184,7 +185,7 @@ function getOptionData({ policy: OnyxEntry | undefined; parentReportAction: OnyxEntry | undefined; hasViolations: boolean; - hasDraftComment: boolean; + hasDraftComment?: boolean; }): ReportUtils.OptionData | undefined { // When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for // this method to be called after the Onyx data has been cleared out. In that case, it's fine to do @@ -248,7 +249,7 @@ function getOptionData({ // setting it Unread so we add additional condition here to avoid empty chat LHN from being bold. result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID; result.isUnreadWithMention = ReportUtils.isUnreadWithMention(report); - result.hasDraftComment = hasDraftComment; + result.hasDraftComment = hasDraftComment ?? hasValidDraftComment(report.reportID); result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js index b702a66eab99..3266c40610dd 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js @@ -16,8 +16,8 @@ import * as Browser from '@libs/Browser'; import canFocusInputOnScreenFocus from '@libs/canFocusInputOnScreenFocus'; import compose from '@libs/compose'; import * as ComposerUtils from '@libs/ComposerUtils'; -import getDraftComment from '@libs/ComposerUtils/getDraftComment'; import convertToLTRForComposer from '@libs/convertToLTRForComposer'; +import {getDraftComment} from '@libs/DraftCommentStore'; import * as EmojiUtils from '@libs/EmojiUtils'; import focusComposerWithDelay from '@libs/focusComposerWithDelay'; import getPlatform from '@libs/getPlatform'; diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose/ReportActionCompose.js index 4bbf3d393213..c3c3fcac478f 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.js @@ -20,8 +20,8 @@ import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import canFocusInputOnScreenFocus from '@libs/canFocusInputOnScreenFocus'; import compose from '@libs/compose'; -import getDraftComment from '@libs/ComposerUtils/getDraftComment'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; +import {getDraftComment} from '@libs/DraftCommentStore'; import getModalState from '@libs/getModalState'; import * as ReportUtils from '@libs/ReportUtils'; import playSound, {SOUNDS} from '@libs/Sound'; From cdef5260923495d2aaf4e44a2a764613590cf820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 4 Mar 2024 13:25:41 +0100 Subject: [PATCH 07/26] split gORIDs into smaller functions --- src/libs/SidebarUtils.ts | 143 +++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 45 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 419b1e26c256..94912c93083d 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -60,26 +60,16 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 { return 0; } -/** - * @returns An array of reportIDs sorted in the proper order - */ -function getOrderedReportIDs( +function filterReportsToDisplay( + reports: Report[], currentReportId: string | null, - allReports: Record, - betas: Beta[], - policies: Record, - priorityMode: ValueOf, allReportActions: OnyxCollection, transactionViolations: OnyxCollection, - currentPolicyID = '', - policyMemberAccountIDs: number[] = [], -): string[] { - const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; - const isInDefaultMode = !isInGSDMode; - const allReportsDictValues = Object.values(allReports); - - // Filter out all the reports that shouldn't be displayed - let reportsToDisplay = allReportsDictValues.filter((report) => { + betas: Beta[], + policies: Record, + isInGSDMode: boolean, +): Report[] { + return reports.filter((report) => { const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`; const parentReportActions = allReportActions?.[parentReportActionsKey]; const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID); @@ -95,18 +85,18 @@ function getOrderedReportIDs( doesReportHaveViolations, }); }); +} - if (reportsToDisplay.length === 0) { - // Display Concierge chat report when there is no report to be displayed - const conciergeChatReport = allReportsDictValues.find(ReportUtils.isConciergeChatReport); - if (conciergeChatReport) { - reportsToDisplay.push(conciergeChatReport); - } +function filterReportsBelongingToWorkspace(reports: Report[], currentPolicyID: string, policyMemberAccountIDs: number[]): Report[] { + if (currentPolicyID || policyMemberAccountIDs.length > 0) { + return reports.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID)); } + return reports; +} +function groupReports(reports: Report[]) { // The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order: // 1. Pinned/GBR - Always sorted by reportDisplayName - // 2. Drafts - Always sorted by reportDisplayName // 3. Non-archived reports and settled IOUs // - Sorted by lastVisibleActionCreated in default (most recent) view mode // - Sorted by reportDisplayName in GSD (focus) view mode @@ -118,22 +108,13 @@ function getOrderedReportIDs( const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; - if (currentPolicyID || policyMemberAccountIDs.length > 0) { - reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID)); - } // There are a few properties that need to be calculated for the report which are used when sorting reports. - reportsToDisplay.forEach((report) => { - // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. - // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add - // the reportDisplayName property to the report object directly. - // eslint-disable-next-line no-param-reassign - report.displayName = ReportUtils.getReportName(report); - + reports.forEach((report) => { const isPinned = report.isPinned ?? false; const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? ''); if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) { pinnedAndGBRReports.push(report); - } else if (report.hasDraft) { + } else if (hasValidDraftComment(report.reportID)) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { archivedReports.push(report); @@ -142,22 +123,93 @@ function getOrderedReportIDs( } }); - // Sort each group of reports accordingly - pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); - draftReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); + return { + pinnedAndGBRReports, + draftReports, + nonArchivedReports, + archivedReports, + }; +} + +function updateReportsNames(reports: Report[]) { + reports.forEach((report) => { + // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. + // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add + // the reportDisplayName property to the report object directly. + // eslint-disable-next-line no-param-reassign + report.displayName = ReportUtils.getReportName(report); + }); +} + +function sortPinnedAndGBRReports(reports: Report[]) { + return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); +} +function sortDraftReports(reports: Report[]) { + return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); +} + +function sortNonArchivedReports(reports: Report[], isInDefaultMode: boolean) { if (isInDefaultMode) { - nonArchivedReports.sort((a, b) => { + return reports.sort((a, b) => { const compareDates = a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0; + if (compareDates) { + return compareDates; + } const compareDisplayNames = a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0; - return compareDates || compareDisplayNames; + return compareDisplayNames; }); - // For archived reports ensure that most recent reports are at the top by reversing the order - archivedReports.sort((a, b) => (a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0)); - } else { - nonArchivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); - archivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); } + return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); +} + +function sortArchivedReports(reports: Report[], isInDefaultMode: boolean) { + if (isInDefaultMode) { + return reports.sort((a, b) => (a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0)); + } + return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); +} + +/** + * @returns An array of reportIDs sorted in the proper order + */ +function getOrderedReportIDs( + currentReportId: string | null, + allReports: Record, + betas: Beta[], + policies: Record, + priorityMode: ValueOf, + allReportActions: OnyxCollection, + transactionViolations: OnyxCollection, + currentPolicyID = '', + policyMemberAccountIDs: number[] = [], +): string[] { + const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; + const isInDefaultMode = !isInGSDMode; + const allReportsDictValues = Object.values(allReports); + + // Filter out all the reports that shouldn't be displayed + let reportsToDisplay = filterReportsToDisplay(allReportsDictValues, currentReportId, allReportActions, transactionViolations, betas, policies, isInGSDMode); + + if (reportsToDisplay.length === 0) { + // Display Concierge chat report when there is no report to be displayed + const conciergeChatReport = allReportsDictValues.find(ReportUtils.isConciergeChatReport); + if (conciergeChatReport) { + reportsToDisplay.push(conciergeChatReport); + } + } + + reportsToDisplay = filterReportsBelongingToWorkspace(reportsToDisplay, currentPolicyID, policyMemberAccountIDs); + + updateReportsNames(reportsToDisplay); + + const {pinnedAndGBRReports, draftReports, nonArchivedReports, archivedReports} = groupReports(reportsToDisplay); + + // Sort each group of reports accordingly + sortPinnedAndGBRReports(pinnedAndGBRReports); + sortDraftReports(draftReports); + sortNonArchivedReports(nonArchivedReports, isInDefaultMode); + sortArchivedReports(archivedReports, isInDefaultMode); // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. @@ -389,4 +441,5 @@ function getOptionData({ export default { getOptionData, getOrderedReportIDs, + sortDraftReports, }; From 2f0555fb2ab709a48c35ffaf829053d00c36ec1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 5 Mar 2024 10:19:14 +0100 Subject: [PATCH 08/26] fixing complexity --- src/libs/SidebarUtils.ts | 9 ++- src/pages/home/sidebar/SidebarLinksData.js | 76 +++++++++++++--------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 94912c93083d..9da36a23c74d 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -173,7 +173,7 @@ function sortArchivedReports(reports: Report[], isInDefaultMode: boolean) { /** * @returns An array of reportIDs sorted in the proper order */ -function getOrderedReportIDs( +function getGroupedReports( currentReportId: string | null, allReports: Record, betas: Beta[], @@ -183,7 +183,7 @@ function getOrderedReportIDs( transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], -): string[] { +): Report[][] { const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; const isInDefaultMode = !isInGSDMode; const allReportsDictValues = Object.values(allReports); @@ -213,8 +213,7 @@ function getOrderedReportIDs( // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. - const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); - return LHNReports; + return [pinnedAndGBRReports, draftReports, nonArchivedReports, archivedReports]; } /** @@ -440,6 +439,6 @@ function getOptionData({ export default { getOptionData, - getOrderedReportIDs, + getGroupedReports, sortDraftReports, }; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 621294b80044..247f78e6626d 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -1,4 +1,5 @@ import {deepEqual} from 'fast-equals'; +import {keys, map} from 'lodash'; import lodashGet from 'lodash/get'; import lodashMap from 'lodash/map'; import PropTypes from 'prop-types'; @@ -16,6 +17,7 @@ import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; import compose from '@libs/compose'; +import {hasValidDraftComment} from '@libs/DraftCommentStore'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; @@ -93,6 +95,8 @@ const propTypes = { ), }), + reportsDraftComments: PropTypes.objectOf(PropTypes.string), + ...withCurrentUserPersonalDetailsPropTypes, }; @@ -105,9 +109,17 @@ const defaultProps = { policyMembers: {}, transactionViolations: {}, allReportActions: {}, + reportsDraftComments: {}, ...withCurrentUserPersonalDetailsDefaultProps, }; +const useReportsWithDrafts = (reports, drafts) => { + const reportIDsWithDrafts = useMemo(() => map(keys(drafts), (k) => k.replace(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, '')), [drafts]); + const reportsWithDrafts = useMemo(() => reportIDsWithDrafts.filter(hasValidDraftComment).map((reportID) => reports[ONYXKEYS.COLLECTION.REPORT + reportID]), []); + + return reportsWithDrafts; +}; + function SidebarLinksData({ isFocused, allReportActions, @@ -123,6 +135,7 @@ function SidebarLinksData({ policyMembers, transactionViolations, currentUserPersonalDetails, + reportsDraftComments, }) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); @@ -136,40 +149,22 @@ function SidebarLinksData({ const reportIDsRef = useRef(null); const isLoading = isLoadingApp; - const optionListItems = useMemo(() => { - const reportIDs = SidebarUtils.getOrderedReportIDs( - null, - chatReports, - betas, - policies, - priorityMode, - allReportActions, - transactionViolations, - activeWorkspaceID, - policyMemberAccountIDs, - ); - if (deepEqual(reportIDsRef.current, reportIDs)) { - return reportIDsRef.current; - } + const reportsWithDrafts = useReportsWithDrafts(chatReports, reportsDraftComments); - // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 - // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. - // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete - if (!isLoading || !reportIDsRef.current || network.isOffline || (reportIDsRef.current && prevPriorityMode !== priorityMode)) { - reportIDsRef.current = reportIDs; - } - return reportIDsRef.current || []; - }, [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, isLoading, network.isOffline, prevPriorityMode]); + const groupedReports = useMemo( + () => SidebarUtils.getGroupedReports(null, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs), + [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs], + ); // We need to make sure the current report is in the list of reports, but we do not want // to have to re-generate the list every time the currentReportID changes. To do that // we first generate the list as if there was no current report, then here we check if // the current report is missing from the list, which should very rarely happen. In this // case we re-generate the list a 2nd time with the current report included. - const optionListItemsWithCurrentReport = useMemo(() => { - if (currentReportID && !_.contains(optionListItems, currentReportID)) { - return SidebarUtils.getOrderedReportIDs( + const groupedReportsWithCurrentReport = useMemo(() => { + if (currentReportID && groupedReports.some((group) => group.some((report) => report.reportID === currentReportID)) === false) { + return SidebarUtils.getGroupedReports( currentReportID, chatReports, betas, @@ -181,8 +176,27 @@ function SidebarLinksData({ policyMemberAccountIDs, ); } - return optionListItems; - }, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs]); + return groupedReports; + }, [currentReportID, groupedReports, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs]); + + const orderedReportIDs = useMemo(() => { + // Substituting the first group with the reports with drafts + groupedReports[1] = reportsWithDrafts; + + const reportIDs = groupedReportsWithCurrentReport.flat().map((report) => report.reportID); + + if (deepEqual(reportIDsRef.current, reportIDs)) { + return reportIDsRef.current; + } + + // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 + // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. + // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete + if (!isLoading || !reportIDsRef.current || network.isOffline || (reportIDsRef.current && prevPriorityMode !== priorityMode)) { + reportIDsRef.current = reportIDs; + } + return reportIDsRef.current || []; + }, [groupedReportsWithCurrentReport, reportsWithDrafts, isLoading, network.isOffline, prevPriorityMode, priorityMode]); const currentReportIDRef = useRef(currentReportID); currentReportIDRef.current = currentReportID; @@ -202,7 +216,7 @@ function SidebarLinksData({ // Data props: isActiveReport={isActiveReport} isLoading={isLoading} - optionListItems={optionListItemsWithCurrentReport} + optionListItems={orderedReportIDs} activeWorkspaceID={activeWorkspaceID} /> @@ -332,5 +346,9 @@ export default compose( key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, initialValue: {}, }, + reportsDraftComments: { + key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, + initialValue: {}, + }, }), )(SidebarLinksData); From 74c658a3c4357d68bf4afae37e74d72843914251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 8 Mar 2024 15:21:10 +0100 Subject: [PATCH 09/26] Revert "fixing complexity" This reverts commit 2f0555fb2ab709a48c35ffaf829053d00c36ec1a. --- src/libs/SidebarUtils.ts | 9 +-- src/pages/home/sidebar/SidebarLinksData.js | 76 +++++++++------------- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 9da36a23c74d..94912c93083d 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -173,7 +173,7 @@ function sortArchivedReports(reports: Report[], isInDefaultMode: boolean) { /** * @returns An array of reportIDs sorted in the proper order */ -function getGroupedReports( +function getOrderedReportIDs( currentReportId: string | null, allReports: Record, betas: Beta[], @@ -183,7 +183,7 @@ function getGroupedReports( transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], -): Report[][] { +): string[] { const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; const isInDefaultMode = !isInGSDMode; const allReportsDictValues = Object.values(allReports); @@ -213,7 +213,8 @@ function getGroupedReports( // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. - return [pinnedAndGBRReports, draftReports, nonArchivedReports, archivedReports]; + const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); + return LHNReports; } /** @@ -439,6 +440,6 @@ function getOptionData({ export default { getOptionData, - getGroupedReports, + getOrderedReportIDs, sortDraftReports, }; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 247f78e6626d..621294b80044 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -1,5 +1,4 @@ import {deepEqual} from 'fast-equals'; -import {keys, map} from 'lodash'; import lodashGet from 'lodash/get'; import lodashMap from 'lodash/map'; import PropTypes from 'prop-types'; @@ -17,7 +16,6 @@ import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; import compose from '@libs/compose'; -import {hasValidDraftComment} from '@libs/DraftCommentStore'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; @@ -95,8 +93,6 @@ const propTypes = { ), }), - reportsDraftComments: PropTypes.objectOf(PropTypes.string), - ...withCurrentUserPersonalDetailsPropTypes, }; @@ -109,17 +105,9 @@ const defaultProps = { policyMembers: {}, transactionViolations: {}, allReportActions: {}, - reportsDraftComments: {}, ...withCurrentUserPersonalDetailsDefaultProps, }; -const useReportsWithDrafts = (reports, drafts) => { - const reportIDsWithDrafts = useMemo(() => map(keys(drafts), (k) => k.replace(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, '')), [drafts]); - const reportsWithDrafts = useMemo(() => reportIDsWithDrafts.filter(hasValidDraftComment).map((reportID) => reports[ONYXKEYS.COLLECTION.REPORT + reportID]), []); - - return reportsWithDrafts; -}; - function SidebarLinksData({ isFocused, allReportActions, @@ -135,7 +123,6 @@ function SidebarLinksData({ policyMembers, transactionViolations, currentUserPersonalDetails, - reportsDraftComments, }) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); @@ -149,22 +136,40 @@ function SidebarLinksData({ const reportIDsRef = useRef(null); const isLoading = isLoadingApp; + const optionListItems = useMemo(() => { + const reportIDs = SidebarUtils.getOrderedReportIDs( + null, + chatReports, + betas, + policies, + priorityMode, + allReportActions, + transactionViolations, + activeWorkspaceID, + policyMemberAccountIDs, + ); - const reportsWithDrafts = useReportsWithDrafts(chatReports, reportsDraftComments); + if (deepEqual(reportIDsRef.current, reportIDs)) { + return reportIDsRef.current; + } - const groupedReports = useMemo( - () => SidebarUtils.getGroupedReports(null, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs), - [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs], - ); + // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 + // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. + // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete + if (!isLoading || !reportIDsRef.current || network.isOffline || (reportIDsRef.current && prevPriorityMode !== priorityMode)) { + reportIDsRef.current = reportIDs; + } + return reportIDsRef.current || []; + }, [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, isLoading, network.isOffline, prevPriorityMode]); // We need to make sure the current report is in the list of reports, but we do not want // to have to re-generate the list every time the currentReportID changes. To do that // we first generate the list as if there was no current report, then here we check if // the current report is missing from the list, which should very rarely happen. In this // case we re-generate the list a 2nd time with the current report included. - const groupedReportsWithCurrentReport = useMemo(() => { - if (currentReportID && groupedReports.some((group) => group.some((report) => report.reportID === currentReportID)) === false) { - return SidebarUtils.getGroupedReports( + const optionListItemsWithCurrentReport = useMemo(() => { + if (currentReportID && !_.contains(optionListItems, currentReportID)) { + return SidebarUtils.getOrderedReportIDs( currentReportID, chatReports, betas, @@ -176,27 +181,8 @@ function SidebarLinksData({ policyMemberAccountIDs, ); } - return groupedReports; - }, [currentReportID, groupedReports, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs]); - - const orderedReportIDs = useMemo(() => { - // Substituting the first group with the reports with drafts - groupedReports[1] = reportsWithDrafts; - - const reportIDs = groupedReportsWithCurrentReport.flat().map((report) => report.reportID); - - if (deepEqual(reportIDsRef.current, reportIDs)) { - return reportIDsRef.current; - } - - // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 - // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. - // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete - if (!isLoading || !reportIDsRef.current || network.isOffline || (reportIDsRef.current && prevPriorityMode !== priorityMode)) { - reportIDsRef.current = reportIDs; - } - return reportIDsRef.current || []; - }, [groupedReportsWithCurrentReport, reportsWithDrafts, isLoading, network.isOffline, prevPriorityMode, priorityMode]); + return optionListItems; + }, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs]); const currentReportIDRef = useRef(currentReportID); currentReportIDRef.current = currentReportID; @@ -216,7 +202,7 @@ function SidebarLinksData({ // Data props: isActiveReport={isActiveReport} isLoading={isLoading} - optionListItems={orderedReportIDs} + optionListItems={optionListItemsWithCurrentReport} activeWorkspaceID={activeWorkspaceID} /> @@ -346,9 +332,5 @@ export default compose( key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, initialValue: {}, }, - reportsDraftComments: { - key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, - initialValue: {}, - }, }), )(SidebarLinksData); From 024a50c11872a5ae481506b766caf7e697477732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 8 Mar 2024 15:24:38 +0100 Subject: [PATCH 10/26] Revert "split gORIDs into smaller functions" This reverts commit cdef5260923495d2aaf4e44a2a764613590cf820. --- src/libs/SidebarUtils.ts | 143 ++++++++++++--------------------------- 1 file changed, 45 insertions(+), 98 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 94912c93083d..419b1e26c256 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -60,16 +60,26 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 { return 0; } -function filterReportsToDisplay( - reports: Report[], +/** + * @returns An array of reportIDs sorted in the proper order + */ +function getOrderedReportIDs( currentReportId: string | null, - allReportActions: OnyxCollection, - transactionViolations: OnyxCollection, + allReports: Record, betas: Beta[], policies: Record, - isInGSDMode: boolean, -): Report[] { - return reports.filter((report) => { + priorityMode: ValueOf, + allReportActions: OnyxCollection, + transactionViolations: OnyxCollection, + currentPolicyID = '', + policyMemberAccountIDs: number[] = [], +): string[] { + const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; + const isInDefaultMode = !isInGSDMode; + const allReportsDictValues = Object.values(allReports); + + // Filter out all the reports that shouldn't be displayed + let reportsToDisplay = allReportsDictValues.filter((report) => { const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`; const parentReportActions = allReportActions?.[parentReportActionsKey]; const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID); @@ -85,18 +95,18 @@ function filterReportsToDisplay( doesReportHaveViolations, }); }); -} -function filterReportsBelongingToWorkspace(reports: Report[], currentPolicyID: string, policyMemberAccountIDs: number[]): Report[] { - if (currentPolicyID || policyMemberAccountIDs.length > 0) { - return reports.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID)); + if (reportsToDisplay.length === 0) { + // Display Concierge chat report when there is no report to be displayed + const conciergeChatReport = allReportsDictValues.find(ReportUtils.isConciergeChatReport); + if (conciergeChatReport) { + reportsToDisplay.push(conciergeChatReport); + } } - return reports; -} -function groupReports(reports: Report[]) { // The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order: // 1. Pinned/GBR - Always sorted by reportDisplayName + // 2. Drafts - Always sorted by reportDisplayName // 3. Non-archived reports and settled IOUs // - Sorted by lastVisibleActionCreated in default (most recent) view mode // - Sorted by reportDisplayName in GSD (focus) view mode @@ -108,13 +118,22 @@ function groupReports(reports: Report[]) { const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; + if (currentPolicyID || policyMemberAccountIDs.length > 0) { + reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID)); + } // There are a few properties that need to be calculated for the report which are used when sorting reports. - reports.forEach((report) => { + reportsToDisplay.forEach((report) => { + // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. + // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add + // the reportDisplayName property to the report object directly. + // eslint-disable-next-line no-param-reassign + report.displayName = ReportUtils.getReportName(report); + const isPinned = report.isPinned ?? false; const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? ''); if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) { pinnedAndGBRReports.push(report); - } else if (hasValidDraftComment(report.reportID)) { + } else if (report.hasDraft) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { archivedReports.push(report); @@ -123,93 +142,22 @@ function groupReports(reports: Report[]) { } }); - return { - pinnedAndGBRReports, - draftReports, - nonArchivedReports, - archivedReports, - }; -} - -function updateReportsNames(reports: Report[]) { - reports.forEach((report) => { - // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. - // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add - // the reportDisplayName property to the report object directly. - // eslint-disable-next-line no-param-reassign - report.displayName = ReportUtils.getReportName(report); - }); -} - -function sortPinnedAndGBRReports(reports: Report[]) { - return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); -} - -function sortDraftReports(reports: Report[]) { - return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); -} + // Sort each group of reports accordingly + pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); + draftReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); -function sortNonArchivedReports(reports: Report[], isInDefaultMode: boolean) { if (isInDefaultMode) { - return reports.sort((a, b) => { + nonArchivedReports.sort((a, b) => { const compareDates = a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0; - if (compareDates) { - return compareDates; - } const compareDisplayNames = a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0; - return compareDisplayNames; + return compareDates || compareDisplayNames; }); + // For archived reports ensure that most recent reports are at the top by reversing the order + archivedReports.sort((a, b) => (a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0)); + } else { + nonArchivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); + archivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); } - return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); -} - -function sortArchivedReports(reports: Report[], isInDefaultMode: boolean) { - if (isInDefaultMode) { - return reports.sort((a, b) => (a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0)); - } - return reports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0)); -} - -/** - * @returns An array of reportIDs sorted in the proper order - */ -function getOrderedReportIDs( - currentReportId: string | null, - allReports: Record, - betas: Beta[], - policies: Record, - priorityMode: ValueOf, - allReportActions: OnyxCollection, - transactionViolations: OnyxCollection, - currentPolicyID = '', - policyMemberAccountIDs: number[] = [], -): string[] { - const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; - const isInDefaultMode = !isInGSDMode; - const allReportsDictValues = Object.values(allReports); - - // Filter out all the reports that shouldn't be displayed - let reportsToDisplay = filterReportsToDisplay(allReportsDictValues, currentReportId, allReportActions, transactionViolations, betas, policies, isInGSDMode); - - if (reportsToDisplay.length === 0) { - // Display Concierge chat report when there is no report to be displayed - const conciergeChatReport = allReportsDictValues.find(ReportUtils.isConciergeChatReport); - if (conciergeChatReport) { - reportsToDisplay.push(conciergeChatReport); - } - } - - reportsToDisplay = filterReportsBelongingToWorkspace(reportsToDisplay, currentPolicyID, policyMemberAccountIDs); - - updateReportsNames(reportsToDisplay); - - const {pinnedAndGBRReports, draftReports, nonArchivedReports, archivedReports} = groupReports(reportsToDisplay); - - // Sort each group of reports accordingly - sortPinnedAndGBRReports(pinnedAndGBRReports); - sortDraftReports(draftReports); - sortNonArchivedReports(nonArchivedReports, isInDefaultMode); - sortArchivedReports(archivedReports, isInDefaultMode); // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. @@ -441,5 +389,4 @@ function getOptionData({ export default { getOptionData, getOrderedReportIDs, - sortDraftReports, }; From 3330608939a29ed158385e4136755e8abf08d280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 8 Mar 2024 15:40:52 +0100 Subject: [PATCH 11/26] invalidate reports order on drafts change --- src/libs/SidebarUtils.ts | 2 +- src/pages/home/sidebar/SidebarLinksData.js | 23 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 419b1e26c256..923842a04e32 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -133,7 +133,7 @@ function getOrderedReportIDs( const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? ''); if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) { pinnedAndGBRReports.push(report); - } else if (report.hasDraft) { + } else if (hasValidDraftComment(report.reportID)) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { archivedReports.push(report); diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 621294b80044..b0ad6cb32c97 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -93,6 +93,8 @@ const propTypes = { ), }), + reportsDrafts: PropTypes.objectOf(PropTypes.string), + ...withCurrentUserPersonalDetailsPropTypes, }; @@ -105,6 +107,7 @@ const defaultProps = { policyMembers: {}, transactionViolations: {}, allReportActions: {}, + reportsDrafts: {}, ...withCurrentUserPersonalDetailsDefaultProps, }; @@ -123,6 +126,7 @@ function SidebarLinksData({ policyMembers, transactionViolations, currentUserPersonalDetails, + reportsDrafts, }) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); @@ -160,7 +164,20 @@ function SidebarLinksData({ reportIDsRef.current = reportIDs; } return reportIDsRef.current || []; - }, [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, isLoading, network.isOffline, prevPriorityMode]); + }, [ + chatReports, + betas, + policies, + priorityMode, + allReportActions, + transactionViolations, + activeWorkspaceID, + policyMemberAccountIDs, + isLoading, + network.isOffline, + prevPriorityMode, + reportsDrafts, + ]); // We need to make sure the current report is in the list of reports, but we do not want // to have to re-generate the list every time the currentReportID changes. To do that @@ -332,5 +349,9 @@ export default compose( key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, initialValue: {}, }, + reportsDrafts: { + key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, + initialValue: {}, + }, }), )(SidebarLinksData); From bb2da170a561ba2d3d9c0797e69ba985a127b8bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 8 Mar 2024 15:40:59 +0100 Subject: [PATCH 12/26] remove hasDraft from tests --- tests/utils/collections/reports.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/utils/collections/reports.ts b/tests/utils/collections/reports.ts index 34ce66ab495d..60908e72c2de 100644 --- a/tests/utils/collections/reports.ts +++ b/tests/utils/collections/reports.ts @@ -8,7 +8,6 @@ export default function createRandomReport(index: number): Report { chatType: rand(Object.values(CONST.REPORT.CHAT_TYPE)), currency: randCurrencyCode(), displayName: randWord(), - hasDraft: randBoolean(), ownerAccountID: index, isPinned: randBoolean(), isOptimisticReport: randBoolean(), From b166c500ff40573479f6d0aff17bcf3fad10f1b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 21 Mar 2024 17:05:07 +0100 Subject: [PATCH 13/26] fix draftComment computation --- src/components/LHNOptionsList/OptionRowLHNData.tsx | 1 - src/libs/SidebarUtils.ts | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index 45322551a209..7f7e2b4232ca 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -43,7 +43,6 @@ function OptionRowLHNData({ policy, parentReportAction, hasViolations: !!hasViolations, - hasDraftComment: !!comment.trim(), }); if (deepEqual(item, optionItemRef.current)) { return optionItemRef.current; diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 923842a04e32..47c9cbb37ad3 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -176,7 +176,6 @@ function getOptionData({ policy, parentReportAction, hasViolations, - hasDraftComment, }: { report: OnyxEntry; reportActions: OnyxEntry; @@ -185,7 +184,6 @@ function getOptionData({ policy: OnyxEntry | undefined; parentReportAction: OnyxEntry | undefined; hasViolations: boolean; - hasDraftComment?: boolean; }): ReportUtils.OptionData | undefined { // When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for // this method to be called after the Onyx data has been cleared out. In that case, it's fine to do @@ -207,7 +205,6 @@ function getOptionData({ phoneNumber: null, isUnread: null, isUnreadWithMention: null, - hasDraftComment: false, keyForList: null, searchText: null, isPinned: false, @@ -222,6 +219,7 @@ function getOptionData({ isWaitingOnBankAccount: false, isAllowedToComment: true, isDeletedParentAction: false, + hasDraftComment: false, }; const participantPersonalDetailList = Object.values(OptionsListUtils.getPersonalDetailsForAccountIDs(report.participantAccountIDs ?? [], personalDetails)) as PersonalDetails[]; @@ -249,7 +247,7 @@ function getOptionData({ // setting it Unread so we add additional condition here to avoid empty chat LHN from being bold. result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID; result.isUnreadWithMention = ReportUtils.isUnreadWithMention(report); - result.hasDraftComment = hasDraftComment ?? hasValidDraftComment(report.reportID); + result.hasDraftComment = hasValidDraftComment(report.reportID); result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); From 3fddd59e499b80f044bbfa1ee26bbc82da235ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 21 Mar 2024 17:06:32 +0100 Subject: [PATCH 14/26] change name --- src/libs/ReportUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index b89c70bebcd2..1911f838ac24 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3937,11 +3937,11 @@ function shouldReportBeInOptionList({ } // Retrieve the draft comment for the report and convert it to a boolean - const hasDraft = hasValidDraftComment(report.reportID); + const hasDraftComment = hasValidDraftComment(report.reportID); // Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - if (hasDraft || requiresAttentionFromCurrentUser(report)) { + if (hasDraftComment || requiresAttentionFromCurrentUser(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); From c1214ab2335264ecaa93201e4ab72628048c8c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 21 Mar 2024 17:07:41 +0100 Subject: [PATCH 15/26] remain ordering of default Option object --- src/libs/SidebarUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 47c9cbb37ad3..c9a3f0e0ee21 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -205,6 +205,7 @@ function getOptionData({ phoneNumber: null, isUnread: null, isUnreadWithMention: null, + hasDraftComment: false, keyForList: null, searchText: null, isPinned: false, @@ -219,7 +220,6 @@ function getOptionData({ isWaitingOnBankAccount: false, isAllowedToComment: true, isDeletedParentAction: false, - hasDraftComment: false, }; const participantPersonalDetailList = Object.values(OptionsListUtils.getPersonalDetailsForAccountIDs(report.participantAccountIDs ?? [], personalDetails)) as PersonalDetails[]; From 1b245b48e6c689fa8a802eb0a45caea8979d5d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 12:26:02 +0100 Subject: [PATCH 16/26] make saveReportComment delete drafts for empty comments --- src/libs/actions/Report.ts | 7 ++++--- .../ComposerWithSuggestions/ComposerWithSuggestions.js | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 0bf6074909a0..17ab221e28a0 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -1041,9 +1041,10 @@ function togglePinnedState(reportID: string, isPinnedChat: boolean) { /** * Saves the comment left by the user as they are typing. By saving this data the user can switch between chats, close * tab, refresh etc without worrying about loosing what they typed out. + * When empty string or null is passed, it will delete the draft comment from Onyx store. */ -function saveReportComment(reportID: string, comment: string) { - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, comment); +function saveReportDraftComment(reportID: string, comment: string | null) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, comment || null); } /** Saves the number of lines for the comment */ @@ -2906,7 +2907,7 @@ export { subscribeToReportLeavingEvents, unsubscribeFromReportChannel, unsubscribeFromLeavingRoomReportChannel, - saveReportComment, + saveReportDraftComment, saveReportCommentNumberOfLines, broadcastUserIsTyping, broadcastUserIsLeavingRoom, diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js index 3266c40610dd..18ce6478c8a4 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js @@ -188,7 +188,7 @@ function ComposerWithSuggestions({ const debouncedSaveReportComment = useMemo( () => _.debounce((selectedReportID, newComment) => { - Report.saveReportComment(selectedReportID, newComment || ''); + Report.saveReportDraftComment(selectedReportID, newComment); }, 1000), [], ); @@ -288,7 +288,7 @@ function ComposerWithSuggestions({ if (shouldDebounceSaveComment) { debouncedSaveReportComment(reportID, newCommentConverted); } else { - Report.saveReportComment(reportID, newCommentConverted || ''); + Report.saveReportDraftComment(reportID, newCommentConverted); } if (newCommentConverted) { debouncedBroadcastUserIsTyping(reportID); From 059ea4a17d26a11454078cff03f14aab25f64561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 12:39:07 +0100 Subject: [PATCH 17/26] cleanup --- src/components/LHNOptionsList/LHNOptionsList.tsx | 5 +++-- src/components/LHNOptionsList/OptionRowLHN.tsx | 4 ++-- src/components/LHNOptionsList/OptionRowLHNData.tsx | 1 - src/components/LHNOptionsList/types.ts | 7 +++++-- src/components/optionPropTypes.js | 3 --- .../{DraftCommentStore.ts => DraftCommentUtils.ts} | 13 ++++++++++--- src/libs/OptionsListUtils.ts | 3 --- src/libs/ReportUtils.ts | 3 +-- src/libs/SidebarUtils.ts | 4 +--- .../ComposerWithSuggestions.js | 2 +- .../ReportActionCompose/ReportActionCompose.js | 2 +- 11 files changed, 24 insertions(+), 23 deletions(-) rename src/libs/{DraftCommentStore.ts => DraftCommentUtils.ts} (72%) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 27f424ad1b70..79b86b57bdf9 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -6,6 +6,7 @@ import {withOnyx} from 'react-native-onyx'; import withCurrentReportID from '@components/withCurrentReportID'; import usePermissions from '@hooks/usePermissions'; import useThemeStyles from '@hooks/useThemeStyles'; +import * as DraftCommentUtils from '@libs/DraftCommentUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import variables from '@styles/variables'; import CONST from '@src/CONST'; @@ -60,7 +61,7 @@ function LHNOptionsList({ const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : ''; const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null; - const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? ''; + const hasDraftComment = DraftCommentUtils.isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]); const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions); const lastReportAction = sortedReportActions[0]; @@ -87,7 +88,7 @@ function LHNOptionsList({ isFocused={!shouldDisableFocusOptions && reportID === currentReportID} onSelectRow={onSelectRow} preferredLocale={preferredLocale} - comment={itemComment} + hasDraftComment={hasDraftComment} transactionViolations={transactionViolations} canUseViolations={canUseViolations} onLayout={onLayoutItem} diff --git a/src/components/LHNOptionsList/OptionRowLHN.tsx b/src/components/LHNOptionsList/OptionRowLHN.tsx index 923337ba9ada..09811a5af624 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.tsx +++ b/src/components/LHNOptionsList/OptionRowLHN.tsx @@ -28,7 +28,7 @@ import CONST from '@src/CONST'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OptionRowLHNProps} from './types'; -function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, optionItem, viewMode = 'default', style, onLayout = () => {}}: OptionRowLHNProps) { +function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, optionItem, viewMode = 'default', style, onLayout = () => {}, hasDraftComment}: OptionRowLHNProps) { const theme = useTheme(); const styles = useThemeStyles(); const popoverAnchor = useRef(null); @@ -256,7 +256,7 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti /> )} - {optionItem.hasDraftComment && optionItem.isAllowedToComment && ( + {hasDraftComment && optionItem.isAllowedToComment && ( ; - /** Comment added to report */ - comment: string; + /** Whether a report contains a draft */ + hasDraftComment: boolean; /** The receipt transaction from the parent report action */ receiptTransactions: OnyxCollection; @@ -134,6 +134,9 @@ type OptionRowLHNProps = { /** The item that should be rendered */ optionItem?: OptionData; + /** Whether a report contains a draft */ + hasDraftComment: boolean; + onLayout?: (event: LayoutChangeEvent) => void; }; diff --git a/src/components/optionPropTypes.js b/src/components/optionPropTypes.js index 4cfadea33d60..53d8e86b02d6 100644 --- a/src/components/optionPropTypes.js +++ b/src/components/optionPropTypes.js @@ -25,9 +25,6 @@ export default PropTypes.shape({ // reportID (only present when there is a matching report) reportID: PropTypes.string, - // Whether the report has a draft comment or not - hasDraftComment: PropTypes.bool, - // Key used internally by React keyForList: PropTypes.string, diff --git a/src/libs/DraftCommentStore.ts b/src/libs/DraftCommentUtils.ts similarity index 72% rename from src/libs/DraftCommentStore.ts rename to src/libs/DraftCommentUtils.ts index 1c3469db582a..0ee852063b36 100644 --- a/src/libs/DraftCommentStore.ts +++ b/src/libs/DraftCommentUtils.ts @@ -16,7 +16,7 @@ Onyx.connect({ }); /** - * Returns a draft comment from the onyx collection. + * Returns a draft comment from the onyx collection for given reportID. * Note: You should use the HOCs/hooks to get onyx data, instead of using this directly. * A valid use-case of this function is outside React components, like in utility functions. */ @@ -28,8 +28,15 @@ function getDraftComment(reportID: string): OnyxEntry { * Returns true if the report has a valid draft comment. * A valid draft comment is a non-empty string. */ +function isValidDraftComment(comment?: string | null): boolean { + return !!comment?.trim(); +} + +/** + * Returns true if the report has a valid draft comment. + */ function hasValidDraftComment(reportID: string): boolean { - return !!getDraftComment(reportID)?.trim(); + return isValidDraftComment(getDraftComment(reportID)); } -export {getDraftComment, hasValidDraftComment}; +export {getDraftComment, isValidDraftComment, hasValidDraftComment}; diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index fd1409d9e359..ce9962f37e2e 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -35,7 +35,6 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import times from '@src/utils/times'; import Timing from './actions/Timing'; import * as CollectionUtils from './CollectionUtils'; -import {hasValidDraftComment} from './DraftCommentStore'; import * as ErrorUtils from './ErrorUtils'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; @@ -615,7 +614,6 @@ function createOption( login: null, reportID: '', phoneNumber: null, - hasDraftComment: false, keyForList: null, searchText: null, isDefaultRoom: false, @@ -661,7 +659,6 @@ function createOption( result.ownerAccountID = report.ownerAccountID; result.reportID = report.reportID; result.isUnread = ReportUtils.isUnread(report); - result.hasDraftComment = hasValidDraftComment(report.reportID); result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 1911f838ac24..4654875ea229 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -51,7 +51,7 @@ import type IconAsset from '@src/types/utils/IconAsset'; import * as CollectionUtils from './CollectionUtils'; import * as CurrencyUtils from './CurrencyUtils'; import DateUtils from './DateUtils'; -import {hasValidDraftComment} from './DraftCommentStore'; +import {hasValidDraftComment} from './DraftCommentUtils'; import isReportMessageAttachment from './isReportMessageAttachment'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; @@ -377,7 +377,6 @@ type OptionData = { phoneNumber?: string | null; isUnread?: boolean | null; isUnreadWithMention?: boolean | null; - hasDraftComment?: boolean | null; keyForList?: string | null; searchText?: string | null; isIOUReportOwner?: boolean | null; diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index c9a3f0e0ee21..20dd5d1e7ec2 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -13,7 +13,7 @@ import type {ReportActions} from '@src/types/onyx/ReportAction'; import type ReportAction from '@src/types/onyx/ReportAction'; import type DeepValueOf from '@src/types/utils/DeepValueOf'; import * as CollectionUtils from './CollectionUtils'; -import {hasValidDraftComment} from './DraftCommentStore'; +import {hasValidDraftComment} from './DraftCommentUtils'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; import * as Localize from './Localize'; @@ -205,7 +205,6 @@ function getOptionData({ phoneNumber: null, isUnread: null, isUnreadWithMention: null, - hasDraftComment: false, keyForList: null, searchText: null, isPinned: false, @@ -247,7 +246,6 @@ function getOptionData({ // setting it Unread so we add additional condition here to avoid empty chat LHN from being bold. result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID; result.isUnreadWithMention = ReportUtils.isUnreadWithMention(report); - result.hasDraftComment = hasValidDraftComment(report.reportID); result.isPinned = report.isPinned; result.iouReportID = report.iouReportID; result.keyForList = String(report.reportID); diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js index 18ce6478c8a4..571f33ee8d29 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js @@ -17,7 +17,7 @@ import canFocusInputOnScreenFocus from '@libs/canFocusInputOnScreenFocus'; import compose from '@libs/compose'; import * as ComposerUtils from '@libs/ComposerUtils'; import convertToLTRForComposer from '@libs/convertToLTRForComposer'; -import {getDraftComment} from '@libs/DraftCommentStore'; +import {getDraftComment} from '@libs/DraftCommentUtils'; import * as EmojiUtils from '@libs/EmojiUtils'; import focusComposerWithDelay from '@libs/focusComposerWithDelay'; import getPlatform from '@libs/getPlatform'; diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose/ReportActionCompose.js index c3c3fcac478f..e152eeeb9c0d 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.js +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.js @@ -21,7 +21,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions'; import canFocusInputOnScreenFocus from '@libs/canFocusInputOnScreenFocus'; import compose from '@libs/compose'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; -import {getDraftComment} from '@libs/DraftCommentStore'; +import {getDraftComment} from '@libs/DraftCommentUtils'; import getModalState from '@libs/getModalState'; import * as ReportUtils from '@libs/ReportUtils'; import playSound, {SOUNDS} from '@libs/Sound'; From e599e5b48386c56a8be80f62a2c888a41bf84ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 12:44:42 +0100 Subject: [PATCH 18/26] do not save drafts with spaces only --- src/libs/actions/Report.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 17ab221e28a0..7e6abce0903f 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -1044,7 +1044,7 @@ function togglePinnedState(reportID: string, isPinnedChat: boolean) { * When empty string or null is passed, it will delete the draft comment from Onyx store. */ function saveReportDraftComment(reportID: string, comment: string | null) { - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, comment || null); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, comment?.trim() || null); } /** Saves the number of lines for the comment */ From 7e2c3c5335efa9f8a4599c963bcb27761392c469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 13:11:28 +0100 Subject: [PATCH 19/26] prepare draft comment util --- src/libs/DraftCommentUtils.ts | 9 ++++++++- src/libs/actions/Report.ts | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libs/DraftCommentUtils.ts b/src/libs/DraftCommentUtils.ts index 0ee852063b36..87019bf746ce 100644 --- a/src/libs/DraftCommentUtils.ts +++ b/src/libs/DraftCommentUtils.ts @@ -39,4 +39,11 @@ function hasValidDraftComment(reportID: string): boolean { return isValidDraftComment(getDraftComment(reportID)); } -export {getDraftComment, isValidDraftComment, hasValidDraftComment}; +/** + * Prepares a draft comment by trimming it and returning null if it's empty. + */ +function prepareDraftComment(comment: string | null) { + return comment?.trim() || null; +} + +export {getDraftComment, isValidDraftComment, hasValidDraftComment, prepareDraftComment}; diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 7e6abce0903f..0c86ee60b79b 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -44,6 +44,7 @@ import type UpdateRoomVisibilityParams from '@libs/API/parameters/UpdateRoomVisi import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types'; import * as CollectionUtils from '@libs/CollectionUtils'; import DateUtils from '@libs/DateUtils'; +import {prepareDraftComment} from '@libs/DraftCommentUtils'; import * as EmojiUtils from '@libs/EmojiUtils'; import * as Environment from '@libs/Environment/Environment'; import * as ErrorUtils from '@libs/ErrorUtils'; @@ -1044,7 +1045,7 @@ function togglePinnedState(reportID: string, isPinnedChat: boolean) { * When empty string or null is passed, it will delete the draft comment from Onyx store. */ function saveReportDraftComment(reportID: string, comment: string | null) { - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, comment?.trim() || null); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`, prepareDraftComment(comment)); } /** Saves the number of lines for the comment */ From a3cd97df1a185a08543d5b144a7fff14b47cb667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 14:04:50 +0100 Subject: [PATCH 20/26] waitForCollectionCallback in DraftCommentUtils --- src/libs/DraftCommentUtils.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libs/DraftCommentUtils.ts b/src/libs/DraftCommentUtils.ts index 87019bf746ce..a0af2106cec9 100644 --- a/src/libs/DraftCommentUtils.ts +++ b/src/libs/DraftCommentUtils.ts @@ -1,18 +1,14 @@ -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; -const draftCommentMap: Record> = {}; +let draftCommentMap: OnyxCollection = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, - callback: (value, key) => { - if (!key) { - return; - } - - const reportID = key.replace(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, ''); - draftCommentMap[reportID] = value; + callback: (nextVal) => { + draftCommentMap = nextVal; }, + waitForCollectionCallback: true, }); /** @@ -20,8 +16,8 @@ Onyx.connect({ * Note: You should use the HOCs/hooks to get onyx data, instead of using this directly. * A valid use-case of this function is outside React components, like in utility functions. */ -function getDraftComment(reportID: string): OnyxEntry { - return draftCommentMap[reportID]; +function getDraftComment(reportID: string): OnyxEntry | null | undefined { + return draftCommentMap?.[ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT + reportID]; } /** From da1d82300ed8a4ea64914d8fd640e11b7732af76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 14:05:19 +0100 Subject: [PATCH 21/26] rename draftCommentMap to draftCommentCollection --- src/libs/DraftCommentUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/DraftCommentUtils.ts b/src/libs/DraftCommentUtils.ts index a0af2106cec9..13ed3a571e25 100644 --- a/src/libs/DraftCommentUtils.ts +++ b/src/libs/DraftCommentUtils.ts @@ -2,11 +2,11 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; -let draftCommentMap: OnyxCollection = {}; +let draftCommentCollection: OnyxCollection = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, callback: (nextVal) => { - draftCommentMap = nextVal; + draftCommentCollection = nextVal; }, waitForCollectionCallback: true, }); @@ -17,7 +17,7 @@ Onyx.connect({ * A valid use-case of this function is outside React components, like in utility functions. */ function getDraftComment(reportID: string): OnyxEntry | null | undefined { - return draftCommentMap?.[ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT + reportID]; + return draftCommentCollection?.[ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT + reportID]; } /** From 67a8736cde319a088c603f4492624a039f3d52bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 19:54:09 +0100 Subject: [PATCH 22/26] fix tests --- tests/unit/OptionsListUtilsTest.js | 1 - tests/unit/SidebarFilterTest.ts | 12 +++++++----- tests/unit/SidebarOrderTest.ts | 23 ++++++++++++++--------- tests/utils/LHNTestUtils.tsx | 3 +-- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index 7244b7830a29..e385fb3e96b7 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -17,7 +17,6 @@ describe('OptionsListUtils', () => { participantAccountIDs: [2, 1], visibleChatMemberAccountIDs: [2, 1], reportName: 'Iron Man, Mister Fantastic', - hasDraft: true, type: CONST.REPORT.TYPE.CHAT, }, 2: { diff --git a/tests/unit/SidebarFilterTest.ts b/tests/unit/SidebarFilterTest.ts index 58ec66698b83..a1dd6cd226be 100644 --- a/tests/unit/SidebarFilterTest.ts +++ b/tests/unit/SidebarFilterTest.ts @@ -24,6 +24,7 @@ const ONYXKEYS = { REPORT: 'report_', REPORT_ACTIONS: 'reportActions_', POLICY: 'policy_', + REPORT_DRAFT_COMMENT: 'reportDraftComment_', }, NETWORK: 'network', } as const; @@ -110,7 +111,6 @@ xdescribe('Sidebar', () => { // Given a new report with a draft text const report: Report = { ...LHNTestUtils.getFakeReport([1, 2], 0), - hasDraft: true, }; const reportCollectionDataSet: ReportCollectionDataSet = { @@ -124,6 +124,7 @@ xdescribe('Sidebar', () => { Onyx.multiSet({ [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`]: 'This is a draft message', ...reportCollectionDataSet, }), ) @@ -330,9 +331,9 @@ xdescribe('Sidebar', () => { // const boolArr = [false, false, false, false, false]; it(`the booleans ${JSON.stringify(boolArr)}`, () => { - const [isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned, hasDraft] = boolArr; + const [isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned] = boolArr; const report2: Report = { - ...LHNTestUtils.getAdvancedFakeReport(isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned, hasDraft), + ...LHNTestUtils.getAdvancedFakeReport(isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned), policyID: policy.policyID, }; LHNTestUtils.getDefaultRenderedSidebarLinks(report1.reportID); @@ -453,7 +454,6 @@ xdescribe('Sidebar', () => { // Given a draft report and a pinned report const draftReport = { ...LHNTestUtils.getFakeReport([1, 2]), - hasDraft: true, }; const pinnedReport = { ...LHNTestUtils.getFakeReport([3, 4]), @@ -474,6 +474,7 @@ xdescribe('Sidebar', () => { [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${draftReport.reportID}`]: 'draft report message', ...reportCollectionDataSet, }), ) @@ -676,7 +677,7 @@ xdescribe('Sidebar', () => { it(`the booleans ${JSON.stringify(boolArr)}`, () => { const [isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned, hasDraft] = boolArr; const report2 = { - ...LHNTestUtils.getAdvancedFakeReport(isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned, hasDraft), + ...LHNTestUtils.getAdvancedFakeReport(isArchived, isUserCreatedPolicyRoom, hasAddWorkspaceError, isUnread, isPinned), policyID: policy.policyID, }; LHNTestUtils.getDefaultRenderedSidebarLinks(report1.reportID); @@ -696,6 +697,7 @@ xdescribe('Sidebar', () => { [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, [`${ONYXKEYS.COLLECTION.POLICY}${policy.policyID}`]: policy, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report2.reportID}`]: hasDraft ? 'report2 draft' : null, ...reportCollectionDataSet, }), ) diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 27da8348f43d..9b03ce4b2597 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -25,6 +25,7 @@ const ONYXKEYS = { REPORT: 'report_', REPORT_ACTIONS: 'reportActions_', POLICY: 'policy_', + REPORT_DRAFT_COMMENT: 'reportDraftComment_', }, NETWORK: 'network', IS_LOADING_REPORT_DATA: 'isLoadingReportData', @@ -163,7 +164,6 @@ describe('Sidebar', () => { // And the currently viewed report is the first report const report1 = { ...LHNTestUtils.getFakeReport([1, 2], 3), - hasDraft: true, }; const report2 = LHNTestUtils.getFakeReport([3, 4], 2); const report3 = LHNTestUtils.getFakeReport([5, 6], 1); @@ -190,6 +190,7 @@ describe('Sidebar', () => { [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report1.reportID}`]: 'report1 draft', ...reportCollectionDataSet, }), ) @@ -455,7 +456,6 @@ describe('Sidebar', () => { const report1 = LHNTestUtils.getFakeReport([1, 2], 3); const report2: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([3, 4], 2), - hasDraft: true, }; const report3 = LHNTestUtils.getFakeReport([5, 6], 1); @@ -481,6 +481,7 @@ describe('Sidebar', () => { [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + [ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT + report2.reportID]: 'This is a draft', ...reportCollectionDataSet, }), ) @@ -513,7 +514,6 @@ describe('Sidebar', () => { // And the report has a draft const report: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([1, 2]), - hasDraft: true, }; const reportCollectionDataSet: ReportCollectionDataSet = { @@ -528,6 +528,7 @@ describe('Sidebar', () => { [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`]: 'This is a draft', ...reportCollectionDataSet, }), ) @@ -538,7 +539,7 @@ describe('Sidebar', () => { }) // When the draft is removed - .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, {hasDraft: null})) + .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report.reportID}`, null)) // Then the pencil icon goes away .then(() => { @@ -599,7 +600,6 @@ describe('Sidebar', () => { }; const report2: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([3, 4], 2), - hasDraft: true, }; const report3: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([5, 6], 1), @@ -643,6 +643,7 @@ describe('Sidebar', () => { [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, [ONYXKEYS.SESSION]: {accountID: currentlyLoggedInUserAccountID}, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report2.reportID}`]: 'Report2 draft comment', ...reportCollectionDataSet, }), ) @@ -734,19 +735,15 @@ describe('Sidebar', () => { // and they all have drafts const report1: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([1, 2], 3), - hasDraft: true, }; const report2: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([3, 4], 2), - hasDraft: true, }; const report3: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([5, 6], 1), - hasDraft: true, }; const report4: OnyxTypes.Report = { ...LHNTestUtils.getFakeReport([7, 8], 0), - hasDraft: true, }; LHNTestUtils.getDefaultRenderedSidebarLinks('0'); @@ -757,6 +754,12 @@ describe('Sidebar', () => { [`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3, }; + const reportDraftCommentCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report1.reportID}`]: 'report1 draft', + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report2.reportID}`]: 'report2 draft', + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report3.reportID}`]: 'report3 draft', + }; + return ( waitForBatchedUpdates() // When Onyx is updated with the data and the sidebar re-renders @@ -765,6 +768,7 @@ describe('Sidebar', () => { [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, [ONYXKEYS.IS_LOADING_APP]: false, + ...reportDraftCommentCollectionDataSet, ...reportCollectionDataSet, }), ) @@ -781,6 +785,7 @@ describe('Sidebar', () => { // When a new report is added .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4)) + .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report4.reportID}`, 'report4 draft')) // Then they are still in alphabetical order .then(() => { diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index 44bfcd46d399..bd9269f9f44b 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -216,7 +216,7 @@ function getFakeReportAction(actor = 'email1@test.com', millisecondsInThePast = }; } -function getAdvancedFakeReport(isArchived: boolean, isUserCreatedPolicyRoom: boolean, hasAddWorkspaceError: boolean, isUnread: boolean, isPinned: boolean, hasDraft: boolean): Report { +function getAdvancedFakeReport(isArchived: boolean, isUserCreatedPolicyRoom: boolean, hasAddWorkspaceError: boolean, isUnread: boolean, isPinned: boolean): Report { return { ...getFakeReport([1, 2], 0, isUnread), type: CONST.REPORT.TYPE.CHAT, @@ -225,7 +225,6 @@ function getAdvancedFakeReport(isArchived: boolean, isUserCreatedPolicyRoom: boo stateNum: isArchived ? CONST.REPORT.STATE_NUM.APPROVED : 0, errorFields: hasAddWorkspaceError ? {1708946640843000: {addWorkspaceRoom: 'blah'}} : undefined, isPinned, - hasDraft, }; } From eff88ded64107c8c8ee43ff37ac829e516024d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 20:21:38 +0100 Subject: [PATCH 23/26] add draftComments to FlashList extraData --- src/components/LHNOptionsList/LHNOptionsList.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 70ccb17ed93c..79934bb02c65 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -110,7 +110,10 @@ function LHNOptionsList({ ], ); - const extraData = useMemo(() => [reportActions, reports, policy, personalDetails, data.length], [reportActions, reports, policy, personalDetails, data.length]); + const extraData = useMemo( + () => [reportActions, reports, policy, personalDetails, data.length, draftComments], + [reportActions, reports, policy, personalDetails, data.length, draftComments], + ); return ( From 4b56f6fe4cf9117aa660185649c0414cd2ba169f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 22 Mar 2024 20:28:46 +0100 Subject: [PATCH 24/26] fix minor things --- src/libs/DraftCommentUtils.ts | 2 ++ src/pages/home/sidebar/SidebarLinksData.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/libs/DraftCommentUtils.ts b/src/libs/DraftCommentUtils.ts index 13ed3a571e25..325da93d235e 100644 --- a/src/libs/DraftCommentUtils.ts +++ b/src/libs/DraftCommentUtils.ts @@ -39,6 +39,8 @@ function hasValidDraftComment(reportID: string): boolean { * Prepares a draft comment by trimming it and returning null if it's empty. */ function prepareDraftComment(comment: string | null) { + // logical OR is used to convert empty string to null + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing return comment?.trim() || null; } diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index b0ad6cb32c97..da17afd5b6ee 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -151,6 +151,8 @@ function SidebarLinksData({ transactionViolations, activeWorkspaceID, policyMemberAccountIDs, + // Passing it down as we need to re-calculate the list when the drafts are updated + reportsDrafts, ); if (deepEqual(reportIDsRef.current, reportIDs)) { From fa7415cce101f5af699b13796adb70b24ef9e95a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 28 Mar 2024 10:40:32 +0100 Subject: [PATCH 25/26] fix tests --- tests/unit/SidebarOrderTest.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 9b03ce4b2597..2758d43fb81e 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -784,8 +784,12 @@ describe('Sidebar', () => { }) // When a new report is added - .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4)) - .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report4.reportID}`, 'report4 draft')) + .then(() => + Promise.all([ + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4), + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report4.reportID}`, 'report4 draft'), + ]), + ) // Then they are still in alphabetical order .then(() => { From b52bb0bf9b6be930b7811d35a14bcd01fbed22cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 4 Apr 2024 09:34:03 +0200 Subject: [PATCH 26/26] fix list reordering --- src/pages/home/sidebar/SidebarLinksData.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 2567aef459d6..34af75308ac4 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -370,6 +370,7 @@ export default compose( _.isEqual(prevProps.policyMembers, nextProps.policyMembers) && _.isEqual(prevProps.transactionViolations, nextProps.transactionViolations) && _.isEqual(prevProps.currentUserPersonalDetails, nextProps.currentUserPersonalDetails) && - prevProps.currentReportID === nextProps.currentReportID, + prevProps.currentReportID === nextProps.currentReportID && + _.isEqual(prevProps.reportsDrafts, nextProps.reportsDrafts), ), );