talk-android icon indicating copy to clipboard operation
talk-android copied to clipboard

Refactor drafts to save both Messages & Replies to Room Storage

Open rapterjet2004 opened this issue 6 months ago • 5 comments

  • fixes #5027

--

  • Added a new field to Conversation, MessageDraft which holds the state of any message draft. Can be easily extended to support things like file URI's, in the future, as it's saved as TEXT in Json format.
  • State is saved to Room onPause
  • State is restored from Room onCreate
  • Reply state is cleared only when the message is sent, or the clear button is clicked. It remains through lifecycle changes.
  • Input should be focused if messageText != ""
  • upsertConversations was edited to support syncing data sourced from the server, without overwriting data sourced from the user. We can now store any arbitrary conversation metadata without messing up the sync.

TODO

  • [x] MessageDraft data class + Converter
  • [x] Database migration
  • [x] Mapper functions
  • [x] update dao save/retrieve drafts from storage
  • [x] Track state of draft in ChatViewModel
  • [x] refactor MessageInputFragment to source from ViewModel rather than SharedPreferences
    • [x] Save reply state
    • [x] Save cursor state
    • [x] Save text state
  • [x] Test reply images

🏁 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
  • [x] 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

rapterjet2004 avatar Jun 05 '25 21:06 rapterjet2004

I figured it would be simpler to clear the reply state when exiting ChatActivity. That way the reply view remains upon orientation change but is removed when exiting to ConversationListActivity.

rapterjet2004 avatar Jun 17 '25 17:06 rapterjet2004

I figured it would be simpler to clear the reply state when exiting ChatActivity. That way the reply view remains upon orientation change but is removed when exiting to ConversationListActivity.

Having the reply parent removed when leaving the chat is not expected. It should be possible to have drafts for each conversation including reply parent messages saved. I think best is to avoid the handling with with single sharedPreferences values.

Maybe we could use room for this. Adding isDraft to a chat message is an option, while making use of the parent value. Maybe @Ivansss or @SystemKeeper could give an insight how you handle message drafts on iOS? Do you use the DB table as for the other messages?

mahibi avatar Jun 18 '25 09:06 mahibi

Maybe @Ivansss or @SystemKeeper could give an insight how you handle message drafts on iOS? Do you use the DB table as for the other messages?

Pretty simple right now, there's a column pendingMessage on the room table where we store the conversations. On leaving a room, we save pending messages, on entering we restore, on sending we leave basically. Currently we don't keep replies, but that is something that bothers me for a while already...

SystemKeeper avatar Jun 18 '25 09:06 SystemKeeper

@rapterjet2004 whats the state of the PR? can you update the labels or let me know if it's to review again?

mahibi avatar Jun 24 '25 08:06 mahibi

i get a

IllegalStateException: Migration didn't properly handle: Conversations(com.nextcloud.talk.data.database.model.ConversationEntity).

grafik

so i suggest it should be MessageDraft? instead MessageDraft

Also: for this simple change in DB, you can also use an auto-migration (we often did manual migrations when we could have used auto-migration)..

mahibi avatar Jul 03 '25 09:07 mahibi

Error was caused by the ConversationDao function upsertConversations(accountId, list) as I was incorrectly implementing the upsert functionality. I was deleting the conversation and reinserting them, instead of updating, which causes the ForeignKey in ChatMessageEntity to trigger a cascade which deletes all the ChatMessages associated with the conversation every time the conversation was synced. My bad should have caught that.

rapterjet2004 avatar Jul 14 '25 18:07 rapterjet2004

@sowjanyakch can you please also review and test?

mahibi avatar Jul 17 '25 08:07 mahibi

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/5037-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

github-actions[bot] avatar Jul 25 '25 11:07 github-actions[bot]

Codacy

Lint

TypemasterPR
Warnings9898
Errors00

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1616
Dodgy code6161
Internationalization33
Malicious code vulnerability33
Performance44
Security11
Total9494

github-actions[bot] avatar Jul 25 '25 12:07 github-actions[bot]