-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fixed custom report deploy blockers #86824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
JS00001
merged 29 commits into
Expensify:main
from
mukhrr:fix/custom-report-columns-bugs
Apr 30, 2026
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
24e6657
Compute effective columns for report details when no custom columns a…
mukhrr 3e39290
Conditionally show comments column in report details based on whether…
mukhrr 25a25fd
Wait for transactions to load before rendering ColumnsSettingsList to…
mukhrr 4626386
Update tests to use shouldShowCommentsColumn parameter instead of inc…
mukhrr fb8e52a
Merge remote-tracking branch 'origin' into fix/custom-report-columns-…
mukhrr e58c116
Revert "Remove unused Columns button and related imports from MoneyRe…
mukhrr a75dbc8
Pass report currency to resolveTransactionCardFields for proper amoun…
mukhrr 8df328e
Use modifiedAmount/modifiedCurrency when determining if currency conv…
mukhrr e5b7d57
Use groupAmount variable and check for non-zero value instead of trut…
mukhrr 1314b3c
Store current display amount/currency in originalAmount/originalCurre…
mukhrr b52ecb8
Remove report currency conversion logic from resolveTransactionCardFi…
mukhrr 529fba0
Split currency conversion logic in TotalCell to handle convertedAmoun…
mukhrr ff8c002
Consolidate tax column visibility logic to show both TAX_RATE and TAX…
mukhrr 07d9e79
Compute effective columns from getColumnsToShow when no custom column…
mukhrr 1267c72
Add TOTAL column to transaction views and update related logic
mukhrr 79db6c6
Enhance TransactionUtils tests for exchange rate handling
mukhrr f78015c
Merge remote-tracking branch 'origin' into fix/custom-report-columns-…
mukhrr 1146baf
Merge remote-tracking branch 'origin' into fix/custom-report-columns-…
mukhrr 7429e38
Merge remote-tracking branch 'origin' into fix/custom-report-columns-…
mukhrr dd2a3f5
fixed exchange rate column when default currency is different
mukhrr e47b148
remove duplicate reportCurrency
mukhrr 4ec4cd7
reverted original amount to search page
mukhrr 9e62df3
Merge remote-tracking branch 'origin' into fix/custom-report-columns-…
mukhrr 6736f01
removed convertedAmount rounds to amount coincidence bug
mukhrr 1829021
made total visible by default
mukhrr dcf8ca5
Merge branch 'main' into fix/custom-report-columns-bugs
mukhrr b5d7a95
fixed negative values as Total
mukhrr 4c09ab8
show total as unconverted amount on offline
mukhrr 017a242
Merge remote-tracking branch 'origin/main' into fix/custom-report-col…
mukhrr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5035,6 +5035,7 @@ function getColumnsToShow({ | |
| isExpenseReportViewFromIOUReport = false, | ||
| shouldShowBillableColumn = false, | ||
| shouldShowReimbursableColumn = false, | ||
| shouldShowCommentsColumn = false, | ||
| reportCurrency, | ||
| shouldUseStrictDefaultExpenseColumns = false, | ||
| }: { | ||
|
|
@@ -5047,8 +5048,10 @@ function getColumnsToShow({ | |
| isExpenseReportViewFromIOUReport?: boolean; | ||
| shouldShowBillableColumn?: boolean; | ||
| shouldShowReimbursableColumn?: boolean; | ||
| shouldUseStrictDefaultExpenseColumns?: boolean; | ||
| shouldShowCommentsColumn?: boolean; | ||
| reportCurrency?: string; | ||
| shouldUseStrictDefaultExpenseColumns?: boolean; | ||
| policy?: OnyxTypes.Policy; | ||
| }): SearchColumnType[] { | ||
| if (type === CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT) { | ||
| const defaultReportColumns: SearchColumnType[] = [ | ||
|
|
@@ -5162,11 +5165,11 @@ function getColumnsToShow({ | |
| [CONST.SEARCH.TABLE_COLUMNS.TAX_RATE]: false, | ||
| [CONST.SEARCH.TABLE_COLUMNS.TAX_AMOUNT]: false, | ||
| [CONST.SEARCH.TABLE_COLUMNS.EXCHANGE_RATE]: false, | ||
| [CONST.SEARCH.TABLE_COLUMNS.ORIGINAL_AMOUNT]: false, | ||
| [CONST.SEARCH.TABLE_COLUMNS.REIMBURSABLE]: shouldShowReimbursableColumn, | ||
| [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, | ||
| [CONST.SEARCH.TABLE_COLUMNS.TOTAL]: true, | ||
| [CONST.SEARCH.TABLE_COLUMNS.COMMENTS]: shouldShowCommentsColumn, | ||
| } | ||
| : { | ||
| [CONST.SEARCH.TABLE_COLUMNS.AVATAR]: true, | ||
|
|
@@ -5227,7 +5230,7 @@ function getColumnsToShow({ | |
| } | ||
| } | ||
|
|
||
| if (!addedColumns.has(CONST.SEARCH.TABLE_COLUMNS.COMMENTS)) { | ||
| if (shouldShowCommentsColumn && !addedColumns.has(CONST.SEARCH.TABLE_COLUMNS.COMMENTS)) { | ||
| result.push(CONST.SEARCH.TABLE_COLUMNS.COMMENTS); | ||
| } | ||
|
|
||
|
|
@@ -5290,19 +5293,28 @@ function getColumnsToShow({ | |
| columns[CONST.SEARCH.TABLE_COLUMNS.CARD] = true; | ||
| } | ||
|
|
||
| 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; | ||
| } | ||
|
Comment on lines
5295
to
5302
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change caused regression - Tax columns shown when split expense Please fix as follow-up. |
||
|
|
||
| const hasExchangeRate = getExchangeRate(transaction, reportCurrency) !== ''; | ||
| if (hasExchangeRate || transaction.originalAmount) { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.ORIGINAL_AMOUNT] = true; | ||
| if (hasExchangeRate) { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.EXCHANGE_RATE] = true; | ||
| } | ||
| // Expense report view: TOTAL (workspace currency) is always shown; add AMOUNT | ||
| // (transaction's own currency) when a conversion exists so both are visible. | ||
| // Search page: show ORIGINAL_AMOUNT column (transaction's original amount). | ||
| if (hasExchangeRate) { | ||
| if (isExpenseReportView) { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.TOTAL_AMOUNT] = true; | ||
| } else { | ||
| columns[CONST.SEARCH.TABLE_COLUMNS.ORIGINAL_AMOUNT] = true; | ||
| } | ||
| } | ||
|
|
||
| if (!Array.isArray(data)) { | ||
| const report = data[`${ONYXKEYS.COLLECTION.REPORT}${transaction.reportID}`]; | ||
|
|
@@ -5385,8 +5397,11 @@ function getColumnsToShow({ | |
| CONST.SEARCH.TABLE_COLUMNS.TYPE, | ||
| CONST.SEARCH.TABLE_COLUMNS.DATE, | ||
| CONST.SEARCH.TABLE_COLUMNS.STATUS, | ||
| // TOTAL_AMOUNT (Amount) is data-driven in expense report view: shown only when a | ||
| // conversion exists. In search view, TOTAL_AMOUNT is always-true via the default | ||
| // columns map, so we don't need to list it here as non-data for either surface. | ||
| CONST.SEARCH.TABLE_COLUMNS.TOTAL, | ||
| CONST.SEARCH.TABLE_COLUMNS.COMMENTS, | ||
| CONST.SEARCH.TABLE_COLUMNS.TOTAL_AMOUNT, | ||
| CONST.SEARCH.TABLE_COLUMNS.REIMBURSABLE, | ||
| CONST.SEARCH.TABLE_COLUMNS.BILLABLE, | ||
| CONST.SEARCH.TABLE_COLUMNS.BASE_62_REPORT_ID, | ||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Setting the amount column to
falseforce hides it on the expense report page, even when it is selected in the column configuration.Steps:
Amount.Current: The
Amountcolumn is not displayed.Expected: The user should be able to see the
Amountcolumn 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahmedGaber93 this seems expected and aligns with OldDot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!