fixed custom report deploy blockers#86824
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
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 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 |
How, if that's true?
Do you need to sign-in afresh in a different device/browser or does just operating another tab reveal the bug too? |
Is openapp returning incorrect data? |
The affected columns are hidden on staging unless enabled this way.
It needs a fresh sign-in.
I think it's |
|
@QichenZhu is this because we don't send the convertedAmount? |
@JS00001 yes, it misses |
|
@mukhrr is that true? |
|
Ooh I see where its getting overridden |
|
One sec, will work on a BE change |
|
@QichenZhu meanwhile can we proceed with FE changes? |
|
@QichenZhu @mukhrr the BE PR is deployed, can you recheck the issue please? |
|
Bug: exchange rate and total columns disappear for certain amounts Steps
Expected resultExchange rate and total columns show for all NZD amounts regardless of value. Actual resultExchange rate and total columns disappear for NZ$16.95 and NZ$16.96, but display correctly for other values. 86824-bug.mov |
|
@mukhrr I think thats a FE bug ^ |
|
Yeah that's a FE bug. I'm sure it will be fixed quickly, but we seem to be in a cycle of fixing known bugs while new ones surface. Can we put the feature behind a beta and find more resources to help stabilize it? |
@QichenZhu this is now fixed and I agree with your idea. |
|
🚧 @trjExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
I dont think we should put this behind a beta. However, if you'd like, we could ask QA to run through the test steps and see any issues they may find before we merge? |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
That would be great! Before that, let's figure out why we are in this cycle. For example, the most recent bug came from changes made last week to fix another bug, which feels like a result of AI misuse. If so, it might be time to switch models. |
|
@trjExpensify yeah it makes sense. Now it should be okay cc @QichenZhu |
| <AmountCell | ||
| total={Math.abs(convertedAmount)} |
There was a problem hiding this comment.
But do we really need to show positive since it was created as negative originally? cc @trjExpensify
There was a problem hiding this comment.
Classic shows both fields as negative, so we should make Total negative, not the other way around.
There was a problem hiding this comment.
No, I mean negative. The parentheses represent a negative value.
There was a problem hiding this comment.
okay 👍. Let's also have Tom confirm it
There was a problem hiding this comment.
parenthesis does represent a negative value in Classic. 👍 In NewDot we display a negative expense with a minus sign.
| case CONST.SEARCH.TABLE_COLUMNS.TOTAL: { | ||
| const convertedAmount = transactionItem.convertedAmount ?? 0; |
There was a problem hiding this comment.
Bug: expenses created offline show zero as Total.
@trjExpensify should we show Total as blank or the unconverted amount for an offline expense?










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