Conversation
|
@inimaga I'd really appreciate if we get this merged ASAP, pls! Hopefully, before this PR gets reverted. cc @trjExpensify |
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.
|
…r for tax rate sorting
…MoneyRequestReportTableHeader
… COMMENTS column test descriptions
…NT, and TAX_AMOUNT column sorting
|
Do we need @QichenZhu to review this? |
I think so. |
|
Okay, let's jump on that ASAP to fix these blockers. |
|
@JS00001 I'm going to put you on this for internal as it's from the columns PR. 👍 |
As far as I know, it needs backend changes. |
Hmm, after checking several times, you seem to be right. My first conclusion seems correct here #83959 (comment) (strikethroughed one) |
|
@trjExpensify @JS00001, can we hide the Exchange rate column for now since the backend doesn't support it yet? |
|
@JS00001 - don't we have
|
luacmartins
left a comment
There was a problem hiding this comment.
Nice! I mostly reviewed to check that there wasn't any performance regression to navigating to Search and I can confirm this didn't regress performance
@QichenZhu thanks for review and feedbacks! I think I should fix this. |
|
@mukhrr we have a few failing checks |
|
Yes I am working on this yet |
… columns on default search page
|
@QichenZhu mind checking one more time, pls?! I also fixed hiding empty columns based on #83974 (comment) |
…s in custom reports based on transaction data
They fix this @QichenZhu About Card column, yeah I couln't reproduce it |
|
@trjExpensify @JS00001, is it possible to set up an account for @mukhrr to test company card expenses? |
|
On staging and dev do you not all see the "Mock bank" option to connect to? |
|
Yes but how to create expenses with card? |
|
Via Plaid Go to Workspaces and create or open a USD workspace. Tap Company cards. Tap Add cards. Proceed with United States. Proceed with Direct feed. Select Other and then Next. This opens the Plaid UI. Tap Continue as guest. Select any bank. I typically do US Bank. Enter the Plaid test credentials: username: user_good password: pass_good If prompted for a security code, enter: credential_good. Select the Plaid credit card ending x3333. This card has test transactions you can use to test/demo features on the reports page that require transactions. If you need a 2nd card, the checking account ending in x0000 has transactions as well. Follow the remaining steps to connect the card(s) in Plaid. Select a statement close date. Tap Assign card and follow the steps to assign the card(s). Select From the beginning to import historical transactions. You can now use this card and its transactions to test/demo third-party cards, including features like Statements on the Reports page. |
|
@mukhrr conflicts |
|
@QichenZhu it seems this bug pre-existing before the PR and I am curios which one is correct. Report layout or expenses page
|
|
@mukhrr, even if the bug existed before, this PR will unmask it, so we need to either resolve it ourselves or wait until it is fixed. |
… card names from cardList
@QichenZhu Yes, absolutely agree with that. I assume expenses page shows card column correclty and applied it for report layout too
|
There was a problem hiding this comment.
Good job! The card name issue is resolved.
However, the search page doesn't show deleted cards as deleted after applying this PR. Please see the comment below.
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
…ns in MoneyRequestReportTransactionList





Explanation of Change
Fixed Issues
$ #82252
$ #83962
$ #83963
$ #83959
$ #83985
$ #83988
$ #83974
$ https://github.com/Expensify/Expensify/issues/614169
PROPOSAL: #83959 (comment)
Tests
#83962
#83963
#83959
#83985
Go to one of Expenses layout
Comment -> Go Back to Report layout
Verify chat bubble exists as an comment indicator
Verify that no errors appear in the JS console
Offline tests
QA Steps
The same as tests
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