Skip to content

Let marking unread, favoriting and archiving survive being offline or killed - #6719

Open
AndyScherzinger wants to merge 1 commit into
masterfrom
feat/noid/durableConversationActions
Open

AndyScherzinger wants to merge 1 commit into
masterfrom
feat/noid/durableConversationActions

Conversation

@AndyScherzinger

@AndyScherzinger AndyScherzinger commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Stacked on #6717 — review that one first. #6715 and #6694 have merged; this branch still targets feat/noid/optimisticReminders and will be retargeted to master once #6717 lands.

Marking a conversation unread, favoriting it and archiving it were optimistic but not durable. The local entry changed at once, then the request had two attempts in the view model's scope, and anything that stopped it there reverted the change. Two cases that made worse:

  • Offline. The tap applied, both attempts failed within about a second, and the row flipped back — the app knew it had no connection and still threw the change away.
  • The process going away. Archiving a conversation and immediately swiping the app off the recents list left the change in the local database with its in-memory guard gone, so the next room list sync quietly put the old value back.

feat(conversation list): let unread, favorite and archive outlive the app — the three actions now hand the request to a new ConversationActionWorker, the way the read marker already goes through ReadMarkerSyncWorker. The worker carries a NetworkType.CONNECTED constraint, so a change made offline waits for a connection instead of being discarded, and it retries with exponential backoff across process death. Work is unique per conversation and action with REPLACE, so favoriting and unfavoriting in quick succession sends only the last intent, while archiving and marking unread on the same conversation stay independent.

This is only safe because each of these sets a value rather than moving one. Verified against a Nextcloud 36 dev instance running Talk main: POST …/favorite, POST …/archive and DELETE …/chat/{token}/read each answer 200 twice in a row and leave the same state, so a retry can never overshoot. That is the opposite of the note-to-self share in #6717, which has no such guarantee and therefore reads back before retrying.

Failure handling keeps both halves. While the list is still on screen it observes the work and, when the worker has used up its attempts, puts the conversation back and shows the error snackbar exactly as before. When the list is gone the worker releases the pending guard instead, and the next room list sync restores the server state.

Known limitation, deliberately not solved here. The pending guard lives in memory in ConversationListUpdater. If the process dies before the worker runs, the guard is gone, so a room list sync between the restart and the worker finishing can show the old value for a moment before the worker's success brings it back. Making the guard durable means persisting it next to the conversation entry, which is a schema change and its own review.

🖼️ Screenshots

No UI was added or changed. The archive snackbar now appears when the row moves rather than when the server answers.

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed — five tests cover what the work is named, which decides what replaces what, and the action round trip through the work input; both are silent to break
  • 🔖 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

@AndyScherzinger AndyScherzinger added this to the 25.1.0 milestone Sep 18, 2026
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 93bea38 to 0e97e99 Compare September 24, 2026 14:27
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch 2 times, most recently from 1af1d9f to d969345 Compare September 24, 2026 14:32
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 0e97e99 to 0f19149 Compare September 24, 2026 15:01
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from d969345 to 02badb4 Compare September 24, 2026 15:03
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 0f19149 to bf571a9 Compare September 24, 2026 16:41
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 02badb4 to bf571a9 Compare September 24, 2026 16:42
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from bf571a9 to a0740e5 Compare October 1, 2026 19:02
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 2a32aa9 to c980541 Compare October 1, 2026 19:03
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from a0740e5 to 08df497 Compare October 2, 2026 16:57
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from c980541 to 71269d5 Compare October 2, 2026 17:00
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 08df497 to 00d3a61 Compare October 5, 2026 22:32
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 71269d5 to d080ec3 Compare October 5, 2026 22:35
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 00d3a61 to 8200d2c Compare October 5, 2026 23:20
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from d080ec3 to 1837ca3 Compare October 5, 2026 23:21
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 8200d2c to 57e7e04 Compare October 5, 2026 23:48
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 1837ca3 to 828811a Compare October 5, 2026 23:52
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 57e7e04 to 2fb50b7 Compare October 6, 2026 05:50
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 828811a to bdda654 Compare October 6, 2026 05:52
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 2fb50b7 to 85883e2 Compare October 6, 2026 08:59
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from bdda654 to bbc0813 Compare October 6, 2026 09:02
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 85883e2 to c7fd3fb Compare October 10, 2026 16:05
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from bbc0813 to 174f1e7 Compare October 10, 2026 16:07
@AndyScherzinger

Copy link
Copy Markdown
Member Author

@coderabbitai review

@AndyScherzinger

Copy link
Copy Markdown
Member Author

@mahibi this PR now sits on top of your work in #6715, and rebasing it surfaced a design question I don't think I should answer on my own, because it's about which of our two contracts should win.

What happens

This branch (B8) moves favoriting, archiving and marking-unread out of the view model and into ConversationActionWorker, so that a change the user made survives the process being killed and is sent once connectivity comes back. Marking read is untouched and still goes straight to the repository.

Your new ConversationsListViewModelOptimisticActionsTest asserts the opposite mechanism — it stubs conversationsRepository and verifies the view model calls it directly, then verifies the view model releases the pending guard itself. Once this branch is rebased onto #6715, five of those tests fail:

ConversationsListViewModelOptimisticActionsTest
  a favorite the server accepts stays applied         FAILED
  a favorite the server refuses is taken back         FAILED
  an unfavorite the server refuses is taken back      FAILED
  marking unread that the server refuses is taken back FAILED
  an archive the server refuses is taken back         FAILED

They fail with TimeoutCancellationException rather than an assertion error, because handOver reaches for NextcloudTalkApplication.sharedApplication!! and WorkManager.getInstance(...), and under Robolectric with a plain Application neither is there. marking read that the server refuses is taken back still passes, which is exactly the action this branch does not move.

Worth saying clearly: git merges this without reporting a conflict, and GitHub shows the PR as mergeable. The incompatibility is only visible when the tests run.

What is actually in conflict

The user-visible contract your tests pin down is unchanged by this branch — the change is still applied at once, still taken back when it finally fails, and the pending guard is still released. What differs is the mechanism, and one consequence of it:

  • the request is sent by the worker rather than by the view model, so the view model no longer touches conversationsRepository for these three actions
  • the guard is released by the worker (including when it gives up after its retries), not by the view model
  • a change now outlives the screen and the process, so "the server refuses it" and "we finally gave up on it" become distinguishable states

The question

Which contract do you want to keep?

  1. Worker wins. Your five tests get re-pointed at a small seam — ConversationActionScheduler.send(...): Boolean — so the view model can be tested without WorkManager, and the assertions about optimistic apply / revert survive as they are. The guard-release assertions move to worker-level coverage, since that is where the behaviour went. I have this written and working already and can push it here.
  2. Direct request wins. This branch drops the worker for these three actions and B8 is limited to something else, or dropped.
  3. Something in between — for example only archive going through the worker.

I'd lean towards 1, because the offline case is the whole point of B8 and the seam makes the view model testable rather than less tested. But it is your call, since the tests and the reasoning behind them are yours. Happy to take 2 without argument if you think the added indirection is not worth it.

@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 415f322 to 465d034 Compare October 15, 2026 11:00
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from ff5a37a to 67b9320 Compare October 15, 2026 11:00
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 465d034 to 3d8dd2a Compare October 15, 2026 11:48
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 67b9320 to 9168d65 Compare October 15, 2026 11:48
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 3d8dd2a to eebc67e Compare October 15, 2026 16:21
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 9168d65 to 90b15c5 Compare October 15, 2026 16:21
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from eebc67e to 7026c43 Compare October 15, 2026 17:29
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 90b15c5 to 8c978cc Compare October 15, 2026 17:29
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 7026c43 to 5886cba Compare October 15, 2026 21:20
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 8c978cc to 917264d Compare October 15, 2026 21:20
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 5886cba to c6db06c Compare October 16, 2026 07:00
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 917264d to b58125a Compare October 16, 2026 07:00
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from c6db06c to 6840143 Compare October 16, 2026 11:28
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from b58125a to bb86a52 Compare October 16, 2026 11:28
@mahibi
mahibi force-pushed the feat/noid/optimisticReminders branch from 6840143 to aa0d8cc Compare October 16, 2026 12:12
Base automatically changed from feat/noid/optimisticReminders to master October 17, 2026 08:44
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from bb86a52 to 4c9b0d6 Compare October 17, 2026 10:06
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

📱 QA build

Download app-qa-debug.apk
QR code Open the QR code for this download
Commit 54a048d
Version 6719
Available until 7 days after this build

The QA build installs alongside a released Nextcloud app, so you can keep
using your existing install while testing.

Downloading the file requires a GitHub account, so open this link on the
device you want to test on, or transfer the APK to it.

@AndyScherzinger AndyScherzinger added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 17, 2026
@AndyScherzinger
AndyScherzinger marked this pull request as ready for review October 17, 2026 10:47
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch 4 times, most recently from 7c0e1e0 to 2d096f0 Compare October 18, 2026 22:41
… app

All three reverted themselves when the request did not come back, so a tap
taken offline flickered and a tap taken just before the app was killed was
lost. They now go to a worker, the way the read marker does, which waits for
a connection and survives the process.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/durableConversationActions branch from 2d096f0 to 54a048d Compare October 19, 2026 12:21
@github-actions

Copy link
Copy Markdown
Contributor

Codacy

Lint

TypemasterPR
Warnings138138
Errors1515

SpotBugs

CategoryBaseNew
Bad practice77
Correctness88
Dodgy code4040
Internationalization33
Malicious code vulnerability33
Performance88
Security1111
Total8080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant