Skip to content

Fix-55026: Make shouldShowAddMissingDetails condition consistent #55510

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 13 commits into from
Feb 28, 2025
Merged

Fix-55026: Make shouldShowAddMissingDetails condition consistent #55510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9586af7
add unit test and modify the logic
DylanDylann Jan 24, 2025
7721461
revert the prev logic
DylanDylann Jan 24, 2025
606ce73
update condition
DylanDylann Jan 24, 2025
059d264
update UTs
DylanDylann Jan 24, 2025
5738fb6
short-hand syntax
DylanDylann Jan 24, 2025
05b2f04
merge main
DylanDylann Feb 14, 2025
806e621
merge main
DylanDylann Feb 25, 2025
b56cc53
add card state check
DylanDylann Feb 25, 2025
1e0f35c
fix lint
DylanDylann Feb 25, 2025
37d173f
merge main
DylanDylann Feb 25, 2025
d229e2a
Update src/pages/MissingPersonalDetails/MissingPersonalDetailsMagicCo…
DylanDylann Feb 27, 2025
9e77b64
Update src/pages/MissingPersonalDetails/MissingPersonalDetailsMagicCo…
DylanDylann Feb 27, 2025
d546e46
Update src/pages/MissingPersonalDetails/MissingPersonalDetailsMagicCo…
DylanDylann Feb 27, 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
39 changes: 12 additions & 27 deletions src/components/ReportActionItem/IssueCardMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import RenderHTML from '@components/RenderHTML';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import {getPolicy, getWorkspaceAccountID, isPolicyAdmin} from '@libs/PolicyUtils';
import {getCardIssuedMessage, getOriginalMessage, isActionOfType, shouldShowAddMissingDetails} from '@libs/ReportActionsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand All @@ -23,42 +23,27 @@ type IssueCardMessageProps = {
function IssueCardMessage({action, policyID}: IssueCardMessageProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS);
const [session] = useOnyx(ONYXKEYS.SESSION);
const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID ?? '-1');
const workspaceAccountID = getWorkspaceAccountID(policyID);
const [cardList = {}] = useOnyx(ONYXKEYS.CARD_LIST);
const [session] = useOnyx(ONYXKEYS.SESSION);
const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`);

const assigneeAccountID = (ReportActionsUtils.getOriginalMessage(action) as IssueNewCardOriginalMessage)?.assigneeAccountID;

const missingDetails =
!privatePersonalDetails?.legalFirstName ||
!privatePersonalDetails?.legalLastName ||
!privatePersonalDetails?.dob ||
!privatePersonalDetails?.phoneNumber ||
isEmptyObject(privatePersonalDetails?.addresses) ||
privatePersonalDetails.addresses.length === 0;

const isAssigneeCurrentUser = !isEmptyObject(session) && session.accountID === assigneeAccountID;

const cardIssuedActionOriginalMessage = ReportActionsUtils.isActionOfType(
const assigneeAccountID = (getOriginalMessage(action) as IssueNewCardOriginalMessage)?.assigneeAccountID;
const cardIssuedActionOriginalMessage = isActionOfType(
action,
CONST.REPORT.ACTIONS.TYPE.CARD_ISSUED,
CONST.REPORT.ACTIONS.TYPE.CARD_ISSUED_VIRTUAL,
CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS,
)
? ReportActionsUtils.getOriginalMessage(action)
? getOriginalMessage(action)
: undefined;
const cardID = cardIssuedActionOriginalMessage?.cardID ?? -1;
const isPolicyAdmin = PolicyUtils.isPolicyAdmin(PolicyUtils.getPolicy(policyID));
const card = isPolicyAdmin ? cardsList?.[cardID] : cardList[cardID];
const shouldShowAddMissingDetailsButton = !isEmptyObject(card) && action?.actionName === CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS && missingDetails && isAssigneeCurrentUser;
const cardID = cardIssuedActionOriginalMessage?.cardID ?? CONST.DEFAULT_NUMBER_ID;
const card = isPolicyAdmin(getPolicy(policyID)) ? cardsList?.[cardID] : cardList[cardID];
const isAssigneeCurrentUser = !isEmptyObject(session) && session.accountID === assigneeAccountID;
const shouldShowAddMissingDetailsButton = isAssigneeCurrentUser && shouldShowAddMissingDetails(action?.actionName, card);

return (
<>
<RenderHTML
html={`<muted-text>${ReportActionsUtils.getCardIssuedMessage({reportAction: action, shouldRenderHTML: true, policyID, shouldDisplayLinkToCard: !!card})}</muted-text>`}
/>
<RenderHTML html={`<muted-text>${getCardIssuedMessage({reportAction: action, shouldRenderHTML: true, policyID, card})}</muted-text>`} />
{shouldShowAddMissingDetailsButton && (
<Button
onPress={() => Navigation.navigate(ROUTES.MISSING_PERSONAL_DETAILS)}
Expand Down
35 changes: 18 additions & 17 deletions src/libs/ReportActionsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Locale, OnyxInputOrEntry, PersonalDetailsList, PrivatePersonalDetails} from '@src/types/onyx';
import type {Card, Locale, OnyxInputOrEntry, PersonalDetailsList, PrivatePersonalDetails} from '@src/types/onyx';
import type {JoinWorkspaceResolution, OriginalMessageChangeLog, OriginalMessageExportIntegration} from '@src/types/onyx/OriginalMessage';
import type {PolicyReportFieldType} from '@src/types/onyx/Policy';
import type Report from '@src/types/onyx/Report';
Expand Down Expand Up @@ -2116,17 +2116,29 @@ function isCardIssuedAction(reportAction: OnyxEntry<ReportAction>) {
);
}

function shouldShowAddMissingDetails(actionName?: ReportActionName, card?: Card) {
const missingDetails =
!privatePersonalDetails?.legalFirstName ||
!privatePersonalDetails?.legalLastName ||
!privatePersonalDetails?.dob ||
!privatePersonalDetails?.phoneNumber ||
isEmptyObject(privatePersonalDetails?.addresses) ||
privatePersonalDetails.addresses.length === 0;

return actionName === CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS && (card?.state === CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED || missingDetails);
}

function getCardIssuedMessage({
reportAction,
shouldRenderHTML = false,
policyID = '-1',
shouldDisplayLinkToCard = false,
card,
personalDetails,
}: {
reportAction: OnyxEntry<ReportAction>;
shouldRenderHTML?: boolean;
policyID?: string;
shouldDisplayLinkToCard?: boolean;
card?: Card;
personalDetails?: Partial<PersonalDetailsList>;
}) {
const cardIssuedActionOriginalMessage = isActionOfType(
Expand All @@ -2150,24 +2162,12 @@ function getCardIssuedMessage({
const assignee = shouldRenderHTML ? `<mention-user accountID="${assigneeAccountID}"/>` : assigneeDetails?.firstName ?? assigneeDetails?.login ?? '';
const navigateRoute = isPolicyAdmin ? ROUTES.EXPENSIFY_CARD_DETAILS.getRoute(policyID, String(cardID)) : ROUTES.SETTINGS_DOMAINCARD_DETAIL.getRoute(String(cardID));
const expensifyCardLink =
shouldRenderHTML && shouldDisplayLinkToCard
? `<a href='${environmentURL}/${navigateRoute}'>${translateLocal('cardPage.expensifyCard')}</a>`
: translateLocal('cardPage.expensifyCard');

@getusha getusha Oct 2, 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.

Looks like this caused a regression here: #68123
We failed to consider a case where the card is disabled.

shouldRenderHTML && !!card ? `<a href='${environmentURL}/${navigateRoute}'>${translateLocal('cardPage.expensifyCard')}</a>` : translateLocal('cardPage.expensifyCard');
const companyCardLink = shouldRenderHTML
? `<a href='${environmentURL}/${ROUTES.SETTINGS_WALLET}'>${translateLocal('workspace.companyCards.companyCard')}</a>`
: translateLocal('workspace.companyCards.companyCard');

const missingDetails =
!privatePersonalDetails?.legalFirstName ||
!privatePersonalDetails?.legalLastName ||
!privatePersonalDetails?.dob ||
!privatePersonalDetails?.phoneNumber ||
isEmptyObject(privatePersonalDetails?.addresses) ||
privatePersonalDetails.addresses.length === 0;

const isAssigneeCurrentUser = currentUserAccountID === assigneeAccountID;

const shouldShowAddMissingDetailsMessage = !isAssigneeCurrentUser || (reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS && missingDetails);
const shouldShowAddMissingDetailsMessage = !isAssigneeCurrentUser || shouldShowAddMissingDetails(reportAction?.actionName, card);
switch (reportAction?.actionName) {
case CONST.REPORT.ACTIONS.TYPE.CARD_ISSUED:
return translateLocal('workspace.expensifyCard.issuedCard', {assignee});
Expand Down Expand Up @@ -2322,6 +2322,7 @@ export {
getActionableJoinRequestPendingReportAction,
getReportActionsLength,
wasActionCreatedWhileOffline,
shouldShowAddMissingDetails,
getWorkspaceCategoryUpdateMessage,
getWorkspaceUpdateFieldMessage,
getWorkspaceNameUpdatedMessage,
Expand Down
7 changes: 5 additions & 2 deletions src/pages/MissingPersonalDetails/MissingPersonalDetailsMagicCodeModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {clearPersonalDetailsErrors} from '@libs/actions/PersonalDetails';
import {requestValidateCodeAction} from '@libs/actions/User';
import {getLatestError} from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

Expand All @@ -19,11 +20,13 @@ type MissingPersonalDetailsMagicCodeModalProps = {
function MissingPersonalDetailsMagicCodeModal({onClose, isValidateCodeActionModalVisible, handleSubmitForm}: MissingPersonalDetailsMagicCodeModalProps) {
const {translate} = useLocalize();
const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS);
const [cardList] = useOnyx(ONYXKEYS.CARD_LIST);
const [account] = useOnyx(ONYXKEYS.ACCOUNT);
const [validateCodeAction] = useOnyx(ONYXKEYS.VALIDATE_ACTION_CODE);
const privateDetailsErrors = privatePersonalDetails?.errors ?? undefined;
const validateLoginError = getLatestError(privateDetailsErrors);
const primaryLogin = account?.primaryLogin ?? '';
const areAllCardsShipped = Object.values(cardList ?? {})?.every((card) => card?.state !== CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED);

const missingDetails =
!privatePersonalDetails?.legalFirstName ||
Expand All @@ -34,13 +37,13 @@ function MissingPersonalDetailsMagicCodeModal({onClose, isValidateCodeActionModa
privatePersonalDetails.addresses.length === 0;

useEffect(() => {
if (missingDetails || !!privateDetailsErrors) {
if (missingDetails || !!privateDetailsErrors || !areAllCardsShipped) {
return;
}

clearDraftValues(ONYXKEYS.FORMS.PERSONAL_DETAILS_FORM);
Navigation.goBack();
}, [missingDetails, privateDetailsErrors]);
}, [missingDetails, privateDetailsErrors, areAllCardsShipped]);

const onBackButtonPress = () => {
onClose?.();
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ function BaseReportActionContextMenu({
openOverflowMenu,
setIsEmojiPickerActive,
moneyRequestAction,
hasCard: !!card,
card,
};

if ('renderContent' in contextAction) {
Expand Down
8 changes: 4 additions & 4 deletions src/pages/home/report/ContextMenu/ContextMenuActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ import {
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ROUTES from '@src/ROUTES';
import type {Beta, Download as DownloadOnyx, OnyxInputOrEntry, ReportAction, ReportActionReactions, Report as ReportType, Transaction, User} from '@src/types/onyx';
import type {Beta, Card, Download as DownloadOnyx, OnyxInputOrEntry, ReportAction, ReportActionReactions, Report as ReportType, Transaction, User} from '@src/types/onyx';
import type IconAsset from '@src/types/utils/IconAsset';
import KeyboardUtils from '@src/utils/keyboard';
import type {ContextMenuAnchor} from './ReportActionContextMenu';
Expand Down Expand Up @@ -179,7 +179,7 @@ type ContextMenuActionPayload = {
setIsEmojiPickerActive?: (state: boolean) => void;
anchorRef?: MutableRefObject<View | null>;
moneyRequestAction: ReportAction | undefined;
hasCard?: boolean;
card?: Card;
};

type OnPress = (closePopover: boolean, payload: ContextMenuActionPayload, selection?: string, reportID?: string, draftMessage?: string) => void;
Expand Down Expand Up @@ -459,7 +459,7 @@ const ContextMenuActions: ContextMenuAction[] = [
// If return value is true, we switch the `text` and `icon` on
// `ContextMenuItem` with `successText` and `successIcon` which will fall back to
// the `text` and `icon`
onPress: (closePopover, {reportAction, transaction, selection, report, reportID, hasCard}) => {
onPress: (closePopover, {reportAction, transaction, selection, report, reportID, card}) => {
const isReportPreviewAction = isReportPreviewActionReportActionsUtils(reportAction);
const messageHtml = getActionHtml(reportAction);
const messageText = getReportActionMessageText(reportAction);
Expand Down Expand Up @@ -599,7 +599,7 @@ const ContextMenuActions: ContextMenuAction[] = [
const {label, errorMessage} = getOriginalMessage(reportAction) ?? {label: '', errorMessage: ''};
setClipboardMessage(translateLocal('report.actions.type.integrationSyncFailed', {label, errorMessage}));
} else if (isCardIssuedAction(reportAction)) {
setClipboardMessage(getCardIssuedMessage({reportAction, shouldRenderHTML: true, policyID: report?.policyID, shouldDisplayLinkToCard: hasCard}));
setClipboardMessage(getCardIssuedMessage({reportAction, shouldRenderHTML: true, policyID: report?.policyID, card}));
} else if (isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.DELETE_INTEGRATION)) {
setClipboardMessage(getRemovedConnectionMessage(reportAction));
} else if (content) {
Expand Down
78 changes: 78 additions & 0 deletions tests/unit/ReportActionsUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,4 +636,82 @@ describe('ReportActionsUtils', () => {
expect(expectedFragments).toEqual([{text: expectedMessage, html: `<muted-text>${expectedMessage}</muted-text>`, type: 'COMMENT'}]);
});
});

describe('shouldShowAddMissingDetails', () => {
it('should return true if personal detail is not completed', async () => {
const card = {
cardID: 1,
state: CONST.EXPENSIFY_CARD.STATE.STATE_DEACTIVATED,
bank: 'vcf',
domainName: 'expensify',
lastUpdated: '2022-11-09 22:27:01.825',
fraud: CONST.EXPENSIFY_CARD.FRAUD_TYPES.DOMAIN,
};
const mockPersonalDetail = {
address: {
street: '123 Main St',
city: 'New York',
state: 'NY',
postalCode: '10001',
},
};
await Onyx.set(ONYXKEYS.PRIVATE_PERSONAL_DETAILS, mockPersonalDetail);
const res = ReportActionsUtils.shouldShowAddMissingDetails(CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS, card);
expect(res).toEqual(true);
});
it('should return true if card state is STATE_NOT_ISSUED', async () => {
const card = {
cardID: 1,
state: CONST.EXPENSIFY_CARD.STATE.STATE_NOT_ISSUED,
bank: 'vcf',
domainName: 'expensify',
lastUpdated: '2022-11-09 22:27:01.825',
fraud: CONST.EXPENSIFY_CARD.FRAUD_TYPES.DOMAIN,
};
const mockPersonalDetail = {
addresses: [
{
street: '123 Main St',
city: 'New York',
state: 'NY',
postalCode: '10001',
},
],
legalFirstName: 'John',
legalLastName: 'David',
phoneNumber: '+162992973',
dob: '9-9-2000',
};
await Onyx.set(ONYXKEYS.PRIVATE_PERSONAL_DETAILS, mockPersonalDetail);
const res = ReportActionsUtils.shouldShowAddMissingDetails(CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS, card);
expect(res).toEqual(true);
});
it('should return false if no condition is matched', async () => {
const card = {
cardID: 1,
state: CONST.EXPENSIFY_CARD.STATE.OPEN,
bank: 'vcf',
domainName: 'expensify',
lastUpdated: '2022-11-09 22:27:01.825',
fraud: CONST.EXPENSIFY_CARD.FRAUD_TYPES.DOMAIN,
};
const mockPersonalDetail = {
addresses: [
{
street: '123 Main St',
city: 'New York',
state: 'NY',
postalCode: '10001',
},
],
legalFirstName: 'John',
legalLastName: 'David',
phoneNumber: '+162992973',
dob: '9-9-2000',
};
await Onyx.set(ONYXKEYS.PRIVATE_PERSONAL_DETAILS, mockPersonalDetail);
const res = ReportActionsUtils.shouldShowAddMissingDetails(CONST.REPORT.ACTIONS.TYPE.CARD_MISSING_ADDRESS, card);
expect(res).toEqual(false);
});
});
});