Skip to content

Fix: onboarding Track-business users land on Home instead of Inbox #97009

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

Draft
MelvinBot wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix: onboarding Track-business users land on Home instead of Inbox #97009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 3 additions & 3 deletions src/components/SidePanel/RHPVariantTest/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import SidePanelActions from '@libs/actions/SidePanel';
import isReportTopmostSplitNavigator from '@libs/Navigation/helpers/isReportTopmostSplitNavigator';
import getCentralPaneReportID from '@libs/Navigation/helpers/getCentralPaneReportID';
import Navigation from '@libs/Navigation/Navigation';

import CONST from '@src/CONST';
Expand Down Expand Up @@ -67,7 +67,7 @@ const shouldOpenRHPVariant: ShouldOpenRHPVariant = (variantOverride) => {
const handleRHPVariantNavigation: HandleRHPVariantNavigation = (onboardingPolicyID, variantOverride) => {
const variant = variantOverride ?? onboardingRHPVariant;
if (variant === CONST.ONBOARDING_RHP_VARIANT.TRACK_EXPENSES_WITH_CONCIERGE) {
const shouldPreserveRevealedReport = isReportTopmostSplitNavigator();
const shouldPreserveRevealedReport = !!getCentralPaneReportID();
if (!shouldPreserveRevealedReport) {
Navigation.navigate(ROUTES.HOME);
}
Expand All @@ -78,7 +78,7 @@ const handleRHPVariantNavigation: HandleRHPVariantNavigation = (onboardingPolicy
const isRHPHomePage = variant === CONST.ONBOARDING_RHP_VARIANT.RHP_HOME_PAGE;

if (isRHPHomePage) {
const shouldPreserveRevealedReport = isReportTopmostSplitNavigator();
const shouldPreserveRevealedReport = !!getCentralPaneReportID();
if (!shouldPreserveRevealedReport) {
Navigation.navigate(ROUTES.HOME);
}
Expand Down
4 changes: 2 additions & 2 deletions src/libs/navigateAfterOnboarding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Onyx from 'react-native-onyx';
import {setDisableDismissOnEscape} from './actions/Modal';
import SidePanelActions from './actions/SidePanel';
import {setOnboardingRHPVariant} from './actions/Welcome';
import isReportTopmostSplitNavigator from './Navigation/helpers/isReportTopmostSplitNavigator';
import getCentralPaneReportID from './Navigation/helpers/getCentralPaneReportID';
import {dismissOnboardingModalBeforeExit} from './Navigation/helpers/OnboardingNavigationUtils';
import shouldOpenOnAdminRoom from './Navigation/helpers/shouldOpenOnAdminRoom';
import Navigation from './Navigation/Navigation';
Expand Down Expand Up @@ -101,7 +101,7 @@ function navigateAfterOnboarding(
);
if (reportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID));
} else if (!isReportTopmostSplitNavigator()) {
} else if (!getCentralPaneReportID()) {
// Navigate to home to trigger guard evaluation
Navigation.navigate(ROUTES.HOME);
}
Expand Down
22 changes: 12 additions & 10 deletions tests/unit/components/SidePanel/RHPVariantTest.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import ROUTES from '@src/ROUTES';

import type * as RHPVariantTest from '../../../../src/components/SidePanel/RHPVariantTest/index';

const mockIsReportTopmostSplitNavigator = jest.fn(() => false);
const mockGetCentralPaneReportID = jest.fn<string | undefined, []>(() => undefined);

jest.mock('@expensify/react-native-hybrid-app', () => ({
__esModule: true,
Expand Down Expand Up @@ -35,9 +35,9 @@ jest.mock('react-native-onyx', () => ({
},
}));

jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => ({
jest.mock('@libs/Navigation/helpers/getCentralPaneReportID', () => ({
__esModule: true,
default: () => mockIsReportTopmostSplitNavigator(),
default: () => mockGetCentralPaneReportID(),
}));

jest.mock('@libs/Navigation/Navigation', () => ({
Expand All @@ -59,35 +59,37 @@ const {handleRHPVariantNavigation} = jest.requireActual<typeof RHPVariantTest>('
describe('handleRHPVariantNavigation', () => {
beforeEach(() => {
jest.clearAllMocks();
mockIsReportTopmostSplitNavigator.mockReturnValue(false);
mockGetCentralPaneReportID.mockReturnValue(undefined);
});

it('preserves the topmost report for the rhpHomePage variant', () => {
mockIsReportTopmostSplitNavigator.mockReturnValue(true);
it('preserves the revealed report for the rhpHomePage variant', () => {
mockGetCentralPaneReportID.mockReturnValue('reportID');

handleRHPVariantNavigation('policyID', CONST.ONBOARDING_RHP_VARIANT.RHP_HOME_PAGE);

expect(Navigation.navigate).not.toHaveBeenCalled();
expect(SidePanelActions.openSidePanel).toHaveBeenCalledWith(true);
});

it('navigates home for the rhpHomePage variant when no report is topmost', () => {
it('navigates home for the rhpHomePage variant when no report is revealed', () => {
handleRHPVariantNavigation('policyID', CONST.ONBOARDING_RHP_VARIANT.RHP_HOME_PAGE);

expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.HOME);
expect(SidePanelActions.openSidePanel).toHaveBeenCalledWith(true);
});

it('preserves the topmost report for the trackExpensesWithConcierge variant and opens the side panel on top of it', () => {
mockIsReportTopmostSplitNavigator.mockReturnValue(true);
it('preserves the revealed report for the trackExpensesWithConcierge variant and opens the side panel on top of it', () => {
mockGetCentralPaneReportID.mockReturnValue('reportID');

handleRHPVariantNavigation('policyID', CONST.ONBOARDING_RHP_VARIANT.TRACK_EXPENSES_WITH_CONCIERGE);

expect(Navigation.navigate).not.toHaveBeenCalled();
expect(SidePanelActions.openSidePanel).toHaveBeenCalledWith(true);
});

it('navigates home for the trackExpensesWithConcierge variant when no report is topmost', () => {
it('navigates home for the trackExpensesWithConcierge variant when the Inbox tab is topmost but no report is revealed', () => {
// Reproduces the reported bug: the Reports split navigator is topmost showing only the empty Inbox
// sidebar (getCentralPaneReportID returns undefined), so onboarding must still land the user on Home.
handleRHPVariantNavigation('policyID', CONST.ONBOARDING_RHP_VARIANT.TRACK_EXPENSES_WITH_CONCIERGE);

expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.HOME);
Expand Down
23 changes: 17 additions & 6 deletions tests/unit/navigateAfterOnboardingTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const REPORT_ID = '3';
const USER_ID = '4';
const mockFindLastAccessedReport = jest.fn<OnyxEntry<Report>, Parameters<typeof ReportUtils.findLastAccessedReport>>();
const mockShouldOpenOnAdminRoom = jest.fn();
const mockIsReportTopmostSplitNavigator = jest.fn(() => false);
const mockGetCentralPaneReportID = jest.fn<string | undefined, []>(() => undefined);

jest.mock('@expensify/react-native-hybrid-app', () => ({
__esModule: true,
Expand Down Expand Up @@ -74,9 +74,9 @@ jest.mock('@libs/Navigation/helpers/shouldOpenOnAdminRoom', () => ({
default: () => mockShouldOpenOnAdminRoom() as boolean,
}));

jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => ({
jest.mock('@libs/Navigation/helpers/getCentralPaneReportID', () => ({
__esModule: true,
default: () => mockIsReportTopmostSplitNavigator(),
default: () => mockGetCentralPaneReportID(),
}));

describe('navigateAfterOnboarding', () => {
Expand All @@ -88,7 +88,7 @@ describe('navigateAfterOnboarding', () => {

beforeEach(async () => {
jest.clearAllMocks();
mockIsReportTopmostSplitNavigator.mockReturnValue(false);
mockGetCentralPaneReportID.mockReturnValue(undefined);
return Onyx.clear();
});

Expand All @@ -109,14 +109,25 @@ describe('navigateAfterOnboarding', () => {
expect(navigate).toHaveBeenCalledWith(ROUTES.HOME);
});

it('should preserve the topmost report if onboardingAdminsChatReportID is not provided on larger screens', () => {
it('should preserve the revealed report if onboardingAdminsChatReportID is not provided on larger screens', () => {
const navigate = jest.spyOn(Navigation, 'navigate');
mockIsReportTopmostSplitNavigator.mockReturnValue(true);
// A real report is revealed in the topmost split navigator, so we should not yank the user to Home.
mockGetCentralPaneReportID.mockReturnValue(REPORT_ID);

navigateAfterOnboarding(false, true, '', {}, undefined, undefined);
expect(navigate).not.toHaveBeenCalled();
});

it('should navigate to home when the Inbox tab is topmost but no report is revealed on larger screens', () => {
const navigate = jest.spyOn(Navigation, 'navigate');
// The Reports split navigator can be topmost showing only the empty Inbox sidebar (no report revealed).
// getCentralPaneReportID returns undefined in that case, so we must still land the onboarding user on Home.
mockGetCentralPaneReportID.mockReturnValue(undefined);

navigateAfterOnboarding(false, true, '', {}, undefined, undefined);
expect(navigate).toHaveBeenCalledWith(ROUTES.HOME);
});

it('should not navigate to last accessed report if it is a concierge chat on small screens', async () => {
const navigate = jest.spyOn(Navigation, 'navigate');
const lastAccessedReport = {
Expand Down
Loading