fix: exclude thread reports from isChatUsedForOnboarding admin room fallback#83305
Conversation
|
@abzokhattab 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] |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
…allback When the onboarding NVP has no chatReportID, isChatUsedForOnboarding falls back to isAdminRoom(). Thread reports inherit chatType POLICY_ADMINS from their parent, causing the function to incorrectly return true for threads. This adds a !isChatThread() guard to the isAdminRoom fallback path so that threads inside the admins room are no longer treated as onboarding chats. Also adds a companion unit test to verify this behavior.
35aab29 to
4fc08c9
Compare
|
@abzokhattab |
this step is wrong ... we are not testing this on staging .. can you please change it |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-02-26.at.02.39.19.movAndroid: mWeb ChromeScreen.Recording.2026-02-26.at.02.37.20.moviOS: HybridAppScreen.Recording.2026-02-26.at.02.40.08.moviOS: mWeb SafariScreen.Recording.2026-02-26.at.02.42.24.movMacOS: Chrome / SafariScreen.Recording.2026-02-26.at.02.34.45.mov |
|
Updated. |
|
@abzokhattab
|
|
🚧 @srikarparsi has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/srikarparsi in version: 9.3.32-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.32-3 🚀
|
Explanation of Change
When the onboarding NVP exists but lacks a
chatReportID(transient state during guided setup),isChatUsedForOnboardingfalls back toisAdminRoom(). Thread reports created in the #admins room inheritchatType: POLICY_ADMINSfrom their parent, soisAdminRoom()returnstruefor those threads too. This causes the "Register for webinar" button (and other onboarding UI like the LHN tooltip and FreeTrial badge) to incorrectly appear in thread headers.This PR adds a
!isChatThread(optionOrReport)guard to theisAdminRoomfallback path insideisChatUsedForOnboarding(). A thread is never the onboarding chat itself — it is always a child of one — so this change is semantically correct and fixes the root cause at the utility function level rather than patching individual call sites.A companion unit test is also added to verify that a report with both
chatType: POLICY_ADMINSandparentReportID/parentReportActionIDset correctly returnsfalse.Fixed Issues
$ #81069
PROPOSAL: #81069 (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
Android: mWeb Chrome
iOS: Native
iOS mWeb Safari
macOS Chrome & macOS Safari