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

Remove export ReportUtil.getReport function #43632

Merged
merged 22 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@ import * as FileUtils from '@libs/fileDownload/FileUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import CONST from '@src/CONST';
import type {Report, ReportAction, ReportActions} from '@src/types/onyx';
import type {ReportAction, ReportActions} from '@src/types/onyx';
import type {Note} from '@src/types/onyx/Report';

/**
* Constructs the initial component state from report actions
*/
function extractAttachments(
type: ValueOf<typeof CONST.ATTACHMENT_TYPE>,
{
report,
privateNotes,
accountID,
parentReportAction,
reportActions,
}: {report?: OnyxEntry<Report>; accountID?: number; parentReportAction?: OnyxEntry<ReportAction>; reportActions?: OnyxEntry<ReportActions>},
}: {privateNotes?: Record<number, Note>; accountID?: number; parentReportAction?: OnyxEntry<ReportAction>; reportActions?: OnyxEntry<ReportActions>},
) {
const privateNotes = report?.privateNotes;
const targetNote = privateNotes?.[Number(accountID)]?.note ?? '';
const attachments: Attachment[] = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let targetAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {report, accountID});
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report?.privateNotes, accountID});
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions});
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let targetAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {report, accountID});
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report?.privateNotes, accountID});
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined});
}
Expand Down
12 changes: 9 additions & 3 deletions src/libs/ModifiedExpenseMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ import Onyx from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PolicyTagList, ReportAction} from '@src/types/onyx';
import type {PolicyTagList, Report, ReportAction} from '@src/types/onyx';
import type {ModifiedExpense} from '@src/types/onyx/OriginalMessage';
import * as CurrencyUtils from './CurrencyUtils';
import DateUtils from './DateUtils';
import getReportPolicyID from './getReportPolicyID';
import * as Localize from './Localize';
import * as PolicyUtils from './PolicyUtils';
import * as TransactionUtils from './TransactionUtils';
Expand All @@ -24,6 +23,13 @@ Onyx.connect({
},
});

let allReports: OnyxCollection<Report>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => (allReports = value),
});

/**
* Builds the partial message fragment for a modified field on the expense.
*/
Expand Down Expand Up @@ -110,7 +116,7 @@ function getForReportAction(reportID: string | undefined, reportAction: OnyxEntr
return '';
}
const reportActionOriginalMessage = reportAction?.originalMessage as ModifiedExpense | undefined;
const policyID = getReportPolicyID(reportID) ?? '-1';
const policyID = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.policyID ?? '-1';

const removalFragments: string[] = [];
const setFragments: string[] = [];
Expand Down
12 changes: 6 additions & 6 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,9 @@ Onyx.connect({
});

/**
* Get the report given a reportID
* Get the report or draft report given a reportID
*/
function getReport(reportID: string | undefined): OnyxEntry<Report> {
function getReportOrDraftReport(reportID: string | undefined): OnyxEntry<Report> {
if (!allReports && !allReportsDraft) {
return undefined;
}
Expand Down Expand Up @@ -605,7 +605,7 @@ function getLastActorDisplayName(lastActorDetails: Partial<PersonalDetails> | nu
* Update alternate text for the option when applicable
*/
function getAlternateText(option: ReportUtils.OptionData, {showChatPreviewLine = false, forcePolicyNamePreview = false}: PreviewConfig) {
const report = getReport(option.reportID);
const report = getReportOrDraftReport(option.reportID);
const isAdminRoom = ReportUtils.isAdminRoom(report);
const isAnnounceRoom = ReportUtils.isAnnounceRoom(report);

Expand Down Expand Up @@ -681,7 +681,7 @@ function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails
const properSchemaForMoneyRequestMessage = ReportUtils.getReportPreviewMessage(report, lastReportAction, true, false, null, true);
lastMessageTextFromReport = ReportUtils.formatReportLastMessageText(properSchemaForMoneyRequestMessage);
} else if (ReportActionUtils.isReportPreviewAction(lastReportAction)) {
const iouReport = getReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction));
const iouReport = getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction));
const lastIOUMoneyReportAction = allSortedReportActions[iouReport?.reportID ?? '-1']?.find(
(reportAction, key) =>
ReportActionUtils.shouldReportActionBeVisible(reportAction, key) &&
Expand Down Expand Up @@ -866,7 +866,7 @@ function createOption(
* Get the option for a given report.
*/
function getReportOption(participant: Participant): ReportUtils.OptionData {
const report = getReport(participant.reportID);
const report = getReportOrDraftReport(participant.reportID);

// For 1:1 chat, we don't want to include currentUser as participants in order to not mark 1:1 chats as having multiple participants
const isOneOnOneChat = ReportUtils.isOneOnOneChat(report);
Expand Down Expand Up @@ -906,7 +906,7 @@ function getReportOption(participant: Participant): ReportUtils.OptionData {
* Get the option for a policy expense report.
*/
function getPolicyExpenseReportOption(participant: Participant | ReportUtils.OptionData): ReportUtils.OptionData {
const expenseReport = ReportUtils.isPolicyExpenseChat(participant) ? getReport(participant.reportID) : null;
const expenseReport = ReportUtils.isPolicyExpenseChat(participant) ? getReportOrDraftReport(participant.reportID) : null;

const visibleParticipantAccountIDs = Object.entries(expenseReport?.participants ?? {})
.filter(([, reportParticipant]) => reportParticipant && !reportParticipant.hidden)
Expand Down
18 changes: 6 additions & 12 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import * as store from './actions/ReimbursementAccount/store';
import * as CurrencyUtils from './CurrencyUtils';
import DateUtils from './DateUtils';
import {hasValidDraftComment} from './DraftCommentUtils';
import originalGetReportPolicyID from './getReportPolicyID';
import isReportMessageAttachment from './isReportMessageAttachment';
import localeCompare from './LocaleCompare';
import * as LocalePhoneNumber from './LocalePhoneNumber';
Expand Down Expand Up @@ -1228,7 +1227,8 @@ function isClosedExpenseReportWithNoExpenses(report: OnyxEntry<Report>): boolean
/**
* Whether the provided report is an archived room
*/
function isArchivedRoom(report: OnyxInputOrEntry<Report> | EmptyObject, reportNameValuePairs?: OnyxInputOrEntry<ReportNameValuePairs> | EmptyObject): boolean {
function isArchivedRoom(reportOrID: OnyxInputOrEntry<Report> | EmptyObject | string, reportNameValuePairs?: OnyxInputOrEntry<ReportNameValuePairs> | EmptyObject): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

I was looking at this one (and also isInvoiceRoom above) and I don't really like that the first param could be either a reportID (string) or a report (object). It makes the code feel much less uncertain and if I was looking at it, I'd be wondering when I should use a reportID vs just a report. Some suggestions I have:

  • Keep it two separate parameters for clarity (reportID, report)
  • Make two separate methods
  • Anywhere that is only using the reportID, have it get the report from the collection first and then these methods would only have a report parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was looking at this one (and also isInvoiceRoom above) and I don't really like that the first param could be either a reportID (string) or a report (object). It makes the code feel much less uncertain and if I was looking at it, I'd be wondering when I should use a reportID vs just a report. Some suggestions I have:

It's the same way we use for other functions like isMoneyRequestReport, isMoneyRequest,...

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I don't think that is a pattern that we should be following, so let's not do it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tgolen I updated with two separate methods.

const report = typeof reportOrID === 'string' ? getReport(reportOrID) : reportOrID;
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
if (reportNameValuePairs) {
return reportNameValuePairs.isArchived;
}
Expand Down Expand Up @@ -5537,7 +5537,8 @@ function getAllPolicyReports(policyID: string): Array<OnyxEntry<Report>> {
/**
* Returns true if Chronos is one of the chat participants (1:1)
*/
function chatIncludesChronos(report: OnyxInputOrEntry<Report> | EmptyObject): boolean {
function chatIncludesChronos(reportOrID: OnyxInputOrEntry<Report> | EmptyObject | string): boolean {
const report = typeof reportOrID === 'string' ? getReport(reportOrID) : reportOrID;
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
const participantAccountIDs = Object.keys(report?.participants ?? {}).map(Number);
return participantAccountIDs.includes(CONST.ACCOUNT_ID.CHRONOS);
}
Expand Down Expand Up @@ -5683,13 +5684,6 @@ function getReportIDFromLink(url: string | null): string {
return reportID;
}

/**
* Get the report policyID given a reportID
*/
function getReportPolicyID(reportID?: string): string | undefined {
return originalGetReportPolicyID(reportID);
}

/**
* Check if the chat report is linked to an iou that is waiting for the current user to add a credit bank account.
*/
Expand Down Expand Up @@ -6038,7 +6032,8 @@ function getAddWorkspaceRoomOrChatReportErrors(report: OnyxEntry<Report>): Error
/**
* Return true if the expense report is marked for deletion.
*/
function isMoneyRequestReportPendingDeletion(report: OnyxEntry<Report> | EmptyObject): boolean {
function isMoneyRequestReportPendingDeletion(reportOrID: OnyxEntry<Report> | EmptyObject | string): boolean {
const report = typeof reportOrID === 'string' ? getReport(reportOrID) : reportOrID;
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
if (!isMoneyRequestReport(report)) {
return false;
}
Expand Down Expand Up @@ -7084,7 +7079,6 @@ export {
getReportNotificationPreference,
getReportOfflinePendingActionAndErrors,
getReportParticipantsTitle,
getReportPolicyID,
getReportPreviewMessage,
getReportRecipientAccountIDs,
getRoom,
Expand Down
18 changes: 9 additions & 9 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,9 @@ Onyx.connect({
});

/**
* Get the report given a reportID
* Get the report or draft report given a reportID
*/
function getReport(reportID: string | undefined): OnyxEntry<OnyxTypes.Report> {
function getReportOrDraftReport(reportID: string | undefined): OnyxEntry<OnyxTypes.Report> {
if (!allReports && !allReportsDraft) {
return undefined;
}
Expand Down Expand Up @@ -2328,7 +2328,7 @@ function createDistanceRequest(
) {
// If the report is an iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function
const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
const currentChatReport = isMoneyRequestReport ? getReport(report?.chatReportID) : report;
const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report?.chatReportID) : report;
const moneyRequestReportID = isMoneyRequestReport ? report?.reportID : '';

const optimisticReceipt: Receipt = {
Expand Down Expand Up @@ -3439,7 +3439,7 @@ function requestMoney(
) {
// If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function
const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
const currentChatReport = isMoneyRequestReport ? getReport(report?.chatReportID) : report;
const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report?.chatReportID) : report;
const moneyRequestReportID = isMoneyRequestReport ? report?.reportID : '';
const isMovingTransactionFromTrackExpense = IOUUtils.isMovingTransactionFromTrackExpense(action);

Expand Down Expand Up @@ -3617,7 +3617,7 @@ function trackExpense(
linkedTrackedExpenseReportID?: string,
) {
const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
const currentChatReport = isMoneyRequestReport ? getReport(report.chatReportID) : report;
const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report.chatReportID) : report;
const moneyRequestReportID = isMoneyRequestReport ? report.reportID : '';
const isMovingTransactionFromTrackExpense = IOUUtils.isMovingTransactionFromTrackExpense(action);

Expand Down Expand Up @@ -6188,7 +6188,7 @@ function hasIOUToApproveOrPay(chatReport: OnyxEntry<OnyxTypes.Report> | EmptyObj
const chatReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`] ?? {};

return Object.values(chatReportActions).some((action) => {
const iouReport = getReport(action.childReportID ?? '-1');
const iouReport = getReportOrDraftReport(action.childReportID ?? '-1');
const policy = PolicyUtils.getPolicy(iouReport?.policyID);
const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy) || canApproveIOU(iouReport, chatReport, policy);
return action.childReportID?.toString() !== excludedIOUReportID && action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && shouldShowSettlementButton;
Expand All @@ -6204,7 +6204,7 @@ function approveMoneyRequest(expenseReport: OnyxTypes.Report | EmptyObject, full
}
const optimisticApprovedReportAction = ReportUtils.buildOptimisticApprovedReportAction(total, expenseReport.currency ?? '', expenseReport.reportID);
const optimisticNextStep = NextStepUtils.buildNextStep(expenseReport, CONST.REPORT.STATUS_NUM.APPROVED);
const chatReport = getReport(expenseReport.chatReportID);
const chatReport = getReportOrDraftReport(expenseReport.chatReportID);

const optimisticReportActionsData: OnyxUpdate = {
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -6330,7 +6330,7 @@ function approveMoneyRequest(expenseReport: OnyxTypes.Report | EmptyObject, full

function submitReport(expenseReport: OnyxTypes.Report) {
const currentNextStep = allNextSteps[`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`] ?? null;
const parentReport = getReport(expenseReport.parentReportID);
const parentReport = getReportOrDraftReport(expenseReport.parentReportID);
const policy = PolicyUtils.getPolicy(expenseReport.policyID);
const isCurrentUserManager = currentUserPersonalDetails.accountID === expenseReport.managerID;
const isSubmitAndClosePolicy = PolicyUtils.isSubmitAndClose(policy);
Expand Down Expand Up @@ -6662,7 +6662,7 @@ function replaceReceipt(transactionID: string, file: File, source: string) {
*/
function setMoneyRequestParticipantsFromReport(transactionID: string, report: OnyxEntry<OnyxTypes.Report>): Participant[] {
// If the report is iou or expense report, we should get the chat report to set participant for request money
const chatReport = ReportUtils.isMoneyRequestReport(report) ? getReport(report?.chatReportID) : report;
const chatReport = ReportUtils.isMoneyRequestReport(report) ? getReportOrDraftReport(report?.chatReportID) : report;
const currentUserAccountID = currentUserPersonalDetails.accountID;
const shouldAddAsReport = !isEmptyObject(chatReport) && ReportUtils.isSelfDM(chatReport);
let participants: Participant[] = [];
Expand Down
16 changes: 2 additions & 14 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,18 +269,6 @@ Onyx.connect({
callback: (val) => (quickAction = val),
});

/**
* Get the report given a reportID
*/
function getReport(reportID: string | undefined): OnyxEntry<Report> {
if (!currentReportData || !reportID) {
return undefined;
}

const report = currentReportData?.[reportID];
return report;
}

function clearGroupChat() {
Onyx.set(ONYXKEYS.NEW_GROUP_CHAT_DRAFT, null);
}
Expand Down Expand Up @@ -490,7 +478,7 @@ function addActions(reportID: string, text = '', file?: FileObject) {
lastReadTime: currentTime,
};

const report = getReport(reportID);
const report = currentReportData?.[reportID];

if (!isEmptyObject(report) && ReportUtils.getReportNotificationPreference(report) === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
optimisticReport.notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS;
Expand Down Expand Up @@ -2601,7 +2589,7 @@ function joinRoom(report: OnyxEntry<Report>) {
}

function leaveGroupChat(reportID: string) {
const report = getReport(reportID);
const report = currentReportData?.[reportID];
if (!report) {
Log.warn('Attempting to leave Group Chat that does not existing locally');
return;
Expand Down
16 changes: 2 additions & 14 deletions src/libs/actions/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,6 @@ Onyx.connect({
callback: (value) => (allReports = value),
});

/**
* Get the report given a reportID
*/
function getReport(reportID: string | undefined): OnyxEntry<OnyxTypes.Report> {
if (!allReports) {
return undefined;
}

const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
return report;
}

/**
* Clears out the task info from the store
*/
Expand Down Expand Up @@ -1020,15 +1008,15 @@ function canModifyTask(taskReport: OnyxEntry<OnyxTypes.Report>, sessionAccountID
return true;
}

if (!ReportUtils.canWriteInReport(getReport(taskReport?.reportID))) {
if (!ReportUtils.canWriteInReport(taskReport)) {
return false;
}

return !isEmptyObject(taskReport) && ReportUtils.isAllowedToComment(taskReport);
}

function clearTaskErrors(reportID: string) {
const report = getReport(reportID);
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];

// Delete the task preview in the parent report
if (report?.pendingFields?.createChat === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) {
Expand Down
33 changes: 0 additions & 33 deletions src/libs/getReportPolicyID.ts

This file was deleted.

Loading
Loading