Skip to content

Conversation

rapterjet2004
Copy link
Contributor

@rapterjet2004 rapterjet2004 commented Jun 5, 2025

--

  • 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

  • MessageDraft data class + Converter
  • Database migration
  • Mapper functions
  • update dao save/retrieve drafts from storage
  • Track state of draft in ChatViewModel
  • refactor MessageInputFragment to source from ViewModel rather than SharedPreferences
    • Save reply state
    • Save cursor state
    • Save text state
  • 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
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

@rapterjet2004 rapterjet2004 self-assigned this Jun 5, 2025
@rapterjet2004 rapterjet2004 added the 2. developing Work in progress label Jun 5, 2025
@rapterjet2004 rapterjet2004 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 10, 2025
mahibi

This comment was marked as resolved.

@rapterjet2004 rapterjet2004 requested a review from mahibi June 17, 2025 17:03
@rapterjet2004
Copy link
Contributor Author

rapterjet2004 commented Jun 17, 2025

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.

@mahibi
Copy link
Collaborator

mahibi commented Jun 18, 2025

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?

@@ -295,11 +311,20 @@ class MessageInputFragment : Fragment() {

private fun restoreState() {
if (binding.fragmentMessageInputView.inputEditText.text.isEmpty()) {
Log.d("Julius", "State restored from: ${chatActivity.localClassName}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove

@SystemKeeper
Copy link

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 rapterjet2004 changed the title Allow replies to maintain state on orientation change Refactor drafts to save both Messages & Replies to Room Storage Jun 18, 2025
@mahibi
Copy link
Collaborator

mahibi commented Jun 24, 2025

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

@rapterjet2004 rapterjet2004 added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 24, 2025
@rapterjet2004 rapterjet2004 requested a review from mahibi June 26, 2025 14:08
@rapterjet2004 rapterjet2004 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 26, 2025
@mahibi

This comment was marked as resolved.

Copy link
Collaborator

@mahibi mahibi left a comment

Choose a reason for hiding this comment

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

see comment

mahibi

This comment was marked as resolved.

@rapterjet2004
Copy link
Contributor Author

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.

Copy link
Collaborator

@mahibi mahibi left a comment

Choose a reason for hiding this comment

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

seems to work now. See comments for some improvement suggestions.

esp. runBlocking should be avoided. (I know.. we also use it in other places and it should also be improved there)

@mahibi
Copy link
Collaborator

mahibi commented Jul 17, 2025

@sowjanyakch can you please also review and test?

@mahibi mahibi added this to the 22.0.0 milestone Jul 17, 2025
@rapterjet2004 rapterjet2004 requested a review from mahibi July 18, 2025 14:18
@mahibi mahibi mentioned this pull request Jul 25, 2025
59 tasks
Copy link
Contributor

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.

Copy link
Contributor

Codacy

Lint

TypemasterPR
Warnings9898
Errors00

SpotBugs

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

@mahibi mahibi merged commit 479bbc8 into master Jul 25, 2025
14 of 16 checks passed
@mahibi mahibi deleted the issue-5027 branch July 25, 2025 12:13
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Screen rotation with quoted message
3 participants