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

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

Merged
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 3 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 @@ -917,7 +917,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;
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}
Copy link
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}

Copy link
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

Copy link
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

Copy link
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));
};

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 @@ -37,7 +41,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'));
Card.clearIssueNewCardFlow();
};

Expand Down
15 changes: 13 additions & 2 deletions src/pages/workspace/expensifyCard/issueNew/IssueNewCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
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(() => {
Expand All @@ -36,7 +42,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
Loading