Skip to content

Put every optimistic action on one helper and fix the ones that failed silently - #6715

Draft
AndyScherzinger wants to merge 5 commits into
feat/noid/optimisticReactionsfrom
feat/noid/optimisticConversationActions
Draft

AndyScherzinger wants to merge 5 commits into
feat/noid/optimisticReactionsfrom
feat/noid/optimisticConversationActions

Conversation

@AndyScherzinger

@AndyScherzinger AndyScherzinger commented Sep 17, 2026

Copy link
Copy Markdown
Member

Stacked on #6694 — review that one first, this branch targets feat/noid/optimisticReactions and will be retargeted to master once it merges.

#6694 shipped five optimistic chat actions, each with its own hand-rolled apply/retry/revert block. This branch pulls that shape into one helper, puts the conversation list and conversation info actions on it, and fixes the two places where an action told the user it had worked when it had not.

refactor(chat): extract the optimistic action pattern into one helperoptimisticAction(apply, isConfirmed, request) in utils/OptimisticAction.kt. apply writes the local state and returns its own undo action, or null when nothing changed, in which case nothing is undone later either. A transient failure is retried once, any failure or a cancellation takes the change back, and the undo runs under NonCancellable so a revert cannot itself be cancelled half-way. Migrating OfflineFirstChatRepository and ReactionsRepositoryImpl onto it removes 112 lines with no behaviour change.

perf(conversations): keep list and info actions correct when they fail — mark as read, mark as unread, favorite, archive and the important and sensitive toggles now go through the same helper. The two toggles gain a retry they never had; all six gain cancellation safety, so leaving the screen mid-request no longer leaves the change applied although the server may never have heard of it.

fix(conversation info): tell the user when a setting did not saveDatabaseStorageModule.saveString and saveBoolean caught every failure and only logged it, so the notification level, the message expiration and the call notification switch kept showing a setting the server had refused, with no error anywhere. The failure now reaches the view model, which puts the previous value back and shows the error snackbar.

perf(conversation list): drop a deleted conversation from the list at once — the row stayed in the list until the worker had finished and the list had been fetched again. It now disappears on confirmation and comes back if the delete fails, reusing the guard that hides a conversation being left.

test(utils): cover the optimistic action contract directlyoptimisticAction was only exercised through the repositories using it. Seven tests now assert the rules every optimistic action depends on: applied before the answer, taken back on a refusal, taken back on a refusal the server reports in the payload, taken back when the screen is left mid-request, nothing taken back when nothing was applied, and one hiccup retried without losing the change.

🖼️ Screenshots

No UI was added or changed — the same controls, the same snackbars, applied earlier.

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed — not backported, this goes into 25.1.0
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Four call sites had each grown their own copy of "apply locally, retry once on a
transient failure, revert when it finally fails, revert when cancelled", with the
retry predicate spelled out twice and one of them shipping a guard bug.

optimisticAction() now carries that shape, including the case where the server
refuses in the payload rather than in the status code, and the case where an
answer means the change was already applied - which must not revert anything.

No behaviour change; the repository tests of reactions, deletions, edits, pinning
and dismissals carry the migration.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Marking a conversation read or unread, favoriting, archiving and the
important and sensitive toggles now go through the shared optimistic
helper, so every one of them retries a connection problem once and takes
its change back when the screen is left mid-request.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The notification level, the message expiration and the call notification
switch logged their failure and left the screen showing a setting the
server never accepted. They now go back to the previous value and say so.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
… once

Deleting left the row in place until the worker had finished and the list
had been fetched again. It now disappears on confirmation, reusing the
guard that hides a conversation being left, and comes back if the delete
fails.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The helper was only exercised through the repositories that use it, so the
rules every optimistic action depends on - applied before the answer, taken
back on a refusal or a cancellation, kept after one hiccup - had no test of
their own.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
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.

1 participant