Skip to content

Part 1: Remove Onyx.connect() for the key: ONYXKEYS.PERSONAL_DETAILS_LIST in src/libs/actions/Report.ts #86840

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

Merged
AndrewGable merged 13 commits into from
Apr 21, 2026
Merged

Part 1: Remove Onyx.connect() for the key: ONYXKEYS.PERSONAL_DETAILS_LIST in src/libs/actions/Report.ts #86840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
be107e4
feat: refactor
lorretheboy Apr 1, 2026
9c018d7
chore: code comment
lorretheboy Apr 1, 2026
556fc05
chore: run prettier
lorretheboy Apr 1, 2026
cb7590e
fix: test
lorretheboy Apr 1, 2026
259d77d
fix: conflicts
lorretheboy Apr 2, 2026
5d700bf
fix: codex review
lorretheboy Apr 2, 2026
d48958b
fix: conflicts
lorretheboy Apr 8, 2026
d428ecf
fix: test
lorretheboy Apr 8, 2026
7576b00
fix: conflicts
lorretheboy Apr 16, 2026
b2c0f59
fix: types
lorretheboy Apr 16, 2026
83ab877
fix: update tests
lorretheboy Apr 16, 2026
cf66cfd
fix: conflicts
lorretheboy Apr 21, 2026
754c28d
Merge branch 'main' of https://github.com/Expensify/App into fix-part…
lorretheboy Apr 21, 2026
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
8 changes: 5 additions & 3 deletions src/components/PromotedActionsBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {joinRoom, navigateToAndOpenReport, navigateToAndOpenReportWithAccountIDs
import {callFunctionIfActionIsAllowed} from '@userActions/Session';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type {PersonalDetailsList} from '@src/types/onyx';
import type Beta from '@src/types/onyx/Beta';
import type OnyxReport from '@src/types/onyx/Report';
import Button from './Button';
Expand All @@ -32,6 +33,7 @@ type PromotedActionsType = Record<BasePromotedActions, (report: OnyxReport) => P
login?: string;
currentUserAccountID: number;
introSelected: OnyxEntry<IntroSelected>;
personalDetails: OnyxEntry<PersonalDetailsList>;
isSelfTourViewed: boolean | undefined;
betas: OnyxEntry<Beta[]>;
}) => PromotedAction;
Expand Down Expand Up @@ -69,7 +71,7 @@ const PromotedActions = {
joinRoom(report, currentUserAccountID);
}),
}),
message: ({reportID, accountID, login, currentUserAccountID, introSelected, isSelfTourViewed, betas}) => ({
message: ({reportID, accountID, login, personalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas}) => ({
key: CONST.PROMOTED_ACTIONS.MESSAGE,
icon: 'CommentBubbles',
translationKey: 'common.message',
Expand All @@ -81,11 +83,11 @@ const PromotedActions = {

// The accountID might be optimistic, so we should use the login if we have it
if (login) {
navigateToAndOpenReport([login], currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
navigateToAndOpenReport([login], personalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
return;
}
if (accountID) {
navigateToAndOpenReportWithAccountIDs([accountID], currentUserAccountID, introSelected, isSelfTourViewed, betas);
navigateToAndOpenReportWithAccountIDs([accountID], currentUserAccountID, introSelected, isSelfTourViewed, betas, personalDetails);
}
},
}),
Expand Down
2 changes: 1 addition & 1 deletion src/components/Search/SearchPageHeader/useSearchPageInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ function useSearchPageInput({queryJSON, onSearch, onSubmit}: UseSearchPageInputP
} else if (item?.reportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(item?.reportID));
} else if ('login' in item) {
navigateToAndOpenReport(item.login ? [item.login] : [], currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
navigateToAndOpenReport(item.login ? [item.login] : [], personalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/components/Search/SearchRouter/SearchRouter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla
if (item?.reportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(item.reportID));
} else if ('login' in item) {
navigateToAndOpenReport(item.login ? [item.login] : [], currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
navigateToAndOpenReport(item.login ? [item.login] : [], personalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas, false);
}
});
onRouterClose();
Expand All @@ -324,6 +324,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla
[
autocompleteSubstitutions,
onRouterClose,
personalDetails,
onSearchQueryChange,
submitSearch,
textInputValue,
Expand Down
101 changes: 82 additions & 19 deletions src/libs/actions/Report/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@
type?: string;
};

type ParticipantInfo = {
login: string;
accountID?: number;
};

type OpenReportActionParams = {
/** The ID of the report to open */
reportID: string | undefined;
Expand All @@ -286,8 +291,12 @@
/** The ID used to fetch a specific range of report actions related to the current reportActionID when opening a chat */
reportActionID?: string;

/** The list of users that are included in a new chat, not including the user creating it */
participantLoginList?: string[];
/** The list of participants */
participants?: ParticipantInfo[];

// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
/** The personal details of the participants */
personalDetails?: OnyxEntry<PersonalDetailsList>;

/** The optimistic report object created when making a new chat, saved as optimistic data */
newReportObject?: OptimisticChatReport;
Expand All @@ -298,9 +307,6 @@
/** Whether or not this report is being opened from a deep link */
isFromDeepLink?: boolean;

/** The list of accountIDs that are included in a new chat, not including the user creating it */
participantAccountIDList?: number[];

/** Whether this is a new thread being created */
isNewThread?: boolean;

Expand Down Expand Up @@ -381,7 +387,7 @@
// map of reportID to all reportActions for that report
const allReportActions: OnyxCollection<ReportActions> = {};

Onyx.connect({

Check warning on line 390 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!key || !actions) {
Expand All @@ -393,7 +399,7 @@
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 402 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -402,7 +408,7 @@
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {};
Onyx.connect({

Check warning on line 411 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand All @@ -417,7 +423,7 @@
});

let onboarding: OnyxEntry<Onboarding>;
Onyx.connect({

Check warning on line 426 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_ONBOARDING,
callback: (val) => {
if (Array.isArray(val)) {
Expand Down Expand Up @@ -1174,6 +1180,13 @@
guidedSetupData: string;
};

function buildParticipantInfoFromLogins(logins: string[], accountIDs?: number[]): ParticipantInfo[] {
return logins.map((login, index) => ({
login,
accountID: accountIDs?.[index],
}));
}

/**
* Prepares guided setup data for an OpenReport request when onboarding is not yet completed.
* Returns the onyx data arrays and guidedSetupData string to include in the API parameters,
Expand Down Expand Up @@ -1252,11 +1265,11 @@
reportID,
introSelected,
reportActionID,
participantLoginList = [],
participants = [],
newReportObject,
parentReportActionID,
isFromDeepLink = false,
participantAccountIDList = [],
personalDetails,
isNewThread = false,
transaction,
transactionViolations,
Expand All @@ -1272,6 +1285,10 @@
return;
}

const participantLoginList = participants.map((p) => p.login).filter((login) => !!login);
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
const participantAccountIDList = participants.map((p) => p.accountID).filter((id): id is number => id !== undefined);

const optimisticReport = reportActionsExist(reportID)
? {}
: {
Expand Down Expand Up @@ -1524,7 +1541,8 @@

let emailCreatingAction: string = CONST.REPORT.OWNER_EMAIL_FAKE;
if (newReportObject.ownerAccountID && newReportObject.ownerAccountID !== CONST.REPORT.OWNER_ACCOUNT_ID_FAKE) {
emailCreatingAction = allPersonalDetails?.[newReportObject.ownerAccountID]?.login ?? '';
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
emailCreatingAction = (personalDetails ?? allPersonalDetails)?.[newReportObject.ownerAccountID]?.login ?? '';
}
const optimisticCreatedAction = buildOptimisticCreatedReportAction(emailCreatingAction);
optimisticData.push(
Expand Down Expand Up @@ -1562,7 +1580,7 @@
const participantAccountIDs = PersonalDetailsUtils.getAccountIDsByLogins(participantLoginList);
for (const [index, login] of participantLoginList.entries()) {
const accountID = participantAccountIDs.at(index) ?? -1;
const isOptimisticAccount = !allPersonalDetails?.[accountID];
const isOptimisticAccount = !(personalDetails ?? allPersonalDetails)?.[accountID];

if (!isOptimisticAccount) {
continue;
Expand Down Expand Up @@ -1894,6 +1912,8 @@
iouReportAction?: OnyxEntry<ReportAction>,
transaction?: Transaction,
transactionViolations?: TransactionViolations,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
): OptimisticChatReport | undefined {
// Determine if we need selfDM report (for track expenses or unreported transactions)
const isTrackExpense = !iouReport && ReportActionsUtils.isTrackExpenseAction(iouReportAction);
Expand Down Expand Up @@ -1931,7 +1951,9 @@
openReport({
reportID: optimisticTransactionThreadReportID,
introSelected,
participantLoginList: currentUserLogin ? [currentUserLogin] : [],
participants: currentUserLogin ? [{login: currentUserLogin, accountID: currentUserAccountID}] : [],
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails: personalDetails ?? allPersonalDetails,
newReportObject: optimisticTransactionThread,
parentReportActionID: iouReportAction?.reportActionID,
transaction,
Expand Down Expand Up @@ -1981,6 +2003,7 @@
*/
function navigateToAndOpenReport(
userLogins: string[],
personalDetails: OnyxEntry<PersonalDetailsList>,
currentUserAccountID: number,
introSelected: OnyxEntry<IntroSelected>,
isSelfTourViewed: boolean | undefined,
Expand All @@ -1998,7 +2021,16 @@
currentUserAccountID,
});
// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport({reportID: newChat?.reportID, introSelected, reportActionID: '', participantLoginList: userLogins, newReportObject: newChat, isSelfTourViewed, betas});
openReport({
reportID: newChat?.reportID,
introSelected,
reportActionID: '',
participants: buildParticipantInfoFromLogins(userLogins),
personalDetails,
newReportObject: newChat,
isSelfTourViewed,
betas,
});
}
const report = isEmptyObject(chat) ? newChat : chat;

Expand Down Expand Up @@ -2037,8 +2069,17 @@
introSelected: OnyxEntry<IntroSelected>,
isSelfTourViewed: boolean | undefined,
betas: OnyxEntry<Beta[]>,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
) {
let newChat: OptimisticChatReport | undefined;
const participants = participantAccountIDs.map((accountID): ParticipantInfo => {
return {
login: '',
accountID,
};
});

const chat = getChatByParticipants([...participantAccountIDs, currentUserAccountID]);
if (!chat) {
newChat = buildOptimisticChatReport({
Expand All @@ -2052,7 +2093,9 @@
isSelfTourViewed,
newReportObject: newChat,
parentReportActionID: '0',
participantAccountIDList: participantAccountIDs,
participants,
Copy link
Copy Markdown
Member

@parasharrajat parasharrajat Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not passing the login list here earlier. But now it will be mapped from the personal details Collection. Then that value will be passed to the API and used further in this function. Is it fine?

Copy link
Copy Markdown
Member

@parasharrajat parasharrajat Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump @lorretheboy

Copy link
Copy Markdown
Contributor Author

@lorretheboy lorretheboy Apr 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe no new behavior is introduced. For sure, I excluded them

Copy link
Copy Markdown
Member

@parasharrajat parasharrajat Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THanks, I will check.

Copy link
Copy Markdown
Member

@parasharrajat parasharrajat Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see that now. Login is empty string.

// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails: personalDetails ?? allPersonalDetails,
betas,
});
}
Expand All @@ -2077,8 +2120,10 @@
currentUserAccountID: number,
introSelected: OnyxEntry<IntroSelected>,
betas: OnyxEntry<Beta[]>,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
) {
const report = childReport ?? createChildReport(childReport, parentReportAction, parentReport, currentUserAccountID, introSelected, betas);
const report = childReport ?? createChildReport(childReport, parentReportAction, parentReport, currentUserAccountID, introSelected, betas, personalDetails);

Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID, undefined, undefined, Navigation.getActiveRoute()));
}
Expand All @@ -2095,6 +2140,8 @@
currentUserAccountID: number,
introSelected: OnyxEntry<IntroSelected>,
betas: OnyxEntry<Beta[]>,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
): Report {
const participantAccountIDs = [...new Set([currentUserAccountID, Number(parentReportAction.actorAccountID)])];
// Threads from DMs and selfDMs don't have a chatType. All other threads inherit the chatType from their parent
Expand Down Expand Up @@ -2124,11 +2171,16 @@
};
}

const participantLogins = PersonalDetailsUtils.getLoginsByAccountIDs(Object.keys(newChat.participants ?? {}).map(Number));
const participantAccountIDsForDetails = Object.keys(newChat.participants ?? {}).map(Number);
const participantLogins = PersonalDetailsUtils.getLoginsByAccountIDs(participantAccountIDsForDetails);
const participants = buildParticipantInfoFromLogins(participantLogins);

openReport({
reportID: newChat.reportID,
introSelected,
participantLoginList: participantLogins,
participants,
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails: personalDetails ?? allPersonalDetails,
newReportObject: newChat,
parentReportActionID: parentReportAction.reportActionID,
isNewThread: true,
Expand Down Expand Up @@ -3009,6 +3061,8 @@
isSelfTourViewed: boolean | undefined,
betas: OnyxEntry<Beta[]>,
prevNotificationPreference?: NotificationPreference,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
) {
if (childReportID) {
openReport({reportID: childReportID, introSelected, betas, isSelfTourViewed});
Expand Down Expand Up @@ -3047,10 +3101,13 @@
});

const participantLogins = PersonalDetailsUtils.getLoginsByAccountIDs(participantAccountIDs);
const participants = buildParticipantInfoFromLogins(participantLogins);
openReport({
reportID: newChat.reportID,
introSelected,
participantLoginList: participantLogins,
participants,
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails: personalDetails ?? allPersonalDetails,
newReportObject: newChat,
parentReportActionID: parentReportAction.reportActionID,
isSelfTourViewed,
Expand Down Expand Up @@ -3460,6 +3517,8 @@
checkIfCurrentPageActive = () => true,
linkToOptions?: LinkToOptions,
reportActionID?: string,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
) {
// If conciergeReportID contains a concierge report ID, we navigate to the concierge chat using the stored report ID.
// Otherwise, we would find the concierge chat and navigate to it.
Expand All @@ -3471,7 +3530,8 @@
if (!checkIfCurrentPageActive()) {
return;
}
navigateToAndOpenReport([CONST.EMAIL.CONCIERGE], currentUserAccountID, introSelected, isSelfTourViewed, betas, shouldDismissModal);
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
navigateToAndOpenReport([CONST.EMAIL.CONCIERGE], personalDetails ?? allPersonalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas, shouldDismissModal);
});
} else if (shouldDismissModal) {
Navigation.dismissModalWithReport({reportID: conciergeReportID, reportActionID});
Expand Down Expand Up @@ -3937,14 +3997,17 @@
betas: OnyxEntry<Beta[]>,
shouldPopToTop = false,
shouldDeleteChildReports = false,
// TODO: personalDetails should be a required field in follow-up PRs https://github.com/Expensify/App/issues/73656
personalDetails?: OnyxEntry<PersonalDetailsList>,
) {
// Dismiss the current report screen and replace it with Concierge Chat
if (shouldPopToTop) {
Navigation.popToSidebar();
} else {
Navigation.goBack();
}
navigateToConciergeChat(conciergeReportID, introSelected, currentUserAccountID, isSelfTourViewed, betas, false);
// TODO: allPersonalDetails fallback should be removed in follow-up PRs https://github.com/Expensify/App/issues/73656
navigateToConciergeChat(conciergeReportID, introSelected, currentUserAccountID, isSelfTourViewed, betas, false, undefined, undefined, undefined, personalDetails ?? allPersonalDetails);
// eslint-disable-next-line @typescript-eslint/no-deprecated
InteractionManager.runAfterInteractions(() => {
deleteReport(reportID, shouldDeleteChildReports);
Expand Down Expand Up @@ -7285,7 +7348,7 @@
});
}

export type {Video, GuidedSetupData, TaskForParameters, IntroSelected, OpenReportActionParams};
export type {Video, GuidedSetupData, TaskForParameters, IntroSelected, OpenReportActionParams, ParticipantInfo};

export {
addAttachmentWithComment,
Expand Down
3 changes: 2 additions & 1 deletion src/pages/NewChatPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ function NewChatPage({ref}: NewChatPageProps) {
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [isSelfTourViewed] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {selector: hasSeenTourSelector});
const selectionListRef = useRef<SelectionListWithSectionsHandle | null>(null);
const allPersonalDetails = usePersonalDetails();

const [reportAttributesDerivedFull] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES);

Expand Down Expand Up @@ -379,7 +380,7 @@ function NewChatPage({ref}: NewChatPageProps) {
return;
}
KeyboardUtils.dismiss().then(() => {
singleExecution(() => navigateToAndOpenReport([login], currentUserAccountID, introSelected, isSelfTourViewed, betas))();
singleExecution(() => navigateToAndOpenReport([login], allPersonalDetails, currentUserAccountID, introSelected, isSelfTourViewed, betas))();
});
};

Expand Down
4 changes: 3 additions & 1 deletion src/pages/ProfilePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ function ProfilePage({route}: ProfilePageProps) {

// If it's a self DM, we only want to show the Message button if the self DM report exists because we don't want to optimistically create a report for self DM
if ((!isCurrentUser || report) && !isAnonymousUserSession()) {
promotedActions.push(PromotedActions.message({reportID: report?.reportID, accountID, login: loginParams, currentUserAccountID, introSelected, isSelfTourViewed, betas}));
promotedActions.push(
PromotedActions.message({reportID: report?.reportID, personalDetails, accountID, login: loginParams, currentUserAccountID, introSelected, isSelfTourViewed, betas}),
);
}

return (
Expand Down
Loading
Loading