Suppress promo/training/onboarding modals during supportal sessions - #96284
Conversation
Extend the app-review supportal/copilot gate from #96231 into a single shared guard (shouldSuppressPromotionalUI) and reuse it across all promo, training, and onboarding entry points so this UI is hidden for supportal and copilot sessions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@linhvovan29546 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] |
|
This PR adds a new |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 020ab35c31
ℹ️ 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".
| }, [hasCompletedOnboarding]); | ||
|
|
||
| const isAllOnyxLoaded = !isLoadingOnyxValue(isLoadingAppMetadata, accountMetadata, dismissedProductTrainingMetadata, tryNewDotMetadata, onboardingMetadata); | ||
| const isAllOnyxLoaded = !isLoadingOnyxValue(isLoadingAppMetadata, dismissedProductTrainingMetadata, tryNewDotMetadata, onboardingMetadata); |
There was a problem hiding this comment.
In copilot sessions where ONYXKEYS.ACCOUNT is still loading, useShouldSuppressPromotionalUI() evaluates the missing account as false; because this readiness check no longer includes the account metadata, isEligible can become true and schedule the AI promo before delegatedAccess.delegate arrives. That regresses the previous guard, which waited for accountMetadata, so large/slow copilot accounts can still see the promo modal this change is meant to suppress.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 75f3a59. Restored the account load-metadata gate in isAllOnyxLoaded so eligibility waits for ONYXKEYS.ACCOUNT to load before scheduling the promo; suppression itself still flows through useShouldSuppressPromotionalUI.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Sorry, this should have been created as a draft, not ready for a review yet |
- Use a default export for the shouldSuppressPromotionalUI selector to satisfy import/prefer-default-export - Swap the two Onyx.connectWithoutView subscriptions in PromotionalUIUtils for Onyx.connect Co-authored-by: Cursor <cursoragent@cursor.com>
Delete PromotionalUIUtils (which used Onyx.connect, forbidden by the no-onyx-connect lint rule) and instead reuse existing subscriptions: - Welcome guards read supportal state from their existing session var - OnboardingGuard reads it from a new GuardContext.isSupportalSession computed in guards/index.ts from its existing session subscription - Report reuses NetworkStore.isSupportAuthToken for the change-policy modal Co-authored-by: Cursor <cursoragent@cursor.com>
Compute suppression in DynamicReportChangeWorkspacePage via useShouldSuppressPromotionalUI (useOnyx) and pass it into changeReportPolicy, removing the NetworkStore read from the Report action. Welcome guards now read supportal state from GuardContext.isSupportalSession in evaluate(). Co-authored-by: Cursor <cursoragent@cursor.com>
useShouldSuppressPromotionalUI now subscribes with primitive-returning selectors (isSupportalSessionSelector, isActingAsDelegateSelector) so consumers only re-render when the derived boolean can change (PERF-11). Removes the now-unused combined selector and its test. Co-authored-by: Cursor <cursoragent@cursor.com>
Covers regular sessions, support auth token, mid-transition isSupportAuthTokenUsed, and copilot (delegate) sessions. Co-authored-by: Cursor <cursoragent@cursor.com>
The guard's evaluate() reads context.isSupportalSession, so the test must pass the flag via the guard context rather than the module-level session. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Covers support auth token, mid-transition isSupportAuthTokenUsed, and non-supportal/empty/undefined sessions. Co-authored-by: Cursor <cursoragent@cursor.com>
Restore the account load-metadata gate in useAIFeaturesPromoModal so the promo isn't scheduled before ACCOUNT loads (which would miss the copilot/delegate signal in slow sessions). Suppression still flows through useShouldSuppressPromotionalUI. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 512bc6a7c0
ℹ️ 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".
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| const isInvitedOrGroupMember = (hasNonPersonalPolicy || wasInvitedToNewDot) ?? false; | ||
| if (isMigratedUser || isInvitedOrGroupMember) { | ||
| if (isMigratedUser || isInvitedOrGroupMember || shouldSuppressPromotionalUI) { |
There was a problem hiding this comment.
In copilot sessions where ONYXKEYS.ACCOUNT is still loading, useShouldSuppressPromotionalUI() reports false because the delegate selector treats missing account data as non-delegate, and this effect does not wait for that key before it can call startOnboardingFlow() directly. Since the comment below notes this bypasses OnboardingGuard, a slow delegate account with incomplete onboarding can still be reset into onboarding before delegatedAccess.delegate arrives; please gate this decision on the suppression key metadata or make the suppression hook fail closed while loading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@mountiny Could you please take a look at this comment?
There was a problem hiding this comment.
Fixed in f81c7b5 by making the suppression hook fail closed: useShouldSuppressPromotionalUI() now returns true (suppress) while SESSION/ACCOUNT are still loading (via isLoadingOnyxValue on their metadata). So a copilot/supportal session is never briefly treated as normal, and this effect won't startOnboardingFlow() before delegatedAccess.delegate arrives. Added unit tests for both loading cases.
|
I don't have access to a real Supportal session to test this directly, so I verified the behavior by mocking Supportal (and Copilot) state locally to exercise the exact code paths this PR changes. The suppression logic behaved as expected in every case I tested. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApptelegram-cloud-document-5-6278098973831994522.1.mp4Android: mWeb Chrometelegram-cloud-document-5-6278098973831994521.1.mp4iOS: HybridAppScreen.Recording.2026-07-23.at.18.47.58.movScreen.Recording.2026-07-23.at.23.06.19.moviOS: mWeb SafariScreen.Recording.2026-07-23.at.23.06.19.movMacOS: Chrome / SafariScreen.Recording.2026-07-23.at.22.43.07.movScreen.Recording.2026-07-23.at.22.44.59.mov |
|
I will test the supportal on the adhoc |
|
Supportalled to a fresh account that should have onboarding modal - not shown ✅ |
Return true (suppress) while SESSION/ACCOUNT are still loading so a supportal/copilot session is never briefly treated as normal - e.g. a copilot on an un-onboarded account could otherwise be reset into onboarding before delegatedAccess.delegate loads. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@linhvovan29546 can you take a final look please? |
| const [isActingAsDelegate, accountMetadata] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isActingAsDelegateSelector}); | ||
| const shouldSuppressPromotionalUI = useShouldSuppressPromotionalUI(); | ||
| // Only used for its load metadata: the promo must wait for ACCOUNT to load so a copilot/delegate session is detected before eligibility is decided | ||
| const [, accountMetadata] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isActingAsDelegateSelector}); |
There was a problem hiding this comment.
Now that useShouldSuppressPromotionalUI already handles the metadata. I think this code is now dead weight and can probably be removed
There was a problem hiding this comment.
Good call — done in 206ddeb.
Since useShouldSuppressPromotionalUI() now fails closed (returns true while SESSION/ACCOUNT are still loading), the !shouldSuppressPromotionalUI term in isEligible already blocks the promo until ACCOUNT has loaded. That made the separate ACCOUNT read — kept only for its load metadata in isAllOnyxLoaded — redundant, so I removed it along with the now-unused isActingAsDelegateSelector import, and updated the comment on the suppression hook line.
fmt and lint-changed pass; the remaining typecheck-tsgo errors are pre-existing and unrelated to this file.
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
@linhvovan29546 ready for final tests |
|
@mountiny EsLint failed |
|
@MelvinBot please fix the failing eslint ci |
… test Co-authored-by: Vit Horacek <mountiny@users.noreply.github.com>
|
Fixed the failing ESLint check in bd82470. The error was Verified locally with |
|
@stitesExpensify 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] |
|
🚧 cristipaval 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/cristipaval in version: 9.4.45-0 🚀
|
Help site review — no changes requiredI reviewed the changes in this PR against Expensify's help site content under Why: This PR suppresses promotional, training, and onboarding UI (proactive app review, AI Features Promo, product-training tooltips, migrated-user/submit-plan welcome, onboarding flow, track-training, change-policy/multi-scan/hold-reject educational modals) specifically during supportal (Expensify staff support-login) and copilot sessions. It's a session-scoped UX refinement for internal/agent sessions — it doesn't add, remove, or change any customer-facing feature, workflow, setting, pricing, or documented behavior. Details
Since no documentation changes are needed, I did not create a draft help site PR. |
|
Hi @mountiny. What account should we use as a customer ND account? |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.4.45-14 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Promotional, training, and onboarding UI is disruptive during ND Supportal sessions (Expensify staff logged into a customer account). It blocks navigation, slows initial load, and is embarrassing during customer demos. Supportal agents also can't act on these modals — e.g. the proactive app review modal returns a 411 and re-appears in a loop.
#96231 already gated the proactive app review modal on supportal/copilot. This PR reuses that same check across every promo/training/onboarding entry point, so all of this UI is suppressed for supportal (and copilot) sessions.
How it is shared:
isSupportalSessionSelectorinsrc/selectors/Session.tsis the single supportal predicate (also reused by the existingisSupportalSession()action accessor).useShouldSuppressPromotionalUI(supportal OR copilot) is used by React entry points. It readsSESSION/ACCOUNTviauseOnyxselectors and fails closed — it returnstrue(suppress) while those keys are still loading, so a supportal/copilot session is never briefly treated as normal.GuardContext.isSupportalSession, computed once insrc/libs/Navigation/guards/index.ts.OnboardingGuardadditionally suppresses copilot via its existingACCOUNTsubscription; the migrated-user and submit-plan welcome guards are supportal-only by design (see PR discussion).DynamicReportChangeWorkspacePage(React), which passes the suppression flag into the action rather than reading Onyx at module level.Wired into: proactive app review, AI features promo, product-training tooltips, migrated-user welcome, submit-plan welcome, onboarding (guard + hook + navigator mount), track-training modal, change-policy educational modal, multi-scan educational modal, and hold/reject educational modals (the underlying hold/reject action still runs; only the educational modal is skipped).
SupportalPermissionDeniedModal(the 411 reactive modal) is intentionally unchanged.Fixed Issues
$ #96282
PROPOSAL:
Tests
authTokenType=support).SupportalPermissionDeniedModalstill appears (411 behavior unchanged).Offline tests
Same as tests. This change only gates modal/UI visibility based on session and delegate state; it does not alter any network-dependent behavior for normal users.
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 / Safari