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.

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

fix navigation for different assignees in confirmation flow #56732

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
+32 −14
Open

fix navigation for different assignees in confirmation flow #56732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1dd72aa
fix navigation for different assignees in confirmation flow
twilight2294 Feb 12, 2025
ba7bcf0
fix failing test
twilight2294 Feb 12, 2025
7eb930a
Merge branch 'Expensify:main' into issue/56009
twilight2294 Feb 14, 2025
9da4a6b
Merge branch 'Expensify:main' into issue/56009
twilight2294 Feb 16, 2025
cf36251
Merge branch 'Expensify:main' into issue/56009
twilight2294 Feb 18, 2025
60a1c89
use a `useRef` to store the first value of the card assignee email
twilight2294 Feb 18, 2025
4722cfd
make use of `useState`
twilight2294 Feb 18, 2025
867522c
move `useRef` usage to `AssignCardFeedPage`
twilight2294 Feb 18, 2025
f5a57e5
Merge branch 'issue/56009' of https://github.com/twilight2294/App int…
twilight2294 Feb 18, 2025
9a7fa81
fix named imports `ESLint`
twilight2294 Feb 18, 2025
8f34ad6
remove unnecessary console log
twilight2294 Feb 18, 2025
3e49e20
Merge branch 'Expensify:main' into issue/56009
twilight2294 Feb 20, 2025
85638f4
Merge branch 'Expensify:main' into issue/56009
twilight2294 Feb 21, 2025
a177e5c
Revert all changes after cf362518984492f51112009286e966b775319433 , e…
twilight2294 Feb 21, 2025
baad25d
update route to add new param `workspaceMemberAccountID`
twilight2294 Feb 21, 2025
57f24c1
fix eslint
twilight2294 Feb 21, 2025
f328608
fix lint
twilight2294 Feb 21, 2025
e15accf
gracefully exit if value if undefined
twilight2294 Feb 21, 2025
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
8 changes: 7 additions & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,13 @@ const ROUTES = {
},
WORKSPACE_COMPANY_CARDS_ASSIGN_CARD: {
route: 'settings/workspaces/:policyID/company-cards/:feed/assign-card',
getRoute: (policyID: string, feed: string, backTo?: string) => getUrlWithBackToParam(`settings/workspaces/${policyID}/company-cards/${feed}/assign-card`, backTo),
getRoute: (policyID: string, feed: string, backTo?: string, workspaceMemberAccountID?: number) =>
getUrlWithBackToParam(
workspaceMemberAccountID
? `settings/workspaces/${policyID}/company-cards/${feed}/assign-card?workspaceMemberAccountID=${encodeURIComponent(workspaceMemberAccountID)}`
: `settings/workspaces/${policyID}/company-cards/${feed}/assign-card`,
backTo,
),
},
WORKSPACE_COMPANY_CARD_DETAILS: {
route: 'settings/workspaces/:policyID/company-cards/:bank/:cardID',
Expand Down
1 change: 1 addition & 0 deletions src/libs/Navigation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ type SettingsNavigatorParamList = {
policyID: string;
feed: CompanyCardFeed;
backTo?: Routes;
workspaceMemberAccountID?: string;
};
[SCREENS.WORKSPACE.COMPANY_CARDS_SETTINGS_FEED_NAME]: {
policyID: string;
Expand Down
2 changes: 2 additions & 0 deletions src/pages/workspace/companyCards/assignCard/AssignCardFeedPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ function AssignCardFeedPage({route, policy}: AssignCardFeedPageProps) {
const feed = route.params?.feed;
const backTo = route.params?.backTo;
const policyID = policy?.id;
const workspaceMemberAccountID = route.params?.workspaceMemberAccountID;
const [isActingAsDelegate] = useOnyx(ONYXKEYS.ACCOUNT, {selector: (account) => !!account?.delegatedAccess?.delegate});

useEffect(() => {
Expand Down Expand Up @@ -77,6 +78,7 @@ function AssignCardFeedPage({route, policy}: AssignCardFeedPageProps) {
<ConfirmationStep
policyID={policyID}
backTo={backTo}
workspaceMemberAccountID={workspaceMemberAccountID}
/>
Comment on lines 78 to 82
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed that we pass route params here. In that case we shouldn't use the current approach as we are still passing through AssignCardFeedPage. Instead we should store the first valid assignee email and compare it with the current one and if they are not the same let's not pass the backTo param. Something like this:

    const firstAssigneeEmail = useRef(issueNewCard?.data.assigneeEmail);
    if (!firstAssigneeEmail.current) {
        firstAssigneeEmail.current = issueNewCard?.data.assigneeEmail;
    }
    const shouldUseBackToParam = !firstAssigneeEmail.current || firstAssigneeEmail.current === issueNewCard?.data.assigneeEmail;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt I tried this one and Confirmation page re-renders each time clearing the firstAssigneeEmail ref, we had to pass it via AssignCardFeedPage only for the value of the ref to persist, you can take a look at 867522c commit of the previous implementation.

Its late for me now, but i will again try to take a look at this tomorrow morning, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

That code should go in AssignCardFeedPage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright!, I am OoO for today, I will update this PR tomorrow morning, thanks for the patience

);
default:
Expand Down
33 changes: 21 additions & 12 deletions src/pages/workspace/companyCards/assignCard/ConfirmationStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import * as CardUtils from '@libs/CardUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import {maskCardNumber} from '@libs/CardUtils';
import {getPersonalDetailByEmail} from '@libs/PersonalDetailsUtils';
import Navigation from '@navigation/Navigation';
import * as CompanyCards from '@userActions/CompanyCards';
import {assignWorkspaceCompanyCard, clearAssignCardStepAndData, setAssignCardStepAndData} from '@userActions/CompanyCards';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand All @@ -25,33 +25,42 @@ type ConfirmationStepProps = {

/** Route to go back to */
backTo?: Route;

/** Workspace member account id */
workspaceMemberAccountID?: string;
};

function ConfirmationStep({policyID, backTo}: ConfirmationStepProps) {
function ConfirmationStep({policyID, backTo, workspaceMemberAccountID}: ConfirmationStepProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const {isOffline} = useNetwork();

const [assignCard] = useOnyx(ONYXKEYS.ASSIGN_CARD);

const data = assignCard?.data;
const cardholderName = PersonalDetailsUtils.getPersonalDetailByEmail(data?.email ?? '')?.displayName ?? '';

const cardholderName = getPersonalDetailByEmail(data?.email ?? '')?.displayName ?? '';
const cardholderAccountID = getPersonalDetailByEmail(data?.email ?? '')?.accountID?.toString() ?? '';
const submit = () => {
if (!policyID) {
return;
}
CompanyCards.assignWorkspaceCompanyCard(policyID, data);
Navigation.navigate(backTo ?? ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID));
CompanyCards.clearAssignCardStepAndData();
assignWorkspaceCompanyCard(policyID, data);

if (backTo) {
Navigation.navigate(workspaceMemberAccountID === cardholderAccountID ? backTo : ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID));
} else {
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID));
}

clearAssignCardStepAndData();
};

const editStep = (step: AssignCardStep) => {
CompanyCards.setAssignCardStepAndData({currentStep: step, isEditing: true});
setAssignCardStepAndData({currentStep: step, isEditing: true});
};

const handleBackButtonPress = () => {
CompanyCards.setAssignCardStepAndData({currentStep: CONST.COMPANY_CARD.STEP.TRANSACTION_START_DATE});
setAssignCardStepAndData({currentStep: CONST.COMPANY_CARD.STEP.TRANSACTION_START_DATE});
};

return (
Expand All @@ -77,7 +86,7 @@ function ConfirmationStep({policyID, backTo}: ConfirmationStepProps) {
/>
<MenuItemWithTopDescription
description={translate('workspace.companyCards.card')}
title={CardUtils.maskCardNumber(data?.cardNumber ?? '', data?.bankName)}
title={maskCardNumber(data?.cardNumber ?? '', data?.bankName)}
shouldShowRightIcon
onPress={() => editStep(CONST.COMPANY_CARD.STEP.CARD)}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/members/WorkspaceMemberNewCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function WorkspaceMemberNewCardPage({route, personalDetails}: WorkspaceMemberNew
isEditing: false,
});
Navigation.setNavigationActionToMicrotaskQueue(() =>
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_ASSIGN_CARD.getRoute(policyID, selectedFeed, ROUTES.WORKSPACE_MEMBER_DETAILS.getRoute(policyID, accountID))),
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_ASSIGN_CARD.getRoute(policyID, selectedFeed, ROUTES.WORKSPACE_MEMBER_DETAILS.getRoute(policyID, accountID), accountID)),
);
}
};
Expand Down
Loading