Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,11 @@ function isThreadParentMessage(reportAction: OnyxEntry<ReportAction>, reportID:
*
* @deprecated Use Onyx.connect() or withOnyx() instead
*/
function getParentReportAction(report: OnyxEntry<Report>, allReportActionsParam?: OnyxCollection<ReportActions>): ReportAction | Record<string, never> {
function getParentReportAction(report: OnyxEntry<Report>): ReportAction | Record<string, never> {
if (!report?.parentReportID || !report.parentReportActionID) {
return {};
}
return (allReportActionsParam ?? allReportActions)?.[report.parentReportID]?.[report.parentReportActionID] ?? {};
return allReportActions?.[report.parentReportID]?.[report.parentReportActionID] ?? {};
}

/**
Expand Down
16 changes: 4 additions & 12 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3381,8 +3381,8 @@ function canSeeDefaultRoom(report: OnyxEntry<Report>, policies: OnyxCollection<P
return Permissions.canUseDefaultRooms(betas ?? []);
}

function canAccessReport(report: OnyxEntry<Report>, policies: OnyxCollection<Policy>, betas: OnyxEntry<Beta[]>, allReportActions?: OnyxCollection<ReportActions>): boolean {
if (isThread(report) && ReportActionsUtils.isPendingRemove(ReportActionsUtils.getParentReportAction(report, allReportActions))) {
function canAccessReport(report: OnyxEntry<Report>, policies: OnyxCollection<Policy>, betas: OnyxEntry<Beta[]>): boolean {
if (isThread(report) && ReportActionsUtils.isPendingRemove(ReportActionsUtils.getParentReportAction(report))) {
return false;
}

Expand Down Expand Up @@ -3411,15 +3411,7 @@ function shouldHideReport(report: OnyxEntry<Report>, currentReportId: string): b
* This logic is very specific and the order of the logic is very important. It should fail quickly in most cases and also
* filter out the majority of reports before filtering out very specific minority of reports.
*/
function shouldReportBeInOptionList(
report: OnyxEntry<Report>,
currentReportId: string,
isInGSDMode: boolean,
betas: Beta[],
policies: OnyxCollection<Policy>,
allReportActions?: OnyxCollection<ReportActions>,
excludeEmptyChats = false,
) {
function shouldReportBeInOptionList(report: OnyxEntry<Report>, currentReportId: string, isInGSDMode: boolean, betas: Beta[], policies: OnyxCollection<Policy>, excludeEmptyChats = false) {
const isInDefaultMode = !isInGSDMode;
// Exclude reports that have no data because there wouldn't be anything to show in the option item.
// This can happen if data is currently loading from the server or a report is in various stages of being created.
Expand All @@ -3442,7 +3434,7 @@ function shouldReportBeInOptionList(
) {
return false;
}
if (!canAccessReport(report, policies, betas, allReportActions)) {
if (!canAccessReport(report, policies, betas)) {
return false;
}

Expand Down
6 changes: 2 additions & 4 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function getOrderedReportIDs(
betas: Beta[],
policies: Record<string, Policy>,
priorityMode: ValueOf<typeof CONST.PRIORITY_MODE>,
allReportActions: OnyxCollection<ReportActions>,
allReportActions: OnyxCollection<ReportAction[]>,
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.

I'm curious about this change, the structure should be OnyxCollection<ReportActions>.

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.

Yes, but in the component where this method is used , there is a selector in withOnyx which is mapping value to type ReportAction[]

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.

In the original issue I put a screenshot (I am also a creator of this issue XD) I found it when migrating ReportUtils

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.

Ok, I understand now. So maybe we should use Record<string, ReportAction[]> like you used in the other file?

): string[] {
// Generate a unique cache key based on the function arguments
const cachedReportsKey = JSON.stringify(
Expand Down Expand Up @@ -149,9 +149,7 @@ function getOrderedReportIDs(
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);
// Filter out all the reports that shouldn't be displayed
const reportsToDisplay = allReportsDictValues.filter((report) =>
ReportUtils.shouldReportBeInOptionList(report, currentReportId ?? '', isInGSDMode, betas, policies, allReportActions, true),
);
const reportsToDisplay = allReportsDictValues.filter((report) => ReportUtils.shouldReportBeInOptionList(report, currentReportId ?? '', isInGSDMode, betas, policies, true));

if (reportsToDisplay.length === 0) {
// Display Concierge chat report when there is no report to be displayed
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/withReportOrNotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default function (
const shouldShowFullScreenLoadingIndicator = props.isLoadingReportData !== false && (!Object.entries(props.report ?? {}).length || !props.report?.reportID);

const shouldShowNotFoundPage =
!Object.entries(props.report ?? {}).length || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas, {});
!Object.entries(props.report ?? {}).length || !props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas);

// If the content was shown but it's not anymore that means the report was deleted and we are probably navigating out of this screen.
// Return null for this case to avoid rendering FullScreenLoadingIndicator or NotFoundPage when animating transition.
Expand Down
4 changes: 2 additions & 2 deletions tests/perf-test/SidebarUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import {PersonalDetails} from '@src/types/onyx';
import Policy from '@src/types/onyx/Policy';
import Report from '@src/types/onyx/Report';
import ReportAction, {ReportActions} from '@src/types/onyx/ReportAction';
import ReportAction from '@src/types/onyx/ReportAction';
import createCollection from '../utils/collections/createCollection';
import createPersonalDetails from '../utils/collections/personalDetails';
import createRandomPolicy from '../utils/collections/policies';
Expand Down Expand Up @@ -86,7 +86,7 @@ test('getOrderedReportIDs on 5k reports', async () => {
},
],
]),
) as unknown as OnyxCollection<ReportActions>;
) as unknown as OnyxCollection<ReportAction[]>;

Onyx.multiSet({
...mockedResponseMap,
Expand Down