Skip to content

Commit b4d3bd7

Browse files
tgolentgolen
authored
Merge pull request #89450 from shubham1206agra/refactor-isResolvedActionableWhisper
Refactor: simplified isResolvedActionableWhisper to remove dependency from ONYXKEYS.COLLECTION.REPORT_ACTIONS Onyx data
2 parents 02461ab + 05d7cc3 commit b4d3bd7

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
@@ -1130,7 +1130,7 @@ const supportedActionTypes = new Set<ReportActionName>([...Object.values(otherAc
11301130
* Checks whether an action is actionable track expense and resolved.
11311131
*
11321132
*/
1133-
function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>, allActionsForReport?: OnyxEntry<ReportActions>): boolean {
1133+
function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>): boolean {
11341134
const originalMessage = getOriginalMessage(reportAction);
11351135
if (!originalMessage || typeof originalMessage !== 'object') {
11361136
return false;
@@ -1145,25 +1145,6 @@ function isResolvedActionableWhisper(reportAction: OnyxEntry<ReportAction>, allA
11451145
return false;
11461146
}
11471147

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

@@ -1189,13 +1170,7 @@ function isResolvedConciergeDescriptionOptions(reportAction: OnyxEntry<ReportAct
11891170
* Checks if a reportAction is fit for display, meaning that it's not deprecated, is of a valid
11901171
* and supported type, it's not deleted and also not closed.
11911172
*/
1192-
function shouldReportActionBeVisible(
1193-
reportAction: OnyxEntry<ReportAction>,
1194-
key: string | number,
1195-
canUserPerformWriteAction?: boolean,
1196-
reportsParam?: OnyxCollection<Report>,
1197-
allActionsForReport?: OnyxEntry<ReportActions>,
1198-
): boolean {
1173+
function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key: string | number, canUserPerformWriteAction?: boolean, reportsParam?: OnyxCollection<Report>): boolean {
11991174
if (!reportAction) {
12001175
return false;
12011176
}
@@ -1275,7 +1250,7 @@ function shouldReportActionBeVisible(
12751250
}
12761251

12771252
// If action is actionable whisper and resolved by user, then we don't want to render anything
1278-
if (isActionableWhisper(reportAction) && isResolvedActionableWhisper(reportAction, allActionsForReport)) {
1253+
if (isActionableWhisper(reportAction) && isResolvedActionableWhisper(reportAction)) {
12791254
return false;
12801255
}
12811256

@@ -1319,27 +1294,22 @@ function isReportActionVisible(
13191294
return false;
13201295
}
13211296

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

13341304
if (visibleReportActions && reportID) {
13351305
const reportCache = visibleReportActions[reportID];
13361306
if (!reportCache) {
1337-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1307+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13381308
}
13391309
const staticVisibility = reportCache[reportAction.reportActionID];
13401310
// If action is not in derived value cache, fall back to runtime calculation
13411311
if (staticVisibility === undefined) {
1342-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1312+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13431313
}
13441314
if (!staticVisibility) {
13451315
return false;
@@ -1349,7 +1319,7 @@ function isReportActionVisible(
13491319
}
13501320
return true;
13511321
}
1352-
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction, undefined, reportActionsForReport);
1322+
return shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canUserPerformWriteAction);
13531323
}
13541324

13551325
/**

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

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

0 commit comments

Comments
 (0)