fix(drawer): show Settings when the account roster is empty (#360) - #361
Merged
Conversation
The drawer footer rendered Settings and the account avatar inside one
`accounts.firstOrNull { it.isActive }?.let`, so an empty roster removed
both. On phone the footer is the only route to Settings, which makes the
server/account screens, sign-out and the diagnostic-log export
unreachable in exactly the states that need them.
isLoggedIn is a pure token check, independent of the roster, so the two
can disagree. An iOS reinstall guarantees it: the token and server URL
live in the Keychain and survive app deletion, while the roster lives in
DataStore and is wiped. The rebuild path needs a live getUser(), so a
user pointed at an unreachable server can only reach the screen that
would fix the server by first reaching the server.
Render Settings unconditionally and gate only AccountChip, which is the
sole part that needs a resolved account. The trailing inset becomes
conditional too, so Settings lines up with the Files/Agents rows above
it when no chip is drawn.
Contributor
Android debug APKArtifact:
|
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
1 participant
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.
Closes #360.
Summary
The drawer footer rendered the Settings row and the account avatar inside a single
accounts.firstOrNull { it.isActive }?.let, so an empty account roster removed both. That footer is the only route to Settings on either layout — the phone drawer and the tablet sidebar both renderDrawerContent, and the chat overflow menu has no Settings item — so an empty roster leaves the server-connection screen, sign-out, and the data and diagnostic-log exports unreachable.Why the roster can be empty while signed in
isLoggedInis a pure token check and does not consult the roster, so the two can disagree. An iOS reinstall guarantees it: the access token and the server URL live in the Keychain and survive app deletion, while the roster lives in DataStore and is wiped. The rebuild path,AuthRepository.restoreAccountIfNeeded, needs a livegetUser()— so a user left pointed at an unreachable server can reach the screen that would fix the server only by first reaching the server.The gate was not deliberate. Settings was an ungated footer item alongside Agents, Skills and Files until #221 moved it into the footer slot to share a row with the account avatar, where it was swept under the avatar's null check. (#215 had introduced that slot and its null check a day earlier, but held only the avatar.)
Changes
AccountChip— the sole part that needs a resolved account.Warmingwindow Settings now renders immediately rather than popping in once the roster resolves.Testing
:feature:conversations:detektMetadataCommonMainand:feature:conversations:testDebugUnitTestpass.feature/conversationshas no Compose test source set, and adding one is disproportionate to the remaining branch, whichAccountChip's non-nullaccountparameter makes compiler-enforced.Notes
Reaching Settings is necessary but not sufficient to recover from this state. Sign-out skips its local teardown while identity is
Resolved(null), so the tokens survive it:AuthRepositoryImpl.logoutskipsaccountSwitcher.remove()in theaccount == nullbranch, and that is the only thing that clears tokens and emits session-expired. The server-side revocation still fires. That, and lettingrestoreAccountIfNeededfall back to an identity derived from the persisted server URL, are tracked separately on #360 and deliberately not in this PR — the latter touches tenancy scoping.