Add {report:debitedAmount} and {report:creditedAmount} formula tokens - #100060
Add {report:debitedAmount} and {report:creditedAmount} formula tokens#100060ishpaul777 wants to merge 4 commits into
Conversation
Keep NewDot titles and HelpDot in lockstep with Classic: {report:debitedAmount} and {report:creditedAmount}, empty when the reimbursement has no FX split.
Concierge reviewer checklist:
For more detailed instructions on completing this checklist, see How do I review a HelpDot PR as a Concierge Team member? |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@parasharrajat @RachCHopkins One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Fix CONSISTENCY-16 semicolon-joined comment and extract the shared debited/credited amount formatting into one helper (CONSISTENCY-3).
|
HelpDot Documentation Review Overall Assessment Scores Summary
Key Findings
Recommendations
Files Reviewed
Note: The remaining changed files (src/libs/Formula.ts, tests/unit/FormulaTest.ts, cspell.json) are implementation/config and outside the scope of this documentation review. |
| | `{report:type}` | Expense Report | Report type (Expense Report, Invoice, Bill) | | ||
| | `{report:reimbursable}` | $143.43 | Reimbursable amount | | ||
| | `{report:debitedAmount}` | $82.50 | Amount taken from the company bank account for a cross-border reimbursement | | ||
| | `{report:creditedAmount}` | C$110.00 | Amount deposited to the employee bank account for a cross-border reimbursement | |
There was a problem hiding this comment.
Terminology: In New Expensify articles, prefer "member" over "employee" per the naming conventions. Consider "Amount deposited to the member's bank account for a cross-border reimbursement" to align with Expensify NewDot terminology.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2f06a846
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Empty is a real value for submit and reimbursement-amount tokens; keep it. | ||
| if (value === '' && !isSubmissionInfoPart(part) && !isReimbursementAmountPart(part)) { | ||
| value = part.definition; |
There was a problem hiding this comment.
Preserve reimbursement tokens when formatting fails
When an amount and source currency exist but formatting returns an empty string—for example, {report:debitedAmount:UNKNOWN} uses an invalid display currency—this blanket exemption treats the empty value as resolved. Consequently, hasUnresolvedTokens remains false and computeOptimisticReportName() accepts a title with the amount silently removed instead of retaining the server-rendered title. Distinguish an intentionally missing reimbursement amount from formatting failures so the latter preserve the raw token.
Useful? React with 👍 / 👎.
Title and field formulas are frozen once a report reaches approved, and the amounts only exist after the payment converts during reimbursement, so a title using these tokens is always empty. They stay in the export reference.
Explanation of Change
Teaches the NewDot formula engine
{report:debitedAmount}and{report:creditedAmount}so it resolves the same token set as the Auth and Web-Expensify engines. Both amounts are already on the report in Onyx. A missing amount resolves to empty, which covers same-currency payments and reports that have not been reimbursed yet. A trailing currency that needs conversion is left as the raw token so the backend fills it in, which is how{report:total:EUR}already behaves here.These tokens are primarily for custom CSV and PDF export templates, which the Web-Expensify engine renders. On the client they matter in two places: a report field formula that also references
{field:...}is resolved live in the report view, and an optimistic report title is computed locally, where an unknown token would otherwise be left on screen as raw placeholder text.HelpDot lists the tokens in the Expensify Classic export article. They are deliberately not listed in the report title articles, because Auth freezes title and field formulas once a report reaches approved and these amounts only exist after the payment converts during reimbursement, so a title using them would always be empty.
Ship with Auth#24161 and Web-Expensify#55803.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/655941
PROPOSAL:
Tests
Paymentwith the formula{field:Employee ID} paid {report:debitedAmount} / received {report:creditedAmount}, whereEmployee IDis another report field on the same workspace.Paymentfield shows no amounts and no raw{report:debitedAmount}or{report:creditedAmount}text.Paid {report:debitedAmount}, create a new report, and confirm the title contains no raw token text.Offline tests
QA Steps
Paymentwith the formula{field:Employee ID} paid {report:debitedAmount} / received {report:creditedAmount}, whereEmployee IDis another report field on the same workspace.Paymentfield shows no amounts and no raw{report:debitedAmount}or{report:creditedAmount}text.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari