Skip to content

Commit

Permalink
fixing complexity
Browse files Browse the repository at this point in the history
  • Loading branch information
@kacper-mikolajczak
kacper-mikolajczak committed Mar 5, 2024
1 parent cdef526 commit 2f0555f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 34 deletions.
9 changes: 4 additions & 5 deletions src/libs/SidebarUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, Report>,
betas: Beta[],
Expand All @@ -183,7 +183,7 @@ function getOrderedReportIDs(
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
): string[] {
): Report[][] {
const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);
Expand Down Expand Up @@ -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];
}

/**
Expand Down Expand Up @@ -440,6 +439,6 @@ function getOptionData({

export default {
getOptionData,
getOrderedReportIDs,
getGroupedReports,
sortDraftReports,
};
76 changes: 47 additions & 29 deletions src/pages/home/sidebar/SidebarLinksData.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -93,6 +95,8 @@ const propTypes = {
),
}),

reportsDraftComments: PropTypes.objectOf(PropTypes.string),

...withCurrentUserPersonalDetailsPropTypes,
};

Expand All @@ -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,
Expand All @@ -123,6 +135,7 @@ function SidebarLinksData({
policyMembers,
transactionViolations,
currentUserPersonalDetails,
reportsDraftComments,
}) {
const styles = useThemeStyles();
const {activeWorkspaceID} = useActiveWorkspace();
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -202,7 +216,7 @@ function SidebarLinksData({
// Data props:
isActiveReport={isActiveReport}
isLoading={isLoading}
optionListItems={optionListItemsWithCurrentReport}
optionListItems={orderedReportIDs}
activeWorkspaceID={activeWorkspaceID}
/>
</View>
Expand Down Expand Up @@ -332,5 +346,9 @@ export default compose(
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
initialValue: {},
},
reportsDraftComments: {
key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT,
initialValue: {},
},
}),
)(SidebarLinksData);

0 comments on commit 2f0555f

Please sign in to comment.