Skip to content

[No QA] [Workspace Feeds] Clean up for workspace feeds #48323

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

Merged
mountiny merged 5 commits into from
Sep 2, 2024
Merged

[No QA] [Workspace Feeds] Clean up for workspace feeds #48323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e53f0bf
Uncomment isMonthlySettlementAllowed related code
VickyStash Aug 30, 2024
881bc10
Go back to the workspace member details page if the card creation fro…
VickyStash Aug 30, 2024
d2c093b
Prevent user from using deep link to access WorkspaceSettlementFreque…
VickyStash Aug 30, 2024
2e293ac
Code improvements
VickyStash Sep 2, 2024
f34d40b
Merge branch 'refs/heads/main' into feature/47308-feeds-follow-ups
VickyStash Sep 2, 2024
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
2 changes: 1 addition & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ const ROUTES = {
},
WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW: {
route: 'settings/workspaces/:policyID/expensify-card/issue-new',
getRoute: (policyID: string) => `settings/workspaces/${policyID}/expensify-card/issue-new` as const,
getRoute: (policyID: string, backTo?: string) => getUrlWithBackToParam(`settings/workspaces/${policyID}/expensify-card/issue-new`, backTo),
},
WORKSPACE_EXPENSIFY_CARD_BANK_ACCOUNT: {
route: 'settings/workspaces/:policyID/expensify-card/choose-bank-account',
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 @@ -676,6 +676,7 @@ type SettingsNavigatorParamList = {
};
[SCREENS.WORKSPACE.EXPENSIFY_CARD_ISSUE_NEW]: {
policyID: string;
backTo?: Routes;
};
[SCREENS.WORKSPACE.EXPENSIFY_CARD_BANK_ACCOUNT]: {
policyID: string;
Expand Down
8 changes: 3 additions & 5 deletions src/pages/workspace/expensifyCard/WorkspaceCardSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ function WorkspaceCardSettingsPage({route}: WorkspaceCardSettingsPageProps) {
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);

const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;
// const isMonthlySettlementAllowed = cardSettings?.isMonthlySettlementAllowed ?? true;
const isMonthlySettlementAllowed = cardSettings?.isMonthlySettlementAllowed ?? false;
const settlementFrequency = cardSettings?.monthlySettlementDate ? CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.MONTHLY : CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;
// TODO: replace this line with the following line and uncomment the previous comment
const isSettlementFrequencyBlocked = settlementFrequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;
// const isSettlementFrequencyBlocked = !isMonthlySettlementAllowed && settlementFrequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;
const isSettlementFrequencyBlocked = !isMonthlySettlementAllowed && settlementFrequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;
const bankAccountNumber = bankAccountList?.[paymentBankAccountID.toString()]?.accountData?.accountNumber ?? '';

return (
Expand Down Expand Up @@ -66,7 +64,7 @@ function WorkspaceCardSettingsPage({route}: WorkspaceCardSettingsPageProps) {
description={translate('workspace.expensifyCard.settlementFrequency')}
title={translate(`workspace.expensifyCard.frequency.${settlementFrequency}`)}
shouldShowRightIcon={settlementFrequency !== CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY}
// interactive={!isSettlementFrequencyBlocked}
interactive={!isSettlementFrequencyBlocked}

@DylanDylann DylanDylann Aug 30, 2024

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.

NAB: Maybe add disabled props

Suggested change
interactive={!isSettlementFrequencyBlocked}
interactive={!isSettlementFrequencyBlocked}
disabled={isSettlementFrequencyBlocked}

@VickyStash VickyStash Aug 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann It adds the opacity, I'm not sure that it's the expected behaviour

image

@DylanDylann DylanDylann Aug 30, 2024

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.

Yeah, It's only my suggestion and also be a minor thing

@MariaHCD MariaHCD Sep 2, 2024

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.

I don't think the disabled prop is necessary in this case

onPress={() => Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_SETTINGS_FREQUENCY.getRoute(policyID))}
hintText={
isSettlementFrequencyBlocked ? (
Expand Down
7 changes: 6 additions & 1 deletion src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,17 @@ function WorkspaceExpensifyCardListPage({route, cardsList}: WorkspaceExpensifyCa

const sortedCards = useMemo(() => CardUtils.sortCardsByCardholderName(cardsList, personalDetails), [cardsList, personalDetails]);

const issueCard = () => {
const activeRoute = Navigation.getActiveRoute();
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID, activeRoute));
};
Comment on lines +52 to +55

@allroundexperts allroundexperts Aug 14, 2025

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.

We should have cleared the ISSUE_NEW_EXPENSIFY_CARD_FORM data when issuing a card. Not doing so caused #66841


const getHeaderButtons = () => (
<View style={[styles.w100, styles.flexRow, styles.gap2, shouldUseNarrowLayout && styles.mb3]}>
<Button
medium
success
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID))}
onPress={issueCard}
icon={Expensicons.Plus}
text={translate('workspace.expensifyCard.issueCard')}
style={shouldUseNarrowLayout && styles.flex1}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/workspace/expensifyCard/WorkspaceSettlementFrequencyPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function WorkspaceSettlementFrequencyPage({route}: WorkspaceSettlementFrequencyP

const shouldShowMonthlyOption = cardSettings?.isMonthlySettlementAllowed ?? false;
const selectedFrequency = cardSettings?.monthlySettlementDate ? CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.MONTHLY : CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;
const isSettlementFrequencyBlocked = !shouldShowMonthlyOption && selectedFrequency === CONST.EXPENSIFY_CARD.FREQUENCY_SETTING.DAILY;

const data = useMemo(() => {
const options = [];
Expand Down Expand Up @@ -62,6 +63,7 @@ function WorkspaceSettlementFrequencyPage({route}: WorkspaceSettlementFrequencyP
<AccessOrNotFoundWrapper
accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
policyID={policyID}
shouldBeBlocked={isSettlementFrequencyBlocked}
featureName={CONST.POLICY.MORE_FEATURES.ARE_EXPENSIFY_CARDS_ENABLED}
>
<ScreenWrapper
Expand Down
10 changes: 7 additions & 3 deletions src/pages/workspace/expensifyCard/issueNew/ConfirmationStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@ import * as Card from '@userActions/Card';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Route} from '@src/ROUTES';
import type {IssueNewCardStep} from '@src/types/onyx/Card';

type ConfirmationStepProps = {
// ID of the policy that the card will be issued under
/** ID of the policy that the card will be issued under */
policyID: string;

/** Route to navigate to */
backTo?: Route;
};

function ConfirmationStep({policyID}: ConfirmationStepProps) {
function ConfirmationStep({policyID, backTo}: ConfirmationStepProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const {isOffline} = useNetwork();
Expand All @@ -43,7 +47,7 @@ function ConfirmationStep({policyID}: ConfirmationStepProps) {

const submit = () => {
Card.issueExpensifyCard(policyID, CONST.COUNTRY.US, data);
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD.getRoute(policyID ?? '-1'));
Navigation.navigate(backTo ?? ROUTES.WORKSPACE_EXPENSIFY_CARD.getRoute(policyID ?? '-1'));

@s77rt s77rt May 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We shouldn't navigate to the backTo param if we started issuing the card from the member's details page but changed the card holder. After changing the card holder it does not make sense to return to the previous card holder details page.

Coming from #56009

Card.clearIssueNewCardFlow();
};

Expand Down
16 changes: 13 additions & 3 deletions src/pages/workspace/expensifyCard/issueNew/IssueNewCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useEffect} from 'react';
import {useOnyx} from 'react-native-onyx';
import type {SettingsNavigatorParamList} from '@navigation/types';
import type {WithPolicyAndFullscreenLoadingProps} from '@pages/workspace/withPolicyAndFullscreenLoading';
import withPolicyAndFullscreenLoading from '@pages/workspace/withPolicyAndFullscreenLoading';
import * as Card from '@userActions/Card';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import AssigneeStep from './AssigneeStep';
import CardNameStep from './CardNameStep';
import CardTypeStep from './CardTypeStep';
import ConfirmationStep from './ConfirmationStep';
import LimitStep from './LimitStep';
import LimitTypeStep from './LimitTypeStep';

function IssueNewCardPage({policy}: WithPolicyAndFullscreenLoadingProps) {
type IssueNewCardPageProps = WithPolicyAndFullscreenLoadingProps & StackScreenProps<SettingsNavigatorParamList, typeof SCREENS.WORKSPACE.EXPENSIFY_CARD_ISSUE_NEW>;

function IssueNewCardPage({policy, route}: IssueNewCardPageProps) {
const [issueNewCard] = useOnyx(ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD);

const {currentStep} = issueNewCard ?? {};

const policyID = policy?.id ?? '-1';
const backTo = route?.params?.backTo;

// TODO: add logic to skip Assignee step when the flow is started from the member's profile page
useEffect(() => {
Card.startIssueNewCardFlow(policyID);
}, [policyID]);
Expand All @@ -36,7 +41,12 @@ function IssueNewCardPage({policy}: WithPolicyAndFullscreenLoadingProps) {
case CONST.EXPENSIFY_CARD.STEP.CARD_NAME:
return <CardNameStep />;
case CONST.EXPENSIFY_CARD.STEP.CONFIRMATION:
return <ConfirmationStep policyID={policyID} />;
return (
<ConfirmationStep
policyID={policyID}
backTo={backTo}
/>
);
default:
return <AssigneeStep policy={policy} />;
}
Expand Down
4 changes: 3 additions & 1 deletion src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,16 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
);

const navigateToIssueNewCard = useCallback(() => {
const activeRoute = Navigation.getActiveRoute();

Card.setIssueNewCardStepAndData({
step: CONST.EXPENSIFY_CARD.STEP.CARD_TYPE,
data: {
assigneeEmail: memberLogin,
},
isEditing: false,
});
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID));
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID, activeRoute));
}, [memberLogin, policyID]);

const openRoleSelectionModal = useCallback(() => {
Expand Down
1 change: 1 addition & 0 deletions src/pages/workspace/withPolicy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type PolicyRoute = RouteProp<
| typeof SCREENS.WORKSPACE.REPORT_FIELDS_VALUE_SETTINGS
| typeof SCREENS.WORKSPACE.ACCOUNTING.CARD_RECONCILIATION
| typeof SCREENS.WORKSPACE.RULES
| typeof SCREENS.WORKSPACE.EXPENSIFY_CARD_ISSUE_NEW
>;

function getPolicyIDFromRoute(route: PolicyRoute): string {
Expand Down