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/50248 handle card failure data #50644

Merged
merged 6 commits into from
Oct 21, 2024
Merged

Fix/50248 handle card failure data #50644

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f7ba709
fix: do not show cards section when the domain is not provisioned
koko57 Oct 10, 2024
e3fc12c
fix: show an error when the card cannot be issued
koko57 Oct 11, 2024
f23fde6
fix: eslint
koko57 Oct 11, 2024
293158d
Merge branch 'main' into fix/50248-handle-card-failure-data
koko57 Oct 16, 2024
3b444bf
fix: apply requested changes
koko57 Oct 16, 2024
a80d894
fix: resolve conflicts
koko57 Oct 18, 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
62 changes: 58 additions & 4 deletions src/libs/actions/Card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,11 @@ function revealVirtualCardDetails(cardID: number, validateCode: string): Promise
];

// eslint-disable-next-line rulesdir/no-api-side-effects-method
API.makeRequestWithSideEffects(SIDE_EFFECT_REQUEST_COMMANDS.REVEAL_EXPENSIFY_CARD_DETAILS, parameters, {optimisticData, successData, failureData})
API.makeRequestWithSideEffects(SIDE_EFFECT_REQUEST_COMMANDS.REVEAL_EXPENSIFY_CARD_DETAILS, parameters, {
optimisticData,
successData,
failureData,
})
.then((response) => {
if (response?.jsonCode !== CONST.JSON_CODE.SUCCESS) {
if (response?.jsonCode === CONST.JSON_CODE.INCORRECT_MAGIC_CODE) {
Expand Down Expand Up @@ -336,7 +340,7 @@ function getCardDefaultName(userName?: string) {
}

function setIssueNewCardStepAndData({data, isEditing, step}: IssueNewCardFlowData) {
Onyx.merge(ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD, {data, isEditing, currentStep: step});
Onyx.merge(ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD, {data, isEditing, currentStep: step, errors: null});
}

function clearIssueNewCardFlow() {
Expand Down Expand Up @@ -625,6 +629,40 @@ function issueExpensifyCard(policyID: string, feedCountry: string, data?: IssueN

const {assigneeEmail, limit, limitType, cardTitle, cardType} = data;

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD,
value: {
isLoading: true,
errors: null,
mountiny marked this conversation as resolved.
Show resolved Hide resolved
isSuccessful: null,
},
},
];

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD,
value: {
isLoading: false,
isSuccessful: true,
},
},
];

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD,
value: {
isLoading: false,
errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),
},
},
];

const parameters = {
policyID,
assigneeEmail,
Expand All @@ -634,14 +672,30 @@ function issueExpensifyCard(policyID: string, feedCountry: string, data?: IssueN
};

if (cardType === CONST.EXPENSIFY_CARD.CARD_TYPE.PHYSICAL) {
API.write(WRITE_COMMANDS.CREATE_EXPENSIFY_CARD, {...parameters, feedCountry});
API.write(
WRITE_COMMANDS.CREATE_EXPENSIFY_CARD,
{...parameters, feedCountry},
{
optimisticData,
successData,
failureData,
},
);
return;
}

const domainAccountID = PolicyUtils.getWorkspaceAccountID(policyID);

// eslint-disable-next-line rulesdir/no-multiple-api-calls
API.write(WRITE_COMMANDS.CREATE_ADMIN_ISSUED_VIRTUAL_CARD, {...parameters, domainAccountID});
API.write(
WRITE_COMMANDS.CREATE_ADMIN_ISSUED_VIRTUAL_CARD,
{...parameters, domainAccountID},
{
optimisticData,
successData,
failureData,
},
);
}

function openCardDetailsPage(cardID: number) {
Expand Down
30 changes: 20 additions & 10 deletions src/pages/workspace/expensifyCard/issueNew/ConfirmationStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, {useEffect, useRef} from 'react';
import {View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import Button from '@components/Button';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import InteractiveStepSubHeader from '@components/InteractiveStepSubHeader';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
Expand All @@ -13,6 +13,7 @@ import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import {getTranslationKeyForLimitType} from '@libs/CardUtils';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import * as ErrorUtils from '@libs/ErrorUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import Navigation from '@navigation/Navigation';
import * as Card from '@userActions/Card';
Expand All @@ -38,19 +39,28 @@ function ConfirmationStep({policyID, backTo}: ConfirmationStepProps) {
const [issueNewCard] = useOnyx(ONYXKEYS.ISSUE_NEW_EXPENSIFY_CARD);

const data = issueNewCard?.data;
const isSuccessful = issueNewCard?.isSuccessful;

const submitButton = useRef<View>(null);

useEffect(() => {
submitButton.current?.focus();
}, []);

const submit = () => {
Card.issueExpensifyCard(policyID, CONST.COUNTRY.US, data);
useEffect(() => {
if (!isSuccessful) {
return;
}
Navigation.navigate(backTo ?? ROUTES.WORKSPACE_EXPENSIFY_CARD.getRoute(policyID ?? '-1'));
Card.clearIssueNewCardFlow();
}, [backTo, policyID, isSuccessful]);

const submit = () => {
Card.issueExpensifyCard(policyID, CONST.COUNTRY.US, data);
};

const errorMessage = ErrorUtils.getLatestErrorMessage(issueNewCard);

const editStep = (step: IssueNewCardStep) => {
Card.setIssueNewCardStepAndData({step, isEditing: true});
};
Expand Down Expand Up @@ -115,14 +125,14 @@ function ConfirmationStep({policyID, backTo}: ConfirmationStepProps) {
onPress={() => editStep(CONST.EXPENSIFY_CARD.STEP.CARD_NAME)}
/>
<View style={[styles.mh5, styles.pb5, styles.mt3, styles.flexGrow1, styles.justifyContentEnd]}>
<Button
ref={submitButton}
<FormAlertWithSubmitButton
buttonRef={submitButton}
message={errorMessage}
isAlertVisible={!!errorMessage}
isDisabled={isOffline}
success
large
style={[styles.w100]}
onPress={submit}
text={translate('workspace.card.issueCard')}
isLoading={issueNewCard?.isLoading}
onSubmit={submit}
buttonText={translate('workspace.card.issueCard')}
/>
</View>
</ScrollView>
Expand Down
6 changes: 6 additions & 0 deletions src/pages/workspace/expensifyCard/issueNew/IssueNewCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ function IssueNewCardPage({policy, route}: IssueNewCardPageProps) {
Card.startIssueNewCardFlow(policyID);
}, [policyID]);

useEffect(() => {
return () => {
Card.clearIssueNewCardFlow();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to clear data here? We already cleared data if the API failed

Copy link
Contributor Author

@koko57 koko57 Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I noticed that when we close RHP during the flow and go to another workspace and start the flow for issuing the card we have the data from the previous flow. This may lead to an error when i.e. we have chosen an assignee that is not a member of the new workspace. To avoid such errors, I think it's better to clear the flow everytime it's closed.

cc @mountiny

};
}, []);

const getCurrentStep = () => {
switch (currentStep) {
case CONST.EXPENSIFY_CARD.STEP.ASSIGNEE:
Expand Down
6 changes: 5 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 @@ -63,6 +63,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const [expensifyCardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`);
const [allCardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}`);
const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);
const [isRemoveMemberConfirmModalVisible, setIsRemoveMemberConfirmModalVisible] = useState(false);
const [isRoleSelectionModalVisible, setIsRoleSelectionModalVisible] = useState(false);

Expand All @@ -80,6 +81,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
const ownerDetails = personalDetails?.[policy?.ownerAccountID ?? -1] ?? ({} as PersonalDetails);
const policyOwnerDisplayName = ownerDetails.displayName ?? policy?.owner ?? '';
const companyCards = CardUtils.getMemberCards(policy, allCardsList, accountID);
const paymentAccountID = cardSettings?.paymentBankAccountID ?? 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nab just to make it clearer

Suggested change
const paymentAccountID = cardSettings?.paymentBankAccountID ?? 0;
const paymentBankAccountID = cardSettings?.paymentBankAccountID ?? 0;


// TODO: for now enabled for testing purposes. Change this to check for the actual multiple feeds when API is ready
const hasMultipleFeeds = policy?.areCompanyCardsEnabled;
Expand Down Expand Up @@ -206,6 +208,8 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
return <NotFoundPage />;
}

const shouldShowCardsSection = (policy?.areExpensifyCardsEnabled && !!paymentAccountID) ?? policy?.areCompanyCardsEnabled;

return (
<AccessOrNotFoundWrapper
policyID={policyID}
Expand Down Expand Up @@ -293,7 +297,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
onRoleChange={changeRole}
onClose={() => setIsRoleSelectionModalVisible(false)}
/>
{(policy?.areExpensifyCardsEnabled ?? policy?.areCompanyCardsEnabled) && (
{shouldShowCardsSection && (
<>
<View style={[styles.ph5, styles.pv3]}>
<Text style={StyleUtils.combineStyles([styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting])}>
Expand Down
9 changes: 9 additions & 0 deletions src/types/onyx/Card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,15 @@ type IssueNewCard = {

/** Whether the user is editing step */
isEditing: boolean;

/** Whether the request is being processed */
isLoading?: boolean;

/** Error message */
errors?: OnyxCommon.Errors;

/** Whether the request was successful */
isSuccessful?: boolean;
};

/** List of Expensify cards */
Expand Down
Loading