Skip to content

Bugfix/6052/fix to immediately show messages after sending#6104

Open
mahibi wants to merge 2 commits intomasterfrom
bugfix/6052/fixToImmediatelyShowMessagesAfterSending
Open

Bugfix/6052/fix to immediately show messages after sending#6104
mahibi wants to merge 2 commits intomasterfrom
bugfix/6052/fixToImmediatelyShowMessagesAfterSending

Conversation

@mahibi
Copy link
Copy Markdown
Collaborator

@mahibi mahibi commented Apr 22, 2026

fix #6052

see commit message 193e96d for explanation why it failed on some instances and not on others.

Also show sent icon when message status is SENT_PENDING_ACK

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

mahibi added 2 commits April 22, 2026 15:07
The temporary local message id that is calculated via removeYearFromTimestamp creates id's like 3221969 for today.
The filtering of getMessagesEqualOrNewerThan in ChatMessagesDao.kt however was filtering out id's that are lower than oldestMessageId.

For small instances this worked fine as it included the temporary local timestamp. However for large instance with a huge amount of messages the temporary messages were not included as their id's were too low compared to the actual id's that the server uses for messages.
Only after the insurance request was triggered, the messages appeared again as their id's were replaced by the server response.

The fix is to include the temporary messages regardless of their local temporary id.

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi mahibi added this to the 24.0.0 milestone Apr 22, 2026
@mahibi mahibi self-assigned this Apr 22, 2026
Copilot AI review requested due to automatic review settings April 22, 2026 13:37
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes #6052 by ensuring newly sent (temporary / pending-ack) messages appear immediately and have an appropriate status icon.

Changes:

  • Adjusted the messages query to include temporary messages regardless of oldestMessageId.
  • Updated status icon resolution to show a “sent” icon for SENT_PENDING_ACK.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt Expands the WHERE clause so temporary messages are returned even when they fall outside the id >= oldestMessageId window.
app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt Adds SENT_PENDING_ACK handling and simplifies the status-icon selection logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

WHERE internalConversationId = :internalConversationId
AND (:threadId IS NULL OR threadId = :threadId)
AND id >= :oldestMessageId
AND (id >= :oldestMessageId OR isTemporary = 1)
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an OR condition like this commonly prevents SQLite from using an index on id efficiently, which can degrade performance as the table grows. If this query is on a hot path, consider rewriting to preserve index usage (e.g., UNION ALL of two indexed queries: one for id >= :oldestMessageId and one for isTemporary = 1, keeping the same conversation/thread filters) and then ordering the combined result.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Codacy

Lint

TypemasterPR
Warnings8887
Errors00

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1010
Dodgy code5353
Internationalization33
Malicious code vulnerability33
Performance44
Security11
Total8080

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.

posting messages don't show up in the chat (or very late / when reentering the chat)

2 participants