Skip to content

Commit 829358c

Browse files
Refactor: simplified isResolvedActionableWhisper to remove dependency from ONYXKEYS.COLLECTION.REPORT_ACTIONS Onyx data
1 parent 1dd01da commit 829358c

3 files changed

Lines changed: 10 additions & 100 deletions

File tree

src/libs/ReportActionsUtils.ts

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,7 @@ const supportedActionTypes = new Set<ReportActionName>([...Object.values(otherAc
11331133
* Checks whether an action is actionable track expense and resolved.
11341134
*
11351135
*/
1136-
function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>, allActionsForReport?: OnyxEntry<ReportActions>): boolean {
1136+
function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>): boolean {
11371137
const originalMessage = getOriginalMessage(reportAction);
11381138
if (!originalMessage || typeof originalMessage !== 'object') {
11391139
return false;
@@ -1148,25 +1148,6 @@ function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>, allA
11481148
return false;
11491149
}
11501150

1151-
// For mention whispers, only treat as deleted if the parent comment is also deleted.
1152-
// This distinguishes cascade deletion (parent deleted -> whisper should hide) from
1153-
// the backend's one-per-user cleanup (parent still exists -> whisper should stay visible).
1154-
if (reportAction?.reportActionID && (isActionableMentionWhisper(reportAction) || isActionableReportMentionWhisper(reportAction))) {
1155-
const reportID = reportAction.reportID;
1156-
const actions = allActionsForReport ?? (reportID ? allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] : undefined);
1157-
if (actions) {
1158-
// Prefer the stored reportActionID from the whisper's originalMessage when available (set for
1159-
// whispers created during message edits, which don't follow the parentID+1 ID convention).
1160-
// Fall back to offset arithmetic for legacy whispers that predate this field.
1161-
const storedParentID = isActionableMentionWhisper(reportAction) ? (originalMessage as {parentReportActionID?: string}).parentReportActionID : undefined;
1162-
const parentActionID = storedParentID ?? String(BigInt(reportAction.reportActionID) - (isActionableReportMentionWhisper(reportAction) ? 2n : 1n));
1163-
const parentAction = actions[parentActionID];
1164-
if (parentAction && !isDeletedAction(parentAction)) {
1165-
return false;
1166-
}
1167-
}
1168-
}
1169-
11701151
return true;
11711152
}
11721153

@@ -1192,13 +1173,7 @@ function isResolvedConciergeDescriptionOptions(reportAction: OnyxEntry<ReportAct
11921173
* Checks if a reportAction is fit for display, meaning that it's not deprecated, is of a valid
11931174
* and supported type, it's not deleted and also not closed.
11941175
*/
1195-
function shouldReportActionBeVisible(
1196-
reportAction: OnyxEntry<ReportAction>,
1197-
key: string | number,
1198-
canUserPerformWriteAction?: boolean,
1199-
reportsParam?: OnyxCollection<Report>,
1200-
allActionsForReport?: OnyxEntry<ReportActions>,
1201-
): boolean {
1176+
function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key: string | number, canUserPerformWriteAction?: boolean, reportsParam?: OnyxCollection<Report>): boolean {
12021177
if (!reportAction) {
12031178
return false;
12041179
}
@@ -1278,7 +1253,7 @@ function shouldReportActionBeVisible(
12781253
}
12791254

12801255
// If action is actionable whisper and resolved by user, then we don't want to render anything
1281-
if (isActionableWhisper(reportAction) && isResolvedActionableWhisper(reportAction, allActionsForReport)) {
1256+
if (isActionableWhisper(reportAction) && isResolvedActionableWhisper(reportAction)) {
12821257
return false;
12831258
}
12841259

@@ -1322,27 +1297,22 @@ function isReportActionVisible(
13221297
return false;
13231298
}
13241299

1325-
// Look up all actions for this report so the parent-check in isResolvedActionableWhisper
1326-
// can determine whether a whisper's `deleted` flag reflects a real cascade deletion
1327-
// (parent comment deleted) vs. the backend's one-per-user cleanup (parent still exists).
1328-
const reportActionsForReport = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
1329-
13301300
// Actions with pendingAction are optimistic or in-flight, so their visibility may differ
13311301
// from what's cached in visibleReportActions (which reflects persisted Onyx data).
13321302
// We must recalculate visibility at runtime to ensure accuracy for these transient states.
13331303
if (reportAction.pendingAction) {
1334-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1304+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13351305
}
13361306

13371307
if (visibleReportActions && reportID) {
13381308
const reportCache = visibleReportActions[reportID];
13391309
if (!reportCache) {
1340-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1310+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13411311
}
13421312
const staticVisibility = reportCache[reportAction.reportActionID];
13431313
// If action is not in derived value cache, fall back to runtime calculation
13441314
if (staticVisibility === undefined) {
1345-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1315+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13461316
}
13471317
if (!staticVisibility) {
13481318
return false;
@@ -1352,7 +1322,7 @@ function isReportActionVisible(
13521322
}
13531323
return true;
13541324
}
1355-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1325+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13561326
}
13571327

13581328
/**

src/libs/actions/OnyxDerived/configs/visibleReportActions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export default createOnyxDerivedValueConfig({
7777
if (shouldSkipCachingAction(action)) {
7878
continue;
7979
}
80-
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports, reportActions);
80+
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports);
8181
}
8282
}
8383
}
@@ -114,7 +114,7 @@ export default createOnyxDerivedValueConfig({
114114
delete reportVisibility[action.reportActionID];
115115
continue;
116116
}
117-
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports, reportActions);
117+
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports);
118118
}
119119
}
120120
}
@@ -164,7 +164,7 @@ export default createOnyxDerivedValueConfig({
164164
continue;
165165
}
166166

167-
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports, reportActions);
167+
reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined, allReports);
168168
}
169169
}
170170

tests/unit/ReportActionsUtilsTest.ts

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import {
5050
isIOUActionMatchingTransactionList,
5151
isNewerReportAction,
5252
isReportActionVisibleAsLastAction,
53-
isResolvedActionableWhisper,
5453
shouldHideNewMarker,
5554
} from '../../src/libs/ReportActionsUtils';
5655
import {buildOptimisticCreatedReportForUnapprovedAction} from '../../src/libs/ReportUtils';
@@ -1046,65 +1045,6 @@ describe('ReportActionsUtils', () => {
10461045
// Then the whisper with deleted set should be filtered out, leaving only the ADD_COMMENT
10471046
expect(result).toStrictEqual([input.at(0)]);
10481047
});
1049-
1050-
it('should keep ACTIONABLE_MENTION_WHISPER visible when deleted is set but parent comment is not deleted', () => {
1051-
// Given a parent ADD_COMMENT (ID N) and an ACTIONABLE_MENTION_WHISPER (ID N+1) whose
1052-
// originalMessage.deleted is set (e.g. from the backend one-per-user cleanup rule).
1053-
// Use sequential IDs so the parent check can find the parent via whisperID - 1.
1054-
const parentID = '1000000000000000';
1055-
const whisperID = '1000000000000001';
1056-
1057-
const parentAction: ReportAction = {
1058-
created: '2024-11-19 08:04:13.728',
1059-
reportActionID: parentID,
1060-
reportID: '1',
1061-
actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
1062-
originalMessage: {
1063-
html: '<mention-user accountID="18414674"/>',
1064-
whisperedTo: [],
1065-
lastModified: '2024-11-19 08:04:13.728',
1066-
},
1067-
message: [
1068-
{
1069-
html: '<mention-user accountID="18414674"/>',
1070-
text: '@someone',
1071-
type: 'COMMENT',
1072-
whisperedTo: [],
1073-
},
1074-
],
1075-
};
1076-
1077-
const whisperAction: ReportAction = {
1078-
created: '2024-11-19 08:04:13.730',
1079-
reportActionID: whisperID,
1080-
reportID: '1',
1081-
actionName: CONST.REPORT.ACTIONS.TYPE.ACTIONABLE_MENTION_WHISPER,
1082-
originalMessage: {
1083-
inviteeAccountIDs: [18414674],
1084-
lastModified: '2024-11-19 08:04:25.813',
1085-
whisperedTo: [18301266],
1086-
deleted: '2024-11-19 08:04:27.000',
1087-
},
1088-
message: [
1089-
{
1090-
html: "Heads up, <mention-user accountID=18414674></mention-user> isn't a member of this room.",
1091-
text: "Heads up, isn't a member of this room.",
1092-
type: 'COMMENT',
1093-
},
1094-
],
1095-
};
1096-
1097-
const allActionsForReport: ReportActions = {
1098-
[parentID]: parentAction,
1099-
[whisperID]: whisperAction,
1100-
};
1101-
1102-
// When checking whether the whisper is resolved, providing the full action set
1103-
const result = isResolvedActionableWhisper(whisperAction, allActionsForReport);
1104-
1105-
// Then the whisper should NOT be treated as resolved because its parent is still present and not deleted
1106-
expect(result).toBe(false);
1107-
});
11081048
});
11091049

11101050
describe('hasRequestFromCurrentAccount', () => {

0 commit comments

Comments
 (0)