Strip accounting-style parentheses from expense report names in LHN#83604
Draft
Strip accounting-style parentheses from expense report names in LHN#83604
Conversation
The backend formats negative expense amounts with accounting-style parentheses (e.g., "($25.00)") in report.reportName. This caused the LHN preview to display amounts in brackets. Add stripAccountingBrackets helper to normalize these in getMoneyRequestReportName and computeReportName. Also fix buildOptimisticExpenseReport to use Math.abs for the display amount. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
7 tasks
- Change String#replace() to String#replaceAll() in stripAccountingBrackets to satisfy unicorn/prefer-string-replace-all - Fix import ordering in getClipboardTextTest.ts for Prettier Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
Contributor
Author
|
Fixed the failing checks:
|
Contributor
Author
|
Fixed the failing checks in commit
All lint, typecheck, and test checks are now passing. Only builds and Snyk scans are still pending (infrastructure checks, not related to code changes). |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
The backend (OldDot/PHP) formats negative expense amounts using accounting-style parentheses (e.g.,
($25.00)) in thereport.reportNamefield. When this server-provided name is used as the LHN preview text for expense reports, the parenthesized amount shows up in the UI.This PR adds a
stripAccountingBracketshelper inReportNameUtils.tsthat strips accounting-style parentheses from currency amounts. It's applied in bothgetMoneyRequestReportNameandcomputeReportName— the two places wherereport.reportNameis returned directly for expense reports.Additionally,
buildOptimisticExpenseReportinReportUtils.tswas formatting the stored negative total directly viaconvertToDisplayString(storedTotal), which could produce-$25.00in the optimistic report name. This is fixed to useMath.abs(storedTotal)so the display amount is always positive.Fixed Issues
$ #83269
PROPOSAL: #83269 (comment)
Tests
$25.00not($25.00))Offline tests
This change only affects how report names are displayed — it strips formatting from server-provided strings. No network requests are made, so offline behavior is unchanged.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A — text-only change, no UI layout affected
Android: mWeb Chrome
N/A — text-only change, no UI layout affected
iOS: Native
N/A — text-only change, no UI layout affected
iOS: mWeb Safari
N/A — text-only change, no UI layout affected
MacOS: Chrome / Safari
N/A — text-only change, no UI layout affected