-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix report field is enabled when there are no outstanding reports #79415
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
Fix report field is enabled when there are no outstanding reports #79415
Conversation
|
@aimane-chnaif 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] |
|
Additional video: others.mp4 |
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.
|
|
Please confirm that this doesn't reintroduce the following issues: |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1.mp42.mp4@aimane-chnaif all fine |
|
One more to confirm no regression - #76205 |
|
Please fix conflict |
| // 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 = | ||
| (isFromGlobalCreate && !isPerDiemRequest ? shouldReportBeEditableFromFAB : 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.
Updated to handle #77963. It also fixes an issue where reports from all policy is shown in the edit report page.
|
Please merge main to fix lint |
|
There's conflict with #79591. Update accordingly. |
…chat-doesnt-have-open-report
|
@aimane-chnaif done |
Reviewer Checklist
Screenshots/VideosAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb Safari |
|
Is this bug or expected? (happening on production) Screen.Recording.2026-01-18.at.11.14.40.am.movReport is not selected as default when there is outstanding report in selected workspace |
| transactionIDs={transaction ? [transaction.transactionID] : []} | ||
| selectedReportID={selectedReportID} | ||
| selectedPolicyID={!isEditing && !isFromGlobalCreate ? reportOrDraftReport?.policyID : undefined} | ||
| selectedPolicyID={(!isEditing && !isFromGlobalCreate) || isPerDiemTransaction ? reportOrDraftReport?.policyID : undefined} |
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.
I am not sure if we should fix the bug which is out of scope. (happening on production)
This still doesn't fix the issue when create per diem from FAB.
reportOrDraftReport is undefined. Should we use perDiemOriginalPolicy?.id instead?
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.
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.
I am not sure if we should fix the bug which is out of scope
I'm up to you
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.
I am still seeing other workspace. Let's use perDiemOriginalPolicy?.id if you agree
Screen.Recording.2026-01-18.at.11.50.46.am.mov
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.
Oh, I forget to turn off the Submission frequency.
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.
Let's use perDiemOriginalPolicy?.id
Hmm, it's undefined for non perdiem request. I think let's handle this on a separate issue so we can test this more and not increase the scope of this PR.
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.
I meant to update like this:
isPerDiemTransaction ? perDiemOriginalPolicy.id : !isEditing && !isFromGlobalCreate ? reportOrDraftReport?.policyID : undefined
Either revert or update
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.
Oh, make sense. That works. Updated!
I think it's expected if Submission frequency is turned off |
|
|
||
| return outstandingReports.filter((report) => { | ||
| const reportNameValuePair = reportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID}`]; | ||
| return !isArchivedReport(reportNameValuePair) && isReportOutstanding(report, report?.policyID, reportNameValuePairs, false); |
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.
Does new hook also count for archived report?
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.
ok, isReportOutstanding has that check.
|
|
||
| const shouldReportBeEditableFromFAB = isUnreported ? allOutstandingReports.length >= 1 : allOutstandingReports.length > 1; | ||
|
|
||
| const outstandingReports = useOutstandingReports(undefined, isFromGlobalCreate && !isPerDiemRequest ? undefined : policyID, ownerAccountID, false); |
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.
Is it correct that last param (isEditing) is always false?
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.
Do you have a repro steps? |
I am not able to reproduce anymore after relogin. Not blocker. |
|
Let's just address #79415 (comment) |
Super helpful, thanks! |
JmillsExpensify
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.
Product changes look good. Approving!
|
@bernhardoj conflicts, otherwise LGTM |
…chat-doesnt-have-open-report
7c58e13
|
@grgia fixed |
|
✋ 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/grgia in version: 9.3.7-0 🚀
|
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.7-3 🚀
|






Explanation of Change
Fixed Issues
$ #78788
PROPOSAL: #78788 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Precondition:
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
android.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios.mweg.mp4
MacOS: Chrome / Safari
web.mp4