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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
rename function
  • Loading branch information
nkdengineer committed Jun 17, 2024
commit 395f4a092604ccbc5e6eea8c63eabb6132d87b06
Original file line number Diff line number Diff line change
@@ -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[] = [];

Original file line number Diff line number Diff line change
@@ -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});
}
2 changes: 1 addition & 1 deletion src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
@@ -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});
}
12 changes: 9 additions & 3 deletions src/libs/ModifiedExpenseMessage.ts
Original file line number Diff line number Diff line change
@@ -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';
@@ -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.
*/
@@ -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[] = [];
12 changes: 6 additions & 6 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
@@ -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;
}
@@ -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);

@@ -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) &&
@@ -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);
@@ -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)
18 changes: 6 additions & 12 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
@@ -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';
@@ -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;
}
@@ -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);
}
@@ -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.
*/
@@ -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;
}
@@ -7078,7 +7073,6 @@ export {
getReportNotificationPreference,
getReportOfflinePendingActionAndErrors,
getReportParticipantsTitle,
getReportPolicyID,
getReportPreviewMessage,
getReportRecipientAccountIDs,
getRoom,
18 changes: 9 additions & 9 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
@@ -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;
}
@@ -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 = {
@@ -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);

@@ -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);

@@ -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;
@@ -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,
@@ -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);
@@ -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[] = [];
16 changes: 2 additions & 14 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
@@ -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);
}
@@ -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;
@@ -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;
16 changes: 2 additions & 14 deletions src/libs/actions/Task.ts
Original file line number Diff line number Diff line change
@@ -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
*/
@@ -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) {
33 changes: 0 additions & 33 deletions src/libs/getReportPolicyID.ts

This file was deleted.

Loading
Loading