fix(auth): show the link-sent confirmation on anonymous upgrade, with e2e tests - #2484
Open
demolaf wants to merge 2 commits into
Open
fix(auth): show the link-sent confirmation on anonymous upgrade, with e2e tests#2484demolaf wants to merge 2 commits into
demolaf wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the email link sign-in logic and significantly expands its test coverage. In FirebaseAuthScreen.kt, a check is added to prevent resetting the backstack to AuthRoute.Success on notification trailing events. Additionally, email link sign-in tests are extracted from EmailAuthScreenTest.kt into a dedicated EmailLinkAuthScreenTest.kt suite with comprehensive end-to-end scenarios. Feedback includes addressing a potential infinite loop in Robolectric tests due to virtual clock behavior when using Thread.sleep() with System.currentTimeMillis(), and safely checking if lateinit properties are initialized in tearDown() to avoid masking setup failures.
demolaf
force-pushed
the
feat/require-navigation-followups
branch
from
September 6, 2026 21:51
04ed088 to
d9b2cc2
Compare
demolaf
force-pushed
the
fix/email-link-sign-in-confirmation
branch
from
September 6, 2026 21:54
c5e1b56 to
9118458
Compare
demolaf
force-pushed
the
feat/require-navigation-followups
branch
from
September 7, 2026 10:40
d9b2cc2 to
598ea96
Compare
demolaf
force-pushed
the
fix/email-link-sign-in-confirmation
branch
3 times, most recently
from
September 7, 2026 11:36
e0fee1c to
3072f52
Compare
demolaf
force-pushed
the
feat/require-navigation-followups
branch
2 times, most recently
from
September 7, 2026 14:27
42afa07 to
cab710b
Compare
demolaf
changed the base branch from
feat/require-navigation-followups
to
version-10.0.0-beta05
September 7, 2026 15:01
demolaf
force-pushed
the
fix/email-link-sign-in-confirmation
branch
from
September 7, 2026 15:01
3072f52 to
f6fd6a7
Compare
demolaf
force-pushed
the
fix/email-link-sign-in-confirmation
branch
from
September 7, 2026 15:25
f6fd6a7 to
2371997
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sending an email link while an anonymous user was signed in showed no confirmation. Retracting the
EmailSignInLinkSentnotification toIdlemakesauthStateFlow()'s combine fall back to reporting the session, so the pre-existing anonymous session arrived as a freshSuccessand reset the back stack to the success route, tearing the dialog down with the screen that owned it.FirebaseAuthScreen.kt: the steps that own a "link sent" confirmation are no longer reset off by a re-asserted session — only a real credential exchange, which is the only thing carrying anAuthResult, still completes to the success route. Applied to theRequiresEmailVerification/RequiresProfileCompletionbranch too, sinceauthUserStateroutes an unverified password user there rather than toSuccess. The guard is stateless, so it holds across activity recreation and is unaffected byStateFlowconflation. Same reasoning as the existingPhone.EnterPhoneNumberexemption in theIdlebranch.Email link sign-in had a single e2e test buried in
EmailAuthScreenTest. It now has its ownEmailLinkAuthScreenTestwith nine, covering the cross-device prompt and completion, the method-picker hand-off, anonymous upgrade, and the wrong-device, foreign-anonymous-id and different-anonymous-user guards — with the anonymous upgrade case asserting the flow stayed on the email link step, verified to fail on the old code and pass with the fix. Link delivery now fails rather thanAssume-skips on timeout, so a slow emulator can no longer report the class green. 966 unit and 71 e2e tests pass.This is an interim fix. The underlying cause — one-shot outcomes modelled as
AuthStatevalues, which is what makes retraction expose the session at all — is tracked separately and would delete this guard along with five other workarounds.Maintainer note: Fixes internal CPRN-422