Skip to content
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.

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

fix: redirect to proper place after upgrade #46617

Merged
merged 11 commits into from
Aug 12, 2024
4 changes: 3 additions & 1 deletion src/components/ConnectToNetSuiteButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ function ConnectToNetSuiteButton({policyID, shouldDisconnectIntegrationBeforeCon
<Button
onPress={() => {
if (!isControlPolicy(policy)) {
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.netsuite.alias));
Navigation.navigate(
ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.netsuite.alias, ROUTES.POLICY_ACCOUNTING_NETSUITE_TOKEN_INPUT.getRoute(policyID)),
);
return;
}

Expand Down
8 changes: 7 additions & 1 deletion src/components/ConnectToSageIntacctButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,13 @@ function ConnectToSageIntacctButton({policyID, shouldDisconnectIntegrationBefore
<Button
onPress={() => {
if (!isControlPolicy(policy)) {
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.intacct.alias));
Navigation.navigate(
ROUTES.WORKSPACE_UPGRADE.getRoute(
policyID,
CONST.UPGRADE_FEATURE_INTRO_MAPPING.intacct.alias,
ROUTES.POLICY_ACCOUNTING_SAGE_INTACCT_PREREQUISITES.getRoute(policyID),
),
);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/Policy/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2642,7 +2642,7 @@ function enableExpensifyCard(policyID: string, enabled: boolean) {
}
}

function enablePolicyReportFields(policyID: string, enabled: boolean) {
function enablePolicyReportFields(policyID: string, enabled: boolean, disableRedirect = false) {
const onyxData: OnyxData = {
optimisticData: [
{
Expand Down Expand Up @@ -2685,7 +2685,7 @@ function enablePolicyReportFields(policyID: string, enabled: boolean) {

API.write(WRITE_COMMANDS.ENABLE_POLICY_REPORT_FIELDS, parameters, onyxData);

if (enabled && getIsNarrowLayout()) {
if (enabled && getIsNarrowLayout() && !disableRedirect) {
navigateWhenEnableFeature(policyID);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/categories/CategoryGLCodePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function CategoryGLCodePage({route}: EditCategoryPageProps) {
if (newGLCode !== glCode) {
Category.setPolicyCategoryGLCode(route.params.policyID, categoryName, newGLCode);
}
Navigation.goBack();
Navigation.dismissModal();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this PR related with this issue
We upgraded navigation event for editGLCode
But not for onBackButtonPress

},
[categoryName, glCode, route.params.policyID],
);
Expand Down
4 changes: 2 additions & 2 deletions src/pages/workspace/categories/CategoryPayrollCodePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ function CategoryPayrollCodePage({route}: EditCategoryPageProps) {
if (newPayrollCode !== payrollCode) {
Category.setPolicyCategoryPayrollCode(route.params.policyID, categoryName, newPayrollCode);
}
Navigation.goBack(ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, route.params.categoryName));
Navigation.dismissModal();
},
[categoryName, payrollCode, route.params.categoryName, route.params.policyID],
[categoryName, payrollCode, route.params.policyID],
);

return (
Expand Down
16 changes: 14 additions & 2 deletions src/pages/workspace/categories/CategorySettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,13 @@ function CategorySettingsPage({route, policyCategories, navigation}: CategorySet
description={translate(`workspace.categories.glCode`)}
onPress={() => {
if (!isControlPolicy(policy)) {
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(route.params.policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.glAndPayrollCodes.alias));
Navigation.navigate(
ROUTES.WORKSPACE_UPGRADE.getRoute(
route.params.policyID,
CONST.UPGRADE_FEATURE_INTRO_MAPPING.glAndPayrollCodes.alias,
ROUTES.WORKSPACE_CATEGORY_GL_CODE.getRoute(route.params.policyID, policyCategory.name),
),
);
return;
}
Navigation.navigate(ROUTES.WORKSPACE_CATEGORY_GL_CODE.getRoute(route.params.policyID, policyCategory.name));
Expand All @@ -155,7 +161,13 @@ function CategorySettingsPage({route, policyCategories, navigation}: CategorySet
description={translate(`workspace.categories.payrollCode`)}
onPress={() => {
if (!isControlPolicy(policy)) {
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(route.params.policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.glAndPayrollCodes.alias));
Navigation.navigate(
ROUTES.WORKSPACE_UPGRADE.getRoute(
route.params.policyID,
CONST.UPGRADE_FEATURE_INTRO_MAPPING.glAndPayrollCodes.alias,
ROUTES.WORKSPACE_CATEGORY_PAYROLL_CODE.getRoute(route.params.policyID, policyCategory.name),
),
);
return;
}
Navigation.navigate(ROUTES.WORKSPACE_CATEGORY_PAYROLL_CODE.getRoute(route.params.policyID, policyCategory.name));
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/tags/TagGLCodePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function TagGLCodePage({route, policyTags}: EditTagGLCodePageProps) {
if (newGLCode !== glCode) {
Tag.setPolicyTagGLCode(route.params.policyID, tagName, orderWeight, newGLCode);
}
Navigation.goBack();
Navigation.dismissModal();
},
[glCode, route.params.policyID, tagName, orderWeight],
);
Expand Down
8 changes: 7 additions & 1 deletion src/pages/workspace/tags/TagSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,13 @@ function TagSettingsPage({route, policyTags, navigation}: TagSettingsPageProps)

const navigateToEditGlCode = () => {
if (!PolicyUtils.isControlPolicy(policy)) {
Navigation.navigate(ROUTES.WORKSPACE_UPGRADE.getRoute(route.params.policyID, CONST.UPGRADE_FEATURE_INTRO_MAPPING.glCodes.alias));
Navigation.navigate(
ROUTES.WORKSPACE_UPGRADE.getRoute(
route.params.policyID,
CONST.UPGRADE_FEATURE_INTRO_MAPPING.glCodes.alias,
ROUTES.WORKSPACE_TAG_GL_CODE.getRoute(policy?.id ?? '', route.params.orderWeight, route.params.tagName),
),
);
return;
}
Navigation.navigate(ROUTES.WORKSPACE_TAG_GL_CODE.getRoute(route.params.policyID, route.params.orderWeight, currentPolicyTag.name));
Expand Down
6 changes: 3 additions & 3 deletions src/pages/workspace/upgrade/UpgradeConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import ROUTES from '@src/ROUTES';

type Props = {
policyName: string;
policyID: string;
onConfirmUpgrade: () => void;
};

function UpgradeConfirmation({policyName, policyID}: Props) {
function UpgradeConfirmation({policyName, onConfirmUpgrade}: Props) {
const {translate} = useLocalize();
const styles = useThemeStyles();

Expand All @@ -31,7 +31,7 @@ function UpgradeConfirmation({policyName, policyID}: Props) {
</>
}
shouldShowButton
onButtonPress={() => Navigation.goBack(ROUTES.WORKSPACE_PROFILE.getRoute(policyID))}
onButtonPress={onConfirmUpgrade}
buttonText={translate('workspace.upgrade.completed.gotIt')}
/>
);
Expand Down
22 changes: 16 additions & 6 deletions src/pages/workspace/upgrade/UpgradeIntro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {ValueOf} from 'type-fest';
import Avatar from '@components/Avatar';
import Badge from '@components/Badge';
import Button from '@components/Button';
import Icon from '@components/Icon';
import * as Expensicon from '@components/Icon/Expensicons';
import * as Illustrations from '@components/Icon/Illustrations';
import Text from '@components/Text';
Expand All @@ -26,18 +27,27 @@ function UpgradeIntro({feature, onUpgrade, buttonDisabled, loading}: Props) {
const styles = useThemeStyles();
const {isExtraSmallScreenWidth, isSmallScreenWidth} = useResponsiveLayout();
const {translate} = useLocalize();
const iconSrc = feature.icon in Illustrations ? Illustrations[feature.icon as keyof typeof Illustrations] : Expensicon[feature.icon as keyof typeof Expensicon];
const isIllustration = feature.icon in Illustrations;
const iconSrc = isIllustration ? Illustrations[feature.icon as keyof typeof Illustrations] : Expensicon[feature.icon as keyof typeof Expensicon];
const iconAdditionalStyles = feature.id === CONST.UPGRADE_FEATURE_INTRO_MAPPING.approvals.id ? styles.br0 : undefined;

return (
<View style={styles.p5}>
<View style={styles.workspaceUpgradeIntroBox({isExtraSmallScreenWidth, isSmallScreenWidth})}>
<View style={[styles.mb3, styles.flexRow, styles.justifyContentBetween]}>
<Avatar
type={CONST.ICON_TYPE_AVATAR}
source={iconSrc}
iconAdditionalStyles={iconAdditionalStyles}
/>
{!isIllustration ? (
<Avatar
source={iconSrc}
type={CONST.ICON_TYPE_AVATAR}
/>
) : (
<Icon
src={iconSrc}
width={48}
height={48}
additionalStyles={iconAdditionalStyles}
/>
)}
<Badge
icon={Expensicon.Unlock}
text={translate('workspace.upgrade.upgradeToUnlock')}
Expand Down
47 changes: 39 additions & 8 deletions src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {useNavigation} from '@react-navigation/native';
import type {StackScreenProps} from '@react-navigation/stack';
import React from 'react';
import React, {useCallback, useEffect} from 'react';
import {useOnyx} from 'react-native-onyx';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
Expand All @@ -8,6 +9,7 @@ import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
import {isControlPolicy} from '@libs/PolicyUtils';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import CONST from '@src/CONST';
import * as Policy from '@src/libs/actions/Policy/Policy';
Expand All @@ -19,22 +21,51 @@ import UpgradeIntro from './UpgradeIntro';
type WorkspaceUpgradePageProps = StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.UPGRADE>;

function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
const navigation = useNavigation();
const styles = useThemeStyles();
const policyID = route.params.policyID;
const feature = Object.values(CONST.UPGRADE_FEATURE_INTRO_MAPPING).find((f) => f.alias === route.params.featureName);
const {translate} = useLocalize();
const [policy] = useOnyx(`policy_${policyID}`);
const {isOffline} = useNetwork();

if (!feature || !policy) {
return <NotFoundPage />;
}
const isUpgraded = React.useMemo(() => isControlPolicy(policy), [policy]);

const upgradeToCorporate = () => {
Policy.upgradeToCorporate(policy.id, feature.id);
if (!policy || !feature) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not have a way in the UI to upgrade without having to click on a feature first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this just to keep typescript happy since feature or policy could be null if the user opens the upgrade page via a deep link with incorrect params.

return;
}

Policy.upgradeToCorporate(policy.id, feature.name);
};

const isUpgraded = policy.type === CONST.POLICY.TYPE.CORPORATE;
const confirmUpgrade = useCallback(() => {
if (!feature) {
return;
}
switch (feature.id) {
case CONST.UPGRADE_FEATURE_INTRO_MAPPING.reportFields.id:
Policy.enablePolicyReportFields(policyID, true, true);
return Navigation.navigate(ROUTES.WORKSPACE_MORE_FEATURES.getRoute(policyID));
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, This caused this issue: #47683. More info in this proposal: #47683 (comment)

default:
return route.params.backTo ? Navigation.navigate(route.params.backTo) : Navigation.goBack();
}
}, [feature, policyID, route.params.backTo]);

useEffect(() => {
const unsubscribeListener = navigation.addListener('blur', () => {
if (!isUpgraded) {
return;
}
confirmUpgrade();
Copy link
Contributor

@DylanDylann DylanDylann Aug 21, 2024

Choose a reason for hiding this comment

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

@allroundexperts Hi, why do we need to listen blur event here? Could we call confirmUpgrade() right after licking "Got it, thanks" on onConfirmUpgrade callback?

cc @rushatgabhane

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to call the confirmation function if the user doesn't event click on the Got it, thanks! button and just closes the RHP by clicking elsewhere on the screen.

});

return unsubscribeListener;
}, [isUpgraded, confirmUpgrade, navigation]);

if (!feature || !policy) {
return <NotFoundPage />;
}
Comment on lines +66 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from #47904
To handle a case when a regular user with no "upgrade" permissions visits this page, we should have also added a check for the user to be an admin here and in some other places.


return (
<ScreenWrapper
Expand All @@ -44,11 +75,11 @@ function WorkspaceUpgradePage({route}: WorkspaceUpgradePageProps) {
>
<HeaderWithBackButton
title={translate('common.upgrade')}
onBackButtonPress={() => Navigation.goBack(route.params.backTo ?? ROUTES.WORKSPACE_PROFILE.getRoute(policyID))}
onBackButtonPress={() => (isUpgraded ? Navigation.dismissModal() : Navigation.goBack())}
/>
{isUpgraded && (
<UpgradeConfirmation
policyID={policy.id}
onConfirmUpgrade={() => Navigation.dismissModal()}
policyName={policy.name}
/>
)}
Expand Down
7 changes: 1 addition & 6 deletions src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2735,15 +2735,10 @@ const styles = (theme: ThemeColors) =>
width: 110,
},

workspaceUpgradeIntroBox: ({isExtraSmallScreenWidth, isSmallScreenWidth}: WorkspaceUpgradeIntroBoxParams): ViewStyle => {
workspaceUpgradeIntroBox: ({isExtraSmallScreenWidth}: WorkspaceUpgradeIntroBoxParams): ViewStyle => {
let paddingHorizontal = spacing.ph5;
let paddingVertical = spacing.pv5;

if (isSmallScreenWidth) {
paddingHorizontal = spacing.ph4;
paddingVertical = spacing.pv4;
}

if (isExtraSmallScreenWidth) {
paddingHorizontal = spacing.ph2;
paddingVertical = spacing.pv2;
Expand Down
Loading