Skip to content
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.

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

[NoQA] [Audit][Implementation] - Increase max cache keys count and use waitForCollectionCallback #38464

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
21 changes: 5 additions & 16 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,6 @@ Onyx.connect({
},
});

const policyExpenseReports: OnyxCollection<Report> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report, key) => {
if (!ReportUtils.isPolicyExpenseChat(report)) {
return;
}
policyExpenseReports[key] = report;
},
});

let allTransactions: OnyxCollection<Transaction> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION,
Expand Down Expand Up @@ -763,13 +752,13 @@ function getReportOption(participant: Participant): ReportUtils.OptionData {
/**
* Get the option for a policy expense report.
*/
function getPolicyExpenseReportOption(report: Report): ReportUtils.OptionData {
const expenseReport = policyExpenseReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`];
function getPolicyExpenseReportOption(participant: Participant | ReportUtils.OptionData): ReportUtils.OptionData {
const expenseReport = ReportUtils.isPolicyExpenseChat(participant) ? ReportUtils.getReport(participant.reportID) : null;

const option = createOption(
expenseReport?.visibleChatMemberAccountIDs ?? [],
allPersonalDetails ?? {},
expenseReport ?? null,
!isEmptyObject(expenseReport) ? expenseReport : null,
{},
{
showChatPreviewLine: false,
Expand All @@ -780,8 +769,8 @@ function getPolicyExpenseReportOption(report: Report): ReportUtils.OptionData {
// Update text & alternateText because createOption returns workspace name only if report is owned by the user
option.text = ReportUtils.getPolicyName(expenseReport);
option.alternateText = Localize.translateLocal('workspace.common.workspace');
option.selected = report.selected;
option.isSelected = report.selected;
option.selected = participant.selected;
option.isSelected = participant.selected;
return option;
}

Expand Down
13 changes: 4 additions & 9 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,12 @@ type MemberChangeMessageElement = MessageTextElement | MemberChangeMessageUserMe

const policyChangeActionsSet = new Set<string>(Object.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG));

const allReports: OnyxCollection<Report> = {};

let allReports: OnyxCollection<Report> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report, key) => {
if (!key || !report) {
return;
}

const reportID = CollectionUtils.extractCollectionItemID(key);
allReports[reportID] = report;
waitForCollectionCallback: true,
callback: (reports) => {
allReports = reports;
},
});

Expand Down
18 changes: 7 additions & 11 deletions src/libs/TaskUtils.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
import type {OnyxEntry} from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Report} from '@src/types/onyx';
import type {Message} from '@src/types/onyx/ReportAction';
import type ReportAction from '@src/types/onyx/ReportAction';
import * as CollectionUtils from './CollectionUtils';
import * as Localize from './Localize';

const allReports: Record<string, Report> = {};
let allReports: OnyxCollection<Report> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report, key) => {
if (!key || !report) {
return;
}
const reportID = CollectionUtils.extractCollectionItemID(key);
allReports[reportID] = report;
waitForCollectionCallback: true,
callback: (reports) => {
allReports = reports;
},
});

Expand All @@ -42,11 +38,11 @@ function getTaskReportActionMessage(action: OnyxEntry<ReportAction>): Pick<Messa
}

function getTaskTitle(taskReportID: string, fallbackTitle = ''): string {
const taskReport = allReports[taskReportID] ?? {};
const taskReport = allReports?.[taskReportID] ?? {};
// We need to check for reportID, not just reportName, because when a receiver opens the task for the first time,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should've been updated to const taskReport = allReports?.[${ONYXKEYS.COLLECTION.REPORT}${taskReportID}] ?? {}; after updating how we key the reports. it caused a regression #40121 More information: #40121 (comment)

// an optimistic report is created with the only property – reportName: 'Chat report',
// and it will be displayed as the task title without checking for reportID to be present.
return Object.hasOwn(taskReport, 'reportID') && taskReport.reportName ? taskReport.reportName : fallbackTitle;
return Object.hasOwn(taskReport, 'reportID') && 'reportName' in taskReport && typeof taskReport.reportName === 'string' ? taskReport.reportName : fallbackTitle;
}

function getTaskCreatedMessage(reportAction: OnyxEntry<ReportAction>) {
Expand Down
18 changes: 4 additions & 14 deletions src/libs/actions/PriorityMode.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import debounce from 'lodash/debounce';
import type {OnyxCollection} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import * as CollectionUtils from '@libs/CollectionUtils';
import Log from '@libs/Log';
import * as ReportUtils from '@libs/ReportUtils';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -36,21 +35,12 @@ Onyx.connect({
// eslint-disable-next-line @typescript-eslint/no-use-before-define
const autoSwitchToFocusMode = debounce(tryFocusModeUpdate, 300, {leading: true});

let allReports: OnyxCollection<Report> | undefined;
let allReports: OnyxCollection<Report> | undefined = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report, key) => {
if (!key || !report) {
return;
}

if (!allReports) {
allReports = {};
}

const reportID = CollectionUtils.extractCollectionItemID(key);

allReports[reportID] = report;
waitForCollectionCallback: true,
callback: (reports) => {
allReports = reports;

// Each time a new report is added we will check to see if the user should be switched
autoSwitchToFocusMode();
Expand Down
7 changes: 2 additions & 5 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ Onyx.connect({
}
const reportID = CollectionUtils.extractCollectionItemID(key);
currentReportData[reportID] = report;
// eslint-disable-next-line @typescript-eslint/no-use-before-define
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
handleReportChanged(report);
},
});

Expand Down Expand Up @@ -1123,11 +1125,6 @@ function handleReportChanged(report: OnyxEntry<Report>) {
}
}

Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: handleReportChanged,
});

/** Deletes a comment from the report, basically sets it as empty string */
function deleteReportComment(reportID: string, reportAction: ReportAction) {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
Expand Down
11 changes: 4 additions & 7 deletions src/libs/actions/Welcome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,13 @@ Onyx.connect({
},
});

const allReports: OnyxCollection<Report> = {};
let allReports: OnyxCollection<Report> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
initWithStoredValues: false,
callback: (val, key) => {
if (!val || !key) {
return;
}

allReports[key] = {...allReports[key], ...val};
waitForCollectionCallback: true,
callback: (reports) => {
allReports = reports;
},
});

Expand Down
2 changes: 1 addition & 1 deletion src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function () {
keys: ONYXKEYS,

// Increase the cached key count so that the app works more consistently for accounts with large numbers of reports
maxCachedKeysCount: 10000,
maxCachedKeysCount: 20000,
safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
captureMetrics: Metrics.canCaptureOnyxMetrics(),
initialKeyStates: {
Expand Down
Loading