From d301a133ec603c14039beebed4597cad88f5ec8c Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Wed, 4 Sep 2024 13:29:30 -0700 Subject: [PATCH 1/4] Create all onboarding tasks with Concierge --- src/libs/actions/Report.ts | 23 ++----------------- .../BaseOnboardingPersonalDetails.tsx | 6 +---- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index bd31d3832605..1f87c65fcf84 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2164,17 +2164,6 @@ function navigateToConciergeChat(shouldDismissModal = false, checkIfCurrentPageA } } -/** - * Navigates to the 1:1 system chat - */ -function navigateToSystemChat() { - const systemChatReport = ReportUtils.getSystemChat(); - - if (systemChatReport?.reportID) { - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(systemChatReport.reportID)); - } -} - /** Add a policy report (workspace room) optimistically and navigate to it. */ function addPolicyReport(policyReport: ReportUtils.OptimisticChatReport) { const createdReportAction = ReportUtils.buildOptimisticCreatedReportAction(CONST.POLICY.OWNER_EMAIL_FAKE); @@ -3329,13 +3318,7 @@ function completeOnboarding( adminsChatReportID?: string, onboardingPolicyID?: string, ) { - const isAccountIDOdd = AccountUtils.isAccountIDOddNumber(currentUserAccountID ?? 0); - const targetEmail = isAccountIDOdd ? CONST.EMAIL.NOTIFICATIONS : CONST.EMAIL.CONCIERGE; - - // If the target report isn't opened, the permission field will not exist. So we should add the fallback permission for task report - const fallbackPermission = isAccountIDOdd ? [CONST.REPORT.PERMISSIONS.READ] : [CONST.REPORT.PERMISSIONS.READ, CONST.REPORT.PERMISSIONS.WRITE]; - - const actorAccountID = PersonalDetailsUtils.getAccountIDsByLogins([targetEmail])[0]; + const actorAccountID = PersonalDetailsUtils.getAccountIDsByLogins([CONST.EMAIL.CONCIERGE])[0]; const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]); const {reportID: targetChatReportID = '', policyID: targetChatPolicyID = ''} = targetChatReport ?? {}; @@ -3386,7 +3369,7 @@ function completeOnboarding( targetChatPolicyID, CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); - const taskCreatedAction = ReportUtils.buildOptimisticCreatedReportAction(targetEmail); + const taskCreatedAction = ReportUtils.buildOptimisticCreatedReportAction(CONST.EMAIL.CONCIERGE); const taskReportAction = ReportUtils.buildOptimisticTaskCommentReportAction( currentTask.reportID, task.title, @@ -3450,7 +3433,6 @@ function completeOnboarding( }, isOptimisticReport: true, managerID: currentUserAccountID, - permissions: targetChatReport?.permissions ?? fallbackPermission, }, }, { @@ -4109,7 +4091,6 @@ export { saveReportActionDraft, deleteReportComment, navigateToConciergeChat, - navigateToSystemChat, addPolicyReport, deleteReport, navigateToConciergeChatAndDeleteReport, diff --git a/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx b/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx index d2dbd7eff953..c5dab0f8a247 100644 --- a/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx +++ b/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx @@ -76,11 +76,7 @@ function BaseOnboardingPersonalDetails({ // Only navigate to concierge chat when central pane is visible // Otherwise stay on the chats screen. if (!shouldUseNarrowLayout && !route.params?.backTo) { - if (AccountUtils.isAccountIDOddNumber(accountID ?? 0)) { - Report.navigateToSystemChat(); - } else { - Report.navigateToConciergeChat(); - } + Report.navigateToConciergeChat(); } }, [onboardingPurposeSelected, onboardingAdminsChatReportID, onboardingPolicyID, shouldUseNarrowLayout, route.params?.backTo, accountID], From 0e704458c5c42c4b32d6d4d9374e6a8f3c3ea1d5 Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Wed, 4 Sep 2024 13:40:00 -0700 Subject: [PATCH 2/4] Remove unused dep, use Concierge ID directly --- src/libs/actions/Report.ts | 3 +-- .../BaseOnboardingPersonalDetails.tsx | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 1f87c65fcf84..6958e4d84426 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -8,7 +8,6 @@ import Onyx from 'react-native-onyx'; import type {PartialDeep, ValueOf} from 'type-fest'; import type {Emoji} from '@assets/emojis/types'; import type {FileObject} from '@components/AttachmentModal'; -import AccountUtils from '@libs/AccountUtils'; import * as ActiveClientManager from '@libs/ActiveClientManager'; import * as API from '@libs/API'; import type { @@ -3318,7 +3317,7 @@ function completeOnboarding( adminsChatReportID?: string, onboardingPolicyID?: string, ) { - const actorAccountID = PersonalDetailsUtils.getAccountIDsByLogins([CONST.EMAIL.CONCIERGE])[0]; + const actorAccountID = CONST.ACCOUNT_ID.CONCIERGE; const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]); const {reportID: targetChatReportID = '', policyID: targetChatPolicyID = ''} = targetChatReport ?? {}; diff --git a/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx b/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx index c5dab0f8a247..4579059c23bc 100644 --- a/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx +++ b/src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx @@ -6,7 +6,6 @@ import InputWrapper from '@components/Form/InputWrapper'; import type {FormOnyxValues} from '@components/Form/types'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import OfflineIndicator from '@components/OfflineIndicator'; -import {useSession} from '@components/OnyxProvider'; import ScreenWrapper from '@components/ScreenWrapper'; import Text from '@components/Text'; import TextInput from '@components/TextInput'; @@ -15,7 +14,6 @@ import useAutoFocusInput from '@hooks/useAutoFocusInput'; import useLocalize from '@hooks/useLocalize'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useThemeStyles from '@hooks/useThemeStyles'; -import AccountUtils from '@libs/AccountUtils'; import * as ErrorUtils from '@libs/ErrorUtils'; import Navigation from '@libs/Navigation/Navigation'; import * as ValidationUtils from '@libs/ValidationUtils'; @@ -40,7 +38,6 @@ function BaseOnboardingPersonalDetails({ const {shouldUseNarrowLayout, onboardingIsMediumOrLargerScreenWidth} = useResponsiveLayout(); const {inputCallbackRef} = useAutoFocusInput(); const [shouldValidateOnChange, setShouldValidateOnChange] = useState(false); - const {accountID} = useSession(); useEffect(() => { Welcome.setOnboardingErrorMessage(''); @@ -79,7 +76,7 @@ function BaseOnboardingPersonalDetails({ Report.navigateToConciergeChat(); } }, - [onboardingPurposeSelected, onboardingAdminsChatReportID, onboardingPolicyID, shouldUseNarrowLayout, route.params?.backTo, accountID], + [onboardingPurposeSelected, onboardingAdminsChatReportID, onboardingPolicyID, shouldUseNarrowLayout, route.params?.backTo], ); const validate = (values: FormOnyxValues<'onboardingPersonalDetailsForm'>) => { From 891c6ec154ad727d5fe5d50970640f4a92747b98 Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Wed, 4 Sep 2024 14:49:20 -0700 Subject: [PATCH 3/4] Don't show Expensify DM in background behind modal , update docs --- src/libs/ReportUtils.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 7b226b2e5c8e..990b83ff2a38 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -55,7 +55,6 @@ import type {Message, ReportActions} from '@src/types/onyx/ReportAction'; import type {Comment, TransactionChanges, WaypointCollection} from '@src/types/onyx/Transaction'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type IconAsset from '@src/types/utils/IconAsset'; -import AccountUtils from './AccountUtils'; import * as IOU from './actions/IOU'; import * as PolicyActions from './actions/Policy/Policy'; import * as store from './actions/ReimbursementAccount/store'; @@ -5981,7 +5980,10 @@ function shouldReportBeInOptionList({ return false; } - if (report?.participants?.[CONST.ACCOUNT_ID.NOTIFICATIONS] && (!currentUserAccountID || !AccountUtils.isAccountIDOddNumber(currentUserAccountID))) { + // We used to use the system DM for A/B testing onboarding tasks, but now only create them in the Concierge chat. We + // still need to allow existing users who have tasks in the system DM to see them, but otherwise we don't need to + // show that chat + if (report?.participants?.[CONST.ACCOUNT_ID.NOTIFICATIONS] && isEmptyReport(report)) { return false; } @@ -7619,8 +7621,9 @@ function shouldShowMerchantColumn(transactions: Transaction[]) { } /** - * Whether the report is a system chat or concierge chat, depending on the onboarding report ID or fallbacking - * to the user's account ID (used for A/B testing purposes). + * Whether a given report is used for onboarding tasks. In the past, it could be either the Concierge chat or the system + * DM, and we saved the report ID in the user's `onboarding` NVP. As a fallback for users who don't have the NVP, we now + * only use the Concierge chat. */ function isChatUsedForOnboarding(optionOrReport: OnyxEntry | OptionData): boolean { // onboarding can be an array for old accounts and accounts created from olddot @@ -7628,13 +7631,12 @@ function isChatUsedForOnboarding(optionOrReport: OnyxEntry | OptionData) return onboarding.chatReportID === optionOrReport?.reportID; } - return AccountUtils.isAccountIDOddNumber(currentUserAccountID ?? -1) - ? isSystemChat(optionOrReport) - : (optionOrReport as OptionData)?.isConciergeChat ?? isConciergeChatReport(optionOrReport); + return (optionOrReport as OptionData)?.isConciergeChat ?? isConciergeChatReport(optionOrReport); } /** - * Get the report (system or concierge chat) used for the user's onboarding process. + * Get the report used for the user's onboarding process. For most users it is the Concierge chat, however in the past + * we also used the system DM for A/B tests. */ function getChatUsedForOnboarding(): OnyxEntry { return Object.values(ReportConnection.getAllReports() ?? {}).find(isChatUsedForOnboarding); From 6931e93fb6f9677e599227e0f733a24c6924bf02 Mon Sep 17 00:00:00 2001 From: Francois Laithier Date: Wed, 4 Sep 2024 15:12:14 -0700 Subject: [PATCH 4/4] Fix unit tests --- tests/unit/ReportUtilsTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 6161fa57f75c..bdff4323579f 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -884,7 +884,7 @@ describe('ReportUtils', () => { expect(ReportUtils.isChatUsedForOnboarding(LHNTestUtils.getFakeReport())).toBeFalsy(); }); - it('should return true if the user account ID is odd and report is the system chat', async () => { + it('should return false if the user account ID is odd and report is the system chat - only the Concierge chat chat should be the onboarding chat for users without the onboarding NVP', async () => { const accountID = 1; await Onyx.multiSet({ @@ -901,7 +901,7 @@ describe('ReportUtils', () => { chatType: CONST.REPORT.CHAT_TYPE.SYSTEM, }; - expect(ReportUtils.isChatUsedForOnboarding(report)).toBeTruthy(); + expect(ReportUtils.isChatUsedForOnboarding(report)).toBeFalsy(); }); it('should return true if the user account ID is even and report is the concierge chat', async () => {