Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mostRecentReportActionLastModified from ReconnectApp calls #57780

Merged
merged 3 commits into from
Mar 5, 2025
Merged

Remove mostRecentReportActionLastModified from ReconnectApp calls #57780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e47861
remove mostRecentReportActionLastModified from reconnectApp
MarioExpensify Mar 4, 2025
86da403
fix lint
MarioExpensify Mar 4, 2025
31d4387
prettier
MarioExpensify Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/libs/API/parameters/ReconnectAppParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
type ReconnectAppParams = {
mostRecentReportActionLastModified?: string;
updateIDFrom?: number;
policyIDList: string[];
};
Expand Down
40 changes: 0 additions & 40 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,45 +987,6 @@ function getReportAction(reportID: string | undefined, reportActionID: string |
return allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]?.[reportActionID];
}

function getMostRecentReportActionLastModified(): string {
// Start with the oldest date possible
let mostRecentReportActionLastModified = DateUtils.getDBTime(0);

// Flatten all the actions
// Loop over them all to find the one that is the most recent
const flatReportActions = Object.values(allReportActions ?? {})
.flatMap((actions) => (actions ? Object.values(actions) : []))
.filter(Boolean);
flatReportActions.forEach((action) => {
// Pending actions should not be counted here as a user could create a comment or some other action while offline and the server might know about
// messages they have not seen yet.
if (action.pendingAction) {
return;
}

const lastModified = action.lastModified ?? action.created;

if (lastModified < mostRecentReportActionLastModified) {
return;
}

mostRecentReportActionLastModified = lastModified;
});

// We might not have actions so we also look at the report objects to see if any have a lastVisibleActionLastModified that is more recent. We don't need to get
// any reports that have been updated before either a recently updated report or reportAction as we should be up to date on these
Object.values(allReports ?? {}).forEach((report) => {
const reportLastVisibleActionLastModified = report?.lastVisibleActionLastModified ?? report?.lastVisibleActionCreated;
if (!reportLastVisibleActionLastModified || reportLastVisibleActionLastModified < mostRecentReportActionLastModified) {
return;
}

mostRecentReportActionLastModified = reportLastVisibleActionLastModified;
});

return mostRecentReportActionLastModified;
}

/**
* @returns The report preview action or `null` if one couldn't be found
*/
Expand Down Expand Up @@ -2265,7 +2226,6 @@ export {
getReportActionMessageFragments,
getMessageOfOldDotReportAction,
getMostRecentIOURequestActionID,
getMostRecentReportActionLastModified,
getNumberOfMoneyRequests,
getOneTransactionThreadReportID,
getOriginalMessage,
Expand Down
19 changes: 0 additions & 19 deletions src/libs/actions/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import Log from '@libs/Log';
import getCurrentUrl from '@libs/Navigation/currentUrl';
import Navigation from '@libs/Navigation/Navigation';
import Performance from '@libs/Performance';
import {getMostRecentReportActionLastModified} from '@libs/ReportActionsUtils';
import {isLoggingInAsNewUser as isLoggingInAsNewUserSessionUtils} from '@libs/SessionUtils';
import {clearSoundAssetsCache} from '@libs/Sound';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -313,15 +312,6 @@ function reconnectApp(updateIDFrom: OnyxEntry<number> = 0) {
getPolicyParamsForOpenOrReconnect().then((policyParams) => {
const params: ReconnectAppParams = policyParams;

// When the app reconnects we do a fast "sync" of the LHN and only return chats that have new messages. We achieve this by sending the most recent reportActionID.
// we have locally. And then only update the user about chats with messages that have occurred after that reportActionID.
//
// - Look through the local report actions and reports to find the most recently modified report action or report.
// - We send this to the server so that it can compute which new chats the user needs to see and return only those as an optimization.
Timing.start(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION);
params.mostRecentReportActionLastModified = getMostRecentReportActionLastModified();
Timing.end(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION, '', 500);

// Include the update IDs when reconnecting so that the server can send incremental updates if they are available.
// Otherwise, a full set of app data will be returned.
if (updateIDFrom) {
Expand All @@ -345,15 +335,6 @@ function finalReconnectAppAfterActivatingReliableUpdates(): Promise<void | OnyxT
return getPolicyParamsForOpenOrReconnect().then((policyParams) => {
const params: ReconnectAppParams = {...policyParams};

// When the app reconnects we do a fast "sync" of the LHN and only return chats that have new messages. We achieve this by sending the most recent reportActionID.
// we have locally. And then only update the user about chats with messages that have occurred after that reportActionID.
//
// - Look through the local report actions and reports to find the most recently modified report action or report.
// - We send this to the server so that it can compute which new chats the user needs to see and return only those as an optimization.
Timing.start(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION);
params.mostRecentReportActionLastModified = getMostRecentReportActionLastModified();
Timing.end(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION, '', 500);

// It is SUPER BAD FORM to return promises from action methods.
// DO NOT FOLLOW THIS PATTERN!!!!!
// It was absolutely necessary in order to not break the app while migrating to the new reliable updates pattern. This method will be removed
Expand Down
23 changes: 9 additions & 14 deletions tests/perf-test/ReportActionsUtils.perf-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Onyx from 'react-native-onyx';
import {measureFunction} from 'reassure';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import {getLastClosedReportAction, getLastVisibleAction, getLastVisibleMessage, getMostRecentIOURequestActionID, getSortedReportActionsForDisplay} from '@libs/ReportActionsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ReportActions} from '@src/types/onyx/ReportAction';
Expand Down Expand Up @@ -63,7 +63,7 @@ describe('ReportActionsUtils', () => {
*/
test('[ReportActionsUtils] getLastVisibleAction on 10k reportActions', async () => {
await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getLastVisibleAction(reportId));
await measureFunction(() => getLastVisibleAction(reportId));
});

test('[ReportActionsUtils] getLastVisibleAction on 10k reportActions with actionsToMerge', async () => {
Expand All @@ -89,19 +89,19 @@ describe('ReportActionsUtils', () => {
} as unknown as ReportActions;

await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getLastVisibleAction(reportId, true, actionsToMerge));
await measureFunction(() => getLastVisibleAction(reportId, true, actionsToMerge));
});

test('[ReportActionsUtils] getMostRecentIOURequestActionID on 10k ReportActions', async () => {
const reportActionsArray = ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true);
const reportActionsArray = getSortedReportActionsForDisplay(reportActions, true);

await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getMostRecentIOURequestActionID(reportActionsArray));
await measureFunction(() => getMostRecentIOURequestActionID(reportActionsArray));
});

test('[ReportActionsUtils] getLastVisibleMessage on 10k ReportActions', async () => {
await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getLastVisibleMessage(reportId));
await measureFunction(() => getLastVisibleMessage(reportId));
});

test('[ReportActionsUtils] getLastVisibleMessage on 10k ReportActions with actionsToMerge', async () => {
Expand All @@ -127,21 +127,16 @@ describe('ReportActionsUtils', () => {
} as unknown as ReportActions;

await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getLastVisibleMessage(reportId, true, actionsToMerge));
await measureFunction(() => getLastVisibleMessage(reportId, true, actionsToMerge));
});

test('[ReportActionsUtils] getSortedReportActionsForDisplay on 10k ReportActions', async () => {
await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true));
await measureFunction(() => getSortedReportActionsForDisplay(reportActions, true));
});

test('[ReportActionsUtils] getLastClosedReportAction on 10k ReportActions', async () => {
await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getLastClosedReportAction(reportActions));
});

test('[ReportActionsUtils] getMostRecentReportActionLastModified', async () => {
await waitForBatchedUpdates();
await measureFunction(() => ReportActionsUtils.getMostRecentReportActionLastModified());
await measureFunction(() => getLastClosedReportAction(reportActions));
});
});
Loading