fixed custom report deploy blockers#86824
Conversation
melvin-bot
Bot
requested review from
Pujan92 and
trjExpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24e66578cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… transactions have comments
… ensure effective columns are computed with final data
|
@QichenZhu pls, let's try to review and merge this as soon as possible before reverting this big PR #83981 |
…luding COMMENTS in visibleColumns
|
@trjExpensify Plz assign @QichenZhu if they are going to review this PR |
|
@mukhrr can you work on the other two blockers here as well? |
|
Hmm, @mukhrr this PR fixes two issues, and is pretty large, so I don't think that we should CP this one. Do you have a way to easily disable the feature that we can CP? |
I think the rest two are expected. pls see #86784 (comment) and #86807 (comment) |
I think we can remove Columns button for now as we did last time. |
|
@mukhrr great, could you create a PR for that please? |
|
Then we can merge this fix to main right after |
|
@mukhrr lets also fix this here (cc @trjExpensify ) We should be using: |
@JS00001 this is not the issue as deploy blocker, isn't it? |
|
I dont think so, no, its an issue tom just realized |
|
@QichenZhu kindly bump. Let's make this move faster, pls )) |
|
@QichenZhu what's your ETA for the re-review? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp (N/A)Android: mWeb Chrome (N/A)iOS: HybridApp (N/A)iOS: mWeb Safari (N/A)MacOS: Chrome / SafariScreen.Recording.2026-04-28.at.10.44.49.AM.mov
Screen.Recording.2026-04-28.at.10.53.27.AM.mov
|
QichenZhu
left a comment
QichenZhu
left a comment
There was a problem hiding this comment.
Worked well in my testing. Would be good to get QA on the PR branch.
|
will ask qa to review an adhoc |
|
🚧 @JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel! Reports- Missing “Original amount” and “Exchange Rate” columns in multi-currency reportVersion Number: 9.3.62-3 PR:86824 Action Performed:
Expected Result:When expenses are created in different currencies, the report should display the “Original amount” and “Exchange Rate” columns, allowing users to see both the original currency values and their converted amounts Actual Result:The report only displays amounts in CLP and does not show the “Original amount” and “Exchange Rate” columns, even when expenses are created in multiple currencies (EUR and USD). This prevents users from viewing original transaction values and currency conversion details. When expenses are created in different currencies, the report should display the “Original amount” and “Exchange Rate” columns, allowing users to see both the original currency values and their converted amounts. Workaround:Unknown Platforms:Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos671.mp4 |
|
Mhm, what am I missing from watching that video? All of these are there, as they should be:
If anything, putting exchange rate between amount and total could be a shout, but people can re-order and save their column preference. Was that the only one, @m-natarajan? |
|
@trjExpensify yes that was the only one |
|
Niceee! Shall we look to final approve and merge then? 👍 |
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
PR to introduce a HIGH customer request in #migrate. 👍
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.65-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR against the help site articles under No help site changes are required. This PR fixes deploy blockers for the custom report columns feature — all changes are bug fixes to existing behavior:
The relevant help article (Using-Reports-in-New-Expensify.md) already covers column customization via the Columns icon, including show/hide, drag-and-drop reordering, and saving layouts. The user-facing column labels (Total, Exchange rate, Tax rate, etc.) remain unchanged. No new features, workflows, or terminology were introduced that would require documentation updates. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.65-6 🚀
|
|
Deploy Blocker #89490 was identified to be related to this PR. |
|
|
||
| if (transaction.taxCode) { | ||
| // If the transaction has any tax info (code, value, or amount), | ||
| // show both TAX_RATE and TAX_AMOUNT columns. Zero is valid tax data. | ||
| const hasTaxInfo = !!transaction.taxCode || transaction.taxAmount != null || (transaction.taxValue !== undefined && transaction.taxValue !== ''); | ||
| if (hasTaxInfo) { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.TAX_RATE] = true; | ||
| } | ||
|
|
||
| if (transaction.taxAmount) { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.TAX_AMOUNT] = true; | ||
| } |
There was a problem hiding this comment.
This change caused regression - Tax columns shown when split expense
Please fix as follow-up.
| [CONST.SEARCH.TABLE_COLUMNS.BILLABLE]: shouldShowBillableColumn, | ||
| [CONST.SEARCH.TABLE_COLUMNS.COMMENTS]: true, | ||
| [CONST.SEARCH.TABLE_COLUMNS.TOTAL_AMOUNT]: true, | ||
| [CONST.SEARCH.TABLE_COLUMNS.TOTAL_AMOUNT]: false, |
There was a problem hiding this comment.
Bug: Setting the amount column to false force hides it on the expense report page, even when it is selected in the column configuration.
Steps:
- Log in with a new account.
- Create a new workspace.
- Create 2 expenses.
- Open the expense report page.
- Configure the columns to display
Amount.
Current: The Amount column is not displayed.
Expected: The user should be able to see the Amount column since it is selected in the column configuration.
Screen.Recording.2026-05-05.at.10.41.59.PM.mov
There was a problem hiding this comment.
@ahmedGaber93 this seems expected and aligns with OldDot.
2026-04-15_12-51-15.mp4
There was a problem hiding this comment.
Ah, got it. Now we need to have an expense with foreign currency to display the amount column on NewDot.
We need the Amount column to always be visible so that users can edit it inline.
Will discuss this on our PR, thanks!
11 participants
Explanation of Change
Fixed Issues
$ #86786
#86820
#86784
#86807
PROPOSAL:
Tests
#86786
#86820
Verify there is no space between Amount and chevron right icon in the end of row
Verify that no errors appear in the JS console
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
REC-20260303100105.mp4
Android: mWeb Chrome
REC-20260303100105.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_mWeb.mp4
MacOS: Chrome / Safari
web.mp4