Skip to content

Extract LHN alternateText computation into shared ReportAlternateTextUtils - #99061

Merged
mountiny merged 21 commits into
Expensify:mainfrom
callstack-internal:fix/lhn-search-alt-text-parity
Sep 3, 2026
Merged

mountiny merged 21 commits into
Expensify:mainfrom
callstack-internal:fix/lhn-search-alt-text-parity

Conversation

@sosek108

@sosek108 sosek108 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Zero-behavior-change refactor. Extracts the entire LHN alternate-text (row preview) computation out of SidebarUtils.getOptionData into a new pure module, src/libs/ReportAlternateTextUtils.ts:

  • getReportAlternateText — the full special-action chain, actor prefixes, and welcome-message fallbacks, moved byte-for-byte out of SidebarUtils.
  • getLastMessageTextForReport and its dependency closure, moved from OptionsListUtils and re-exported there, so no import site had to change.
  • getWelcomeMessage / getRoomWelcomeMessage, moved from SidebarUtils (still available on its default export).

A new 145-case snapshot parity test (tests/unit/ReportAlternateTextParityTest.ts) guards this: the snapshots were captured against the pre-extraction code in a separate first commit, so CI proves there is zero output diff caused by the extraction itself.

This is groundwork for a follow-up PR that makes Search previews reuse this exact LHN pipeline instead of approximating it.

Fixed Issues

$ #98958
PROPOSAL:

Tests

  1. npm run test -- tests/unit/ReportAlternateTextParityTest.ts — 145 snapshot cases pass.
  2. npm run test -- tests/unit/SidebarUtilsTest.ts tests/unit/OptionsListUtilsTest.tsx — pass.
  3. Open the app, verify LHN rows show identical preview text as before for: plain messages, invite/remove members, renamed room, expense report previews, empty chats (welcome message). No visual change expected anywhere.
  • Verify that no errors appear in the JS console

Offline tests

N/A (pure refactor, no behavior change)

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Same as tests

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Side by side comparison (left is branch, right is staging)

Nagranie.z.ekranu.2026-08-26.o.15.29.02.mov

…arity

# Conflicts:
#	src/libs/OptionsListUtils/index.ts
# Conflicts:
#	src/libs/OptionsListUtils/index.ts
#	src/libs/SidebarUtils.ts
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/libs/OptionsListUtils/getChatPreviewParts.ts 96.15% <ø> (+0.61%) ⬆️
src/libs/OptionsListUtils/index.ts 91.29% <ø> (+3.75%) ⬆️
src/libs/SidebarUtils.ts 90.44% <100.00%> (+11.07%) ⬆️
src/libs/ReportAlternateTextUtils.ts 87.61% <87.61%> (ø)
... and 7 files with indirect coverage changes

@sosek108

Copy link
Copy Markdown
Contributor Author

@codex review
@MelvinBot review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 90f5ba6e1e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@MelvinBot

Copy link
Copy Markdown
Contributor

Code Review — LGTM ✅

This is a clean, genuinely behavior-preserving refactor. I verified the extraction is safe (byte-for-byte on the moved logic), the parity test is a real guard, and the LHN previews still render on web. No blocking issues found.

What this PR does

Extracts the LHN alternate-text (row preview) computation out of SidebarUtils.getOptionData and OptionsListUtils into a new pure module src/libs/ReportAlternateTextUtils.ts, re-exporting where needed so no import site had to change. Groundwork for making Search previews reuse the exact LHN pipeline.

Verification

1. No behavior drift. The moved bodies (getReportAlternateText, getLastMessageTextForReport + its dependency closure, getWelcomeMessage, getRoomWelcomeMessage, and the getChatPreviewParts actor helpers) are identical to their pre-extraction versions on main. The one intentional signature change is correct: the old inline !!reportNameValuePairs?.private_isArchived became a privateIsArchived param, wired correctly at the call site in SidebarUtils.

2. No circular-import hazard. Dependency direction is one-way (OptionsListUtils/getChatPreviewPartsReportAlternateTextUtils); the new module does not import back from them. Its only top-level executable code is two Onyx.connectWithoutView callbacks that run asynchronously, so there's no module-init TDZ risk. The author also proactively broke a re-export cycle by relocating getPersonalDetailsForAccountIDs.

3. Re-exports resolve. getPersonalDetailsForAccountIDs/getPersonalDetailForAccountID moved to PersonalDetailsUtils (verbatim, no divergence) and all ~11 call sites were updated. The deprecatedCachedOneTransactionThreadReportIDs cache + its Onyx connect were moved cleanly with no orphaned duplicate left behind.

4. The parity test is a real guard, not a tautology. Confirmed against the commit graph: the snapshot baseline was captured in a commit where ReportAlternateTextUtils.ts did not yet exist (i.e. against the old inline code), and the snapshot file is byte-identical at HEAD. So the 145-case suite genuinely locks the old output — a valid characterization test.

App test (web)

Drove the LHN on dev NewDot web to confirm previews still render:

Step Status Observation
LHN list loads with preview text under report name ✅ pass Concierge row shows title + timestamp + full preview text
Open a report, return — preview still correct ✅ pass Preview text identical after Inbox → Home → Inbox
No blank / undefined / [object Object] previews ✅ pass Both strings absent from the page
No console errors while browsing LHN ⚠️ partial No visible errors/broken UI, but web session tooling can't inspect the console directly

Note: the test account has only one LHN row (Concierge), so varied preview types (attachments, IOU/expense, task previews) weren't exercised interactively — those are covered by the parity snapshots and existing SidebarUtilsTest/OptionsListUtilsTest instead.

Screenshots

LHN Inbox list showing Concierge row with preview text, report open in main pane

LHN Inbox list showing Concierge row with preview text, report open in main pane

LHN Inbox preview text unchanged after navigating away to Home and back

LHN Inbox preview text unchanged after navigating away to Home and back

Minor observations (non-blocking)

  • The parity snapshots drive only through SidebarUtils.getOptionData and don't exercise the getLastMessageTextForReport path via OptionsListUtils.createOption. Since the moved bodies are byte-identical this doesn't affect correctness, but if you want belt-and-suspenders coverage of that second entry point, consider adding a parity case through createOption too.

Nice groundwork for the Search-preview follow-up. 👍


view run

@sosek108
sosek108 marked this pull request as ready for review August 27, 2026 06:59
@sosek108
sosek108 requested review from a team as code owners August 27, 2026 06:59
@melvin-bot
melvin-bot Bot requested review from a team, garrettmknight and thelullabyy and removed request for a team August 27, 2026 06:59
@melvin-bot

melvin-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

@thelullabyy Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed request for a team August 27, 2026 06:59
@luacmartins

Copy link
Copy Markdown
Contributor

@sosek108 conflicts

# Conflicts:
#	src/libs/OptionsListUtils/index.ts
#	src/libs/SidebarUtils.ts
#	tests/unit/OptionsListUtilsTest.tsx
@sosek108

sosek108 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@sosek108 conflicts

resolved. I'll check again in few hours

@thelullabyy

Copy link
Copy Markdown
Contributor

Test failure doesn't cause by this PR

@melvin-bot
melvin-bot Bot requested a review from francoisl September 2, 2026 19:00
@francoisl

Copy link
Copy Markdown
Contributor

The tests should be fixed on main now.

@sosek108

sosek108 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for approving! I've merged main so checks should pass. Let's see 👀

@mountiny mountiny left a comment

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.

LGTM, mainly moving things around

@mountiny

mountiny commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Given this is zero change refactor but with huge surface, I will merge this now to avoid conflicts and risking we miss something. I hope Frank wont be mad

@mountiny
mountiny merged commit 40616b2 into Expensify:main Sep 3, 2026
34 checks passed
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

OSBotify commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@IuliiaHerets

Copy link
Copy Markdown

Hi @sosek108. Is it No QA PR?

cc @mountiny @luacmartins @francoisl

@sosek108

sosek108 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@IuliiaHerets This PR does not change behavioral part of LHN but I think that it should be tested - compared to staging if there is no change in subtext of LHN entries

@OSBotify

OSBotify commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.70-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

No help site changes are required for this PR, so I did not create a draft docs PR.

This is a zero-behavior-change refactor. It moves the LHN alternate-text (row preview) computation out of SidebarUtils.getOptionData into a new src/libs/ReportAlternateTextUtils.ts, and relocates getLastMessageTextForReport, getWelcomeMessage, and getRoomWelcomeMessage. Nothing a user sees changes.

What I checked
  • Files changed: cspell.json, src/libs/ReportAlternateTextUtils.ts (new), src/libs/SidebarUtils.ts, src/libs/OptionsListUtils/index.ts, src/libs/OptionsListUtils/getChatPreviewParts.ts, plus four test files and one snapshot file.
  • No src/languages/*.ts changes — no user-facing copy was added, removed, or reworded.
  • No new feature, setting, tab, or button — nothing new to document, and no existing label to rename.
  • The cspell.json change only allowlists the new snapshot file.
  • The PR's own parity test (tests/unit/ReportAlternateTextParityTest.ts, 145 snapshot cases) exists specifically to prove the extraction produces identical output.
  • Searched docs/articles for anything describing LHN preview or alternate text. No article documents this text, so there is nothing to update.

Help site articles document what users do and see. A pure internal module extraction has no surface there.

If the planned follow-up PR (making Search previews reuse this pipeline) changes what text users actually see in Search results, that one may well need a docs update — re-run this request on that PR when it opens.

@sosek108, no help site PR was created because no changes are needed. Please confirm you agree, or tell me what user-facing behavior you think changed and I'll take another look.


view run

@IuliiaHerets

Copy link
Copy Markdown

@sosek108 can you please share QA steps then?

@OSBotify

OSBotify commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.72-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

Help site review: no changes required — no draft PR created

This PR is a zero-behavior-change internal refactor, so nothing under App/docs/articles needs to be updated.

Why

  • Nothing user-facing changed. The diff touches only src/libs/* (code movement), tests/unit/*, and one cspell.json ignore entry. No src/languages/*.ts, no components, no new screens, settings, tabs, or buttons.
  • The LHN preview text itself is identical. getReportAlternateText, getLastMessageTextForReport, and getWelcomeMessage / getRoomWelcomeMessage were moved out of SidebarUtils and OptionsListUtils into the new ReportAlternateTextUtils, with OptionsListUtils re-exporting them so no import site changed.
  • That parity is enforced by a test. tests/unit/ReportAlternateTextParityTest.ts snapshots 145 cases captured against the pre-extraction code in a separate first commit, so CI proves zero output diff. thelullabyy also confirmed this in the reviewer checklist.

What I checked in the help site

I searched App/docs/articles for content describing chat list preview text, welcome messages, and room descriptions. The help site documents what those features do for users — not the module that computes the string — and none of that user-facing behavior moved. There is no article whose accuracy is affected by this PR.

Because no changes are required, I did not create a draft help site PR, apply the HelpDot label, or assign one. Those steps were conditional on docs updates being needed.


@sosek108, no help site PR was created because this refactor produces identical user-facing text. If you believe a docs update is still needed here, reply with what behavior you want documented and I'll open the draft PR.


view run

@OSBotify

OSBotify commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.72-2 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants