Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions src/components/PromotedActionsBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {joinRoom, navigateToAndOpenReport, navigateToAndOpenReportWithAccountIDs
import {callFunctionIfActionIsAllowed} from '@userActions/Session';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type Beta from '@src/types/onyx/Beta';
import type OnyxReport from '@src/types/onyx/Report';
import Button from './Button';
import type {ThreeDotsMenuItem} from './HeaderWithBackButton/types';
Expand All @@ -33,6 +34,7 @@ type PromotedActionsType = Record<BasePromotedActions, (report: OnyxReport) => P
currentUserAccountID: number;
introSelected: OnyxEntry<IntroSelected>;
isSelfTourViewed: boolean | undefined;
betas: OnyxEntry<Beta[]>;
}) => PromotedAction;
} & {
[CONST.PROMOTED_ACTIONS.JOIN]: (report: OnyxReport, currentUserAccountID: number) => PromotedAction;
Expand Down Expand Up @@ -64,7 +66,7 @@ const PromotedActions = {
joinRoom(report, currentUserAccountID);
}),
}),
message: ({reportID, accountID, login, currentUserAccountID, introSelected, isSelfTourViewed}) => ({
message: ({reportID, accountID, login, currentUserAccountID, introSelected, isSelfTourViewed, betas}) => ({
key: CONST.PROMOTED_ACTIONS.MESSAGE,
icon: 'CommentBubbles',
translationKey: 'common.message',
Expand All @@ -80,7 +82,7 @@ const PromotedActions = {
return;
}
if (accountID) {
navigateToAndOpenReportWithAccountIDs([accountID], currentUserAccountID, introSelected);
navigateToAndOpenReportWithAccountIDs([accountID], currentUserAccountID, introSelected, betas);
}
},
}),
Expand Down
3 changes: 2 additions & 1 deletion src/libs/actions/Report/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@
// map of reportID to all reportActions for that report
const allReportActions: OnyxCollection<ReportActions> = {};

Onyx.connect({

Check warning on line 376 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 @@ -385,7 +385,7 @@
});

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

Check warning on line 388 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 @@ -394,7 +394,7 @@
});

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

Check warning on line 397 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 @@ -412,7 +412,7 @@
});

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

Check warning on line 415 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 @@ -2061,7 +2061,7 @@
*
* @param participantAccountIDs of user logins to start a chat report with.
*/
function navigateToAndOpenReportWithAccountIDs(participantAccountIDs: number[], currentUserAccountID: number, introSelected: OnyxEntry<IntroSelected>) {
function navigateToAndOpenReportWithAccountIDs(participantAccountIDs: number[], currentUserAccountID: number, introSelected: OnyxEntry<IntroSelected>, betas: OnyxEntry<Beta[]>) {
let newChat: OptimisticChatReport | undefined;
const chat = getChatByParticipants([...participantAccountIDs, currentUserAccountID]);
if (!chat) {
Expand All @@ -2075,6 +2075,7 @@
newReportObject: newChat,
parentReportActionID: '0',
participantAccountIDList: participantAccountIDs,
betas,
});
}
const report = chat ?? newChat;
Expand Down
10 changes: 8 additions & 2 deletions src/libs/actions/replaceOptimisticReportWithActualReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,19 @@ function replaceOptimisticReportWithActualReport(report: Report, draftReportComm
callback();

// We are already on the parent one expense report, so just call the API to fetch report data
openReport({reportID: parentReportID, introSelected: undefined});
// betas is safe to pass as undefined because introSelected is undefined, so the code path
// that uses betas is never reached. Passing it explicitly so the compiler flags this when
// betas becomes required. Refactor issue: https://github.com/Expensify/App/issues/66424
openReport({reportID: parentReportID, introSelected: undefined, betas: undefined});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we don't need the betas here? And if we really don't, wouldn't it be better if that was the default value so we don't have to be explicit about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

betas is safe to pass as undefined here because introSelected is always undefined in these calls, so the code path that uses betas is never reached. We're passing it explicitly (rather than omitting it) so that the compiler will flag these call sites when we make betas required in a future PR. I've added a comment to clarify this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

});
} else {
callback();

// We are already on the parent one expense report, so just call the API to fetch report data
openReport({reportID: parentReportID, introSelected: undefined});
// betas is safe to pass as undefined because introSelected is undefined, so the code path
// that uses betas is never reached. Passing it explicitly so the compiler flags this when
// betas becomes required. Refactor issue: https://github.com/Expensify/App/issues/66424
openReport({reportID: parentReportID, introSelected: undefined, betas: undefined});
}
return;
}
Expand Down
3 changes: 2 additions & 1 deletion src/pages/ProfilePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ function ProfilePage({route}: ProfilePageProps) {
const [account] = useOnyx(ONYXKEYS.ACCOUNT);
const [isDebugModeEnabled = false] = useOnyx(ONYXKEYS.IS_DEBUG_MODE_ENABLED);
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [isSelfTourViewed] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {selector: hasSeenTourSelector});
const guideCalendarLink = account?.guideDetails?.calendarLink ?? '';
const expensifyIcons = useMemoizedLazyExpensifyIcons(['Bug', 'Pencil', 'Phone']);
Expand Down Expand Up @@ -165,7 +166,7 @@ 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}));
promotedActions.push(PromotedActions.message({reportID: report?.reportID, accountID, login: loginParams, currentUserAccountID, introSelected, isSelfTourViewed, betas}));
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function WorkspaceWorkflowsPayerPage({route, policy, personalDetails, isLoadingR
const bankAccountInfo = bankAccountFromList ?? bankAccountConnectedToWorkspace;
const bankAccountID = policyBankAccountID ?? bankAccountInfo?.accountData?.bankAccountID;
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const {isOffline} = useNetwork();
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE);
Expand Down Expand Up @@ -357,7 +358,7 @@ function WorkspaceWorkflowsPayerPage({route, policy, personalDetails, isLoadingR
return;
}
setShowErrorModal(false);
navigateToAndOpenReportWithAccountIDs([policy.ownerAccountID], currentUserPersonalDetails.accountID, introSelected);
navigateToAndOpenReportWithAccountIDs([policy.ownerAccountID], currentUserPersonalDetails.accountID, introSelected, betas);
}}
html={translate('workflowsPayerPage.shareBankAccount.errorDescription', {
admin: selectedPayerDetails?.displayName ?? '',
Expand Down
46 changes: 46 additions & 0 deletions tests/actions/ReportTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5729,6 +5729,52 @@ describe('actions/Report', () => {
});
});

describe('navigateToAndOpenReportWithAccountIDs', () => {
it('should create new chat and pass betas to openReport when no existing chat', async () => {
const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = 'test@user.com';
const PARTICIPANT_ACCOUNT_ID = 2;

await TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN);
await TestHelper.setPersonalDetails(TEST_USER_LOGIN, TEST_USER_ACCOUNT_ID);

const testIntroSelected: OnyxTypes.IntroSelected = {choice: CONST.ONBOARDING_CHOICES.ADMIN};

Report.navigateToAndOpenReportWithAccountIDs([PARTICIPANT_ACCOUNT_ID], TEST_USER_ACCOUNT_ID, testIntroSelected, undefined);
await waitForBatchedUpdates();

TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 1);
expect(Navigation.navigate).toHaveBeenCalled();
});

it('should not call openReport when chat already exists', async () => {
const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = 'test@user.com';
const PARTICIPANT_ACCOUNT_ID = 2;
const EXISTING_REPORT_ID = '456';

await TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN);
await TestHelper.setPersonalDetails(TEST_USER_LOGIN, TEST_USER_ACCOUNT_ID);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${EXISTING_REPORT_ID}`, {
reportID: EXISTING_REPORT_ID,
type: CONST.REPORT.TYPE.CHAT,
participants: {
[TEST_USER_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS},
[PARTICIPANT_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS},
},
});
await waitForBatchedUpdates();

const testIntroSelected: OnyxTypes.IntroSelected = {choice: CONST.ONBOARDING_CHOICES.ADMIN};

Report.navigateToAndOpenReportWithAccountIDs([PARTICIPANT_ACCOUNT_ID], TEST_USER_ACCOUNT_ID, testIntroSelected, undefined);
await waitForBatchedUpdates();

TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 0);
expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute(EXISTING_REPORT_ID));
});
});

describe('getGuidedSetupDataForOpenReport', () => {
const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = 'test@user.com';
Expand Down
25 changes: 23 additions & 2 deletions tests/unit/PromotedActionsBarTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('PromotedActions.message', () => {
currentUserAccountID: 1,
introSelected,
isSelfTourViewed: false,
betas: undefined,
});

action.onSelected();
Expand All @@ -46,11 +47,12 @@ describe('PromotedActions.message', () => {
currentUserAccountID: 1,
introSelected,
isSelfTourViewed: false,
betas: undefined,
});

action.onSelected();

expect(mockNavigateToAndOpenReportWithAccountIDs).toHaveBeenCalledWith([42], 1, introSelected);
expect(mockNavigateToAndOpenReportWithAccountIDs).toHaveBeenCalledWith([42], 1, introSelected, undefined);
});

it('should pass undefined introSelected when not provided', () => {
Expand All @@ -59,11 +61,12 @@ describe('PromotedActions.message', () => {
currentUserAccountID: 1,
introSelected: undefined,
isSelfTourViewed: undefined,
betas: undefined,
});

action.onSelected();

expect(mockNavigateToAndOpenReportWithAccountIDs).toHaveBeenCalledWith([42], 1, undefined);
expect(mockNavigateToAndOpenReportWithAccountIDs).toHaveBeenCalledWith([42], 1, undefined, undefined);
});

it('should navigate to report directly when reportID is provided', () => {
Expand All @@ -72,6 +75,7 @@ describe('PromotedActions.message', () => {
currentUserAccountID: 1,
introSelected: undefined,
isSelfTourViewed: undefined,
betas: undefined,
});

action.onSelected();
Expand All @@ -88,11 +92,28 @@ describe('PromotedActions.message', () => {
currentUserAccountID: 1,
introSelected,
isSelfTourViewed: false,
betas: undefined,
});

action.onSelected();

expect(mockNavigateToAndOpenReport).toHaveBeenCalledWith(['test@example.com'], 1, introSelected, false, false);
expect(mockNavigateToAndOpenReportWithAccountIDs).not.toHaveBeenCalled();
});

it('should pass betas to navigateToAndOpenReportWithAccountIDs when accountID is provided', () => {
const introSelected = {choice: CONST.ONBOARDING_CHOICES.MANAGE_TEAM};
const betas = [CONST.BETAS.ALL];
const action = PromotedActions.message({
accountID: 42,
currentUserAccountID: 1,
introSelected,
isSelfTourViewed: false,
betas,
});

action.onSelected();

expect(mockNavigateToAndOpenReportWithAccountIDs).toHaveBeenCalledWith([42], 1, introSelected, betas);
});
});
Loading