Skip to content

Report the outcome of reminders and shares to notes instead of guessing it - #6717

Draft
AndyScherzinger wants to merge 2 commits into
feat/noid/optimisticConversationActionsfrom
feat/noid/optimisticReminders
Draft

AndyScherzinger wants to merge 2 commits into
feat/noid/optimisticConversationActionsfrom
feat/noid/optimisticReminders

Conversation

@AndyScherzinger

@AndyScherzinger AndyScherzinger commented Sep 18, 2026

Copy link
Copy Markdown
Member

Stacked on #6715, which is stacked on #6694 — review those first. This branch targets feat/noid/optimisticConversationActions and will be retargeted as the stack merges.

Two things in the chat reported the wrong outcome, and both sat behind the last RxJava subscriptions in ChatViewModel that still used the disposableSet the chat throws away on pause.

refactor(chat): move reminders and sharing to notes off RxJavasetReminder, getReminder, deleteReminder, shareToNotes and shareLocationToNotes move down through ChatNetworkDataSource and RetrofitChatNetwork onto four new NcApiCoroutines endpoints. This is not the pure refactor it looks like: disposableSet is disposed in onPause, not in onCleared, so backgrounding the chat while one of these requests was in flight cancelled it silently. On viewModelScope they finish.

getCapabilities, joinRoom and createRoom stay on RxJava on purpose. They sit in that same disposableSet, so moving them is the same behaviour change — and for a join or a capability fetch, what should happen when the chat is backgrounded mid-request is a decision, not a refactor. Neither blocks anything.

fix(chat): tell the user when a reminder or a share to notes failed — a reminder the server refused was written to the log and nowhere else. And showSnackBar fires "Message sent" the moment a share to notes is started, before the answer arrives, so a share that failed was reported to the user as a success. Both now report the failure, through the SharedFlow and snackbar shape #6694 established for reaction failures.

A share that failed for a transient reason is retried, but never blindly. Talk stores a message's referenceId and never looks at it again — verified against a Nextcloud 36 dev instance with Talk main: posting twice with the same reference id returns 201 twice and leaves two messages in the room. So before a second attempt the client asks the note to self whether the message arrived anyway, and only sends it again when it did not. A lookup that cannot be answered counts as "it may be there", because a duplicate note is worse than a missing retry. The location share now carries a reference id too, which the share endpoint has always accepted.

Deliberately without an optimistic reminder state, although the implementation plan asked for one: getReminderExistState has exactly one consumer, the reminder dialog header, and DateTimeCompose refetches the reminder in its constructor every time that dialog opens, while Set and Delete dismiss it immediately. An optimistic value would never be rendered.

🖼️ Screenshots

No UI was added or changed. One new string, nc_message_not_added_to_notes, shown in the existing chat snackbar.

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed — four tests cover holdsShare, the rule that decides whether a failed share is sent again. ChatViewModel takes 14 constructor dependencies plus sharedApplication, so there is no view-model-level test for the snackbar itself; the helper contract these paths rely on is covered in Put every optimistic action on one helper and fix the ones that failed silently #6715
  • 🔖 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

Both used an Observable with a disposable that the chat throws away when it
is paused, so backgrounding the app while the request was in flight dropped
it silently. They now run in the view model scope like every other request
that has to finish.

Assisted-by: Claude Code:claude-opus-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/optimisticReminders branch from 925a2a7 to 1cff49c Compare September 18, 2026 07:22
A reminder the server refused was only logged, and the chat says "Message
sent" the moment a share to notes starts, before the answer arrives - so a
share that failed was reported as a success. Both now show what happened.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant