LHN badge priority: use oldest report action between Task and IOU#91676
Conversation
… and IOU When a chat has both an outstanding task and an IOU action (Submit/Approve/Pay), the badge shown in the LHN should correspond to the oldest report action rather than always showing the Task badge. Previously the task check ran first and returned early, preventing the IOU badge from being evaluated. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
The only failing check is the PR Author Checklist, which is a manual checklist — not a code issue. All other checks are either passing or still pending (tests, typecheck, perf tests, Storybook). There are no code-related check failures to fix. The pending checks should resolve on their own shortly. If any of the currently pending checks fail after they complete, feel free to tag me again and I'll take a look. |
|
@MelvinBot add test steps and check off all items. They're verified. |
|
Done — added test steps (Tests, Offline tests, QA Steps) and checked off all PR Author Checklist items. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ 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". |
| .at(0); | ||
|
|
||
| // If there's a valid IOU action that is older than the task, use the IOU badge instead | ||
| if (hasValidIOUAction && iouReportActionToApproveOrPay?.created && oldestTaskAction?.created && iouReportActionToApproveOrPay.created < oldestTaskAction.created) { |
There was a problem hiding this comment.
There's backend bug.
When I first login or clear cache, childManagerAccountID doesn't exist in task report action before opening report.
So oldestTaskAction is undefined as action.childManagerAccountID === deprecatedCurrentUserAccountID condition isn't met.
Before opening report:
After opening report:
bug.mov
| if (hasValidIOUAction && iouReportActionToApproveOrPay?.created && oldestTaskAction?.created && iouReportActionToApproveOrPay.created < oldestTaskAction.created) { | |
| if (hasValidIOUAction && iouReportActionToApproveOrPay?.created && (!oldestTaskAction || iouReportActionToApproveOrPay.created < oldestTaskAction.created)) { |
Also add unit test for this case.
There was a problem hiding this comment.
Done. Updated the condition to (!oldestTaskAction || iouReportActionToApproveOrPay.created < oldestTaskAction.created) and added a unit test covering the case where childManagerAccountID is missing (so oldestTaskAction is undefined).
…estTaskAction is undefined When childManagerAccountID is not populated (backend bug before opening report), oldestTaskAction is undefined. Update the comparison to fall back to the IOU badge in this case, and add a unit test covering the scenario. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
@MelvinBot mark as ready for review |
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.
|
|
Done — PR has been marked as ready for review. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
@MelvinBot Please unassign me because I am not a reviewer for this PR. |
|
Done! I've removed |
|
@danieldoglas looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
🚧 @danieldoglas has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Explanation of Change
When a workspace chat has both an outstanding task and an IOU action (Submit/Approve/Pay), the LHN badge should display the one corresponding to the oldest report action, since that represents the first required action. Previously, the task badge check in
getReasonAndReportActionThatRequiresAttentionran before the IOU badge check and returned early, so the Task badge always won regardless of which action was created first.This PR restructures the logic to:
createdtimestamp against the IOU candidateThe expense assignee path (processing expense reports) is unchanged.
Fixed Issues
$ #91664
Tests
Offline tests
QA Steps
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))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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
AI Tests
getReasonAndReportActionThatRequiresAttention(13 tests)LHNItemsPresence(21 tests)typecheck-tsgoprettier