improve send status handling

replace sendingFailed with SendStatus add auto migration (incl deleting of column sendingFailed) Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
2025-06-18 19:19:33 +01:00 · 2025-06-17 17:11:39 +02:00 · 2025-06-17 17:11:39 +02:00 · 86bfaa8657
commit 86bfaa8657
parent 8c066eb521
16 changed files with 49 additions and 75 deletions
--- a/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/17.json
+++ b/app/schemas/com.nextcloud.talk.data.source.local.TalkDatabase/17.json
@ -2,7 +2,7 @@
  "formatVersion": 1,
  "database": {
    "version": 17,
-    "identityHash": "c6e75dfc4f897469a82de6aeff6208c1",
+    "identityHash": "5bc4247e179307faa995552da5d34324",
    "entities": [
      {
        "tableName": "User",
@ -445,7 +445,7 @@
      },
      {
        "tableName": "ChatMessages",
-        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`internalId` TEXT NOT NULL, `accountId` INTEGER NOT NULL, `token` TEXT NOT NULL, `id` INTEGER NOT NULL, `internalConversationId` TEXT NOT NULL, `actorDisplayName` TEXT NOT NULL, `message` TEXT NOT NULL, `actorId` TEXT NOT NULL, `actorType` TEXT NOT NULL, `deleted` INTEGER NOT NULL, `expirationTimestamp` INTEGER NOT NULL, `isReplyable` INTEGER NOT NULL, `isTemporary` INTEGER NOT NULL, `lastEditActorDisplayName` TEXT, `lastEditActorId` TEXT, `lastEditActorType` TEXT, `lastEditTimestamp` INTEGER, `markdown` INTEGER, `messageParameters` TEXT, `messageType` TEXT NOT NULL, `parent` INTEGER, `reactions` TEXT, `reactionsSelf` TEXT, `referenceId` TEXT, `sendingFailed` INTEGER NOT NULL, `sendStatus` TEXT, `silent` INTEGER NOT NULL, `systemMessage` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, PRIMARY KEY(`internalId`), FOREIGN KEY(`internalConversationId`) REFERENCES `Conversations`(`internalId`) ON UPDATE CASCADE ON DELETE CASCADE )",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`internalId` TEXT NOT NULL, `accountId` INTEGER NOT NULL, `token` TEXT NOT NULL, `id` INTEGER NOT NULL, `internalConversationId` TEXT NOT NULL, `actorDisplayName` TEXT NOT NULL, `message` TEXT NOT NULL, `actorId` TEXT NOT NULL, `actorType` TEXT NOT NULL, `deleted` INTEGER NOT NULL, `expirationTimestamp` INTEGER NOT NULL, `isReplyable` INTEGER NOT NULL, `isTemporary` INTEGER NOT NULL, `lastEditActorDisplayName` TEXT, `lastEditActorId` TEXT, `lastEditActorType` TEXT, `lastEditTimestamp` INTEGER, `markdown` INTEGER, `messageParameters` TEXT, `messageType` TEXT NOT NULL, `parent` INTEGER, `reactions` TEXT, `reactionsSelf` TEXT, `referenceId` TEXT, `sendStatus` TEXT, `silent` INTEGER NOT NULL, `systemMessage` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, PRIMARY KEY(`internalId`), FOREIGN KEY(`internalConversationId`) REFERENCES `Conversations`(`internalId`) ON UPDATE CASCADE ON DELETE CASCADE )",
        "fields": [
          {
            "fieldPath": "internalId",
@ -581,12 +581,6 @@
            "columnName": "referenceId",
            "affinity": "TEXT"
          },
-          {
-            "fieldPath": "sendingFailed",
-            "columnName": "sendingFailed",
-            "affinity": "INTEGER",
-            "notNull": true
-          },
          {
            "fieldPath": "sendStatus",
            "columnName": "sendStatus",
@ -730,7 +724,7 @@
    ],
    "setupQueries": [
      "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
-      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'c6e75dfc4f897469a82de6aeff6208c1')"
+      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '5bc4247e179307faa995552da5d34324')"
    ]
  }
 }
--- a/app/src/main/java/com/nextcloud/talk/adapters/messages/OutcomingTextMessageViewHolder.kt
+++ b/app/src/main/java/com/nextcloud/talk/adapters/messages/OutcomingTextMessageViewHolder.kt
@ -29,6 +29,7 @@ import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedA
 import com.nextcloud.talk.chat.ChatActivity
 import com.nextcloud.talk.chat.data.ChatMessageRepository
 import com.nextcloud.talk.chat.data.model.ChatMessage
+import com.nextcloud.talk.data.database.model.SendStatus
 import com.nextcloud.talk.data.network.NetworkMonitor
 import com.nextcloud.talk.data.user.model.User
 import com.nextcloud.talk.databinding.ItemCustomOutcomingTextMessageBinding
@ -184,7 +185,7 @@ class OutcomingTextMessageViewHolder(itemView: View) :
        binding.checkMark.visibility = View.INVISIBLE
        binding.sendingProgress.visibility = View.GONE

-        if (message.sendingFailed) {
+        if (message.sendStatus == SendStatus.FAILED) {
            updateStatus(R.drawable.baseline_error_outline_24, context.resources?.getString(R.string.nc_message_failed))
        } else if (message.isTemporary) {
            updateStatus(R.drawable.baseline_schedule_24, context.resources?.getString(R.string.nc_message_sending))
--- a/app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt
+++ b/app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt
@ -200,7 +200,7 @@ class MessageInputFragment : Fragment() {
                    val connectionGained = (!wasOnline && isOnline)
                    Log.d(TAG, "isOnline: $isOnline\nwasOnline: $wasOnline\nconnectionGained: $connectionGained")
                    if (connectionGained) {
-                        chatActivity.messageInputViewModel.sendTempMessages(
+                        chatActivity.messageInputViewModel.sendUnsentMessages(
                            chatActivity.conversationUser!!.getCredentials(),
                            ApiUtils.getUrlForChat(
                                chatActivity.chatApiVersion,
--- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt
+++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt
@ -110,7 +110,7 @@ interface ChatMessageRepository : LifecycleAwareManager {

    suspend fun editTempChatMessage(message: ChatMessage, editedMessageText: String): Flow<Boolean>

-    suspend fun sendTempChatMessages(credentials: String, url: String)
+    suspend fun sendUnsentChatMessages(credentials: String, url: String)

    suspend fun deleteTempMessage(chatMessage: ChatMessage)
 }
--- a/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt
+++ b/app/src/main/java/com/nextcloud/talk/chat/data/model/ChatMessage.kt
@ -14,6 +14,7 @@ import android.util.Log
 import com.bluelinelabs.logansquare.annotation.JsonIgnore
 import com.nextcloud.talk.R
 import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication
+import com.nextcloud.talk.data.database.model.SendStatus
 import com.nextcloud.talk.data.user.model.User
 import com.nextcloud.talk.models.json.chat.ChatUtils.Companion.getParsedMessage
 import com.nextcloud.talk.models.json.chat.ReadStatus
@ -119,7 +120,7 @@ data class ChatMessage(

    var referenceId: String? = null,

-    var sendingFailed: Boolean = true,
+    var sendStatus: SendStatus? = null,

    var silent: Boolean = false

--- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt
+++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt
@ -215,7 +215,8 @@ class OfflineFirstChatRepository @Inject constructor(
                )
            }

-            sendTempChatMessages(credentials, urlForChatting)
+            // this call could be deleted when we have a worker to send messages..
+            sendUnsentChatMessages(credentials, urlForChatting)

            // delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing
            // with them (otherwise there is a race condition).
@ -845,8 +846,6 @@ class OfflineFirstChatRepository @Inject constructor(
            }
        }

-        Log.d(TAG, "sending chat message: " + message)
-
        return flow {
            val response = network.sendChatMessage(
                credentials,
@ -881,7 +880,6 @@ class OfflineFirstChatRepository @Inject constructor(
                    referenceId
                ).firstOrNull()
                failedMessage?.let {
-                    // it.sendingFailed = true
                    it.sendStatus = SendStatus.FAILED
                    chatDao.updateChatMessage(it)

@ -903,7 +901,6 @@ class OfflineFirstChatRepository @Inject constructor(
        referenceId: String
    ): Flow<Result<ChatMessage?>> {
        val messageToResend = chatDao.getTempMessageForConversation(internalConversationId, referenceId).first()
-        // messageToResend.sendingFailed = false
        messageToResend.sendStatus = SendStatus.PENDING
        chatDao.updateChatMessage(messageToResend)

@ -960,8 +957,8 @@ class OfflineFirstChatRepository @Inject constructor(
            }
        }

-    override suspend fun sendTempChatMessages(credentials: String, url: String) {
-        val tempMessages = chatDao.getPendingOrFailedMessagesForConversation(internalConversationId).first()
+    override suspend fun sendUnsentChatMessages(credentials: String, url: String) {
+        val tempMessages = chatDao.getTempUnsentMessagesForConversation(internalConversationId).first()
        tempMessages.sortedBy { it.internalId }.onEach {
            sendChatMessage(
                credentials,
@ -1055,7 +1052,7 @@ class OfflineFirstChatRepository @Inject constructor(
            actorDisplayName = currentUser.displayName!!,
            referenceId = referenceId,
            isTemporary = true,
-            sendingFailed = false,
+            sendStatus = SendStatus.PENDING,
            silent = sendWithoutNotification
        )
        return entity
--- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/MessageInputViewModel.kt
+++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/MessageInputViewModel.kt
@ -169,9 +169,9 @@ class MessageInputViewModel @Inject constructor(
        }
    }

-    fun sendTempMessages(credentials: String, url: String) {
+    fun sendUnsentMessages(credentials: String, url: String) {
        viewModelScope.launch {
-            chatRepository.sendTempChatMessages(
+            chatRepository.sendUnsentChatMessages(
                credentials,
                url
            )
--- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt
@ -56,22 +56,11 @@ interface ChatMessagesDao {
        FROM ChatMessages
        WHERE internalConversationId = :internalConversationId
        AND isTemporary = 1 
-        AND (sendStatus = 'PENDING' OR sendStatus = 'FAILED')
+        AND sendStatus != 'SENT_PENDING_ACK'
        ORDER BY timestamp DESC, id DESC
        """
    )
-    fun getPendingOrFailedMessagesForConversation(internalConversationId: String): Flow<List<ChatMessageEntity>>
-
-    @Query(
-        """
-        SELECT *
-        FROM ChatMessages
-        WHERE internalConversationId = :internalConversationId
-        AND (isTemporary = 1 OR sendStatus = 'SENT_PENDING_ACK')
-        ORDER BY timestamp DESC, id DESC
-        """
-    )
-    fun getTempOrSendingAckMessagesForConversation(internalConversationId: String): Flow<List<ChatMessageEntity>>
+    fun getTempUnsentMessagesForConversation(internalConversationId: String): Flow<List<ChatMessageEntity>>

    @Query(
        """
--- a/app/src/main/java/com/nextcloud/talk/data/database/mappers/ChatMessageMapUtils.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/database/mappers/ChatMessageMapUtils.kt
@ -68,7 +68,7 @@ fun ChatMessageEntity.asModel() =
        isDeleted = deleted,
        referenceId = referenceId,
        isTemporary = isTemporary,
-        sendingFailed = sendingFailed,
+        sendStatus = sendStatus,
        readStatus = ReadStatus.NONE,
        silent = silent
    )
--- a/app/src/main/java/com/nextcloud/talk/data/database/model/ChatMessageEntity.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/database/model/ChatMessageEntity.kt
@ -64,7 +64,6 @@ data class ChatMessageEntity(
    @ColumnInfo(name = "reactions") var reactions: LinkedHashMap<String, Int>? = null,
    @ColumnInfo(name = "reactionsSelf") var reactionsSelf: ArrayList<String>? = null,
    @ColumnInfo(name = "referenceId") var referenceId: String? = null,
-    @ColumnInfo(name = "sendingFailed") var sendingFailed: Boolean = false,
    @ColumnInfo(name = "sendStatus") var sendStatus: SendStatus? = null,
    @ColumnInfo(name = "silent") var silent: Boolean = false,
    @ColumnInfo(name = "systemMessage") var systemMessageType: ChatMessage.SystemMessageType,
--- a/app/src/main/java/com/nextcloud/talk/data/database/model/SendStatus.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/database/model/SendStatus.kt
@ -1,7 +1,7 @@
 /*
 * Nextcloud Talk - Android Client
 *
- * SPDX-FileCopyrightText: 2025 Your Name <your@email.com>
+ * SPDX-FileCopyrightText: 2025 Marcel Hibbe <dev@mhibbe.de>
 * SPDX-License-Identifier: GPL-3.0-or-later
 */

--- a/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/source/local/Migrations.kt
@ -1,18 +1,31 @@
 /*
 * Nextcloud Talk - Android Client
 *
+ * SPDX-FileCopyrightText: 2024-2025 Marcel Hibbe <dev@mhibbe.de>
 * SPDX-FileCopyrightText: 2022 Andy Scherzinger <info@andy-scherzinger.de>
 * SPDX-License-Identifier: GPL-3.0-or-later
 */
 package com.nextcloud.talk.data.source.local

 import android.util.Log
+import androidx.room.DeleteColumn
+import androidx.room.migration.AutoMigrationSpec
 import androidx.room.migration.Migration
 import androidx.sqlite.db.SupportSQLiteDatabase
 import java.sql.SQLException

@Suppress("MagicNumber")
 object Migrations {
+
+    //region Auto migrations
+
+    @DeleteColumn(tableName = "ChatMessages", columnName = "sendingFailed")
+    class AutoMigration16To17 : AutoMigrationSpec
+
+    //endregion
+
+    //region Manual migrations
+
    val MIGRATION_6_8 = object : Migration(6, 8) {
        override fun migrate(db: SupportSQLiteDatabase) {
            Log.i("Migrations", "Migrating 6 to 8")
@ -76,12 +89,7 @@ object Migrations {
        }
    }

-    val MIGRATION_16_17 = object : Migration(16, 17) {
-        override fun migrate(db: SupportSQLiteDatabase) {
-            Log.i("Migrations", "Migrating 16 to 17")
-            addSendStatus(db)
-        }
-    }
+    //endregion

    fun migrateToRoom(db: SupportSQLiteDatabase) {
        db.execSQL(
@ -326,17 +334,6 @@ object Migrations {
        }
    }

-    private fun addSendStatus(db: SupportSQLiteDatabase) {
-        try {
-            db.execSQL(
-                "ALTER TABLE ChatMessages " +
-                    "ADD COLUMN sendStatus TEXT NOT NULL DEFAULT 'PENDING'"
-            )
-        } catch (e: SQLException) {
-            Log.i("Migrations", "Something went wrong when adding column sendStatus to table ChatMessages")
-        }
-    }
-
    fun addTempMessagesSupport(db: SupportSQLiteDatabase) {
        try {
            db.execSQL(
--- a/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/source/local/TalkDatabase.kt
@ -1,7 +1,7 @@
 /*
 * Nextcloud Talk - Android Client
 *
- * SPDX-FileCopyrightText: 2023-2024 Marcel Hibbe <dev@mhibbe.de>
+ * SPDX-FileCopyrightText: 2023-2025 Marcel Hibbe <dev@mhibbe.de>
 * SPDX-FileCopyrightText: 2022 Andy Scherzinger <info@andy-scherzinger.de>
 * SPDX-FileCopyrightText: 2017-2020 Mario Danic <mario@lovelyhq.com>
 * SPDX-License-Identifier: GPL-3.0-or-later
@ -23,6 +23,7 @@ import com.nextcloud.talk.data.database.dao.ConversationsDao
 import com.nextcloud.talk.data.database.model.ChatBlockEntity
 import com.nextcloud.talk.data.database.model.ChatMessageEntity
 import com.nextcloud.talk.data.database.model.ConversationEntity
+import com.nextcloud.talk.data.source.local.Migrations.AutoMigration16To17
 import com.nextcloud.talk.data.source.local.converters.ArrayListConverter
 import com.nextcloud.talk.data.source.local.converters.CapabilitiesConverter
 import com.nextcloud.talk.data.source.local.converters.ExternalSignalingServerConverter
@ -52,7 +53,8 @@ import java.util.Locale
    ],
    version = 17,
    autoMigrations = [
-        AutoMigration(from = 9, to = 10)
+        AutoMigration(from = 9, to = 10),
+        AutoMigration(from = 16, to = 17, spec = AutoMigration16To17::class)
    ],
    exportSchema = true
 )
@ -110,7 +112,7 @@ abstract class TalkDatabase : RoomDatabase() {
            return Room
                .databaseBuilder(context.applicationContext, TalkDatabase::class.java, dbName)
                // comment out openHelperFactory to view the database entries in Android Studio for debugging
-                // .openHelperFactory(factory)
+                .openHelperFactory(factory)
                .addMigrations(
                    Migrations.MIGRATION_6_8,
                    Migrations.MIGRATION_7_8,
@ -120,8 +122,7 @@ abstract class TalkDatabase : RoomDatabase() {
                    Migrations.MIGRATION_12_13,
                    Migrations.MIGRATION_13_14,
                    Migrations.MIGRATION_14_15,
-                    Migrations.MIGRATION_15_16,
-                    Migrations.MIGRATION_16_17
+                    Migrations.MIGRATION_15_16
                )
                .allowMainThreadQueries()
                .addCallback(
--- a/app/src/main/java/com/nextcloud/talk/data/source/local/converters/SendStatusConverter.kt
+++ b/app/src/main/java/com/nextcloud/talk/data/source/local/converters/SendStatusConverter.kt
@ -1,7 +1,7 @@
 /*
 * Nextcloud Talk - Android Client
 *
- * SPDX-FileCopyrightText: 2025 Your Name <your@email.com>
+ * SPDX-FileCopyrightText: 2025 Marcel Hibbe <dev@mhibbe.de>
 * SPDX-License-Identifier: GPL-3.0-or-later
 */

--- a/app/src/main/java/com/nextcloud/talk/ui/dialog/TempMessageActionsDialog.kt
+++ b/app/src/main/java/com/nextcloud/talk/ui/dialog/TempMessageActionsDialog.kt
@ -18,6 +18,7 @@ import com.nextcloud.talk.R
 import com.nextcloud.talk.application.NextcloudTalkApplication
 import com.nextcloud.talk.chat.ChatActivity
 import com.nextcloud.talk.chat.data.model.ChatMessage
+import com.nextcloud.talk.data.database.model.SendStatus
 import com.nextcloud.talk.data.network.NetworkMonitor
 import com.nextcloud.talk.databinding.DialogTempMessageActionsBinding
 import com.nextcloud.talk.ui.theme.ViewThemeUtils
@ -58,9 +59,10 @@ class TempMessageActionsDialog(

    private fun initMenuItems() {
        this.lifecycleScope.launch {
-            initResendMessage(message.sendingFailed && networkMonitor.isOnline.value)
-            initMenuEditMessage(message.sendingFailed || !networkMonitor.isOnline.value)
-            initMenuDeleteMessage(message.sendingFailed || !networkMonitor.isOnline.value)
+            val sendingFailed = message.sendStatus == SendStatus.FAILED
+            initResendMessage(sendingFailed && networkMonitor.isOnline.value)
+            initMenuEditMessage(sendingFailed || !networkMonitor.isOnline.value)
+            initMenuDeleteMessage(sendingFailed || !networkMonitor.isOnline.value)
            initMenuItemCopy()
        }
    }
--- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt
+++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt
@ -30,16 +30,9 @@ class DummyChatMessagesDaoImpl : ChatMessagesDao {
    override fun getTempMessagesForConversation(internalConversationId: String): Flow<List<ChatMessageEntity>> =
        flowOf()

-    override fun getPendingOrFailedMessagesForConversation(
-        internalConversationId: String
-    ): Flow<List<ChatMessageEntity>> {
-        TODO("Not yet implemented")
-    }
-
-    override fun getTempOrSendingAckMessagesForConversation(
-        internalConversationId: String
-    ): Flow<List<ChatMessageEntity>> {
-        TODO("Not yet implemented")
+    override fun getTempUnsentMessagesForConversation(internalConversationId: String): Flow<List<ChatMessageEntity>> {
+        // nothing to return here as long this class is only used for the Search window
+        return flowOf()
    }

    override fun getTempMessageForConversation(