Action badge follow-ups: deep link to reportAction and remove Pinned text#85442
Action badge follow-ups: deep link to reportAction and remove Pinned text#85442
Conversation
Deep link to the specific reportAction when pressing an LHN row with GBR/RBR action badge, instead of scrolling to the bottom. Remove the "Pinned" text label from the pin badge, showing only the pin icon. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
| if (SidebarUtils.shouldShowRedBrickRoad(report, chatReport, reportActionsList, hasAnyViolations, reportErrors, transactions, transactionViolations, !!isReportArchived)) { | ||
| brickRoadStatus = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; | ||
| actionBadge = CONST.REPORT.ACTION_BADGE.FIX; | ||
| actionBadgeReportActionID = greenBadgeReportActionID; |
There was a problem hiding this comment.
@MelvinBot this is completely wrong logic.
Your PR is basically same as #85192 I already created.
The solution I'm looking for is to get correct reportActionID for various types of RBRs.
i.e. RBR when violation exists for expense (shouldDisplayViolationsRBRInLHN = true): how to get reportActionID for report preview with violation?
Can you help with that?
There was a problem hiding this comment.
You're right — the logic here is wrong. Using greenBadgeReportActionID for the RBR case is incorrect because the GBR reportAction (e.g. an approve/pay action) is unrelated to the violation that caused the RBR.
To answer your question about getting the correct reportActionID when shouldDisplayViolationsRBRInLHN is true:
The key insight is that shouldDisplayViolationsRBRInLHN scans all expense reports under the same policyID (via reportsByPolicyID) and finds a potentialReport with violations — but it only returns a boolean, discarding which report matched.
To get the REPORT_PREVIEW action:
-
Refactor
shouldDisplayViolationsRBRInLHN(or create a sibling function) to return the violatingpotentialReport.reportIDinstead of justtrue. -
Then in
getReasonAndReportActionThatHasRedBrickRoad(which already receives the expense chat'sreportActions), find theREPORT_PREVIEWaction whoseoriginalMessage.linkedReportIDmatches that violating report:
const violatingReportID = getViolatingReportIDForLHN(report, transactionViolations);
if (violatingReportID) {
const reportPreviewAction = Object.values(reportActions ?? {}).find(
(action) => isReportPreviewAction(action)
&& getOriginalMessage(action)?.linkedReportID === violatingReportID,
);
return {
reason: CONST.RBR_REASONS.HAS_TRANSACTION_THREAD_VIOLATIONS,
reportAction: reportPreviewAction,
};
}The REPORT_PREVIEW → child report link is via originalMessage.linkedReportID (defined in OriginalMessageReportPreview at src/types/onyx/OriginalMessage.ts:333). There's also an existing pattern for this in getReportPreviewAction at src/libs/actions/IOU/index.ts:1205-1215.
The same approach applies to the HAS_VIOLATIONS reason — find the action whose linked child report has violations.
I'll close this PR since your PR #85192 already takes the correct approach of calling getReasonAndReportActionThatHasRedBrickRoad for the RBR case.
There was a problem hiding this comment.
Implemented the suggestion in 30c7ca1. Here's what changed:
-
ReportUtils.ts— AddedgetViolatingReportIDForLHN, a companion toshouldDisplayViolationsRBRInLHNthat returns the violating child report'sreportID(uses.find()instead of.some()). -
SidebarUtils.ts— ModifiedgetReasonAndReportActionThatHasRedBrickRoadto callgetViolatingReportIDForLHNfor theHAS_TRANSACTION_THREAD_VIOLATIONScase, then finds theREPORT_PREVIEWaction whoseoriginalMessage.linkedReportIDmatches the violating report. -
reportAttributes.ts— ReplacedshouldShowRedBrickRoad(boolean) withgetReasonAndReportActionThatHasRedBrickRoad(returns reason + reportAction), and usesreasonAndReportAction.reportAction?.reportActionIDinstead of the incorrectgreenBadgeReportActionID.
Note: For the HAS_VIOLATIONS and HAS_ERRORS cases, getReasonAndReportActionThatHasRedBrickRoad already returns the appropriate reportAction (from getAllReportActionsErrorsAndReportActionThatRequiresAttention for errors) or undefined when no specific action is identifiable.
Instead of reusing the GBR (green badge) reportActionID for the RBR case, call getReasonAndReportActionThatHasRedBrickRoad to get the actual reportAction that caused the red brick road indicator. For the HAS_TRANSACTION_THREAD_VIOLATIONS case, add getViolatingReportIDForLHN which returns the violating child report's ID, then find the matching REPORT_PREVIEW action via linkedReportID. Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
…tting Co-authored-by: Aimane Chnaif <aimane-chnaif@users.noreply.github.com>
|
Fixed the failing Prettier check: ran |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
The Prettier check failure on |
Explanation of Change
This PR addresses two follow-up fixes for the action badge feature (#84764, PR #85025):
LHN navigation deep links to reportAction — When pressing on a row in the LHN that has a GBR/RBR action badge, the navigation now deep links directly to the specific
reportActionthat requires attention (e.g. theREPORT_PREVIEWaction needing approval/payment/submission), rather than scrolling to the bottom of the chat. This is done by threading theactionBadgeReportActionIDthrough the derived report attributes →OptionData→SidebarLinks.showReportPage→Navigation.navigate.Removed "Pinned" text from badge — The Pinned badge in the LHN now shows only the pin icon without the "Pinned" text label. The non-production path previously rendered a
Badgecomponent with both icon and text; it now renders just the icon, matching the production behavior.Fixed Issues
$ #85436
PROPOSAL:
Tests
Offline tests
Same as Tests — navigation and badge rendering are client-side only.
QA Steps
Same as Tests
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