Skip to content

Cleanup some dependencies on report name value pairs #59962

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
srikarparsi merged 6 commits into from
Apr 10, 2025
Merged

Cleanup some dependencies on report name value pairs #59962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85860a4
Remove unneeded dependency
tgolen Apr 9, 2025
513cafe
Remove unneeded dependency
tgolen Apr 9, 2025
79e2c38
Remove dynamic reference to rNVPs
tgolen Apr 9, 2025
9b9d9d9
Remove unused parameter
tgolen Apr 9, 2025
3e18761
Fix tests
tgolen Apr 9, 2025
cb097d0
Remove unused import
tgolen Apr 9, 2025
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/components/LHNOptionsList/LHNOptionsList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
}
: null;
}
const lastMessageTextFromReport = getLastMessageTextForReport(itemFullReport, lastActorDetails, itemPolicy);
const lastMessageTextFromReport = getLastMessageTextForReport(itemFullReport, lastActorDetails, itemPolicy, itemReportNameValuePairs);

return (
<OptionRowLHNData
Expand Down
6 changes: 3 additions & 3 deletions src/components/OptionListContextProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ function OptionsListContextProvider({children}: OptionsListProviderProps) {
});
const [preferredLocale] = useOnyx(ONYXKEYS.NVP_PREFERRED_LOCALE);
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS);

const personalDetails = usePersonalDetails();
const prevPersonalDetails = usePrevious(personalDetails);
Expand All @@ -72,8 +71,9 @@ function OptionsListContextProvider({children}: OptionsListProviderProps) {

return newOptions;
});
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [reports, reportNameValuePairs, preferredLocale]);

// eslint-disable-next-line react-compiler/react-compiler
}, [reports, personalDetails, preferredLocale]);

/**
* This effect is used to update the options list when personal details change.
Expand Down
14 changes: 12 additions & 2 deletions src/hooks/useReportIDs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ function ReportIDsContextProvider({
}: ReportIDsContextProviderProps) {
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT});
const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS);
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: (c) => mapOnyxCollectionItems(c, policySelector)});
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS);
const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {}});
const draftAmount = Object.keys(reportsDrafts ?? {}).length;
const [betas] = useOnyx(ONYXKEYS.BETAS);
Expand All @@ -71,7 +71,17 @@ function ReportIDsContextProvider({

const getOrderedReportIDs = useCallback(
(currentReportID?: string) =>
SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, transactionViolations, activeWorkspaceID, policyMemberAccountIDs),
SidebarUtils.getOrderedReportIDs(
currentReportID,
chatReports,
betas,
policies,
priorityMode,
transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
reportNameValuePairs,
),
// we need reports draft in deps array to reload the list when a draft is added or removed
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
[chatReports, betas, policies, priorityMode, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, draftAmount, reportNameValuePairs],
Expand Down
15 changes: 10 additions & 5 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
Report,
ReportAction,
ReportActions,
ReportNameValuePairs,
TransactionViolation,
} from '@src/types/onyx';
import type {Attendee, Participant} from '@src/types/onyx/IOU';
Expand Down Expand Up @@ -666,16 +667,19 @@ function getIOUReportIDOfLastAction(report: OnyxEntry<Report>): string | undefin
/**
* Get the last message text from the report directly or from other sources for special cases.
*/
function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails: Partial<PersonalDetails> | null, policy?: OnyxEntry<Policy>): string {
function getLastMessageTextForReport(
report: OnyxEntry<Report>,
lastActorDetails: Partial<PersonalDetails> | null,
policy?: OnyxEntry<Policy>,
reportNameValuePairs?: OnyxInputOrEntry<ReportNameValuePairs>,
): string {
const reportID = report?.reportID;
const lastReportAction = reportID ? lastVisibleReportActions[reportID] : undefined;

// some types of actions are filtered out for lastReportAction, in some cases we need to check the actual last action
const lastOriginalReportAction = reportID ? lastReportActions[reportID] : undefined;
let lastMessageTextFromReport = '';

const reportNameValuePairs = getReportNameValuePairs(reportID);

if (isArchivedNonExpenseReport(report, reportNameValuePairs)) {
const archiveReason =
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Expand Down Expand Up @@ -845,9 +849,10 @@ function createOption(
result.participantsList = personalDetailList;
result.isOptimisticPersonalDetail = personalDetail?.isOptimisticPersonalDetail;
if (report) {
const reportNameValuePairs = getReportNameValuePairs(report.reportID);
result.isChatRoom = reportUtilsIsChatRoom(report);
result.isDefaultRoom = isDefaultRoom(report);
result.private_isArchived = getReportNameValuePairs(report.reportID)?.private_isArchived;
result.private_isArchived = reportNameValuePairs?.private_isArchived;
result.isExpenseReport = isExpenseReport(report);
result.isInvoiceRoom = isInvoiceRoom(report);
result.isMoneyRequestReport = reportUtilsIsMoneyRequestReport(report);
Expand Down Expand Up @@ -882,7 +887,7 @@ function createOption(

const lastActorDetails = report.lastActorAccountID ? personalDetails?.[report.lastActorAccountID] ?? null : null;
const lastActorDisplayName = getLastActorDisplayName(lastActorDetails);
const lastMessageTextFromReport = getLastMessageTextForReport(report, lastActorDetails);
const lastMessageTextFromReport = getLastMessageTextForReport(report, lastActorDetails, undefined, reportNameValuePairs);
let lastMessageText = lastMessageTextFromReport;

const lastAction = lastVisibleReportActions[report.reportID];
Expand Down
8 changes: 4 additions & 4 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ import {
getPolicyName,
getReportDescription,
getReportName,
getReportNameValuePairs,
getReportNotificationPreference,
getReportParticipantsTitle,
getReportSubtitlePrefix,
Expand Down Expand Up @@ -196,6 +195,7 @@ function getOrderedReportIDs(
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>,
): string[] {
Performance.markStart(CONST.TIMING.GET_ORDERED_REPORT_IDS);
const isInFocusMode = priorityMode === CONST.PRIORITY_MODE.GSD;
Expand Down Expand Up @@ -286,14 +286,14 @@ function getOrderedReportIDs(

const isPinned = report?.isPinned ?? false;
const reportAction = getReportAction(report?.parentReportID, report?.parentReportActionID);
const reportNameValuePairs = getReportNameValuePairs(report?.reportID);
const rNVPs = reportNameValuePairs?.[report?.reportID];
if (isPinned || requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndGBRReports.push(miniReport);
} else if (report?.hasErrorsOtherThanFailedReceipt) {
errorReports.push(miniReport);
} else if (hasValidDraftComment(report?.reportID)) {
draftReports.push(miniReport);
} else if (isArchivedNonExpenseReport(report, reportNameValuePairs)) {
} else if (isArchivedNonExpenseReport(report, rNVPs)) {
archivedReports.push(miniReport);
} else {
nonArchivedReports.push(miniReport);
Expand Down Expand Up @@ -518,7 +518,7 @@ function getOptionData({
const lastActorDisplayName = getLastActorDisplayName(lastActorDetails);
let lastMessageTextFromReport = lastMessageTextFromReportProp;
if (!lastMessageTextFromReport) {
lastMessageTextFromReport = getLastMessageTextForReport(report, lastActorDetails, policy);
lastMessageTextFromReport = getLastMessageTextForReport(report, lastActorDetails, policy, reportNameValuePairs);
}

// We need to remove sms domain in case the last message text has a phone number mention with sms domain.
Expand Down
8 changes: 2 additions & 6 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8616,11 +8616,7 @@ function sendMoneyWithWallet(report: OnyxEntry<OnyxTypes.Report>, amount: number
notifyNewAction(params.chatReportID, managerID);
}

function canApproveIOU(
iouReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report> | SearchReport,
policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy> | SearchPolicy,
chatReportRNVP?: OnyxTypes.ReportNameValuePairs,
) {
function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report> | SearchReport, policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy> | SearchPolicy) {

@tgolen tgolen Apr 9, 2025

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.

There wasn't any instance of canApproveIOU() that was passing more than two arguments, so I removed the third parameter entirely.

Trying to get rid of getReportNameValuePairs() is very difficult, so I'm going to slowly try to do it in other PRs. It's because canApproveIOU() is used in many places, so it will take many updates to be able to pass the rNVPs as an argument.

@srikarparsi srikarparsi Apr 10, 2025 •
edited
Loading

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.

Sounds good, I also think we could get a contributor to help with this if we make this external? If there's a reason for it to be internal, I can also help with this

// Only expense reports can be approved
if (!isExpenseReport(iouReport) || !(policy && isPaidGroupPolicy(policy))) {
return false;
Expand All @@ -8636,7 +8632,7 @@ function canApproveIOU(
const isOpenExpenseReport = isOpenExpenseReportReportUtils(iouReport);
const isApproved = isReportApproved({report: iouReport});
const iouSettled = isSettled(iouReport?.reportID);
const reportNameValuePairs = chatReportRNVP ?? getReportNameValuePairs(iouReport?.reportID);
const reportNameValuePairs = getReportNameValuePairs(iouReport?.reportID);
const isArchivedExpenseReport = isArchivedReport(reportNameValuePairs);
const reportTransactions = getReportTransactions(iouReport?.reportID);
const hasOnlyPendingCardOrScanningTransactions = reportTransactions.length > 0 && reportTransactions.every(isPendingCardOrScanningTransaction);
Expand Down