Add Chronos display name constant to fix search showing Chat Report#84391
Add Chronos display name constant to fix search showing Chat Report#84391
Conversation
Add a dedicated effect in OptionsListContextProvider that watches reportAttributes.reports and updates option display names (text) when the derived value provides a name that differs from what is currently stored. This fixes the timing gap where options can be initialized before derived report names are computed, causing reports like Chronos to show "Chat Report" instead of their correct name in search results. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot Well! It is noticed that the effect used for updating the options only for changed reports is already recomputing the report name correctly. So, it does not make sense to add another effect for or updating option display names when reportAttributes provides a name different than what's currently stored. Looks like the alternative solutions are the best way to move forward here. What do you think? |
|
You make a good point. Looking at the existing I agree the alternative solution — mirroring the Concierge pattern for Chronos — is the better approach. It's simpler, avoids an extra effect that iterates all options on every The changes would be:
This matches the pattern already established for Concierge and fixes the issue regardless of personal details loading timing. I can update this PR with that approach if you'd like — just reply with |
|
@MelvinBot implement this |
Mirror the existing Concierge pattern by adding CHRONOS_DISPLAY_NAME and a chatIncludesChronos check in both computeReportName and getReportName. This ensures Chronos always resolves to its correct display name regardless of personal details loading timing, instead of relying on a dedicated effect in OptionsListContextProvider. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Updated the PR to use the alternative approach as discussed. Here's what changed: Removed:
Added:
This mirrors the existing Concierge pattern exactly: when the report includes Chronos as a participant, the display name is set to "Chronos" immediately without depending on personal details being loaded first. |
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Fixed the failing |
Use else-if so the Chronos check doesn't overwrite the Concierge name when both account IDs resolve to the same value (e.g. in dev/test envs). Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Fixed the failing Root cause: In the Fix: Changed the independent |
|
@MelvinBot Please resolve the conflicts |
…layNameInSearch Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Resolved the merge conflicts by merging |
These patches (for sentry 10.24.0 and react-native 7.6.0) were superseded by updated versions on main but remained as merge artifacts. Removing them fixes the validate-patches CI check. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Fixed the failing |
The branch was based on an older version of main that had files since removed. After merging with current main, these 106 stale files were incorrectly retained as merge artifacts. Removing them to fix the Changed files ESLint check, which was flagging ESLint errors in these unrelated files. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
Fixed the failing |
|
A preview of your ExpensifyHelp changes have been deployed to https://df3495e0.helpdot.pages.dev ⚡️ |
|
The previously failing checks (TypeScript, ESLint, Changed files ESLint) were caused by stale files from the old branch base that no longer exist on
|
|
@MelvinBot, The approach of |
|
I have failed to help you. I let you down and for that I am sorry. Please try again later. This may be due to a Claude outage (check here). Failure details: https://github.com/Expensify/Expensify/actions/runs/23304205060 |
|
@MelvinBot Please re-attempt addressing the comment here |
Mirror the Concierge pattern: resolve Chronos display name at the personal details level so OptionListContextProvider picks up the correct name via its personal details change effect, triggering a rerender of the search auto-complete list. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
I investigated the rerender issue and found the root cause. Problem: Why Concierge works: Concierge has an additional check in Fix: Added the same pattern for Chronos in if (passedPersonalDetails?.accountID === CONST.ACCOUNT_ID.CHRONOS) {
displayName = CONST.CHRONOS_DISPLAY_NAME;
}This ensures "Chronos" resolves at the personal details level — just like Concierge — so |
|
The failing check ( Analysis: The perf test Evidence:
This appears to be a flaky perf test. Re-running the checks should resolve it. |
Explanation of Change
When searching for reports like Chronos, the search results display "Chat Report" instead of the correct name. This happens because the display name for 1:1 DMs depends on participant personal details being loaded, but Chronos's personal details may not be available at initialization time — causing the fallback to the server default
report.reportName("Chat Report").Concierge doesn't have this issue because it has a dedicated check that short-circuits to
CONST.CONCIERGE_DISPLAY_NAMEbefore attempting to resolve from personal details. This PR applies the same pattern for Chronos:CHRONOS_DISPLAY_NAME: 'Chronos'constant inCONST/index.tschatIncludesChronoscheck incomputeReportName(ReportNameUtils.ts) — used by derived report attributes and searchgetReportName(ReportUtils.ts) — used by the chat headerThis ensures Chronos always resolves to its correct display name regardless of personal details loading timing.
Fixed Issues
$ #83447
PROPOSAL: #83447 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A - Logic-only change, no UI modifications
Android: mWeb Chrome
N/A - Logic-only change, no UI modifications
iOS: Native
N/A - Logic-only change, no UI modifications
iOS: mWeb Safari
N/A - Logic-only change, no UI modifications
MacOS: Chrome / Safari
N/A - Logic-only change, no UI modifications
Screenshots/Videosundefined