Skip to content

Commit

Permalink
Revert "Merge pull request #44955 from software-mansion-labs/fix-useL…
Browse files Browse the repository at this point in the history
…astAccessedReportID"

This reverts commit 7bbb3fb, reversing
changes made to bf785d9.
  • Loading branch information
francoisl committed Jul 10, 2024
1 parent 88b7838 commit 3dd9e54
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 42 deletions.
148 changes: 148 additions & 0 deletions src/hooks/useLastAccessedReportID.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
import {useCallback, useSyncExternalStore} from 'react';
import type {OnyxCollection} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import {getPolicyEmployeeListByIdWithoutCurrentUser} from '@libs/PolicyUtils';
import * as ReportUtils from '@libs/ReportUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, Report, ReportMetadata} from '@src/types/onyx';
import useActiveWorkspace from './useActiveWorkspace';
import usePermissions from './usePermissions';

/*
* This hook is used to get the lastAccessedReportID.
* This is a piece of data that's derived from a lot of frequently-changing Onyx values: (reports, reportMetadata, policies, etc...)
* We don't want any component that needs access to the lastAccessedReportID to have to re-render any time any of those values change, just when the lastAccessedReportID changes.
* So we have a custom implementation in this file that leverages useSyncExternalStore to connect to a "store" of multiple Onyx values, and re-render only when the one derived value changes.
*/

const subscribers: Array<() => void> = [];

let reports: OnyxCollection<Report> = {};
let reportMetadata: OnyxCollection<ReportMetadata> = {};
let policies: OnyxCollection<Policy> = {};
let accountID: number | undefined;
let isFirstTimeNewExpensifyUser = false;

let reportsConnection: number;
let reportMetadataConnection: number;
let policiesConnection: number;
let accountIDConnection: number;
let isFirstTimeNewExpensifyUserConnection: number;

function notifySubscribers() {
subscribers.forEach((subscriber) => subscriber());
}

function subscribeToOnyxData() {
// eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
reportsConnection = Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
reports = value;
notifySubscribers();
},
});
// eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
reportMetadataConnection = Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
waitForCollectionCallback: true,
callback: (value) => {
reportMetadata = value;
notifySubscribers();
},
});
// eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
policiesConnection = Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (value) => {
policies = value;
notifySubscribers();
},
});
// eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
accountIDConnection = Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (value) => {
accountID = value?.accountID;
notifySubscribers();
},
});
// eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
isFirstTimeNewExpensifyUserConnection = Onyx.connect({
key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER,
callback: (value) => {
isFirstTimeNewExpensifyUser = !!value;
notifySubscribers();
},
});
}

function unsubscribeFromOnyxData() {
if (reportsConnection) {
Onyx.disconnect(reportsConnection);
reportsConnection = 0;
}
if (reportMetadataConnection) {
Onyx.disconnect(reportMetadataConnection);
reportMetadataConnection = 0;
}
if (policiesConnection) {
Onyx.disconnect(policiesConnection);
policiesConnection = 0;
}
if (accountIDConnection) {
Onyx.disconnect(accountIDConnection);
accountIDConnection = 0;
}
if (isFirstTimeNewExpensifyUserConnection) {
Onyx.disconnect(isFirstTimeNewExpensifyUserConnection);
isFirstTimeNewExpensifyUserConnection = 0;
}
}

function removeSubscriber(subscriber: () => void) {
const subscriberIndex = subscribers.indexOf(subscriber);
if (subscriberIndex < 0) {
return;
}
subscribers.splice(subscriberIndex, 1);
if (subscribers.length === 0) {
unsubscribeFromOnyxData();
}
}

function addSubscriber(subscriber: () => void) {
subscribers.push(subscriber);
if (!reportsConnection) {
subscribeToOnyxData();
}
return () => removeSubscriber(subscriber);
}

/**
* Get the last accessed reportID.
*/
export default function useLastAccessedReportID(shouldOpenOnAdminRoom: boolean) {
const {canUseDefaultRooms} = usePermissions();
const {activeWorkspaceID} = useActiveWorkspace();

const getSnapshot = useCallback(() => {
const policyMemberAccountIDs = getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID);
return ReportUtils.findLastAccessedReport(
reports,
!canUseDefaultRooms,
policies,
isFirstTimeNewExpensifyUser,
shouldOpenOnAdminRoom,
reportMetadata,
activeWorkspaceID,
policyMemberAccountIDs,
)?.reportID;
}, [activeWorkspaceID, canUseDefaultRooms, shouldOpenOnAdminRoom]);

// We need access to all the data from these Onyx.connect calls, but we don't want to re-render the consuming component
// unless the derived value (lastAccessedReportID) changes. To address these, we'll wrap everything with useSyncExternalStore
return useSyncExternalStore(addSubscriber, getSnapshot);
}
40 changes: 15 additions & 25 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,26 +551,6 @@ Onyx.connect({
},
});

let allReportMetadata: OnyxCollection<ReportMetadata>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
return;
}
allReportMetadata = value;
},
});

let isFirstTimeNewExpensifyUser = false;
Onyx.connect({
key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER,
callback: (value) => {
isFirstTimeNewExpensifyUser = value ?? false;
},
});

function getCurrentUserAvatar(): AvatarSource | undefined {
return currentUserPersonalDetails?.avatar;
}
Expand Down Expand Up @@ -1157,29 +1137,39 @@ function hasExpensifyGuidesEmails(accountIDs: number[]): boolean {
return accountIDs.some((accountID) => Str.extractEmailDomain(allPersonalDetails?.[accountID]?.login ?? '') === CONST.EMAIL.GUIDES_DOMAIN);
}


function getMostRecentlyVisitedReport(reports: Array<OnyxEntry<Report>>, reportMetadata: OnyxCollection<ReportMetadata>): OnyxEntry<Report> {
const filteredReports = reports.filter(
(report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime),
);
return lodashMaxBy(filteredReports, (a) => new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '').valueOf());
}

function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = false, policyID?: string, excludeReportID?: string): OnyxEntry<Report> {
function findLastAccessedReport(
reports: OnyxCollection<Report>,
ignoreDomainRooms: boolean,
policies: OnyxCollection<Policy>,
isFirstTimeNewExpensifyUser: boolean,
openOnAdminRoom = false,
reportMetadata: OnyxCollection<ReportMetadata> = {},
policyID?: string,
policyMemberAccountIDs: number[] = [],
excludeReportID?: string,
): OnyxEntry<Report> {
// If it's the user's first time using New Expensify, then they could either have:
// - just a Concierge report, if so we'll return that
// - their Concierge report, and a separate report that must have deeplinked them to the app before they created their account.
// If it's the latter, we'll use the deeplinked report over the Concierge report,
// since the Concierge report would be incorrectly selected over the deep-linked report in the logic below.

const policyMemberAccountIDs = PolicyUtils.getPolicyEmployeeListByIdWithoutCurrentUser(allPolicies, policyID, currentUserAccountID);

const allReports = ReportConnection.getAllReports();
let reportsValues = Object.values(allReports ?? {});
let reportsValues = Object.values(reports ?? {});

if (!!policyID || policyMemberAccountIDs.length > 0) {
reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID);
}

let sortedReports = sortReportsByLastRead(reportsValues, reportMetadata);

Check failure on line 1171 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / typecheck

Cannot find name 'sortReportsByLastRead'.

let adminReport: OnyxEntry<Report>;
if (openOnAdminRoom) {
adminReport = reportsValues.find((report) => {
Expand Down
20 changes: 19 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import type {
RecentlyUsedReportFields,
ReportAction,
ReportActionReactions,
ReportMetadata,
ReportUserIsTyping,
} from '@src/types/onyx';
import type {Decision} from '@src/types/onyx/OriginalMessage';
Expand Down Expand Up @@ -224,6 +225,13 @@ Onyx.connect({
},
});

let reportMetadata: OnyxCollection<ReportMetadata> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
waitForCollectionCallback: true,
callback: (value) => (reportMetadata = value),
});

const typingWatchTimers: Record<string, NodeJS.Timeout> = {};

let reportIDDeeplinkedFromOldDot: string | undefined;
Expand Down Expand Up @@ -2595,7 +2603,17 @@ function getCurrentUserAccountID(): number {
}

function navigateToMostRecentReport(currentReport: OnyxEntry<Report>) {
const lastAccessedReportID = ReportUtils.findLastAccessedReport(false, false, undefined, currentReport?.reportID)?.reportID;
const lastAccessedReportID = ReportUtils.findLastAccessedReport(
ReportConnection.getAllReports(),
false,
undefined,
false,
false,
reportMetadata,
undefined,
[],
currentReport?.reportID,
)?.reportID;

if (lastAccessedReportID) {
const lastAccessedReportRoute = ROUTES.REPORT_WITH_ID.getRoute(lastAccessedReportID ?? '-1');
Expand Down
11 changes: 4 additions & 7 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ import ScreenWrapper from '@components/ScreenWrapper';
import TaskHeaderActionButton from '@components/TaskHeaderActionButton';
import type {CurrentReportIDContextValue} from '@components/withCurrentReportID';
import withCurrentReportID from '@components/withCurrentReportID';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useAppFocusEvent from '@hooks/useAppFocusEvent';
import useDeepCompareRef from '@hooks/useDeepCompareRef';
import useIsReportOpenInRHP from '@hooks/useIsReportOpenInRHP';
import useLastAccessedReportID from '@hooks/useLastAccessedReportID';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import usePermissions from '@hooks/usePermissions';
import usePrevious from '@hooks/usePrevious';
import useThemeStyles from '@hooks/useThemeStyles';
import useViewportOffsetTop from '@hooks/useViewportOffsetTop';
Expand Down Expand Up @@ -147,12 +146,10 @@ function ReportScreen({
const prevIsFocused = usePrevious(isFocused);
const firstRenderRef = useRef(true);
const flatListRef = useRef<FlatList>(null);
const {canUseDefaultRooms} = usePermissions();
const reactionListRef = useRef<ReactionListRef>(null);
const {isOffline} = useNetwork();
const isReportOpenInRHP = useIsReportOpenInRHP();
const {isSmallScreenWidth} = useWindowDimensions();
const {activeWorkspaceID} = useActiveWorkspace();
const shouldUseNarrowLayout = isSmallScreenWidth || isReportOpenInRHP;

const [modal] = useOnyx(ONYXKEYS.MODAL);
Expand All @@ -172,6 +169,8 @@ function ReportScreen({
const isLoadingReportOnyx = isLoadingOnyxValue(reportResult);
const permissions = useDeepCompareRef(reportOnyx?.permissions);

// Check if there's a reportID in the route. If not, set it to the last accessed reportID
const lastAccessedReportID = useLastAccessedReportID(!!route.params.openOnAdminRoom);
useEffect(() => {
// Don't update if there is a reportID in the params already
if (route.params.reportID) {
Expand All @@ -183,8 +182,6 @@ function ReportScreen({
return;
}

const lastAccessedReportID = ReportUtils.findLastAccessedReport(!canUseDefaultRooms, !!route.params.openOnAdminRoom, activeWorkspaceID)?.reportID;

// It's possible that reports aren't fully loaded yet
// in that case the reportID is undefined
if (!lastAccessedReportID) {
Expand All @@ -193,7 +190,7 @@ function ReportScreen({

Log.info(`[ReportScreen] no reportID found in params, setting it to lastAccessedReportID: ${lastAccessedReportID}`);
navigation.setParams({reportID: lastAccessedReportID});
}, [activeWorkspaceID, canUseDefaultRooms, navigation, route]);
}, [lastAccessedReportID, navigation, route]);

/**
* Create a lightweight Report so as to keep the re-rendering as light as possible by
Expand Down
12 changes: 3 additions & 9 deletions tests/perf-test/ReportUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ describe('ReportUtils', () => {
keys: ONYXKEYS,
safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
});
});

beforeEach(async () => {
await Onyx.multiSet({
Onyx.multiSet({
...mockedPoliciesMap,
...mockedReportsMap,
});
Expand All @@ -57,17 +55,13 @@ describe('ReportUtils', () => {

test('[ReportUtils] findLastAccessedReport on 2k reports and policies', async () => {
const ignoreDomainRooms = true;
const isFirstTimeNewExpensifyUser = true;
const reports = getMockedReports(2000);
const policies = getMockedPolicies(2000);
const openOnAdminRoom = true;

await Onyx.multiSet({
[ONYXKEYS.COLLECTION.REPORT]: reports,
[ONYXKEYS.COLLECTION.POLICY]: policies,
});

await waitForBatchedUpdates();
await measureFunction(() => ReportUtils.findLastAccessedReport(ignoreDomainRooms, openOnAdminRoom));
await measureFunction(() => ReportUtils.findLastAccessedReport(reports, ignoreDomainRooms, policies, isFirstTimeNewExpensifyUser, openOnAdminRoom));
});

test('[ReportUtils] canDeleteReportAction on 1k reports and policies', async () => {
Expand Down

0 comments on commit 3dd9e54

Please sign in to comment.