Skip to content

Pass hasReportActions to openReport (p4) #96864

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.

Sign up for GitHub

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

Merged
luacmartins merged 2 commits into from
Jul 28, 2026
Merged

Pass hasReportActions to openReport (p4) #96864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f88da6
pass hasReportActions
bernhardoj Jul 23, 2026
9a17e02
Merge branch 'main' into chore/73651-refactor-report-actions-to-use-u…
bernhardoj Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DeepLinkHandler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function DeepLinkHandler({onInitialUrl}: DeepLinkHandlerProps) {
return;
}
hasRefetchedPublicRoom.current = true;
Report.openReport({reportID, introSelected, betas});
Report.openReport({reportID, introSelected, betas, hasReportActions: false});
}, [isLoadingApp, allReports, introSelected, betas]);

return null;
Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/report/withReportAndReportActionOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function WithReportOrNotFoundImpl<TProps extends WithReportAndReportActionOrNotF

const parentReportAction = useParentReportAction(report);
let linkedReportAction: OnyxEntry<OnyxTypes.ReportAction> = reportActions?.[`${props.route.params.reportActionID}`];
const hasReportActions = !!reportActions;

// Handle threads if needed
if (!linkedReportAction?.reportActionID) {
Expand All @@ -72,7 +73,7 @@ function WithReportOrNotFoundImpl<TProps extends WithReportAndReportActionOrNotF
if (!shouldUseNarrowLayout || (!isEmptyObject(report) && !isEmptyObject(linkedReportAction))) {
return;
}
openReport({reportID: props.route.params.reportID, introSelected, betas});
openReport({reportID: props.route.params.reportID, introSelected, betas, hasReportActions});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [shouldUseNarrowLayout, props.route.params.reportID]);

Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/report/withReportOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export default function (shouldRequireReportID = true): <TProps extends WithRepo
const reportID = 'notificationReportID' in params ? params.notificationReportID : params.reportID;
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
const [hasReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: Boolean});
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`);
const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`);
const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`);
Expand All @@ -107,7 +108,7 @@ export default function (shouldRequireReportID = true): <TProps extends WithRepo
return;
}

openReport({reportID, introSelected, betas});
openReport({reportID, introSelected, betas, hasReportActions});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [shouldFetchReport, isReportLoaded, reportID]);

Expand Down
3 changes: 2 additions & 1 deletion src/pages/iou/request/step/withWritableReportOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ function WithWritableReportOrNotFoundImpl<TProps extends WithWritableReportOrNot
}: WithWritableReportOrNotFoundImplProps<TProps>) {
const {route} = props;
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`);
const [hasReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`, {selector: Boolean});
const [isLoadingApp = true] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const [reportDraft] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${route.params.reportID}`);
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
Expand All @@ -104,7 +105,7 @@ function WithWritableReportOrNotFoundImpl<TProps extends WithWritableReportOrNot
if (!!report?.reportID || !route.params.reportID || !!reportDraft || !isEditing) {
return;
}
openReport({reportID: route.params.reportID, introSelected, betas});
openReport({reportID: route.params.reportID, introSelected, betas, hasReportActions});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

Expand Down
13 changes: 12 additions & 1 deletion src/pages/workspace/rooms/WorkspaceRoomsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ function WorkspaceRoomsPage({route}: WorkspaceRoomsPageProps) {
const [betas] = useOnyx(ONYXKEYS.BETAS);

const [policyReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: policyChatRoomsSelector(policyID, reportNameValuePairs)});
const [hasReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {

@github-actions github-actions Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-11 (docs)

This useOnyx subscribes to the entire REPORT_ACTIONS collection (one of the largest collections in the app) and its selector returns a Record<string, boolean> mapped across every policy report. Because a selector is used, Onyx runs deepEqual on this whole map on every report-action change anywhere in the app, and the selector also closes over the large external policyReports dataset and re-iterates it on each of those unrelated updates — compounding the cost. A mapped/transformed collection output like this is exactly the case PERF-11 warns against: the deepEqual is expensive and there is no meaningful re-render savings.

Instead, subscribe without a selector and build the lookup inline (or read presence per-report where it's needed), so Onyx uses the cheap shallowEqual on raw references:

const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);

// where the room action is built:
const hasReportActions = !!reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`];
openReport({reportID: report.reportID, introSelected, betas, shouldMarkAsRead: false, hasReportActions});

This avoids the per-report mapped object and the deepEqual over the full collection.

Reviewed at: 6f88da6 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

@bernhardoj bernhardoj Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the deepEqual will be expensive here because it's a simple boolean map.

selector: (reportActions) => {
return policyReports?.reduce(
(acc, curr) => {
acc[curr.reportID] = !!reportActions?.[curr.reportID];

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Index report actions with the collection key

For admins opening Workspace > Rooms, useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS) returns a collection keyed by the full Onyx key, e.g. ${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID} as used by reportActionsExist, but this selector indexes it with the bare room ID. That makes every cached room evaluate to false, so the new openReport(..., hasReportActions: ...) call behaves as if rooms with existing actions have none and reintroduces the incorrect optimistic report update this parameter is meant to avoid.

Useful? React with 👍 / 👎.

@bernhardoj bernhardoj Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selector returns it with the reportID as the index. Invalid review.

return acc;
},
{} as Record<string, boolean>,
);
},
});

// The newly created room reportID is stored in Onyx right before navigating back here so its row can play the highlight animation.
// It is cleared by the create page once the navigation transition ends (see WorkspaceNewRoomPage), so the animation doesn't replay on a later visit.
Expand All @@ -72,7 +83,7 @@ function WorkspaceRoomsPage({route}: WorkspaceRoomsPageProps) {
// Admins open the details RHP directly instead of the room report, so the report is never fetched via ReportScreen.
// Fetch it here so the RHP has full data (participants, metadata) for Join, Invite and renaming.
// shouldMarkAsRead is false because the user only views the room details, not the conversation itself.
openReport({reportID: report.reportID, introSelected, betas, shouldMarkAsRead: false});
openReport({reportID: report.reportID, introSelected, betas, shouldMarkAsRead: false, hasReportActions: !!hasReportActions?.[report.reportID]});
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.REPORT_DETAILS.getRoute(report.reportID)));
return;
}
Expand Down
Loading