Remove unreported from expense reports#60217
Conversation
|
@abdulrahuman5196 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
| }, | ||
| ]; | ||
|
|
||
| const expenseReportOptions: Array<{status: ExpenseSearchStatus; type: SearchDataTypes; icon: IconAsset; text: TranslationPaths}> = [ |
There was a problem hiding this comment.
I think this would be duplicate declaration to expenseOptions above this with many items in common between them? Could we kindly have the common things in one array have the expenseOptions and expenseReportOptions append the common array and the extra values if required?
There was a problem hiding this comment.
I'd prefer to keep them separate for separation of concerns since they are ultimately different data types and can have uniquer filters. You suggestion would introduce specific logic to determine which filter should be shown on top of the current factory/strategy pattern design we currently have, which keeps things separate and clean.
|
Bump @abdulrahuman5196 |
|
checking now |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeScreen.Recording.2025-04-23.at.7.03.51.PM.mp4iOS: NativeScreen.Recording.2025-04-23.at.6.59.21.PM.mp4iOS: mWeb SafariScreen.Recording.2025-04-23.at.7.00.19.PM.mp4MacOS: Chrome / SafariScreen.Recording.2025-04-23.at.6.44.54.PM.mp4MacOS: DesktopScreen.Recording.2025-04-23.at.6.47.28.PM.mp4 |
|
I am unable to check in android native due to android build issues at my end, but it should work fine since its not a platform specific change and all other platforms are tested. And the performance test failure shouldn't be related to this AFAIK. So I think we should be good. |
abdulrahuman5196
left a comment
There was a problem hiding this comment.
Changes looks good and works well. Reviewers checklist is also complete.
All yours. @JS00001
🎀 👀 🎀
C+ Reviewed
|
@MarioExpensify Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
JS00001
left a comment
There was a problem hiding this comment.
LGTM, waiting on gh actions
|
Perf test failing bc of conflicts |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.32-0 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.32-8 🚀
|
Explanation of Change
It doesn't make sense for the
Expense reportspage to show theUnreportedfilter. So this PR removes it from that page.Fixed Issues
$ #60341
Tests
Reports > Expense reportsUnreportedfilterOffline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Screen.Recording.2025-04-16.at.4.02.43.PM.mov
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop