From 26810a949d069d63f6b1d98525e9259bb5db6b70 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 12 Apr 2024 15:25:42 +0700 Subject: [PATCH 1/8] Update report option when personal detail is changed --- src/components/OptionListContextProvider.tsx | 43 +++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index a83eeda5a419..067dee830c80 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -6,7 +6,7 @@ import * as OptionsListUtils from '@libs/OptionsListUtils'; import type {OptionList} from '@libs/OptionsListUtils'; import * as ReportUtils from '@libs/ReportUtils'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Report} from '@src/types/onyx'; +import type {PersonalDetails, Report} from '@src/types/onyx'; import {usePersonalDetails} from './OnyxProvider'; type OptionsListContextProps = { @@ -37,6 +37,13 @@ const OptionsListContext = createContext({ areOptionsInitialized: false, }); +const isEqualPersonalDetail = (prevPersonalDetail: PersonalDetails | null, personalDetail: PersonalDetails | null) => ( + prevPersonalDetail?.firstName === personalDetail?.firstName && + prevPersonalDetail?.lastName === personalDetail?.lastName && + prevPersonalDetail?.login === personalDetail?.login && + prevPersonalDetail?.displayName === personalDetail?.displayName + ); + function OptionsListContextProvider({reports, children}: OptionsListProviderProps) { const areOptionsInitialized = useRef(false); const [options, setOptions] = useState({ @@ -45,6 +52,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp }); const personalDetails = usePersonalDetails(); + const prevPersonalDetails = useRef(personalDetails); const prevReports = usePrevious(reports); /** @@ -96,7 +104,6 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp newOptions.reports.push(reportOption); return newOptions; }); - // eslint-disable-next-line react-hooks/exhaustive-deps }, [reports]); /** @@ -108,14 +115,46 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp return; } + const newReportOptions: Array<{ + replaceIndex: number; + newReportOption: OptionsListUtils.SearchOption; + }> = []; + + Object.keys(personalDetails).forEach((accoutID) => { + const prevPersonalDetail = prevPersonalDetails.current[accoutID]; + const personalDetail = personalDetails[accoutID]; + + if (isEqualPersonalDetail(prevPersonalDetail, personalDetail)) { + return; + } + + Object.values(reports ?? {}) + .filter((report) => report?.participantAccountIDs?.includes(Number(accoutID))) + .forEach((report) => { + if (!report) { + return; + } + const newReportOption = OptionsListUtils.createOptionFromReport(report, personalDetails); + const replaceIndex = options.reports.findIndex((option) => option.reportID === report.reportID); + newReportOptions.push({ + newReportOption, + replaceIndex, + }); + }); + }); + // since personal details are not a collection, we need to recreate the whole list from scratch const newPersonalDetailsOptions = OptionsListUtils.createOptionList(personalDetails).personalDetails; setOptions((prevOptions) => { const newOptions = {...prevOptions}; newOptions.personalDetails = newPersonalDetailsOptions; + newReportOptions.forEach((newReportOption) => (newOptions.reports[newReportOption.replaceIndex] = newReportOption.newReportOption)); return newOptions; }); + + prevPersonalDetails.current = personalDetails; + // eslint-disable-next-line react-hooks/exhaustive-deps }, [personalDetails]); const loadOptions = useCallback(() => { From 7f41cf81464cd4d25f5cf3b684ad3b13a4da267b Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 12 Apr 2024 16:39:50 +0700 Subject: [PATCH 2/8] remove unnecessary change --- src/components/OptionListContextProvider.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index 067dee830c80..dff85fd8753b 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -104,6 +104,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp newOptions.reports.push(reportOption); return newOptions; }); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [reports]); /** From 8cef4549a071cce524078add24e3bf74a1588eac Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Fri, 12 Apr 2024 17:08:03 +0700 Subject: [PATCH 3/8] fix selfDM case --- src/components/OptionListContextProvider.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index dff85fd8753b..e63ce2c63faa 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -130,7 +130,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp } Object.values(reports ?? {}) - .filter((report) => report?.participantAccountIDs?.includes(Number(accoutID))) + .filter((report) => (report?.participantAccountIDs?.includes(Number(accoutID)) || (ReportUtils.isSelfDM(report) && report?.ownerAccountID === Number(accoutID)))) .forEach((report) => { if (!report) { return; From 2cfca3186a945025d3c44e3a7a0ac58f190389f9 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Tue, 16 Apr 2024 15:56:16 +0700 Subject: [PATCH 4/8] fix lint --- src/components/OptionListContextProvider.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index e63ce2c63faa..405bb989aec9 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -130,7 +130,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp } Object.values(reports ?? {}) - .filter((report) => (report?.participantAccountIDs?.includes(Number(accoutID)) || (ReportUtils.isSelfDM(report) && report?.ownerAccountID === Number(accoutID)))) + .filter((report) => (Boolean(report?.participantAccountIDs?.includes(Number(accoutID))) || (ReportUtils.isSelfDM(report) && report?.ownerAccountID === Number(accoutID)))) .forEach((report) => { if (!report) { return; From f3abe819f807dc010ee1e551921906c80331abbe Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Tue, 16 Apr 2024 16:05:01 +0700 Subject: [PATCH 5/8] run prettier --- src/components/OptionListContextProvider.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index 405bb989aec9..94fe51711e02 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -37,12 +37,11 @@ const OptionsListContext = createContext({ areOptionsInitialized: false, }); -const isEqualPersonalDetail = (prevPersonalDetail: PersonalDetails | null, personalDetail: PersonalDetails | null) => ( - prevPersonalDetail?.firstName === personalDetail?.firstName && - prevPersonalDetail?.lastName === personalDetail?.lastName && - prevPersonalDetail?.login === personalDetail?.login && - prevPersonalDetail?.displayName === personalDetail?.displayName - ); +const isEqualPersonalDetail = (prevPersonalDetail: PersonalDetails | null, personalDetail: PersonalDetails | null) => + prevPersonalDetail?.firstName === personalDetail?.firstName && + prevPersonalDetail?.lastName === personalDetail?.lastName && + prevPersonalDetail?.login === personalDetail?.login && + prevPersonalDetail?.displayName === personalDetail?.displayName; function OptionsListContextProvider({reports, children}: OptionsListProviderProps) { const areOptionsInitialized = useRef(false); @@ -130,7 +129,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp } Object.values(reports ?? {}) - .filter((report) => (Boolean(report?.participantAccountIDs?.includes(Number(accoutID))) || (ReportUtils.isSelfDM(report) && report?.ownerAccountID === Number(accoutID)))) + .filter((report) => Boolean(report?.participantAccountIDs?.includes(Number(accoutID))) || (ReportUtils.isSelfDM(report) && report?.ownerAccountID === Number(accoutID))) .forEach((report) => { if (!report) { return; From e3519979e7b69d61f3eba3637ad6eb1af24e65d6 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Wed, 17 Apr 2024 13:00:27 +0700 Subject: [PATCH 6/8] safely get personal detail --- src/components/OptionListContextProvider.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index 94fe51711e02..5d4e704262f5 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -121,8 +121,8 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp }> = []; Object.keys(personalDetails).forEach((accoutID) => { - const prevPersonalDetail = prevPersonalDetails.current[accoutID]; - const personalDetail = personalDetails[accoutID]; + const prevPersonalDetail = prevPersonalDetails.current?.[accoutID]; + const personalDetail = personalDetails?.[accoutID]; if (isEqualPersonalDetail(prevPersonalDetail, personalDetail)) { return; From 1fecd977f9b534f119955a62bd4c7ecf31135bd2 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Wed, 17 Apr 2024 13:07:31 +0700 Subject: [PATCH 7/8] update to use usePrevious hook --- src/components/OptionListContextProvider.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index 5d4e704262f5..473c0f5bb083 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -51,7 +51,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp }); const personalDetails = usePersonalDetails(); - const prevPersonalDetails = useRef(personalDetails); + const prevPersonalDetails = usePrevious(personalDetails); const prevReports = usePrevious(reports); /** @@ -121,7 +121,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp }> = []; Object.keys(personalDetails).forEach((accoutID) => { - const prevPersonalDetail = prevPersonalDetails.current?.[accoutID]; + const prevPersonalDetail = prevPersonalDetails?.[accoutID]; const personalDetail = personalDetails?.[accoutID]; if (isEqualPersonalDetail(prevPersonalDetail, personalDetail)) { @@ -153,7 +153,6 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp return newOptions; }); - prevPersonalDetails.current = personalDetails; // eslint-disable-next-line react-hooks/exhaustive-deps }, [personalDetails]); From d726f3f254e239a57dfeb26cc172350a30c3c0d2 Mon Sep 17 00:00:00 2001 From: nkdengineer Date: Mon, 22 Apr 2024 14:53:43 +0700 Subject: [PATCH 8/8] add explaination comment --- src/components/OptionListContextProvider.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/OptionListContextProvider.tsx b/src/components/OptionListContextProvider.tsx index 473c0f5bb083..f823c5410ae6 100644 --- a/src/components/OptionListContextProvider.tsx +++ b/src/components/OptionListContextProvider.tsx @@ -153,6 +153,7 @@ function OptionsListContextProvider({reports, children}: OptionsListProviderProp return newOptions; }); + // This effect is used to update the options list when personal details change so we ignore all dependencies except personalDetails // eslint-disable-next-line react-hooks/exhaustive-deps }, [personalDetails]);