Refactor getReportName to prepare for formula computation

[neil-marcellini]

Problem

The getReportName function is messy and will be tricky to refactor to include custom report and formula computation using Onyx derived values.

Solution

Systematically refactor it to separate responsibilities.

1. The logic of calculation report’s name will be extracted to new method computeReportName
2. reportAttributes derived value will be responsible for computation of report name for every report in Onyx using compureReportName
3. getReportName will be simplified only to returning reportName from derived value

Issue Owner

Current Issue Owner: @laurenreidexpensify

[sosek108]

@neil-marcellini To speed up work on this @kstroz will help me with the refactor. Please assign him to this task

[sosek108]

@neil-marcellini Can you please give access to @kstroz to the Design Doc?

[sosek108]

Sorry, no major update here. I had been distracted by other tasks on Expensify. @kstroz took the time to review the project.

Tomorrow we'll update Design Doc with a solid plan how refactor will be implemented.

@kstroz remember to leave daily update on the progress

[kstroz]

Hey, happy to help with this feature! Here’s the PR list I’d create, feel free to correct me if I wrote something unclear or if you’d change anything, as I’m still getting familiar with the project.

1. Simplify getReportName and create computeReportName()

Simplify getReportName:

• Create new function that will allow us to migrate existing getReportName step by step for the time of migration.
• New logic: check if reportAttribute for a given report exists in the attributes store and if so return its reportName value. Otherwise return reportName accessed from report.

Change this component to use new implementation to spot any issues (its subtitle of room members page)

• src/pages/RoomMembersPage.tsx
  • prop
    • Simple call with StringUtils.lineBreaksToSpaces

Create computeReportName() which contain old logic from getReportName()

• src/libs/actions/OnyxDerived/configs/reportAttributes.ts
  • createOnyxDerivedValueConifg() -> compute()
    • Here we should use new computeReportName() instead of getReportName()

Total usage: 2

2. Core Utilities - ReportUtils Internal

• src/libs/ReportUtils.ts
  • parseReportActionHtmlToText()
    • Converts HTML with tag to plain text
  • getSearchReportName()
    • Gets report name in search contexts
  • getParentNavigationSubtitle()
    • Gets subtitle info for navigating to parent report
  • getMovedTransactionMessage()
    • Generates message shown when a transaction is moved to another report
  • getChatListItemReportName()
    • Gets report name for displaying in search/chat list items
  • getMoneyReportPreviewName()
    • Gets the display name for money/expense report preview actions

Note: I’m not 100% sure if we want to include all of these in a single PR.
Total usage: 6

3. Sidebar & List Utilities

• src/libs/SidebarUtils.ts 
  • categorizeReportsForLHN()
    • Simple call for displayName in report sorting in LHN
  • getOptionData()
    • Getting room name from last action report
    • Complex call with policy and invoice receiver policy
  • getRoomWelcomeMessage()
    • Simple call for welcome message
• src/libs/OptionsListUtils/index.ts
  • getLastMessageTextForReport()
    • For policy expense chat IOU report
  • createOption()
    • For participant display
  • getReportOption()
    • For invoice room text
  • getReportDisplayOption()
    • For invoice room text

Why together: These are tightly coupled utilities used for list rendering and sidebar display. Similar usage patterns.
Total usage: 8

4. Action & Search & Query Utils

• src/libs/SearchUIUtils.ts
  • getTaskSections()
    • Getting parent report name with policy and archive status
• src/libs/SearchQueryUtils.ts
  • getFilterDisplayValue()
    • For IN filter display value
• src/libs/actions/Task.ts
  • getShareDestintation()
    • This function is used for task sharing/destination selection. 

Why together: Relatively simple usages.
Total usage: 3

5. Report Action Utilities

• src/libs/ReportActionsUtils.ts
  • getMemeberChangeMessageElements()
    • Used as callback getReportNameCallback
  • getMemberChangeMessageFragment()
    • Used as callback getReportNameCallback passed to getMemeberChangeMessageElements
• src/libs/ModifiedExpenseMessage.ts
  • getMovedFromOrToReportMessage()
    • For modified expense messages
• src/pages/home/report/ContextMenu/ContextMenuActions.tsx
  • getMemberChangeMessageFragment()
    • Passed as function reference to getMemberChangeMessageFragment

Why together: All related to report actions and their messages. Note the function reference case needs special handling.
Total usage: 4

6. Money Request Components

• src/components/MoneyRequestConfirmationListFooter.tsx
  • const reportName
    • Getting selected report name
• src/components/ReportActionItem/MoneyRequestView.tsx
  • prop
    • Displaying parent report in menu item

Why together: All related to money request confirmation flow.
Total usage: 2

7. Report Header Components

• src/pages/home/HeaderView.tsx
  • const title
    • Complex call with policy, parentReportAction, personalDetails, invoiceReceiverPolicy, isReportArchived
• src/components/AvatarWithDisplayName.tsx
  • const title
    • Very complex call with all parameters including reportAttributes

Why together: Both are header/title display components with complex parameter usage.
Total usage: 2

8. Report Details Pages

• src/pages/ReportDetailsPage.tsx
  • const reportName
    • With reportAttributes, parsed with Parser.htmlToText
• src/pages/ReportParticipantsPage.tsx
  • prop
    • With reportAttributes, using StringUtils.lineBreaksToSpaces

Why together: All are report detail/member management pages with similar usage patterns.
Total usage: 2

9. IOU/Expense Pages

• src/pages/iou/SplitExpenseEditPage.tsx
  • const reportName
    • With report and policy
• src/pages/TransactionMerge/MergeTransactionsListContent.tsx
  • const headerContent -> prop
    • For merge transaction description

Why together: Both related to expense/transaction editing flows.
Total usage: 2

10. Modal & Media Components

• src/pages/media/AttachmentModalScreen/routes/report/ReportAvatarModalContent.tsx
  • const attachment
    • For modal header title
• src/pages/home/sidebar/FloatingActionButtonAndPopover.tsx
  • const quickActionSubtitle
    • Complex call for quick action subtitle
  • const quickActionMenuItems
    • For scan receipt description

Why together: Both modal/popover UI components.
Total usage: 3

11. Workspace & Misc Utilities

• src/pages/workspace/WorkspaceInitialPage.tsx
  • prop * For menu item title
• src/libs/MergeTransactionUtils.ts 
  • getDisplayValue()
    • For report field display
• src/libs/Notification/LocalNotification/BrowserNotifications.ts
  • pushReportCommentNotification()
    • For notification display

Why together: Miscellaneous utilities not fitting other categories, relatively simple usages.
Total usage: 3

12. Remaining Pages & Components

• src/pages/ShareCodePage.tsx
  • const title
    • Simple call with report as argument
• src/pages/RoomInvitePage.tsx
  • const reportName
    • Simple call with report as argument
• src/pages/TripChatNameEditPage.tsx
  • const currentChatName
    • Simple call with report as argument
• src/components/ReportWelcomeText.tsx
  • const reportName
    • Simple call with report as argument
• src/pages/home/report/ReportActionItemMessage.tsx
  • const fragment
    • Passed as callback
• src/pages/Search/AdvancedSearchFilters.tsx
  • getFlterInDisplayTitle
    • Simple call used in map

Why together: Remaining pages with simple usages.
Total usage: 6

13. Add tests

• Add performance tests
• Add unit tests

[sosek108]

Thanks @kstroz! Overall great summary

Quick notes:
1.
perf and unit tests can be one PR, but I do honestly think that at least unit should be provided together with implementation of computeReportName or new getReportName
2.

Either create new function that will allow us to migrate existing getReportName step by step or add argument to existing one that will allow to freely change between old and new logic for the time of migration.

I personally think that create new function definition will be more maintanable and writing tests would be easier. In the end we need only to remove old method and potentially rename the new.
3.
First PR should also introduce computeReportName which contains logic extracted from old getReportName. So in the end of first PR we have working logic that can be used both in derived values and in UI refactors
4.
First PR can have one correct implementation in UI to check if its working perfectly correct - QA Team will be able to spot problems from day 0. @neil-marcellini what do you think?
5.
This is to @neil-marcellini. What do you think about this PR granulation?

[sosek108]

@kstroz

one more thing to check. getReportName is dependant on variables that are not present in reportAttributes's dependencies. Eg. parentReportActionParam, invoiceReceiverPolicy. How we will handle that?

function getReportName(
    report: OnyxEntry<Report>,
    policy?: OnyxEntry<Policy> | SearchPolicy,
    parentReportActionParam?: OnyxInputOrEntry<ReportAction>,
    personalDetails?: Partial<PersonalDetailsList>,
    invoiceReceiverPolicy?: OnyxEntry<Policy> | SearchPolicy,
    reportAttributes?: ReportAttributesDerivedValue['reports'],
    transactions?: SearchTransaction[],
    isReportArchived?: boolean,
    reports?: SearchReport[],
    policies?: SearchPolicy[],
)

[kstroz]

@sosek108 I've updated my PR split to include your comments. Whenever you have time, please re-check it. Now I’ll look into the issue you mentioned in the comment below (about the missing dependencies).

[kstroz]

@sosek108 I think for those two arguments, we could create an additional function named getReportNameWithArgs.
From what I’ve checked in the code, we can extract the logic related to those two arguments and either return the result directly or prioritise those arguments if both are needed.
For cases where we need to perform two checks, I’d suggest something like this:

function getReportNameWithArgs(
    report: OnyxEntry<Report>,
    parentReportAction?: OnyxEntry<ReportAction>,
    invoiceReceiverPolicy?: OnyxEntry<Policy>,
    reportAttributes?: ReportAttributesDerivedValue['reports'],
): string {
    return (
        (parentReportAction && getReportNameWithParentReportAction(report, parentReportAction)) ||
        (invoiceReceiverPolicy && getReportNameWithInvoiceReceiverPolicy(report, invoiceReceiverPolicy)) ||
        getReportName(report, reportAttributes)
    );
}

For the rest of the cases, I’d just check if one of the functions returns something and fall back to getReportName().
I’m not sure if this approach works for you, maybe you’ll have a better idea.
I’d prefer to avoid adding this logic to the compute function since it would make it too bloated, but that might also be a possible solution. We can discuss this tomorrow when you're back, just to make sure I didn't miss anything.

[kstroz]

Another idea would be to change the getReportName() signature to getReportName(report, attributes, params?) and pass invoiceReceiverPolicy or parentReportActionParam in params.
To keep getReportName cleaner, since parentReportActionParam is returned in over 30 if statements, we could extract this logic before returning the new logic from design docs (reportAttributes[reportId].reportName || report.reportName).
It might also be a good idea to clean this up a bit, as this is one of the main reasons why getReportName() is currently so long.
We can also separate invoiceReceiverPolicy in the same way, although it’s only used in two if statements within the return. The only concern I have with this extraction is how much the order of the if statements in the current getReportName() matters.