Skip to content

Add a one-click deeplink that signs the user in and creates a Submit workspace - #98887

Open
abzokhattab wants to merge 13 commits into
Expensify:mainfrom
abzokhattab:abzokhattab/98644-submit-onboarding-deeplink
Open

Add a one-click deeplink that signs the user in and creates a Submit workspace#98887
abzokhattab wants to merge 13 commits into
Expensify:mainfrom
abzokhattab:abzokhattab/98644-submit-onboarding-deeplink

Conversation

@abzokhattab

@abzokhattab abzokhattab commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

There's no way to link directly to "start with Submit" from an email today: creating a Submit workspace requires a signed-in user to pick the intent in the onboarding UI (or click "Get started" in Old Dot pricing), neither of which is reproducible with a plain URL.

This PR adds an intent param to the onboarding route so a single link produces "click link → sign in → Submit workspace created". ROUTES.ONBOARDING_ROOT.getRoute() now takes an optional intent and builds onboarding?intent=submit, with the accepted values in the new CONST.ONBOARDING_INTENTS. Those are short, stable aliases rather than the internal onboarding choice strings (newDotEmployer), since they get embedded in emails we can't redeploy.

The intent survives the logged-out → logged-in transition by riding along in the deeplink itself, the same mechanism exitTo=workspace/new already relies on. getOnboardingIntentFromUrl reads it either from a direct link (/onboarding?intent=submit) or from the exitTo of a transition or magic link, encoded or not.

Who the link acts on. Per the Slack discussion on the issue, this is going out to existing users, so it only acts on recipients who have completed guided setup. For them ApplySubmitOnboardingIntent creates the workspace through useAutoCreateSubmitWorkspace — the same hook the onboarding purpose page and the Submit plan welcome modal already use — and skips CompleteGuidedSetup, so their recorded onboarding choice is left alone. Anyone who still has onboarding ahead of them is left to it untouched, since that flow already offers the Submit outcome. That fallback is why nothing here suppresses or reroutes onboarding, and idempotency comes free with the hook, which already skips creation when there's an editable group workspace, an onboarding policy ID, or restricted policy creation.

Where they land is not encoded in the link. The hook finishes by calling navigateToSubmitWorkspaceAfterOnboardingWithMicrotaskQueue — today Spend > Expenses with the side panel open on #admins — so changing that destination later is a one-line change and every link already sitting in an inbox keeps working.

On the route name: onboarding?intent=submit names a flow most recipients never see, but the param genuinely is an onboarding intent (it resolves to the same CONST.ONBOARDING_CHOICES.EMPLOYER the purpose step would set), and for recipients who haven't onboarded it's literally where they land. Happy to rename if you'd prefer something else.

Open questions for the reviewer

  1. Beta gating. The issue lists submit2026 as a beta to respect or graduate. In the current codebase submit2026 is CONST.POLICY.TYPE.SUBMIT — a policy type, not an entry in CONST.BETAS — and Submit workspace creation is gated by business logic (isRestrictedPolicyCreation, existing editable workspaces), not a beta check. So this PR adds no beta gate. Please confirm that matches the intended rollout state.
  2. HybridApp. Verified on web. Native transitions go through a separate HybridApp path (getCurrentUrl() returns '' there), and useOnboardingDeeplinkIntent reads initialURL to cover it, but it needs testing on a HybridApp build.

Fixed Issues

$ #98644
PROPOSAL:

Tests

On iOS, force-close the app before opening a link so it arrives as a cold launch; the intent is applied at most once per app process.

  1. Signed-out recipient (the email-link case)
    1. Sign out of New Expensify.
    2. Request a magic link for an account that has completed onboarding and has no Submit workspace, then open it:
      • Web: https://dev.new.expensify.com:8082/v/<accountID>/<validateCode>?exitTo=onboarding%3Fintent%3Dsubmit
      • iOS: run xcrun simctl openurl booted "new-expensify://v/<accountID>/<validateCode>?exitTo=onboarding%3Fintent%3Dsubmit" in terminal
    3. Verify you're signed in and land on Spend > Expenses with the side panel open on the #admins room. Verify the onboarding flow never appears.
    4. Go to Workspaces and verify exactly one workspace was created and its plan is Submit.
  2. Signed-in recipient
    1. On another account that has finished onboarding and has no Submit workspace, open the link:
      • Web: https://dev.new.expensify.com:8082/onboarding?intent=submit
      • iOS: run xcrun simctl openurl booted "new-expensify://onboarding?intent=submit" in terminal
    2. Verify the same outcome as test 1, and that the Submit plan welcome modal does not appear.
  3. Recipient who never completed onboarding
    1. On a brand new account that has not been through guided setup, open the link.
    2. Verify the normal onboarding flow appears and behaves exactly as it does on main — no step is skipped and no workspace is created behind it.
  4. Idempotency
    1. With the workspace from test 1 or 2 in place, open the same link again. Verify no second workspace is created and onboarding does not start.
    2. On an account that already belongs to a Team or Corporate workspace, open the link. Verify no Submit workspace is created.

Offline tests

Creating the workspace is an optimistic Onyx write, so offline behaviour matches the existing onboarding purpose flow: the workspace appears immediately with a pending state and the request is queued.

QA Steps

Same as the Tests section above.

  • 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
Screen.Recording.2026-08-23.at.16.33.07.mov
Screen.Recording.2026-08-23.at.16.34.35.mov
Screen.Recording.2026-08-23.at.16.35.42.mov
iOS: mWeb Safari
Screen.Recording.2026-08-23.at.16.36.36.mov
Screen.Recording.2026-08-23.at.16.38.04.mov
MacOS: Chrome / Safari
1.mov
2.mov
3.mov
4.mov

Marketing emails need a plain URL that drops the recipient into a ready-to-use
Submit workspace. Today that outcome is only reachable by picking the intent in
the onboarding UI, so it can't be expressed as a link.

Support an `intent` param on the onboarding route and act on it once the
authenticated screens mount, reusing the existing auto-create hook so the
existing-workspace and restricted-policy-creation guards keep repeat clicks
idempotent.
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Coverage Δ
src/CONST/index.ts 94.89% <100.00%> (+0.03%) ⬆️
src/hooks/useOnboardingDeeplinkIntent.ts 100.00% <100.00%> (ø)
src/libs/Navigation/AppNavigator/AuthScreens.tsx 83.15% <100.00%> (+0.17%) ⬆️
src/libs/getOnboardingIntentFromUrl.ts 100.00% <100.00%> (ø)
src/DeepLinkHandler.tsx 85.00% <0.00%> (ø)
src/ROUTES.ts 26.64% <0.00%> (ø)
...AppNavigator/SubmitIntentDeeplinkHandler/index.tsx 75.00% <75.00%> (ø)
...entDeeplinkHandler/ApplySubmitOnboardingIntent.tsx 7.14% <7.14%> (ø)
... and 11 files with indirect coverage changes

Creating the Submit workspace out-of-band left users stranded: the
onboarding navigator mounts straight from the deeplink URL and picks its
entry step from the account's domain, so it settled on the work email
step before the handler ran, and startOnboardingFlow cannot re-route a
navigator that is already in the root state. The workspace ended up
orphaned behind a modal the user could not get past.

Let the deeplink pick the onboarding entry step instead, so the existing
EMPLOYER path creates the workspace and nothing competes for navigation.
Users who already finished onboarding never enter that flow, so for them
the workspace is still created directly.
…rding

The link is going out to existing users, so dropping them into guided setup
asks questions they have already answered. Skip the onboarding UI entirely and
create the workspace outright, leaving them where the "Submit to my employer"
flow normally lands: Spend with the #admins room in the side panel.

Onboarding is suppressed for the session in both places that can start it, the
router and the navigation guard, so nothing pulls the user into the flow while
the workspace is being created.
The link is scoped to existing users with an intent already set, so anyone who
still has guided setup ahead of them is left to the normal onboarding flow,
which already offers the Submit outcome. That removes the need to suppress
onboarding at all, so the router, the navigation guard and the shared
workspace-creation hook go back to their original behaviour.

Also corrects a stale comment describing the post-creation destination as
Categories; the shared helper navigates to Spend with #admins in the side panel.
Use US spelling for "recognize" and drop the percent-encoded exitTo examples,
which cspell reads as the unknown words "Fintent" and "Dsubmit".
@abzokhattab

Copy link
Copy Markdown
Contributor Author

@MelvinBot @codex review the PR, find bugs or regressions

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b21e7156b

ℹ️ 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".

Comment thread src/libs/getOnboardingIntentFromUrl.ts Outdated
Comment thread src/hooks/useOnboardingDeeplinkIntent.ts
@MelvinBot

Copy link
Copy Markdown
Contributor

Review: intent=submit onboarding deeplink

Reviewed the diff and traced the runtime paths (parsing, the deeplink handler, useAutoCreateSubmitWorkspace, the onboarding-navigator gating). No blocking bugs or regressions found — the design is sound: the handler is a sibling that only mounts (and only subscribes to Onyx) once the intent is recognized, idempotency correctly falls out of useAutoCreateSubmitWorkspace's existing-editable-group-policy guard (Submit workspaces are group policies), and completed users never register the onboarding navigator so there's no onboarding flash. CI is green apart from the human checklist items, and the new unit test passes.

Two low-severity notes below — neither blocks merge.

1. getPathWithQuery silently drops the intent for slash-less custom-scheme URLs (latent)

getOnboardingIntentFromUrl.ts:27 strips scheme+host with /^[a-z][\w+.-]*:\/\/[^/]*/i. For a custom-scheme deeplink with no slash after the host — new-expensify://onboarding?intent=submit[^/]* greedily consumes onboarding?intent=submit, the whole string matches, and the function returns '', so the intent is lost.

This isn't a live break for the shipped format: native opens via the /v/<id>/<code>?exitTo=… magic link (only /v is registered as an https deeplink prefix in AndroidManifest.xml, not /onboarding), which is an https URL with a real slash and parses correctly. Since you flagged HybridApp/native as untested in the PR, worth a defensive tweak so a new-expensify://onboarding?… link (or any future slash-less variant) doesn't fail silently — e.g. anchor on the first /, ?, or end-of-host instead of [^/]*.

2. setSubmitMigrationModalShown() is called even when no workspace gets created

In ApplySubmitOnboardingIntent.tsx:51 the NVP write runs unconditionally, before autoCreateSubmitWorkspace decides whether to create anything. For a recipient who already owns a Team/Corporate workspace (test 5), no Submit workspace is created but the migration-modal-shown NVP is still persisted (an extra SetNameValuePair write). No user-visible bug today — SubmitPlanWelcomeModalGuard already gates the modal on groupPolicies.length === 0, so those users would never see it anyway — but it does permanently foreclose that nudge for the narrow case where such a user later loses their group workspace. Fine to leave as-is if that's intended; flagging since it's a side effect that fires on a path that otherwise no-ops.

Things I checked that are fine
  • Idempotency: after the first click, the created Submit workspace makes hasEditableGroupPolicy true, so shouldCreateWorkspace is false on repeat clicks (useAutoCreateSubmitWorkspace.ts:66). ✅
  • Never-onboarded fallback: isOnboardingCompleted === false returns early after latching hasRun, leaving normal onboarding untouched. ✅
  • No onboarding flash for completed users: the onboarding navigator screen is only registered when isOnboardingCompleted === false (AuthScreens.tsx:342), so /onboarding?intent=submit has no route to render into. ✅
  • URL parsing: direct link, encoded/unencoded exitTo, magic link, and the negative cases all resolve correctly given getSearchParamFromPath's double-decode; the unit test coverage matches. ✅
  • Web latch timing: useState(getCurrentUrl) runs synchronously in the sibling handler's first render, before React Navigation rewrites the browser URL. ✅

⚠️ I couldn't get runtime confirmation on web — the prestarted test session rendered a blank page (dev server itself was healthy), so the signed-in create/idempotency flow wasn't exercised end-to-end. Findings above are from code tracing. Worth manually running Tests 1, 4, and 5 before merge, plus a HybridApp/native pass given note #1.

Custom-scheme links like new-expensify://onboarding?intent=submit put the route
where a host would sit, so stripping the origin discarded the route and the
intent was lost. Links opened while the app was already running were missed too,
since the latched URL is stale by then and only secure links were recorded.
@abzokhattab

Copy link
Copy Markdown
Contributor Author

Thanks both — three findings between you, two of which were real native bugs. Fixed in 3215c16.

Fixed: custom-scheme URLs lost the route (Codex P1 #1 / Melvin #1)

Confirmed and reachable, not just latent: new-expensify:// is a registered deeplink prefix in prefixes.ts. In new-expensify://onboarding?intent=submit the route sits exactly where a host sits in a web URL, so stripping the origin discarded it and the intent came back undefined.

The origin is now stripped differently per scheme — host dropped for http(s), scheme only for custom schemes, plus the app://-/ placeholder host. Anchoring the host on [^/?#]* also fixes https://dev.new.expensify.com:8082/..., where the port previously survived. Four new test cases cover these.

Fixed: warm-app links were never seen (Codex P1 #2)

Also correct. InitialURLContextProvider only calls getInitialURL() once, and the warm listener in DeepLinkHandler records into that context only for secure links, so a link opened against an already-running app left urlAtMount stale and the handler never fired.

Extended that same condition to onboarding-intent links rather than adding a parallel mechanism — it is the precedent already in place for exactly this problem.

Not changing: the migration NVP write (Melvin #2)

Correct that setSubmitMigrationModalShown() runs before autoCreateSubmitWorkspace decides whether to create anything, so a recipient who already owns a Team/Corporate workspace gets the NVP without a workspace.

Leaving it. Making it conditional means duplicating !isRestrictedPolicyCreation && !onboardingPolicyID && !hasEditableGroupPolicy in the handler, and that copy would drift from the hook. It also reads as correct to me semantically: the NVP records that the user was offered the Submit migration, and clicking the link is that offer, delivered over email instead of the modal. Suppressing a later nudge for someone who already took the emailed one seems right. Happy to revisit if you disagree.

Unit tests, lint, typecheck and the React Compiler check are green. Native still needs a manual pass, which is where all three of these live.

@abzokhattab abzokhattab changed the title [Submit] Add a one-click deeplink that signs the user in and creates a Submit workspace Add a one-click deeplink that signs the user in and creates a Submit workspace Aug 20, 2026
getRouteFromLink already strips whichever linking-config prefix matched, which
covers the app schemes and the dev server's port, and it is what DeepLinkHandler
uses to read report deeplinks.
@abzokhattab

Copy link
Copy Markdown
Contributor Author

@MelvinBot @codex review the PR, find bugs or regressions

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fad41a199

ℹ️ 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".

Comment thread src/hooks/useOnboardingDeeplinkIntent.ts
cspell reads the percent-encoded literals as the unknown words "Fintent" and
"Dsubmit", and encoding explicitly shows what the link actually carries.
@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 I reviewed the diff and traced the deeplink → workspace-creation path end to end. One real bug (native only) plus two minor notes. The core web flow looks correct.

🐞 Warm-opened native deeplinks lose the intent — the warm-path recording writes to the wrong "initial URL"

The last commit ("Fix native deeplink paths…") added recording of the intent URL when a link is opened while the app is already running:

src/DeepLinkHandler.tsx:148-149

if (hasSecureLinkKey(state.url) || getOnboardingIntentFromUrl(state.url)) {
    onInitialUrl(state.url as Route);
}

But onInitialUrl is wired to Expensify's local initialUrl state, which only feeds NavigationRootsrc/Expensify.tsx:302 (onInitialUrl={setInitialUrl}).

The handler that actually reads the intent reads a different value — the initialURL from InitialURLContextProvider:

src/hooks/useOnboardingDeeplinkIntent.ts:17-20

const {initialURL} = useInitialURLState();
const [urlAtMount] = useState(getCurrentUrl);
return getOnboardingIntentFromUrl(urlAtMount) ?? getOnboardingIntentFromUrl(initialURL);

That context initialURL is set only once, from Linking.getInitialURL() in the provider's mount effect (src/components/InitialURLContextProvider.tsx:40-46) — nothing else ever calls the context's setInitialURL. So on a warm native open, both of the hook's sources are dead:

  • getCurrentUrl() returns '' on native (currentUrl/index.native.ts), and
  • the context initialURL still holds the stale cold-launch URL.

The DeepLinkHandler change records the new URL into NavigationRoot's copy, which useOnboardingDeeplinkIntent never looks at → the intent is lost and no Submit workspace is created for warm-opened native links. This is exactly the case that commit set out to fix.

Web is unaffected (getCurrentUrl() = window.location.href carries it), and native cold launch works (context initialURL = launch URL). Only native warm opens regress — which matches your Open Question #2 that HybridApp is untested.

Fix direction

For the warm path to reach the hook, the intent URL needs to land in the value the hook reads. Either have useOnboardingDeeplinkIntent also read Expensify's local initialUrl (the one NavigationRoot uses), or push the warm URL into InitialURLContextProvider via useInitialURLActions().setInitialURL (which is currently never called). Worth confirming on a HybridApp build with new-expensify://onboarding?intent=submit opened while the app is foregrounded.

Minor notes

  • Workspace name may fall back to email. The effect fires as soon as hasLoadedApp is set and calls autoCreateSubmitWorkspace(firstName ?? '', lastName ?? '', false) (ApplySubmitOnboardingIntent.tsx:46) without waiting for personal details. If PERSONAL_DETAILS_LIST hasn't populated at that instant, the workspace name is derived from the email local-part. Low risk (same pattern as the welcome modal, and details are usually loaded post-HAS_LOADED_APP), but worth a glance.
  • Loose route prefix match. pathWithQuery.startsWith(ROUTES.ONBOARDING_ROOT.route) in getOnboardingIntentFromUrl.ts:28-30 matches any path beginning with the literal onboarding (e.g. onboarding/work-email?intent=submit). No such link is generated today, so this is cosmetic, but a ?///end boundary check would be tighter.

Looks correct ✅

  • Cross-session idempotency: a Submit policy is a group policy (PolicyUtils.isGroupPolicy), so after the first creation hasEditableGroupPolicy short-circuits useAutoCreateSubmitWorkspace, and hasRun/HAS_LOADED_APP cover the in-session case.
  • Never-onboarded users are correctly left alone (if (!isOnboardingCompleted) return;).
  • exitTo parsing (encoded, unencoded, magic link, transition, custom scheme) is well covered by the unit tests.
  • Web happy-path and never-onboarded fallback match the Onyx dumps in your PR description.

I did not spin up a device test since the only open concern is the native/HybridApp warm path, which the web build can't exercise.


view run

The initial URL outlives a sign-out, so remounting on a second account read the
same intent again and created a Submit workspace for that account.
@abzokhattab

Copy link
Copy Markdown
Contributor Author

P1 Consume the intent before a later account signs in

Good catch, this one is real. Fixed in f1887b4.

InitialURLContextProvider sits at the top of App.tsx, above the navigator and the auth boundary, and nothing ever clears initialURL. Signing out and into a second already-onboarded account therefore remounts ApplySubmitOnboardingIntent with the original intent=submit still readable, and the useRef guard is reinitialised along with it — so that account gets a Submit workspace it never asked for.

The guard is now module scope, so it survives the remount and the intent applies at most once per app process.

I went with that over clearing initialURL deliberately. Consuming it is the more precise fix and would keep a genuinely new warm link working, but initialURL is shared with the secure-link check in useOnboardingFlow, useAbsentPageSpan, ScreenWrapper and LogOutPreviousUserPage, and I can't verify I wouldn't disturb one of those on HybridApp. The cost of the contained fix is that clicking the link a second time in the same process is a no-op, which is acceptable given the user already has the workspace by then and creation was idempotent anyway.

Worth noting the same replay characteristic already exists for the secure-link signal in useOnboardingFlow, which reads initialURL the same way. It's harmless there because it only suppresses onboarding rather than writing anything, so I've left it alone.

Keeps the diff to files this feature actually changes.
@abzokhattab

Copy link
Copy Markdown
Contributor Author

@MelvinBot resolve open threads if u tackled them and resolve the conflicts

@MelvinBot

Copy link
Copy Markdown
Contributor

Resolved the 3 open codex threads — each was verified as addressed in the current code (fca471c):

  • Preserve the route in custom-scheme URLs — now goes through getRouteFromLink(), which strips the native scheme prefix while keeping the onboarding?intent=submit route (thread was also already outdated).
  • Observe onboarding links received after native startupDeepLinkHandler now records warm onboarding-intent links into the initial-URL context: hasSecureLinkKey(state.url) || getOnboardingIntentFromUrl(state.url).
  • Consume the intent before a later account signs in — the module-scoped hasAppliedIntent guard consumes the intent once per process, so signing out and into another account can't replay it.

⚠️ I can't resolve the merge conflicts — I didn't open this PR and I'm not listed as an assignee, so per policy I won't push to the branch. To have me do it, add me to the PR assignees and re-run your request. Otherwise you can resolve them yourself with git merge main on the branch (PR is currently CONFLICTING / DIRTY).


view run

…mit-onboarding-deeplink

# Conflicts:
#	src/libs/Navigation/AppNavigator/AuthScreens.tsx
This is the shape the emailed link actually arrives in on mobile, where OldDot
hands over a path rather than a full URL.
@abzokhattab
abzokhattab marked this pull request as ready for review August 23, 2026 14:41
@abzokhattab
abzokhattab requested review from a team as code owners August 23, 2026 14:41
@melvin-bot
melvin-bot Bot requested a review from rojiphil August 23, 2026 14:41
@melvin-bot
melvin-bot Bot requested review from a team and flaviadefaria and removed request for a team August 23, 2026 14:41
@melvin-bot

melvin-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown

@rojiphil 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 the request for review from a team August 23, 2026 14:41
@abzokhattab

Copy link
Copy Markdown
Contributor Author

The PR is ready @hungvu193 @iwiznia @carlosmiceli

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6e5615680

ℹ️ 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".

Comment on lines +35 to +36
if (hasAppliedIntent || !hasLoadedApp || isOnboardingCompleted === undefined || isSupportalSession) {
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Consume ignored intents during supportal sessions

When a warm onboarding link is opened during a supportal session, this condition returns without setting hasAppliedIntent, while DeepLinkHandler has already stored the URL in the session-sticky initial-URL context. When supportal access ends and isSupportalSession changes to false, the same effect runs again and creates a Submit workspace for the restored account. The new module flag does not cover this path because the supportal branch bypasses it; consume the intent when rejecting it rather than deferring it until the session changes.

Useful? React with 👍 / 👎.

Comment on lines +44 to +45
// The deeplink delivers the same outcome as the Submit plan welcome modal, so keep that modal from opening too.
setSubmitMigrationModalShown();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Suppress the welcome guard before loaded-state evaluation

For recipients who also satisfy SubmitPlanWelcomeModalGuard's employer-intent eligibility, the guard's HAS_LOADED_APP subscription queues its proactive navigation in a microtask, whereas this suppression only runs later in a React passive effect. The guard can therefore mount the Submit welcome modal before this call marks it shown; the subsequent workspace navigation dismisses it, but users see an unintended modal flash during the one-click flow. The deeplink needs to suppress or bypass the guard before its app-loaded evaluation runs.

Useful? React with 👍 / 👎.

@abzokhattab

Copy link
Copy Markdown
Contributor Author

@MelvinBot review

@MelvinBot

Copy link
Copy Markdown
Contributor

Code review

Solid, well-scoped PR — the parsing logic is thoroughly unit-tested and the comments explain the "why" throughout. Below are a few things worth a second look before merge. Nothing here is a hard blocker; the guard edge case (#1) is the one most worth deciding on consciously.

Potential issues

1. hasAppliedIntent latches even when nothing was created — a legitimate later account can miss the workspace. In ApplySubmitOnboardingIntent.tsx:35-40 the guard is set to true before the !isOnboardingCompleted early return. So if the intent link is first evaluated while signed into an account that hasn't completed onboarding (guard latches, no workspace), then the user signs into the intended completed-onboarding account in the same process without a restart, creation is skipped because hasAppliedIntent is already true. This is the mirror image of the cross-account replay this guard is meant to prevent — it trades that replay-safety for a missed-creation case. Low probability given the link targets completed users and usually drives a fresh sign-in, but consider latching the guard only once creation actually runs (move hasAppliedIntent = true below the isOnboardingCompleted check), so a no-op evaluation doesn't consume the one shot.

2. ROUTES.ONBOARDING_ROOT.getRoute(intent) has no in-app caller. The new intent param at ROUTES.ts:3967 isn't used anywhere in src (the emailed links are built outside the app). Not a bug — just confirm this surface is intentional and not a leftover, since it can't be exercised or regression-tested from within App.

3. No test around the wiring/guard. getOnboardingIntentFromUrlTest.ts covers the parser well, but the one-shot guard, HAS_LOADED_APP gating, and supportal guard in ApplySubmitOnboardingIntent have the subtle behavior in #1 and are untested. A small component/idempotency test there would lock down the intended semantics. Because hasAppliedIntent is module-global mutable state, such a test would also need a way to reset it between cases.

Minor

Strengths

  • Parser reduces every link shape (web, app://-/, native scheme, encoded/unencoded exitTo, bare HybridApp path) to one code path via getRouteFromLink, with tests for each.
  • Idempotency is handled from multiple angles: waits on HAS_LOADED_APP, defers actual dedup to useAutoCreateSubmitWorkspace, and skips supportal sessions.
  • Keeping the email-facing intent aliases (CONST.ONBOARDING_INTENTS) separate from internal onboarding-choice strings is the right call given they're embedded in non-redeployable emails.

view run

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.

2 participants