Repository navigation
feat(chat): keep a half-typed message when you leave the chat - #1508
Merged
Merged
Conversation
Type half a message, back out to check something, come back: the field is empty. `chatInputState` is a `TextFieldState` held inside `ChatViewModel.State`, so the words never existed anywhere but the screen the pop destroyed. They now go in a `chat_draft` table behind a `ChatDraftStore` in `:shared:chat`, keyed by the same chat id hex `ChatEntityMapper` uses for messages. That includes the locally derived TIP_DM id from `generateChatId`, so a draft typed at someone never messaged before survives the first tip creating the chat with no key migration, and groups need nothing extra. The table arrives as `AutoMigration(from = 34, to = 35)`. It has to: the builder falls back to a destructive migration, and a draft is the one row in this database that cannot be re-fetched from the server afterwards. The rules are the cross-platform design's, pinned by `test-vectors/chat_draft.json` synced into this module's test resources and replayed by `ChatDraftVectorTest`: - Text is stored verbatim; trimming stays at submit. - Text that trims to empty *and* no reply target deletes the row. A missing row and an empty one are different states. - A reply strip aimed with no text yet is still a draft. - Sending clears it the moment the field clears, not on delivery. - An edit persists the new-message draft it displaced, never the edit body. - Restore is silent: the text and the strip come back, focus and the keyboard do not. `chatDraftOf` holds the edit rule so the ViewModel and the vector replay read it from one place. `ChatQuote` gained `senderIdHex` because its two accents are derived from the sender id rather than stored, and a strip rebuilt from a row has to render the same as one the transcript mapped. Writes are debounced 300ms, with immediate flushes from `onCleared` and from process lifecycle STOPPED. The flushes are what make process death survivable, since neither platform guarantees a teardown callback runs first. `saveInBackground` uses the store's own scope because `viewModelScope` is already cancelled by the time `onCleared` runs. Cleared by logout through `RealChatCoordinator.teardown`, by leaving a group, and by blocking, which hides the chat. A failed send needs nothing here: `MessageStatus.FAILED` already renders as "Not sent" with a retry, so the transcript is the record of that text and a draft as well would duplicate it. iOS differs there deliberately, which is why that case sits outside the fixture.
The draft wipe was in teardown(), which is plain sign-out. Everything else in that method stops a job or resets in-memory state; the one call that erased persisted rows was this one, and it contradicted the reason AuthManager gives for keeping the rest: the database is named from the account entropy, so a logout never has to empty a table to keep accounts apart, and the next login reopens the same file. A half-written message is the row a user would most notice losing to a sign-out they expected to be reversible. Moved to clearCache(), alongside the transcript it belongs with, which deleteAndLogout() is the only caller of. Also re-syncs chat_draft.json, which gained two reply-then-edit vectors. Both already pass: entering an edit drops the strip and stashes the text, and cancelling puts the text back without the strip.
Renumbers the chat_draft migration. #1506 landed the chat_metadata viewer state as 34 -> 35 while this branch was open, so chat_draft becomes 35 -> 36 and its exported schema moves to 36.json; 35.json is now that PR's, unchanged. Two AutoMigrations claiming the same version is not a merge conflict Room would have caught at runtime -- it would have taken whichever entry it read first and dropped the other table's DDL. Also adds draftStore to the RealChatCoordinator built by that PR's ViewerStateStreamTest.
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.
Type half a message, back out to check something, come back: the field is empty.
chatInputStateis aTextFieldStateheld insideChatViewModel.State, so the words never existed anywhere but the screen the pop destroyed. The same is true of a background kill, which is indistinguishable from it.Where a draft lives
A
chat_drafttable behind aChatDraftStorein:shared:chat, keyed by the same chat id hexChatEntityMapperuses for messages. That includes the locally derived TIP_DM id fromgenerateChatId, so a draft typed at someone never messaged before survives the first tip creating the chat with no key migration. Groups need nothing extra — they are a chat id like any other.The table arrives as
AutoMigration(from = 35, to = 36)with its exported schema. It has to:FlipcashDatabasebuilds withfallbackToDestructiveMigration(), and a draft is the one row in this database that cannot be re-fetched from the server afterwards. It was 34 -> 35 until #1506 took that version while this branch was open; two entries claiming one version is not something Room rejects, it just reads one and drops the other table's DDL.:shared:chathas no Room on its compile classpath, so the store reaches the table through aChatDraftDataSourcein:shared:persistence:sources, matchingCurrencyCreatorDraftDataSource. The reply target stays an opaque JSON blob on that side — its shape belongs to the chat layer that writes and reads it.The rules
These come from the cross-platform design and are pinned by
test-vectors/chat_draft.json, synced into this module's test resources and replayed case by case inChatDraftVectorTest:chatDraftOfholds the edit rule, so the ViewModel and the vector replay read it from one place rather than two.Wiring
Restore runs in the unified chat-open handler at the point
chatIdresolves, writing into the liveTextFieldStateonDispatchers.Main.immediate— the path the composer pre-fill already documents. Re-entering an already-open chat takes the existing early return and does not restore over what is in the field.Writes are debounced 300ms off a
snapshotFlowof the composer text combined with the reply strip and the edit stash, with immediate flushes fromonClearedand from process lifecycle STOPPED. The flushes are what make process death survivable; the debounce only keeps a database write off the keystroke path.saveInBackgrounduses the store's own scope, because both flush callers run at a moment their own scope is being cancelled.ChatQuotegainedsenderIdHex. ItsaccentandnameAccentare derived from the sender id throughgenerateComplementaryColorPaletterather than stored, so a strip rebuilt from a row needs the id to render the same as one the transcript mapped.Clearing
Leaving a group, on success only. Blocking, which goes through
setChatHidden. Account deletion, throughclearCache— the same path that erases the transcript, and the only caller isdeleteAndLogout.Not logout. A draft belongs to the cache rather than to the session, for the reason
AuthManageralready gives for keeping the rest: the database is named from the account entropy, so signing out never has to empty a table to keep accounts apart, and the next login reopens the same file.ChatCoordinatorTeardownTestpins both halves.No TTL: the store holds text bounded by the composer's existing length limit, times chats typed in.
A failed send
Nothing special on Android.
MessageStatus.FAILEDis already a column rendered as "Not sent" with a retry, so the failed message is the transcript's own record of that text and restoring a draft as well would duplicate it. iOS has no send-status column and so writes the text back — that asymmetry is deliberate and sits outside the shared fixture.What this changed in the design
Three places where the code contradicted the spec, all now corrected upstream rather than worked around here:
Event.EditMessagealready clearsreplyingTo, because reply and edit both own the composer, so the edit stashes the text and the strip goes with it. The fixture gained two vectors for it and this implementation passes them unchanged.One representation note that is not a divergence: the fixture identifies a cited message by UUID and Android's message ids are longs. The id is opaque to every rule being pinned, so
ChatDraftVectorTestcarries it through the round trip rather than reinterpreting it, and asserts the same message comes back.