-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: selected report is not highlighted with a checkmark on confirmation page #67711
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
Conversation
|
@dominictb 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] |
|
@NJ-2020 We have unit tests failure that is related to this PR. Can you check? |
|
@dominictb Done, fixed |
I think these instances in test steps should be |
I don't understand this test case. Can you link a recording here? |
I think the last step should only be: Because there's no difference between self DM and workspace chat in this case. |
|
We also need a test case for editing report. |
|
Screen.Recording.2025-08-12.at.22.09.40.mov |
|
Screen.Recording.2025-08-12.at.22.53.51.mov |
|
Screen.Recording.2025-08-12.at.22.56.41.mov |
|
|
@dominictb I am able to reproduce this on main as well Screen.Recording.2025-08-14.at.08.06.32.mov |
|
|
Updated test cases |
@dominictb I mean this one: Screen.Recording.2025-08-14.at.09.15.03.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-09-10.at.17.55.48-compressed.movAndroid: mWeb ChromeScreen.Recording.2025-09-10.at.18.06.44-compressed.moviOS: HybridAppScreen.Recording.2025-09-10.at.17.24.10-compressed.moviOS: mWeb SafariScreen.Recording.2025-09-10.at.17.44.27-compressed.movMacOS: Chrome / SafariScreen.Recording.2025-09-09.at.15.55.02-compressed.movMacOS: Desktop |
Please update the step 4 of this test case to "Shows all open (draft or un-submitted) reports from that selected workspace" |
|
@dominictb I am not able to reproduce this
Could you please re-try again? Thanks |
Done, updated |
Verify that: Report field is editable + Shows all open (draft or un-submitted) reports + The latest open report from the default workspace is selected by default
Verify that: Report field is editable + Shows all open (draft or un-submitted) reports from the current workspace + The latest open report from the current workspace is selected by default
Verify that: Report field is editable + Shows all open, draft, submitted, or un-submitted reports from all workspaces + The current report is selected by default
Verify that: Report field is editable + Shows all open (draft or un-submitted) reports from that selected workspace + The latest open report from the default workspace is selected by default
|
|
After the above update, we're good to go. Thanks. |
|
For the "Global create, multiple workspaces | multiple reports" case:
Screen.Recording.2025-09-10.at.18.02.19.movI think for the second case, the report field should be editable since we're showing all reports from all workspaces, not just that single workspace. |
|
Cool, @NJ-2020 let's address the comments above. |
|
@dominictb Thank you |
Done, fixed ✅ |
|
Updated test cases Please re-review Thanks |
dominictb
left a comment
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.
🎀👀🎀 LGTM
luacmartins
left a comment
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.
LGTM
|
✋ 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.2.13-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.13-1 🚀
|
| // When creating an expense in an individual report, the report field becomes read-only | ||
| // since the destination is already determined and there's no need to show a selectable list. | ||
| const shouldReportBeEditable = !!firstOutstandingReport && !isMoneyRequestReport(reportID, allReports); | ||
| const shouldReportBeEditable = (isFromGlobalCreate ? allOutstandingReports.length > 1 : availableOutstandingReports.length > 1) && !isMoneyRequestReport(reportID, allReports); |
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.
@NJ-2020 is there a reason for using allOutstandingReports when isFromGlobalCreate = true?
I think we can always use availableOutstandingReports like this:
const shouldReportBeEditable = availableOutstandingReports.length > 1 && !isMoneyRequestReport(reportID, allReports);
I get all eligible reports properly when create from FAB.
Issue: #73609 (comment)
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.
@NJ-2020 bump above question. I see you're active on other PRs
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
@aimane-chnaif When creating an expense from the global action, we need to check all outstanding reports across workspaces so users can select a different report (from another workspace) if needed.
This was added here to fix this bug where workspaces with only one report, the report field aren't editable.
Thanks!







Explanation of Change
Fixed Issues
$ #62601
PROPOSAL: #62601 (comment)
Tests
Precondition: Created 1 workspace
+from global create action and then >Create ExpenseShow more+from global create action and then >Create ExpenseShow more+in the workspace composer chatCreate ExpenseShow more+in the workspace composer chatCreate ExpenseShow more+ > Create ExpenseShow moreCreate Expenseoption on the dropdown menuShow moreSend to someoneand select any workspaceSend to someoneand select any workspaceSend to someoneand select any workspaceOffline tests
Same as Tests
QA Steps
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))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
62601_native_android.mp4
Android: mWeb Chrome
62601_mweb_android.mp4
iOS: Native
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-09-06.at.16.01.06.1.1.mp4
iOS: mWeb Safari
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-09-06.at.13.17.01.1.1.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-09-06.at.13.09.16.1.mp4
MacOS: Desktop
Screen.Recording.2025-09-06.at.13.34.37.1.mp4