feat(totp): allow item selection when totp secret scanned - #79
Merged
Conversation
A deep-linked otpauth:// URI offered an item picker only when the code's issuer matched a stored login by registrable domain, and dropped into a blank form otherwise. The picker is now unconditional and mirrors the passkey flow: the whole item list with its own search, plus a FAB out to a new item. Domain matches become a suggested group at the top of that list rather than a gate. HeaderContent gains a Suggested variant so they group the way pinned items already do, and ItemListScreen takes the ids through a new suggestedItemIds parameter that defaults to empty, leaving the dashboard list unchanged. Suggestions stand down while a search query is active. Matching now runs through resolveTotpDomain instead of the issuer alone, so a code carrying no issuer matches on the domain in its account name. The removed dialog's copy promised that; the code never did it. It also keeps the suggestions consistent with the domain a new item is prefilled with. Ids the list does not hold are ignored rather than fetched: a blank-query list is scoped to the selected vault while getLoginsByTLD searches all of them, and splicing in another vault's item would misrepresent what the list is showing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Back from the form returns to the picker with a clean slate instead of leaving the flow, and the wait before the vault is readable now wears the picker's chrome rather than the form's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
The picker is a destination now, so the login form no longer has to fake one: returning to it pops an entry instead of hand-resetting every field. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Addresses the important and minor findings from the branch review of the TOTP item picker's move to its own navigation destination: - dedupe a double-tap on a picker row with launchSingleTop, since the picker keeps collecting through its exit transition and a second tap inside that window pushed AssignTotpRoute twice - record the invariant that lets Init.Existing safely run parsePendingTotp before initWithId overwrites dialogState, so a future caller does not reintroduce a silently dropped parse error - complete the KDoc on the now-public SelectItemForTotpScreen - drop a dead FakeItemRepository field from its ViewModel test - collapse the duplicated post-unlock destination mapping in MainActivity into destinationAfterUnlock - drop an unused lambda parameter in DashboardGraph - rename a preview function left over from the deleted overlay dialog - remove a stray blank line in feature/item/create's build.gradle.kts Also corrects the design spec's Files section, which claimed no dependency changes were needed even though this branch added implementation(projects.feature.listScreen) to feature/item/create. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
…est's teeth Finishes a fix wave on the TOTP deep link validation gate: - TotpParseErrorDialog now takes an onDismissRequest, wired to the deep link gate's Invalid branch so a back press (which AlertDialog routes through onDismissRequest, not onDismiss) actually closes the gate instead of doing nothing. The two in-app camera scanner call sites keep their old no-op default. - SelectItemForTotpViewModelTest's "an unreadable code leaves the picker with no suggestions" test seeds a login again before asserting on suggestions, so an empty repository cannot make the test pass vacuously. - MainActivity's onRejected callback now logs a warning instead of silently doing nothing when there is no Activity to finish. Not a live bug (LocalActivity.current is never null in production), but a null branch that swallowed the failure would otherwise leave a dead OK button with no trace. - parsePendingTotp's KDoc calls out that it trusts its caller to have validated the uri already, so a new caller passing pendingTotpUri onto DetailPaneInformation.Init needs its own parse gate upstream. Verified :feature:totp and :feature:item:create unit tests plus the full ./gradlew test suite pass. Confirmed the restored seedLogin line matters: breaking SelectItemForTotpViewModel's init to fall through to the suggestion lookup on a failed parse turns the "unreadable code" test red, and reverting brings it back to green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
The import flow was split three ways: :feature:totp validated the deep link, :feature:item:create drew the picker, and :app owned a totpImportGraph holding both halves of the post-gate flow. The picker is TOTP work, so it belongs with the rest of it. totpImportGraph could not move wholesale, because its AssignTotpRoute renders LoginScreen and :feature:item:create already depends on :feature:totp. But the picker never touches that form -- it only emits an ItemId -- so the graph splits cleanly instead: - :feature:totp owns SelectItemForTotpRoute and selectItemForTotpGraph. Both of its callbacks hand the uri back out, so the module never names a login form. - :feature:item:create owns AssignTotpRoute and assignTotpGraph. - :app keeps only the wiring between the two, in navigateToAssignTotp next to navigateToValidatedImport. resolveTotpDomain moves out of :feature:item:core, which is what let the picker follow it: item:core already depends on totp, so importing it back would have cycled. It was pure TOTP logic in the wrong module regardless. The one new edge is :feature:totp -> :feature:list_screen, since the picker's body is ItemListScreen. Every totp consumer inherits it. TestRegistrableDomainResolver backed both the picker test and LoginViewModelTest, which now live in different modules, so it moves to core:util testFixtures as FakeRegistrableDomainResolver rather than being duplicated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
…ew model
totpSecretInformation and totpOriginalUri held a code across calls, but only one
of their seven uses actually needed that.
OnCodesScanned wrote both and then passed the same two values as arguments on
the next line. The override handlers wrapped their work in
totpSecretInformation?.let { } without ever touching it: applyToUi reads before
and after off DialogState.OverrideTotp.fields, so the field was a null guard,
and a redundant one. OverrideTotp is only set at the end of
requestTotpSecretUpdate, whose callers had both just assigned the field
non-null, so the dialog state check above it was already the whole guard.
The one real carry was the deep-linked import onto an existing item, where init
parsed the uri before the suspending load and initWithId folded it in after.
That is now a parameter, and the parse happens where the result is used.
With no caller left that lacks a uri, requestTotpSecretUpdate and
updateUiWithTotpSecretInfo take a non-null originalUri, which retires the
`?: secretInformation.secret` fallback neither could reach.
Confirm and keep on the override dialog had no coverage, so they get tests
before the guard comes off.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
Three modules each grew their own fake, one per testing need: core:util computed an eTLD+1 from the last two labels, autofill looked answers up in a seeded map and recorded what it was asked, and legacy-migration answered example.com to anything containing "example" and null to everything else. The core:util one absorbs all three. A seeded resolutions entry wins, an explicit null included, and anything unseeded falls back to the label heuristic, so a test names only the domains it cares about. resolvedDomains keeps autofill's call log for the tests that assert nothing was resolved. Nothing relied on the semantics that went away. Autofill seeds every domain it resolves, and the two tests that want a null seed one. Every origin the migration tests carry is https://example.com, which the heuristic resolves to the same example.com the fixed answer gave, and no migration test asserts the null branch. The fake now lives in the module that owns the interface, so a fourth need does not grow a fourth fake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH
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.
No description provided.