Skip to content

Removed useless isValidCurrency check #87693

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
shubham1206agra wants to merge 2 commits into
base: main
Choose a base branch
from
+8 −32
Open

Removed useless isValidCurrency check #87693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11a7bcf
Removed useless isValidCurrency check
shubham1206agra Apr 11, 2026
93c4bf8
Fixed tests
shubham1206agra Apr 11, 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
9 changes: 0 additions & 9 deletions src/libs/CurrencyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
let currencyList: OnyxValues[typeof ONYXKEYS.CURRENCY_LIST] = {};

/* eslint-disable rulesdir/prefer-onyx-connect-in-libs -- may refactor to useOnyx/connectWithoutView later */
Onyx.connect({

Check warning on line 12 in src/libs/CurrencyUtils.ts

View workflow job for this annotation

GitHub Actions / ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function

Check warning on line 12 in src/libs/CurrencyUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CURRENCY_LIST,
callback: (val) => {
if (!val || Object.keys(val).length === 0) {
Expand Down Expand Up @@ -201,14 +201,6 @@
.join('');
}

/**
* Checks if passed currency code is a valid currency based on currency list
*/
function isValidCurrencyCode(currencyCode: string): boolean {
const currency = currencyList?.[currencyCode];
return !!currency;
}

export {
getCurrencyDecimals,
getCurrencyUnit,
Expand All @@ -221,6 +213,5 @@
convertAmountToDisplayString,
convertToDisplayStringWithoutCurrency,
convertToDisplayStringWithExplicitCurrency,
isValidCurrencyCode,
convertToShortDisplayString,
};
9 changes: 2 additions & 7 deletions src/libs/Formula.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {ValueOf} from 'type-fest';
import CONST from '@src/CONST';
import type {PersonalDetails, Policy, PolicyReportField, Report, Transaction} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import {convertToDisplayString, convertToDisplayStringWithoutCurrency, isValidCurrencyCode} from './CurrencyUtils';
import {convertToDisplayString, convertToDisplayStringWithoutCurrency} from './CurrencyUtils';
import {formatDate} from './FormulaDatetime';
import getBase62ReportID from './getBase62ReportID';
import Log from './Log';
Expand Down Expand Up @@ -573,11 +573,6 @@ function formatAmount(amount: number | undefined, currency: string | undefined,
return convertToDisplayStringWithoutCurrency(absoluteAmount, currency);
}

// Check if format is a valid currency code (e.g., USD, EUR, eur)
if (!isValidCurrencyCode(trimmedDisplayCurrency)) {
return '';
}

// If a currency conversion is needed (displayCurrency differs from the source),
// return null so the backend can compute it.
// We can only compute the value optimistically when the amount is 0.
Expand All @@ -588,7 +583,7 @@ function formatAmount(amount: number | undefined, currency: string | undefined,
return convertToDisplayString(absoluteAmount, trimmedDisplayCurrency);
}

if (currency && isValidCurrencyCode(currency)) {
if (currency) {
return convertToDisplayString(absoluteAmount, currency, true);
}

Expand Down
10 changes: 0 additions & 10 deletions tests/unit/FormulaTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// eslint-disable-next-line no-restricted-syntax -- disabled because we need CurrencyUtils to mock
import * as CurrencyUtils from '@libs/CurrencyUtils';
import type {FormulaContext} from '@libs/Formula';
import {compute, hasCircularReferences, parse, resolveReportFieldValue} from '@libs/Formula';
// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportActionsUtils to mock
Expand All @@ -18,14 +16,8 @@ jest.mock('@libs/ReportUtils', () => ({
getReportTransactions: jest.fn(),
}));

jest.mock('@libs/CurrencyUtils', () => ({
...jest.requireActual<typeof CurrencyUtils>('@libs/CurrencyUtils'),
isValidCurrencyCode: jest.fn(),
}));

const mockReportActionsUtils = ReportActionsUtils as jest.Mocked<typeof ReportActionsUtils>;
const mockReportUtils = ReportUtils as jest.Mocked<typeof ReportUtils>;
const mockCurrencyUtils = CurrencyUtils as jest.Mocked<typeof CurrencyUtils>;

describe('CustomFormula', () => {
describe('parse()', () => {
Expand Down Expand Up @@ -96,8 +88,6 @@ describe('CustomFormula', () => {
beforeEach(() => {
jest.clearAllMocks();

mockCurrencyUtils.isValidCurrencyCode.mockImplementation((code: string) => ['USD', 'EUR', 'JPY', 'NPR'].includes(code));

const mockReportActions = {
// eslint-disable-next-line @typescript-eslint/naming-convention
'1': {
Expand Down
12 changes: 6 additions & 6 deletions tests/unit/ReportUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10036,15 +10036,15 @@ describe('ReportUtils', () => {
};
const context = createFormulaContext(reportWithNaNTotal, mockPolicy);
const result = compute('{report:total}', context);
// NaN gets formatted as "NaN" by formatAmount, which is a non-empty value
expect(result).toBe('NaN');
// NaN gets formatted as "$NaN" by convertToDisplayString, which is a non-empty value
expect(result).toBe('$NaN');
});

it('should replace {report:total} with formatted amount', () => {
const context = createFormulaContext(mockReport, mockPolicy);
const result = compute('{report:total}', context);
// compute uses convertToDisplayString which includes currency symbol
expect(result).toBe('50.00');
expect(result).toBe('$50.00');
});

it('should replace {report:id} with base62 report ID', () => {
Expand All @@ -10058,7 +10058,7 @@ describe('ReportUtils', () => {
const context = createFormulaContext(mockReport, mockPolicy);
const result = compute(formula, context);
const expectedId = getBase62ReportID(Number(mockReport.reportID));
expect(result).toBe(`Report ${expectedId} has total 50.00`);
expect(result).toBe(`Report ${expectedId} has total $50.00`);
});

it('should handle undefined total by falling back to original definition', () => {
Expand All @@ -10081,8 +10081,8 @@ describe('ReportUtils', () => {
const context = createFormulaContext(reportWithNaNTotal, mockPolicy);
const expectedId = getBase62ReportID(Number(mockReport.reportID));
const result = compute(formula, context);
// NaN gets formatted as "NaN" which is a non-empty value
expect(result).toBe(`ID: ${expectedId}, Total: NaN, Type: Expense Report`);
// NaN gets formatted as "$NaN" which is a non-empty value
expect(result).toBe(`ID: ${expectedId}, Total: $NaN, Type: Expense Report`);
});

it('should handle missing total gracefully', () => {
Expand Down
Loading