Skip to content

Commit

Permalink
Merge pull request #48593 from Expensify/francois-onboarding-concierg…
Browse files Browse the repository at this point in the history
…e-only

Already has all required approvals
  • Loading branch information
francoisl authored Sep 5, 2024
2 parents 8fc5e68 + 6931e93 commit 47b0c1c
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 41 deletions.
18 changes: 10 additions & 8 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -7619,22 +7621,22 @@ 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<Report> | OptionData): boolean {
// onboarding can be an array for old accounts and accounts created from olddot
if (onboarding && !Array.isArray(onboarding) && onboarding.chatReportID) {
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<Report> {
return Object.values(ReportConnection.getAllReports() ?? {}).find(isChatUsedForOnboarding);
Expand Down
24 changes: 2 additions & 22 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -2164,17 +2163,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);
Expand Down Expand Up @@ -3329,13 +3317,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 = CONST.ACCOUNT_ID.CONCIERGE;
const targetChatReport = ReportUtils.getChatByParticipants([actorAccountID, currentUserAccountID]);
const {reportID: targetChatReportID = '', policyID: targetChatPolicyID = ''} = targetChatReport ?? {};

Expand Down Expand Up @@ -3388,7 +3370,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,
Expand Down Expand Up @@ -3452,7 +3434,6 @@ function completeOnboarding(
},
isOptimisticReport: true,
managerID: currentUserAccountID,
permissions: targetChatReport?.permissions ?? fallbackPermission,
},
},
{
Expand Down Expand Up @@ -4111,7 +4092,6 @@ export {
saveReportActionDraft,
deleteReportComment,
navigateToConciergeChat,
navigateToSystemChat,
addPolicyReport,
deleteReport,
navigateToConciergeChatAndDeleteReport,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -40,7 +38,6 @@ function BaseOnboardingPersonalDetails({
const {shouldUseNarrowLayout, onboardingIsMediumOrLargerScreenWidth} = useResponsiveLayout();
const {inputCallbackRef} = useAutoFocusInput();
const [shouldValidateOnChange, setShouldValidateOnChange] = useState(false);
const {accountID} = useSession();

useEffect(() => {
Welcome.setOnboardingErrorMessage('');
Expand Down Expand Up @@ -76,14 +73,10 @@ 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],
[onboardingPurposeSelected, onboardingAdminsChatReportID, onboardingPolicyID, shouldUseNarrowLayout, route.params?.backTo],
);

const validate = (values: FormOnyxValues<'onboardingPersonalDetailsForm'>) => {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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 () => {
Expand Down

0 comments on commit 47b0c1c

Please sign in to comment.