Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/CONST/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,8 @@ const CONST = {
EUR: 'EUR',
},
DEFAULT_CURRENCY_DECIMALS: 2,
// Number of decimals an exchange rate is rounded and padded to for display, matching Expensify Classic.
EXCHANGE_RATE_DISPLAY_DECIMALS: 4,
SCA_CURRENCIES: new Set(['GBP', 'EUR']),
get DIRECT_REIMBURSEMENT_CURRENCIES() {
return [this.CURRENCY.USD, this.CURRENCY.AUD, this.CURRENCY.CAD, this.CURRENCY.GBP, this.CURRENCY.EUR];
Expand Down
2 changes: 1 addition & 1 deletion src/components/TransactionItemRow/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ function TransactionItemRow({
};

const description = getDescription(transactionItem);
const exchangeRateMessage = getExchangeRate(transactionItem, report?.currency ?? policy?.outputCurrency);
const exchangeRateMessage = getExchangeRate(transactionItem, report?.currency ?? policy?.outputCurrency, true);
const cardName = getCompanyCardDescription(translate, transactionItem?.cardName, transactionItem?.cardID, nonPersonalAndWorkspaceCards, transactionItem?.feedCountry);
const isUnreported = transactionItem.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
const shouldShowAttendees = (isUnreported ? !!isAttendeesEnabledForMovingPolicy : shouldShowAttendeesUtils(CONST.IOU.TYPE.SUBMIT, policy)) && transactionAttendees.length > 0;
Expand Down
25 changes: 21 additions & 4 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1481,9 +1481,24 @@ function getTagArrayFromName(tagName: string): string[] {
}

/**
* Returns the exchange rate for a transaction, based on its group or currencyConversionRate
* Caps an exchange rate at 4 decimals for display, matching Expensify Classic, which rounds and pads to
* exactly 4 decimals (`0.272294077603812` -> `0.2723`, `1.5` -> `1.5000`). `toFixed` handles the exponential
* form small rates stringify into (`7.27431439586819e-7` -> `0.0000`), and a finite guard passes a
* non-numeric rate through untouched so we never render `NaN`.
*/
function getExchangeRate(transaction: TransactionWithOptionalSearchFields, reportCurrency?: string) {
function formatExchangeRateForDisplay(rate: string | number): string {
const parsedRate = Number(rate);
return Number.isFinite(parsedRate) ? parsedRate.toFixed(CONST.EXCHANGE_RATE_DISPLAY_DECIMALS) : String(rate);
}

/**
* Returns the exchange rate for a transaction, based on its group or currencyConversionRate.
*
* When `shouldFormatRate` is true (display only), the rate is rounded and padded to exactly 4 decimals
* to match Expensify Classic. The default (false) keeps the raw value so the non-display consumers, the
* search/report sort keys and the emptiness predicate, compare on the full precision exactly as they do today.
*/
function getExchangeRate(transaction: TransactionWithOptionalSearchFields, reportCurrency?: string, shouldFormatRate = false) {
const fromCurrency = getCurrency(transaction);

// On the report view, "unconverted" means the transaction currency matches the report currency.
Expand All @@ -1497,7 +1512,8 @@ function getExchangeRate(transaction: TransactionWithOptionalSearchFields, repor
if (transaction.groupExchangeRate != null && transaction.groupCurrency && fromCurrency !== transaction.groupCurrency) {
const groupRate = Number(transaction.groupExchangeRate);
if (groupRate !== 1) {
return `${transaction.groupExchangeRate} ${fromCurrency}/${transaction.groupCurrency}`;
const rate = shouldFormatRate ? formatExchangeRateForDisplay(transaction.groupExchangeRate) : transaction.groupExchangeRate;
return `${rate} ${fromCurrency}/${transaction.groupCurrency}`;
}
}

Expand All @@ -1509,7 +1525,8 @@ function getExchangeRate(transaction: TransactionWithOptionalSearchFields, repor
if (conversionToCurrency && transaction.currencyConversionRate != null && fromCurrency !== conversionToCurrency) {
const conversionRate = Number(transaction.currencyConversionRate);
if (conversionRate !== 1) {
return `${transaction.currencyConversionRate} ${fromCurrency}/${conversionToCurrency}`;
const rate = shouldFormatRate ? formatExchangeRateForDisplay(transaction.currencyConversionRate) : transaction.currencyConversionRate;
return `${rate} ${fromCurrency}/${conversionToCurrency}`;
}
}

Expand Down
107 changes: 107 additions & 0 deletions tests/unit/TransactionUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4155,6 +4155,113 @@ describe('TransactionUtils', () => {

expect(TransactionUtils.getExchangeRate(transaction, 'EUR')).toBe('');
});

describe('shouldFormatRate (display formatting to 4 decimals, matching Expensify Classic)', () => {
it('rounds a rate with more than 4 decimals rather than truncating', () => {
const transaction = generateTransaction({
currency: 'USD',
groupExchangeRate: 13768.5157822803,
groupCurrency: 'EUR',
amount: -100,
convertedAmount: -1376851,
});

// toFixed(4) rounds .51578… up to .5158 (truncation would give .5157).
expect(TransactionUtils.getExchangeRate(transaction, undefined, true)).toBe('13768.5158 USD/EUR');
});

it('rounds, not truncates, on the values where the two rules differ', () => {
const aed = generateTransaction({
currency: 'AED',
currencyConversionRate: '0.272294077603812',
amount: -100,
convertedAmount: -27,
});
const ron = generateTransaction({
currency: 'RON',
currencyConversionRate: '0.220361392684002',
amount: -100,
convertedAmount: -22,
});

// Classic renders 0.2723 and 0.2204; truncation would give 0.2722 and 0.2203.
expect(TransactionUtils.getExchangeRate(aed, 'USD', true)).toBe('0.2723 AED/USD');
expect(TransactionUtils.getExchangeRate(ron, 'USD', true)).toBe('0.2204 RON/USD');
});

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.

🟡 The exponential case is only tested through the number field, not the string field where the bug was actually reported.

tests/unit/TransactionUtilsTest.ts (the formats an exponential rate to 0.0000 case)

The test drives it through groupExchangeRate: 7.27431439586819e-7, which is typed number. The screenshot in the issue shows the bug arriving through currencyConversionRate, which is typed string, so the real input is the string '7.27431439586819e-07', complete with the zero-padded exponent. Number() parses both, so this passes today, but the two branches read different fields of different types and only one of them is covered for the input class that motivated the issue. One extra case:

it('formats an exponential rate string to 0.0000', () => {
    const transaction = generateTransaction({
        currency: 'IRR',
        currencyConversionRate: '7.27431439586819e-07',
        amount: -100,
        convertedAmount: -1,
    });

    expect(TransactionUtils.getExchangeRate(transaction, 'USD', true)).toBe('0.0000 IRR/USD');
});

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.

Added in eef0158. New case formats an exponential rate string to 0.0000 drives the exponential rate through currencyConversionRate as the zero-padded string '7.27431439586819e-07' — the string field and exact input class from the reported screenshot — alongside the existing number-field case. Both pass.

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.

Done in eef0158. Added a case that drives the exponential input through currencyConversionRate (typed string) using the zero-padded exponent string '7.27431439586819e-07', which is the form the reported bug actually arrived in, and asserts '0.0000 IRR/USD'. All 22 getExchangeRate tests pass.

it('formats an exponential rate to 0.0000 instead of mangling it', () => {
const transaction = generateTransaction({
currency: 'IRR',
groupExchangeRate: 7.27431439586819e-7,
groupCurrency: 'USD',
amount: -100,
convertedAmount: -1,
});

// Classic renders 0.0000; the string-split truncation approach returned 7.2743 here.
expect(TransactionUtils.getExchangeRate(transaction, undefined, true)).toBe('0.0000 IRR/USD');
});

it('formats an exponential rate string to 0.0000', () => {
// The reported bug arrived through currencyConversionRate (typed string), so the real input is the
// zero-padded exponent string rather than the number the groupExchangeRate case above covers.
const transaction = generateTransaction({
currency: 'IRR',
currencyConversionRate: '7.27431439586819e-07',
amount: -100,
convertedAmount: -1,
});

expect(TransactionUtils.getExchangeRate(transaction, 'USD', true)).toBe('0.0000 IRR/USD');
});

it('pads a rate with fewer than 4 decimals to exactly 4', () => {
const transaction = generateTransaction({
currency: 'USD',
groupExchangeRate: 1.5,
groupCurrency: 'EUR',
amount: -100,
convertedAmount: -150,
});

expect(TransactionUtils.getExchangeRate(transaction, undefined, true)).toBe('1.5000 USD/EUR');
});

it('formats the "0.0" string the backend can return to 0.0000', () => {
const transaction = generateTransaction({
currency: 'UZS',
currencyConversionRate: '0.0',
amount: -5000,
convertedAmount: -1,
});

expect(TransactionUtils.getExchangeRate(transaction, 'USD', true)).toBe('0.0000 UZS/USD');
});

it('renders a non-numeric rate verbatim instead of NaN', () => {
const transaction = generateTransaction({
currency: 'USD',
currencyConversionRate: 'invalid',
groupCurrency: 'EUR',
amount: -100,
convertedAmount: -85,
});

// Number('invalid') is NaN, so the finite guard falls back to the raw string.
expect(TransactionUtils.getExchangeRate(transaction, undefined, true)).toBe('invalid USD/EUR');
});

it('leaves the rate untouched without the flag, so the sorts and the emptiness predicate are unaffected', () => {
const transaction = generateTransaction({
currency: 'AED',
currencyConversionRate: '0.272294077603812',
amount: -100,
convertedAmount: -27,
});

expect(TransactionUtils.getExchangeRate(transaction, 'USD')).toBe('0.272294077603812 AED/USD');
});
});
});

describe('mergeProhibitedViolations', () => {
Expand Down
Loading