Skip to content
Open
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
9 changes: 6 additions & 3 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11634,8 +11634,10 @@ type PrepareOnboardingOnyxDataParams = {
selectedInterestedFeatures?: string[];
isInvitedAccountant?: boolean;
onboardingPurposeSelected?: OnboardingPurpose;
isSelfTourViewed?: boolean;
isSelfTourViewed: boolean | undefined;
betas: OnyxEntry<Beta[]>;
// TODO: hasCompletedGuidedSetupFlow will be required eventually. Refactor issue: https://github.com/Expensify/App/issues/66424
hasCompletedGuidedSetupFlow?: boolean;
};

function getBespokeWelcomeMessage(companySize: OnboardingCompanySize | undefined, userReportedIntegration?: OnboardingAccounting): string {
Expand Down Expand Up @@ -11691,6 +11693,7 @@ function prepareOnboardingOnyxData({
onboardingPurposeSelected,
isSelfTourViewed,
betas,
hasCompletedGuidedSetupFlow,
}: PrepareOnboardingOnyxDataParams) {
if (engagementChoice === CONST.ONBOARDING_CHOICES.PERSONAL_SPEND) {
// eslint-disable-next-line no-param-reassign
Expand Down Expand Up @@ -11877,7 +11880,7 @@ function prepareOnboardingOnyxData({

let isTaskAutoCompleted: boolean = task.autoCompleted;

const onboardingSelfTourViewed = isSelfTourViewed ?? onboarding?.selfTourViewed;
const onboardingSelfTourViewed = isSelfTourViewed;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve Onyx fallback for self-tour completion

prepareOnboardingOnyxData() now derives onboardingSelfTourViewed only from the isSelfTourViewed argument, but openReport() still allows callers to omit that field (OpenReportActionParams.isSelfTourViewed?), and several call sites do so (for example AuthScreensInitHandler calls openReport({reportID, introSelected, betas})). In those paths, isSelfTourViewed is undefined even when NVP_ONBOARDING.selfTourViewed is already true, so the VIEW_TOUR task is no longer auto-completed and users can see an incorrect pending onboarding task.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dukenv0307 Could you check this comment?

if (task.type === CONST.ONBOARDING_TASK_TYPE.VIEW_TOUR && onboardingSelfTourViewed) {
// If the user has already viewed the self tour, we mark the task as auto completed
isTaskAutoCompleted = true;
Expand Down Expand Up @@ -12249,7 +12252,7 @@ function prepareOnboardingOnyxData({
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.NVP_ONBOARDING,
value: {hasCompletedGuidedSetupFlow: onboarding?.hasCompletedGuidedSetupFlow ?? null},
value: {hasCompletedGuidedSetupFlow: hasCompletedGuidedSetupFlow ?? onboarding?.hasCompletedGuidedSetupFlow ?? null},
});
}

Expand Down
48 changes: 16 additions & 32 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ describe('ReportUtils', () => {
adminsChatReportID: '1',
// SMALL keeps this in the tasks path; MICRO routes through Phase 1 followups (no tasks generated).
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});

expect(title).toHaveBeenCalledWith(
Expand Down Expand Up @@ -623,6 +624,7 @@ describe('ReportUtils', () => {
adminsChatReportID: '1',
// SMALL keeps this in the tasks path; MICRO routes through Phase 1 followups (no tasks generated).
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});

expect(description).toHaveBeenCalledWith(
Expand Down Expand Up @@ -650,6 +652,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});
expect(result?.guidedSetupData).toHaveLength(0);
// suggestedFollowups beta adds a bespoke Concierge welcome action optimistically for all company sizes
Expand Down Expand Up @@ -678,6 +681,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});
expect(result?.guidedSetupData).toHaveLength(0);
expect(result?.bespokeWelcomeMessage).toContain('growing team');
Expand All @@ -700,6 +704,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.LARGE,
isSelfTourViewed: undefined,
});
expect(result?.guidedSetupData).toHaveLength(0);
expect(result?.bespokeWelcomeMessage).toContain('organization your size');
Expand All @@ -722,6 +727,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.MEDIUM_SMALL,
isSelfTourViewed: undefined,
});
expect(result?.guidedSetupData).toHaveLength(0);
expect(result?.bespokeWelcomeMessage).toContain('growing team');
Expand All @@ -744,6 +750,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.MEDIUM,
isSelfTourViewed: undefined,
});
expect(result?.guidedSetupData).toHaveLength(0);
expect(result?.bespokeWelcomeMessage).toContain('organization your size');
Expand All @@ -767,6 +774,7 @@ describe('ReportUtils', () => {
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
userReportedIntegration: 'quickbooksOnline',
isSelfTourViewed: undefined,
});
expect(result?.bespokeWelcomeMessage).toContain('QuickBooks Online');
expect(result?.bespokeWelcomeMessage).toContain('expenses sync automatically');
Expand All @@ -789,6 +797,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});
// Without the beta, tasks SHOULD be generated (old behavior) — uses SMALL (not MICRO)
// because MICRO + MANAGE_TEAM users bypass the beta gate and get followups directly
Expand Down Expand Up @@ -825,6 +834,7 @@ describe('ReportUtils', () => {
adminsChatReportID,
selectedInterestedFeatures: ['areCompanyCardsEnabled'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});
// Followups path active: no tasks generated, bespoke welcome posted optimistically to #admins.
expect(result?.guidedSetupData.filter((data) => data.type === 'task')).toHaveLength(0);
Expand All @@ -846,6 +856,7 @@ describe('ReportUtils', () => {
adminsChatReportID: '1',
selectedInterestedFeatures: ['categories', 'accounting', 'tags'],
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});

expect(result?.guidedSetupData.filter((data) => data.type === 'task')).toHaveLength(0);
Expand All @@ -869,6 +880,7 @@ describe('ReportUtils', () => {
},
adminsChatReportID: '1',
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});

const personalDetailsCall = mergeSpy.mock.calls.find((call) => call[0] === ONYXKEYS.PERSONAL_DETAILS_LIST);
Expand Down Expand Up @@ -906,6 +918,7 @@ describe('ReportUtils', () => {
adminsChatReportID: '1',
// SMALL keeps the tasks path active; MICRO routes through Phase 1 followups (no tasks generated).
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});

expect(title).toHaveBeenCalledWith(
Expand All @@ -932,6 +945,7 @@ describe('ReportUtils', () => {
},
adminsChatReportID: '1',
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});

expect(result?.guidedSetupData).toHaveLength(0);
Expand All @@ -948,6 +962,7 @@ describe('ReportUtils', () => {
},
adminsChatReportID: '1',
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});

// For LOOKING_AROUND with empty message and no tasks, guidedSetupData should be empty
Expand All @@ -970,6 +985,7 @@ describe('ReportUtils', () => {
},
adminsChatReportID: '1',
companySize: CONST.ONBOARDING_COMPANY_SIZE.MICRO,
isSelfTourViewed: undefined,
});

// Non-LOOKING_AROUND intents with a message should have guidedSetupData entries
Expand Down Expand Up @@ -1036,38 +1052,6 @@ describe('ReportUtils', () => {
expect(viewTourTask).toBeDefined();
expect(viewTourTask?.completedTaskReportActionID).toBeUndefined();
});

it('should auto-complete VIEW_TOUR task when isSelfTourViewed is undefined but onboarding.selfTourViewed is true via Onyx', async () => {
await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {selfTourViewed: true, hasCompletedGuidedSetupFlow: false});
await waitForBatchedUpdates();

const result = prepareOnboardingOnyxData({
introSelected: undefined,
betas: undefined,
engagementChoice: CONST.ONBOARDING_CHOICES.MANAGE_TEAM,
onboardingMessage: {
message: 'This is a test',
tasks: [
{
type: CONST.ONBOARDING_TASK_TYPE.VIEW_TOUR,
title: () => 'View Tour',
description: () => 'Take a tour',
autoCompleted: false,
},
],
},
adminsChatReportID: '1',
// SMALL keeps the tasks path active; MANAGE_TEAM + MICRO routes through Phase 1 followups.
companySize: CONST.ONBOARDING_COMPANY_SIZE.SMALL,
isSelfTourViewed: undefined,
});

const viewTourTask = result?.guidedSetupData.find(
(item): item is Extract<TaskForParameters, {type: 'task'}> => item.type === 'task' && 'task' in item && item.task === CONST.ONBOARDING_TASK_TYPE.VIEW_TOUR,
);
expect(viewTourTask).toBeDefined();
expect(viewTourTask?.completedTaskReportActionID).toBeDefined();
});
});

describe('getIconsForParticipants', () => {
Expand Down
Loading