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

Chore/52820 workspace cards cleanup #52871

Merged
merged 12 commits into from
Nov 22, 2024
Merged

Chore/52820 workspace cards cleanup #52871

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fd029a8
fix: remove unreachable return
koko57 Nov 20, 2024
1cd6d19
feat: skip assignee step when only one workspace member
koko57 Nov 21, 2024
2396fc0
feat: skip the card selection step if only one card available
koko57 Nov 21, 2024
d1b6ef5
feat: skip card selection step when only one card
koko57 Nov 21, 2024
47a9f12
chore: code cleanup
koko57 Nov 21, 2024
6ee3551
feat: create util functions
koko57 Nov 21, 2024
824ad5b
fix: minor fix
koko57 Nov 21, 2024
d71e7f1
fix: minor fix
koko57 Nov 21, 2024
5c82d2c
fix: clear the data when closing the flow
koko57 Nov 22, 2024
d9dd7a3
fix: add missing bank name
koko57 Nov 22, 2024
4137f2f
fix: add card name
koko57 Nov 22, 2024
2570632
fix: delay navigation
koko57 Nov 22, 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
13 changes: 13 additions & 0 deletions src/libs/CardUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {TranslationPaths} from '@src/languages/types';
import type {OnyxValues} from '@src/ONYXKEYS';
import ONYXKEYS from '@src/ONYXKEYS';
import type {BankAccountList, Card, CardFeeds, CardList, CompanyCardFeed, PersonalDetailsList, WorkspaceCardsList} from '@src/types/onyx';
import type {FilteredCardList} from '@src/types/onyx/Card';
import type {CompanyCardNicknames, CompanyFeeds} from '@src/types/onyx/CardFeeds';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type IconAsset from '@src/types/utils/IconAsset';
Expand Down Expand Up @@ -352,6 +353,16 @@ function getSelectedFeed(lastSelectedFeed: OnyxEntry<CompanyCardFeed>, cardFeeds
return lastSelectedFeed ?? defaultFeed;
}

function getFilteredCardList(list?: WorkspaceCardsList) {
const {cardList, ...cards} = list ?? {};
// We need to filter out cards which already has been assigned
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// We need to filter out cards which already has been assigned
// We need to filter out cards which already has been assigned

return Object.fromEntries(Object.entries(cardList ?? {}).filter(([cardNumber]) => !Object.values(cards).find((card) => card.lastFourPAN && cardNumber.endsWith(card.lastFourPAN))));
}

function hasOnlyOneCardToAssign(list: FilteredCardList) {
return Object.keys(list).length === 1;
}

export {
isExpensifyCard,
isCorporateCard,
Expand All @@ -378,4 +389,6 @@ export {
getCorrectStepForSelectedBank,
getCustomOrFormattedFeedName,
removeExpensifyCardFromCompanyCards,
getFilteredCardList,
hasOnlyOneCardToAssign,
};
31 changes: 30 additions & 1 deletion src/pages/workspace/companyCards/WorkspaceCompanyCardsListHeaderButtons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ import * as CardUtils from '@libs/CardUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import Navigation from '@navigation/Navigation';
import variables from '@styles/variables';
import * as CompanyCards from '@userActions/CompanyCards';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {CompanyCardFeed} from '@src/types/onyx';
import type {AssignCardData, AssignCardStep} from '@src/types/onyx/AssignCard';

type WorkspaceCompanyCardsListHeaderButtonsProps = {
/** Current policy id */
Expand All @@ -41,6 +44,32 @@ function WorkspaceCompanyCardsListHeaderButtons({policyID, selectedFeed}: Worksp
const isCustomFeed = CardUtils.isCustomFeed(selectedFeed);
const companyFeeds = CardUtils.getCompanyFeeds(cardFeeds);
const currentFeedData = companyFeeds?.[selectedFeed];
const policy = PolicyUtils.getPolicy(policyID);

const [list] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${selectedFeed}`);
const filteredCardList = CardUtils.getFilteredCardList(list);

const handleAssignCard = () => {
const data: Partial<AssignCardData> = {
bankName: selectedFeed,
};

let currentStep: AssignCardStep = CONST.COMPANY_CARD.STEP.ASSIGNEE;

if (Object.keys(policy?.employeeList ?? {}).length === 1) {
data.email = Object.keys(policy?.employeeList ?? {}).at(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add cardName along with email

currentStep = CONST.COMPANY_CARD.STEP.CARD;

if (CardUtils.hasOnlyOneCardToAssign(filteredCardList)) {
currentStep = CONST.COMPANY_CARD.STEP.TRANSACTION_START_DATE;
data.cardNumber = Object.keys(filteredCardList).at(0);
data.encryptedCardNumber = Object.values(filteredCardList).at(0);
Comment on lines +70 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 I believe that the two fields are different

Screenshot 2024-11-22 at 10 14 44

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you have the key - "480801XXXXX2554" for the card name and the value "v11:47A..." for the encryptedCradNumber

Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 In your code change, you set two fields to the same value

                data.cardNumber = Object.keys(filteredCardList).at(0);
                data.encryptedCardNumber = Object.values(filteredCardList).at(0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Object.keys vs Object.values? how they are the same

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'm getting the first key "480801XXXXX2554" and it's value "v11:47A..." f

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, my bad

Copy link
Contributor

Choose a reason for hiding this comment

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

It is fine

}
}

CompanyCards.setAssignCardStepAndData({data, currentStep});
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_ASSIGN_CARD.getRoute(policyID, selectedFeed));
};

return (
<OfflineWithFeedback
Expand Down Expand Up @@ -79,7 +108,7 @@ function WorkspaceCompanyCardsListHeaderButtons({policyID, selectedFeed}: Worksp
<Button
success
isDisabled={!currentFeedData || !!currentFeedData?.pending || !!currentFeedData?.errors}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_ASSIGN_CARD.getRoute(policyID, selectedFeed))}
onPress={handleAssignCard}
icon={Expensicons.Plus}
text={translate('workspace.companyCards.assignCard')}
style={shouldChangeLayout && styles.flex1}
Expand Down
9 changes: 1 addition & 8 deletions src/pages/workspace/companyCards/assignCard/AssignCardFeedPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useEffect} from 'react';
import React 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 CompanyCards from '@userActions/CompanyCards';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
Expand All @@ -24,10 +23,6 @@ function AssignCardFeedPage({route, policy}: AssignCardFeedPageProps) {
const backTo = route.params?.backTo;
const policyID = policy?.id ?? '-1';

useEffect(() => {
CompanyCards.setAssignCardStepAndData({data: {bankName: feed}});
}, [feed]);

switch (currentStep) {
case CONST.COMPANY_CARD.STEP.ASSIGNEE:
return <AssigneeStep policy={policy} />;
Expand All @@ -52,8 +47,6 @@ function AssignCardFeedPage({route, policy}: AssignCardFeedPageProps) {
default:
return <AssigneeStep policy={policy} />;
}

return <AssigneeStep policy={policy} />;
}

export default withPolicyAndFullscreenLoading(AssignCardFeedPage);
14 changes: 12 additions & 2 deletions src/pages/workspace/companyCards/assignCard/AssigneeStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import * as CardUtils from '@libs/CardUtils';
import {formatPhoneNumber} from '@libs/LocalePhoneNumber';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
Expand All @@ -35,6 +36,10 @@ function AssigneeStep({policy}: AssigneeStepProps) {
const styles = useThemeStyles();
const {isOffline} = useNetwork();
const [assignCard] = useOnyx(ONYXKEYS.ASSIGN_CARD);
const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policy?.id ?? '-1');

const [list] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${assignCard?.data?.bankName ?? ''}`);
const filteredCardList = CardUtils.getFilteredCardList(list);

const isEditing = assignCard?.isEditing;

Expand All @@ -57,8 +62,10 @@ function AssigneeStep({policy}: AssigneeStepProps) {
const personalDetail = PersonalDetailsUtils.getPersonalDetailByEmail(selectedMember);
const memberName = personalDetail?.firstName ? personalDetail.firstName : personalDetail?.login;

const nextStep = CardUtils.hasOnlyOneCardToAssign(filteredCardList) ? CONST.COMPANY_CARD.STEP.TRANSACTION_START_DATE : CONST.COMPANY_CARD.STEP.CARD;

CompanyCards.setAssignCardStepAndData({
currentStep: isEditing ? CONST.COMPANY_CARD.STEP.CONFIRMATION : CONST.COMPANY_CARD.STEP.CARD,
currentStep: isEditing ? CONST.COMPANY_CARD.STEP.CONFIRMATION : nextStep,
data: {
email: selectedMember,
cardName: `${memberName}'s card`,
Expand All @@ -69,7 +76,10 @@ function AssigneeStep({policy}: AssigneeStepProps) {

const handleBackButtonPress = () => {
if (isEditing) {
CompanyCards.setAssignCardStepAndData({currentStep: CONST.COMPANY_CARD.STEP.CONFIRMATION, isEditing: false});
CompanyCards.setAssignCardStepAndData({
currentStep: CONST.COMPANY_CARD.STEP.CONFIRMATION,
isEditing: false,
});
return;
}
Navigation.goBack();
Expand Down
7 changes: 2 additions & 5 deletions src/pages/workspace/companyCards/assignCard/CardSelectionStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,8 @@ function CardSelectionStep({feed, policyID}: CardSelectionStepProps) {

const isEditing = assignCard?.isEditing;
const assigneeDisplayName = PersonalDetailsUtils.getPersonalDetailByEmail(assignCard?.data?.email ?? '')?.displayName ?? '';
const {cardList, ...cards} = list ?? {};
// We need to filter out cards which already has been assigned
const filteredCardList = Object.fromEntries(
Object.entries(cardList ?? {}).filter(([cardNumber]) => !Object.values(cards).find((card) => card.lastFourPAN && cardNumber.endsWith(card.lastFourPAN))),
);
const filteredCardList = CardUtils.getFilteredCardList(list);

const [cardSelected, setCardSelected] = useState(assignCard?.data?.encryptedCardNumber ?? '');
const [shouldShowError, setShouldShowError] = useState(false);

Expand Down
20 changes: 16 additions & 4 deletions src/pages/workspace/members/WorkspaceMemberNewCardPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {CompanyCardFeed} from '@src/types/onyx';
import type {AssignCardData, AssignCardStep} from '@src/types/onyx/AssignCard';

type CardFeedListItem = ListItem & {
/** Card feed value */
Expand All @@ -49,6 +50,9 @@ function WorkspaceMemberNewCardPage({route, personalDetails}: WorkspaceMemberNew
const memberLogin = personalDetails?.[accountID]?.login ?? '';
const availableCompanyCards = CardUtils.removeExpensifyCardFromCompanyCards(cardFeeds);

const [list] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${selectedFeed}`);
const filteredCardList = CardUtils.getFilteredCardList(list);

const handleSubmit = () => {
if (!selectedFeed) {
setShouldShowError(true);
Expand All @@ -64,11 +68,19 @@ function WorkspaceMemberNewCardPage({route, personalDetails}: WorkspaceMemberNew
});
Navigation.navigate(ROUTES.WORKSPACE_EXPENSIFY_CARD_ISSUE_NEW.getRoute(policyID, ROUTES.WORKSPACE_MEMBER_DETAILS.getRoute(policyID, accountID)));
} else {
const data: Partial<AssignCardData> = {
email: memberLogin,
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: Let's add bankName value here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, wouldn't say that is a NAB, I removed setting the feed in the assign card flow

Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 It is a NAB because we already saved feed on the route. But I think we still need to fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, you're saving the feed on the route but not in the Onyx

Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 Let's add cardName along with email

};
let currentStep: AssignCardStep = CONST.COMPANY_CARD.STEP.CARD;

if (CardUtils.hasOnlyOneCardToAssign(filteredCardList)) {
currentStep = CONST.COMPANY_CARD.STEP.TRANSACTION_START_DATE;
data.cardNumber = Object.keys(filteredCardList).at(0);
data.encryptedCardNumber = Object.values(filteredCardList).at(0);
}
CompanyCards.setAssignCardStepAndData({
currentStep: CONST.COMPANY_CARD.STEP.CARD,
data: {
email: memberLogin,
},
currentStep,
data,
isEditing: false,
});
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS_ASSIGN_CARD.getRoute(policyID, selectedFeed, ROUTES.WORKSPACE_MEMBER_DETAILS.getRoute(policyID, accountID)));
Copy link
Contributor

Choose a reason for hiding this comment

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

@koko57 The App displays the asignee step a moment before moving to the correct page. I think we need to wait for Onyx.update before navigating (maybe use setNavigationActionToMicrotaskQueue)

Screen.Recording.2024-11-22.at.11.39.33.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Expand Down
5 changes: 4 additions & 1 deletion src/types/onyx/Card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,5 +180,8 @@ type WorkspaceCardsList = Record<string, Card> & {
cardList?: Record<string, string>;
};

/** Card list with only available card */
type FilteredCardList = Record<string, string>;

export default Card;
export type {ExpensifyCardDetails, CardList, IssueNewCard, IssueNewCardStep, IssueNewCardData, WorkspaceCardsList, CardLimitType};
export type {ExpensifyCardDetails, CardList, IssueNewCard, IssueNewCardStep, IssueNewCardData, WorkspaceCardsList, CardLimitType, FilteredCardList};
Loading