Refactor drafts to save both Messages & Replies to Room Storage
- fixes #5027
--
- Added a new field to Conversation,
MessageDraftwhich 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 != "" upsertConversationswas 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?)
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.
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 toConversationListActivity.
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?
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...
@rapterjet2004 whats the state of the PR? can you update the labels or let me know if it's to review again?
i get a
IllegalStateException: Migration didn't properly handle: Conversations(com.nextcloud.talk.data.database.model.ConversationEntity).
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)..
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.
@sowjanyakch can you please also review and test?
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/5037-talk.apk
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.