Skip to content

Remove unsafe assertions from emoji menus, room creation, and card settlement #103920

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

Open
KJ21-ENG wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Remove unsafe assertions from emoji menus, room creation, and card settlement #103920

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
4313716
Remove type assertions from emoji picker, workspace room and settleme…
KJ21-ENG Oct 13, 2026
73eb411
Make room test observations run after commit
KJ21-ENG Oct 13, 2026
2f216b8
Fix reviewer findings for PR #103920
KJ21-ENG Oct 19, 2026
3e01cdb
Fix upstream CI compatibility for PR #103920
KJ21-ENG Oct 19, 2026
052de6b
Merge main and assert exact settlement calls
KJ21-ENG Oct 20, 2026
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
3 changes: 2 additions & 1 deletion assets/emojis/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type en from '@src/languages/en';
import type IconAsset from '@src/types/utils/IconAsset';

type Emoji = {
Expand All @@ -16,7 +17,7 @@ type Emoji = {
type HeaderEmoji = {
header: true;
icon: IconAsset;
code: string;
code: keyof typeof en.emojiPicker.headers;
};

type PickerEmoji = Emoji | HeaderEmoji;
Expand Down
5 changes: 2 additions & 3 deletions src/components/EmojiPicker/EmojiPickerMenu/index.native.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import type {EmojiPickerList, EmojiPickerListItem} from '@libs/EmojiUtils';
import {getRemovedSkinToneEmoji} from '@libs/EmojiUtils';

import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';

import type {ListRenderItem} from '@shopify/flash-list';
import type {ComponentRef} from 'react';
Expand Down Expand Up @@ -161,11 +160,11 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji, ref}: EmojiPickerMenuPro
ref={getHeaderRef(index)}
accessible
accessibilityRole="header"
accessibilityLabel={translate(`emojiPicker.headers.${code}` as TranslationPaths)}
accessibilityLabel={translate(`emojiPicker.headers.${item.code}`)}
style={[styles.emojiHeaderContainer, target === 'StickyHeader' ? styles.mh4 : {width: windowWidth}]}
onLayout={() => handleHeaderLayout(index)}
>
<Text style={styles.textLabelSupporting}>{translate(`emojiPicker.headers.${code}` as TranslationPaths)}</Text>
<Text style={styles.textLabelSupporting}>{translate(`emojiPicker.headers.${item.code}`)}</Text>
</View>
);
}
Expand Down
5 changes: 2 additions & 3 deletions src/components/EmojiPicker/EmojiPickerMenu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import isEnterWhileComposition from '@libs/KeyboardShortcut/isEnterWhileComposit
import {shouldAutoFocusOnKeyPress} from '@libs/ReportUtils';

import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';

import type {ListRenderItem} from '@shopify/flash-list';
import type {ComponentRef} from 'react';
Expand Down Expand Up @@ -84,7 +83,7 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji, ref}: EmojiPickerMenuPro
}
pendingHeaderFocusIndexRef.current = null;

(headerRef.current as unknown as HTMLElement).focus?.();
headerRef.current.focus();
return true;
}, []);

Expand Down Expand Up @@ -343,7 +342,7 @@ function EmojiPickerMenu({onEmojiSelected, activeEmoji, ref}: EmojiPickerMenuPro
target === 'StickyHeader' ? styles.stickyHeaderEmoji : undefined,
]}
>
<Text style={styles.textLabelSupporting}>{translate(`emojiPicker.headers.${code}` as TranslationPaths)}</Text>
<Text style={styles.textLabelSupporting}>{translate(`emojiPicker.headers.${item.code}`)}</Text>
</View>
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Icon/BankIconsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type IconAsset from '@src/types/utils/IconAsset';

type BankIconParams = {
styles: ThemeStyles;
bankName?: BankName;
bankName?: string;
isCard?: boolean;
maxIconSize?: number;
};
Expand Down
8 changes: 5 additions & 3 deletions src/components/SettlementAccountSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';

import type {BankName} from '@src/types/onyx/Bank';

import React from 'react';
import {View} from 'react-native';

Expand Down Expand Up @@ -31,7 +29,11 @@ type SettlementAccountSelectorProps = {
initiallyFocusedItemKey?: string;
};

function BankAccountListItemLeftElement({bankName}: {bankName: BankName}) {
type BankAccountListItemLeftElementProps = {
bankName: string;
};

function BankAccountListItemLeftElement({bankName}: BankAccountListItemLeftElementProps) {
const styles = useThemeStyles();
const {icon, iconSize, iconStyles} = getBankIcon({bankName, styles});

Expand Down
23 changes: 19 additions & 4 deletions src/pages/workspace/WorkspaceNewRoomPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import BlockingView from '@components/BlockingViews/BlockingView';
import Button from '@components/Button';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
import type {FormOnyxValues} from '@components/Form/types';
import type {FormOnyxValues, FormValue} from '@components/Form/types';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import MenuItemField from '@components/MenuItem/presets/MenuItemField';
import type {AnimatedTextInputRef} from '@components/RNTextInput';
Expand Down Expand Up @@ -44,6 +44,10 @@ import {useIsFocused} from '@react-navigation/core';
import React, {useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';

function findRoomOption<TValue extends string>(options: Array<{value: TValue}>, value: FormValue | undefined) {
return options.find((option) => option.value === value);
}

function EmptyWorkspaceView() {
const styles = useThemeStyles();
const {translate} = useLocalize();
Expand Down Expand Up @@ -346,7 +350,8 @@ function WorkspaceNewRoomPage({ref, policyID: lockedPolicyID}: WorkspaceNewRoomP
label={translate('workspace.common.workspace')}
items={workspaceOptions}
value={policyID}
onValueChange={(value) => setPolicyID(value as typeof policyID)}
valueType="string"
onValueChange={(value) => setPolicyID(value)}
/>
</View>
)}
Expand All @@ -358,7 +363,12 @@ function WorkspaceNewRoomPage({ref, policyID: lockedPolicyID}: WorkspaceNewRoomP
label={translate('writeCapabilityPage.label')}
items={writeCapabilityOptions}
value={writeCapability}
onValueChange={(value) => setWriteCapability(value as typeof writeCapability)}
onValueChange={(value) => {
const selectedOption = findRoomOption(writeCapabilityOptions, value);
if (selectedOption) {
setWriteCapability(selectedOption.value);
}
}}
/>
</View>
)}
Expand All @@ -368,7 +378,12 @@ function WorkspaceNewRoomPage({ref, policyID: lockedPolicyID}: WorkspaceNewRoomP
inputID={INPUT_IDS.VISIBILITY}
label={translate('newRoomPage.visibility')}
items={visibilityOptions}
onValueChange={(value) => setVisibility(value as typeof visibility)}
onValueChange={(value) => {

@github-actions github-actions Bot Oct 16, 2026

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.

❌ CONSISTENCY-3 (docs)

This onValueChange handler duplicates the exact logic introduced for the writeCapability picker just above it: find the option whose value matches, and if it exists, set state to selectedOption.value. The two handlers differ only in the options array (visibilityOptions vs writeCapabilityOptions) and the setter (setVisibility vs setWriteCapability), so the shared shape should be factored out instead of copied.

Extract a small helper and reuse it for both pickers:

const getMatchingOptionValue = <T,>(options: Array<{value: T}>, value: string): T | undefined =>
    options.find((option) => option.value === value)?.value;

// writeCapability picker
onValueChange={(value) => {
    const next = getMatchingOptionValue(writeCapabilityOptions, value);
    if (next) {
        setWriteCapability(next);
    }
}}

// visibility picker
onValueChange={(value) => {
    const next = getMatchingOptionValue(visibilityOptions, value);
    if (next) {
        setVisibility(next);
    }
}}

Reviewed at: 73eb411 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

@ikevin127 ikevin127 Oct 17, 2026

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.

⚠️ Duplicated .find() guard in WorkspaceNewRoomPage.tsx is a valid find by the bot.

@KJ21-ENG KJ21-ENG Oct 19, 2026

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.

Extracted the repeated lookup into one local typed findRoomOption helper used by both pickers. It returns only offered option values; missing and unmatched selections leave the current state unchanged.

@KJ21-ENG KJ21-ENG Oct 19, 2026

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.

Removed the duplicated lookup through one local typed helper shared by both handlers. Added tests for valid, missing, and unmatched picker selections.

const selectedOption = findRoomOption(visibilityOptions, value);
if (selectedOption) {
setVisibility(selectedOption.value);
}
}}
value={visibility}
furtherDetails={visibilityDescription}
shouldShowTooltips={false}
Expand Down
10 changes: 4 additions & 6 deletions src/pages/workspace/expensifyCard/DynamicWorkspaceSettlementAccountPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type {BankName} from '@src/types/onyx/Bank';
import type {ConnectionName} from '@src/types/onyx/Policy';

import {useNavigationState} from '@react-navigation/native';
import {isExpensifyCardContinuousReconciliationEnabledSelector} from '@selectors/Card';
Expand Down Expand Up @@ -104,7 +102,7 @@ function DynamicWorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccou
}, [cardSettings, hasActiveAccountingConnection, continuousReconciliation, reconciliationConnection, fetchPolicyAccountingData]);

const eligibleBankAccountsOptions: BankAccountListItem[] = eligibleBankAccounts.map((bankAccount) => {
const bankName = (bankAccount.accountData?.addressName ?? '') as BankName;
const bankName = bankAccount.accountData?.addressName ?? '';
const bankAccountNumber = bankAccount.accountData?.accountNumber ?? '';
const bankAccountID = bankAccount.accountData?.bankAccountID ?? bankAccount.methodID;

Expand All @@ -121,7 +119,7 @@ function DynamicWorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccou
const fallbackBankAccountOption: BankAccountListItem = {
value: paymentBankAccountID,
text: paymentBankAccountAddressName ?? '',
leftElement: <BankAccountListItemLeftElement bankName={(paymentBankAccountAddressName ?? '') as BankName} />,
leftElement: <BankAccountListItemLeftElement bankName={paymentBankAccountAddressName ?? ''} />,
alternateText: `${translate('workspace.expensifyCard.accountEndingIn')} ${getLastFourDigits(paymentBankAccountNumberFromCardSettings ?? '')}`,
keyForList: paymentBankAccountID?.toString() ?? '',
isSelected: true,
Expand All @@ -141,8 +139,7 @@ function DynamicWorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccou
};

const getCustomListHeaderContent = () => {
const connectionName = reconciliationConnection ?? '';
const connectionParam = getRouteParamForConnection(connectionName as ConnectionName);
const connectionParam = reconciliationConnection ? getRouteParamForConnection(reconciliationConnection) : undefined;

return (
<>
Expand Down Expand Up @@ -194,3 +191,4 @@ function DynamicWorkspaceSettlementAccountPage({route}: WorkspaceSettlementAccou
}

export default DynamicWorkspaceSettlementAccountPage;
export type {WorkspaceSettlementAccountPageProps};
Loading
Loading