Fix approval limit CSV import conversion - #101422
rushatgabhane wants to merge 3 commits into
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
bc2ef2f to
bcbfdc7
Compare
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
bcbfdc7 to
3cde44e
Compare
|
@MelvinBot The current CURRENCY_SYMBOLS regex only supports $, €, £, and ¥, but Expensify supports more currencies. What is the best existing repository or external-library approach to parse and clean currency symbols for every supported currency? |
|
Don't use a symbol whitelist and don't add a library. The authoritative list of every currency Expensify supports is already on the client: Onyx Separately: Recommended implementationDelete import StringUtils from './StringUtils';
import {stripCommaFromAmount, stripSpacesFromAmount} from './MoneyRequestUtils';
/**
* Normalize an amount cell from an imported spreadsheet. Removes the workspace currency's symbol or ISO
* code (prefix/suffix only, where currency markers actually appear) and any Unicode currency sign, plus
* grouping separators and whitespace. Everything else is left intact so validateAmount can reject it.
*/
function normalizeImportedAmount(value: string, currencySymbol?: string, currencyCode?: string): string {
const tokens = [currencySymbol, currencyCode].filter(Boolean).map((token) => StringUtils.escapeRegExp(token as string));
const withoutCurrencyCode = tokens.length > 0 ? value.trim().replace(new RegExp(`^(?:${tokens.join('|')})\\s*|\\s*(?:${tokens.join('|')})$`, 'i'), '') : value;
return stripCommaFromAmount(stripSpacesFromAmount(withoutCurrencyCode.replaceAll(/\p{Sc}/gu, '')));
}Then in both call sites, replace const {getCurrencySymbol, getCurrencyDecimals} = useCurrencyListActions();
const currency = policy?.outputCurrency;
const normalized = normalizeImportedAmount(String(value), getCurrencySymbol(currency ?? ''), currency);
Why this over the alternatives
Also fix: cents conversion is 100× off for zero-decimal currenciesUnrelated to the symbol question, but it's in the same lines. The backend stores 1/100 units for every currency, regardless of the currency's decimal count — see the doc comment on The manual UI path agrees: With approvalLimit = String(convertToBackendAmount(Number.parseFloat(normalizedApprovalLimit)));Keep Test to addThere is currently no test covering the approval-limit CSV column. Next Steps: Reply with |
|
@MelvinBot can you resolve conflicts |
Resolved conflicts in src/libs/importSpreadsheetUtils.ts: main added findColumnName/getCompanyCardColumnMappings while this branch added normalizeImportedAmount. Both are additive, so kept both plus their imports and combined the export list.
|
Done — conflicts resolved and pushed as a merge commit ( One file conflicted: Both sides were purely additive, so I kept both:
The 11 language files auto-merged; How I verified the resolution
Heads-up: I could not run Unrelated to the conflict, two things in this PR still need you:
|
Explanation of Change
Updated member CSV imports so approval limits are converted from major currency units into backend minor units before being stored or sent.
For example, an approval limit of
500is converted to50000cents for USD. The conversion also respects the workspace currency’s decimal places and preserves blank values.Fixed Issues
$ https://github.com/Expensify/App/issues/682002
PROPOSAL:
Tests
500.50000cents and displayed as$500.Offline tests
No offline behavior was changed. Offline testing was not performed.
QA Steps
Approval limit = 500.$500approval limit instead of$5.Screenshots/Videos
No visual or layout changes were made. Screenshots are not applicable.