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 I wonder how you enabled Original amount and Exchange rate on Staging since Columns button is not visible there |
|
@mukhrr I enabled the columns in dev and the settings synced to staging. |
|
I think the difference in UZS/USD exchange rate between NewDot (0.0) and OldDot (0.0001) is just a precision issue of BE, but even if it is 0.0 we should display it. |
@QichenZhu I don't see Original amount enabled here. Does your screenshot tells about all 3 bugs you mentioned?
|
|
@mukhrr that's the very problem. The orignal amount doesn't show. Screen.Recording.2026-04-15.at.10.44.06.PM.mov |
@QichenZhu ah I see. Could you pls share the workspace with mshakhriyorov8+55gg@gmail.com? |
|
I was just talking to @JmillsExpensify about this, and I think we're getting tripped up with
So then it mirrors the below seen on Classic, and doesn't confuse the 
|
|
@mukhrr done! |
So what is expected behavior for Expenses page regarding these amount and original amount columns? |
|
You don't need to make any changes to the Expenses page here, leave that be. |
|
@mukhrr these changes only need to happen on the reports page. The expenses page can stay the same |
|
We don't intend to change the expenses page but we need to verify our changes don't affect it accidentally, so it's better to understand the expected behavior there as well. |
@trjExpensify |
- Introduced a new TOTAL column in various components including SearchTableHeader and TransactionItemRow. - Updated logic to display the TOTAL column based on transaction data, ensuring it reflects converted amounts. - Removed references to ORIGINAL_AMOUNT in favor of the new TOTAL column where applicable. - Adjusted tests to validate visibility and data handling for the TOTAL column in expense reports.
|
@QichenZhu I pushed changes based on latest discussions here. And ensured everything matches with OD before push. |
Haha yeah, other way around like Classic.. you got it! 2026-04-15_12-51-15.mp4 |
| * Returns the exchange rate for a transaction, based on its group or currencyConversionRate | ||
| */ | ||
| function getExchangeRate(transaction: TransactionWithOptionalSearchFields, reportCurrency?: string) { |
There was a problem hiding this comment.
@mukhrr report view displays exchange rate for unconverted expenses
Steps
- Create a workspace with a currency different from your default workspace currency
- Create 2 expenses in the new workspace's currency
- Clear the cache
- Open the report and enable the Amount, Total, and Exchange Rate columns
Expected result
No rate shown for unconverted expenses (Expensify Classic behaviour)
Actual result
Exchange rate column shows the new workspace currency / default workspace currency rate
Screen.Recording.2026-04-16.at.11.50.52.PM.mov
There was a problem hiding this comment.
…bugs # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx # src/libs/SearchUIUtils.ts
|
@QichenZhu ready for another look. |
|
@mukhrr thanks for the update! I think this PR still needs some work before it's ready for review:
Thanks for your patience! Another day often goes by before I get back to you, hopefully this doesn't slow you down. I'd really appreciate it if you can catch such issues earlier on your side. |
…bugs # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
@QichenZhu I was not able to find Spend page. But yeah, I found I missed Original amount and Exchange Rate in Expenses Search page. So I fixed it and assume it's fixed on Spend page also. Could you pls check it once more?! Thank your for your patience too! |
|
Found a weird bug, not sure if it's frontend or backend. If you open a report and log into the same account on another device or incognito window, the report data are corrupted (see video 00:30-00:35). Staging has the same underlying issue but it's not visible to users. Screen.Recording.2026-04-20.at.2.54.14.PM.mov |
5 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