Fix/message deletion issues (#1696)

* SES-2810 - Removing the screenshot privacy toggle * SES-2813 - clickable only when there is a 'follow settings' * SES-2815 - proper icon and spacing for deleted messages * Simplified deletion dialog to be reused for note to self and the rest as only the labels change * SES-2819 - Do not show a reaction on a deleted message * Fixing up deletion details Message view hides reactions completely if the message is marked as deleted All messages can now show the 'Delete' long press option Community messages should be removed completely not marked as deleted * Revert "SES-2819 - Do not show a reaction on a deleted message" This reverts commit 711e31a. * Avoiding adding reactions if the message is marked as deleted
oxen-io · Oct 20, 2024 · 74939da · 74939da
1 parent f6d50ac
commit 74939da
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 144 deletions.
diff --git a/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java b/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java
@@ -127,11 +127,7 @@ private void initializeScreenshotSecurity(boolean isResume) {
     if (!isResume) {
       getWindow().addFlags(WindowManager.LayoutParams.FLAG_SECURE);
     } else {
-      if (TextSecurePreferences.isScreenSecurityEnabled(this)) {
-        getWindow().addFlags(WindowManager.LayoutParams.FLAG_SECURE);
-      } else {
-        getWindow().clearFlags(WindowManager.LayoutParams.FLAG_SECURE);
-      }
+      getWindow().clearFlags(WindowManager.LayoutParams.FLAG_SECURE);
     }
   }
 }
diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt
@@ -41,7 +41,6 @@ import org.thoughtcrime.securesms.components.emoji.EmojiImageView
 import org.thoughtcrime.securesms.components.emoji.RecentEmojiPageModel
 import org.thoughtcrime.securesms.components.menu.ActionItem
 import org.thoughtcrime.securesms.conversation.v2.menus.ConversationMenuItemHelper.userCanBanSelectedUsers
-import org.thoughtcrime.securesms.conversation.v2.menus.ConversationMenuItemHelper.userCanDeleteSelectedItems
 import org.thoughtcrime.securesms.database.MmsSmsDatabase
 import org.thoughtcrime.securesms.database.model.MediaMmsMessageRecord
 import org.thoughtcrime.securesms.database.model.MessageRecord
@@ -559,10 +558,8 @@ class ConversationReactionOverlay : FrameLayout {
             items += ActionItem(R.attr.menu_copy_icon, R.string.accountIDCopy, { handleActionItemClicked(Action.COPY_ACCOUNT_ID) })
         }
         // Delete message
-        if (userCanDeleteSelectedItems(context, message, openGroup, userPublicKey, blindedPublicKey)) {
-            items += ActionItem(R.attr.menu_trash_icon, R.string.delete, { handleActionItemClicked(Action.DELETE) },
-                R.string.AccessibilityId_deleteMessage, message.subtitle, ThemeUtil.getThemedColor(context, R.attr.danger))
-        }
+        items += ActionItem(R.attr.menu_trash_icon, R.string.delete, { handleActionItemClicked(Action.DELETE) },
+            R.string.AccessibilityId_deleteMessage, message.subtitle, ThemeUtil.getThemedColor(context, R.attr.danger))
         // Ban user
         if (userCanBanSelectedUsers(context, message, openGroup, userPublicKey, blindedPublicKey) && !isDeleteOnly) {
             items += ActionItem(R.attr.menu_block_icon, R.string.banUser, { handleActionItemClicked(Action.BAN_USER) })

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationV2Dialogs.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationV2Dialogs.kt
@@ -19,6 +19,8 @@ import network.loki.messenger.R
 import org.session.libsession.utilities.StringSubstitutionConstants.APP_NAME_KEY
 import org.session.libsession.utilities.StringSubstitutionConstants.EMOJI_KEY
 import org.thoughtcrime.securesms.conversation.v2.ConversationViewModel.Commands.*
+import org.thoughtcrime.securesms.conversation.v2.ConversationViewModel.DeleteForEveryoneDialogData
+import org.thoughtcrime.securesms.database.model.MessageRecord
 import org.thoughtcrime.securesms.ui.AlertDialog
 import org.thoughtcrime.securesms.ui.DialogButtonModel
 import org.thoughtcrime.securesms.ui.GetString
@@ -48,9 +50,10 @@ fun ConversationV2Dialogs(
             )
         }
 
-        // delete message(s) for everyone
+        // delete message(s)
         if(dialogsState.deleteEveryone != null){
-            var deleteForEveryone by remember { mutableStateOf(dialogsState.deleteEveryone.defaultToEveryone)}
+            val data = dialogsState.deleteEveryone
+            var deleteForEveryone by remember { mutableStateOf(data.defaultToEveryone)}
 
             AlertDialog(
                 onDismissRequest = {
@@ -59,17 +62,17 @@ fun ConversationV2Dialogs(
                 },
                 title = pluralStringResource(
                     R.plurals.deleteMessage,
-                    dialogsState.deleteEveryone.messages.size,
-                    dialogsState.deleteEveryone.messages.size
+                    data.messages.size,
+                    data.messages.size
                 ),
                 text = pluralStringResource(
                     R.plurals.deleteMessageConfirm,
-                    dialogsState.deleteEveryone.messages.size,
-                    dialogsState.deleteEveryone.messages.size
+                    data.messages.size,
+                    data.messages.size
                 ),
                 content = {
                     // add warning text, if any
-                    dialogsState.deleteEveryone.warning?.let {
+                    data.warning?.let {
                         Text(
                             text = it,
                             textAlign = TextAlign.Center,
@@ -103,9 +106,9 @@ fun ConversationV2Dialogs(
                         ),
                         option = RadioOption(
                             value = Unit,
-                            title = GetString(stringResource(R.string.deleteMessageEveryone)),
+                            title = GetString(data.deleteForEveryoneLabel),
                             selected = deleteForEveryone,
-                            enabled = dialogsState.deleteEveryone.everyoneEnabled
+                            enabled = data.everyoneEnabled
                         )
                     ) {
                         deleteForEveryone = true
@@ -119,9 +122,9 @@ fun ConversationV2Dialogs(
                             // delete messages based on chosen option
                             sendCommand(
                                 if(deleteForEveryone) MarkAsDeletedForEveryone(
-                                    dialogsState.deleteEveryone.copy(defaultToEveryone = deleteForEveryone)
+                                    data.copy(defaultToEveryone = deleteForEveryone)
                                 )
-                                else MarkAsDeletedLocally(dialogsState.deleteEveryone.messages)
+                                else MarkAsDeletedLocally(data.messages)
                             )
                         }
                     ),
@@ -132,76 +135,6 @@ fun ConversationV2Dialogs(
             )
         }
 
-        // delete message(s) for all my devices
-        if(dialogsState.deleteAllDevices != null){
-            var deleteAllDevices by remember { mutableStateOf(dialogsState.deleteAllDevices.defaultToEveryone) }
-
-            AlertDialog(
-                onDismissRequest = {
-                    // hide dialog
-                    sendCommand(HideDeleteAllDevicesDialog)
-                },
-                title = pluralStringResource(
-                    R.plurals.deleteMessage,
-                    dialogsState.deleteAllDevices.messages.size,
-                    dialogsState.deleteAllDevices.messages.size
-                ),
-                text = pluralStringResource(
-                    R.plurals.deleteMessageConfirm,
-                    dialogsState.deleteAllDevices.messages.size,
-                    dialogsState.deleteAllDevices.messages.size
-                ),
-                content = {
-                    TitledRadioButton(
-                        contentPadding = PaddingValues(
-                            horizontal = LocalDimensions.current.xxsSpacing,
-                            vertical = 0.dp
-                        ),
-                        option = RadioOption(
-                            value = Unit,
-                            title = GetString(stringResource(R.string.deleteMessageDeviceOnly)),
-                            selected = !deleteAllDevices
-                        )
-                    ) {
-                        deleteAllDevices = false
-                    }
-
-                    TitledRadioButton(
-                        contentPadding = PaddingValues(
-                            horizontal = LocalDimensions.current.xxsSpacing,
-                            vertical = 0.dp
-                        ),
-                        option = RadioOption(
-                            value = Unit,
-                            title = GetString(stringResource(R.string.deleteMessageDevicesAll)),
-                            selected = deleteAllDevices
-                        )
-                    ) {
-                        deleteAllDevices = true
-                    }
-                },
-                buttons = listOf(
-                    DialogButtonModel(
-                        text = GetString(stringResource(id = R.string.delete)),
-                        color = LocalColors.current.danger,
-                        onClick = {
-                            // delete messages based on chosen option
-                            sendCommand(
-                                if(deleteAllDevices) MarkAsDeletedForEveryone(
-                                    dialogsState.deleteAllDevices.copy(defaultToEveryone = deleteAllDevices)
-                                )
-                                else MarkAsDeletedLocally(dialogsState.deleteAllDevices.messages)
-                            )
-                        }
-                    ),
-                    DialogButtonModel(
-                        GetString(stringResource(R.string.cancel))
-                    )
-                )
-            )
-        }
-
-
         // Clear emoji
         if(dialogsState.clearAllEmoji != null){
             AlertDialog(

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt
@@ -245,12 +245,12 @@ class ConversationViewModel(
 
             // hashes are required if wanting to delete messages from the 'storage server'
             // They are not required for communities OR if all messages are outgoing
-            // also we can only delete deleted messages (marked as deleted) locally
+            // also we can only delete deleted messages and control messages (marked as deleted) locally
             val canDeleteForEveryone = messages.all{ !it.isDeleted && !it.isControlMessage } && (
                     messages.all { it.isOutgoing } ||
                     conversationType == MessageType.COMMUNITY ||
-                            messages.all { lokiMessageDb.getMessageServerHash(it.id, it.isMms) != null
-            })
+                            messages.all { lokiMessageDb.getMessageServerHash(it.id, it.isMms) != null }
+                    )
 
             // There are three types of dialogs for deletion:
             // 1- Delete on device only OR all devices - Used for Note to self
@@ -260,11 +260,16 @@ class ConversationViewModel(
                 // the conversation is a note to self
                 conversationType == MessageType.NOTE_TO_SELF -> {
                     _dialogsState.update {
-                        it.copy(deleteAllDevices = DeleteForEveryoneDialogData(
+                        it.copy(deleteEveryone = DeleteForEveryoneDialogData(
                                 messages = messages,
                                 defaultToEveryone = false,
-                                everyoneEnabled = true,
-                                messageType = conversationType
+                                everyoneEnabled = canDeleteForEveryone,
+                                messageType = conversationType,
+                                deleteForEveryoneLabel = application.getString(R.string.deleteMessageDevicesAll),
+                                warning = if(canDeleteForEveryone) null else
+                                    application.resources.getQuantityString(
+                                        R.plurals.deleteMessageNoteToSelfWarning, messages.count(), messages.count()
+                                    )
                             )
                         )
                     }
@@ -278,6 +283,7 @@ class ConversationViewModel(
                                 messages = messages,
                                 defaultToEveryone = isAdmin.value,
                                 everyoneEnabled = true,
+                                deleteForEveryoneLabel = application.getString(R.string.deleteMessageEveryone),
                                 messageType = conversationType
                             )
                         )
@@ -293,6 +299,7 @@ class ConversationViewModel(
                                 defaultToEveryone = false,
                                 everyoneEnabled = false, // disable 'delete for everyone' - can only delete locally in this case
                                 messageType = conversationType,
+                                deleteForEveryoneLabel = application.getString(R.string.deleteMessageEveryone),
                                 warning = application.resources.getQuantityString(
                                     R.plurals.deleteMessageWarning, messages.count(), messages.count()
                                 )
@@ -548,7 +555,7 @@ class ConversationViewModel(
                     ).show()
                 }
 
-                _dialogsState.update { it.copy(deleteAllDevices = data) }
+                _dialogsState.update { it.copy(deleteEveryone = data) }
             }
 
             // hide loading indicator
@@ -565,11 +572,8 @@ class ConversationViewModel(
             try {
                 repository.deleteCommunityMessagesRemotely(threadId, data.messages)
 
-                // When this is done we simply need to remove the message locally
-                repository.markAsDeletedLocally(
-                    messages = data.messages,
-                    displayedMessage = application.getString(R.string.deleteMessageDeletedGlobally)
-                )
+                // When this is done we simply need to remove the message locally (leave nothing behind)
+                repository.deleteMessages(messages = data.messages, threadId = threadId)
 
                 // show confirmation toast
                 withContext(Dispatchers.Main) {
@@ -731,12 +735,6 @@ class ConversationViewModel(
                 }
             }
 
-            is Commands.HideDeleteAllDevicesDialog -> {
-                _dialogsState.update {
-                    it.copy(deleteAllDevices = null)
-                }
-            }
-
             is Commands.MarkAsDeletedLocally -> {
                 // hide dialog first
                 _dialogsState.update {
@@ -818,15 +816,15 @@ class ConversationViewModel(
     data class DialogsState(
         val openLinkDialogUrl: String? = null,
         val clearAllEmoji: ClearAllEmoji? = null,
-        val deleteEveryone: DeleteForEveryoneDialogData? = null,
-        val deleteAllDevices: DeleteForEveryoneDialogData? = null,
+        val deleteEveryone: DeleteForEveryoneDialogData? = null
     )
 
     data class DeleteForEveryoneDialogData(
         val messages: Set<MessageRecord>,
         val messageType: MessageType,
         val defaultToEveryone: Boolean,
         val everyoneEnabled: Boolean,
+        val deleteForEveryoneLabel: String,
         val warning: String? = null
     )
 
@@ -841,7 +839,6 @@ class ConversationViewModel(
         data class ClearEmoji(val emoji:String, val messageId: MessageId) : Commands()
 
         data object HideDeleteEveryoneDialog : Commands()
-        data object HideDeleteAllDevicesDialog : Commands()
         data object HideClearEmoji : Commands()
 
         data class MarkAsDeletedLocally(val messages: Set<MessageRecord>): Commands()

diff --git a/.../main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuItemHelper.kt b/.../main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuItemHelper.kt
@@ -7,13 +7,6 @@ import org.thoughtcrime.securesms.groups.OpenGroupManager
 
 object ConversationMenuItemHelper {
 
-    @JvmStatic
-    fun userCanDeleteSelectedItems(context: Context, message: MessageRecord, openGroup: OpenGroup?, userPublicKey: String, blindedPublicKey: String?): Boolean {
-        if (openGroup  == null) return message.isOutgoing || !message.isOutgoing
-        if (message.isOutgoing) return true
-        return OpenGroupManager.isUserModerator(context, openGroup.groupId, userPublicKey, blindedPublicKey)
-    }
-
     @JvmStatic
     fun userCanBanSelectedUsers(context: Context, message: MessageRecord, openGroup: OpenGroup?, userPublicKey: String, blindedPublicKey: String?): Boolean {
         if (openGroup == null)  return false

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/ControlMessageView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/ControlMessageView.kt
@@ -87,7 +87,11 @@ class ControlMessageView : LinearLayout {
                         && message.expiryMode != (MessagingModuleConfiguration.shared.storage.getExpirationConfiguration(message.threadId)?.expiryMode ?: ExpiryMode.NONE)
                         && threadRecipient?.isGroupRecipient != true
 
-                    binding.controlContentView.setOnClickListener { disappearingMessages.showFollowSettingDialog(context, message) }
+                    if (followSetting.isVisible) {
+                        binding.controlContentView.setOnClickListener { disappearingMessages.showFollowSettingDialog(context, message) }
+                    } else {
+                        binding.controlContentView.setOnClickListener(null)
+                    }
                 }
             }
             message.isMediaSavedNotification -> {

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt
@@ -234,7 +234,7 @@ class VisibleMessageView : FrameLayout {
         showStatusMessage(message)
 
         // Emoji Reactions
-        if (message.reactions.isNotEmpty()) {
+        if (!message.isDeleted && message.reactions.isNotEmpty()) {
             val capabilities = lokiThreadDb.getOpenGroupChat(threadID)?.server?.let { lokiApiDb.getServerCapabilities(it) }
             if (capabilities.isNullOrEmpty() || capabilities.contains(OpenGroupApi.Capability.REACTIONS.name.lowercase())) {
                 emojiReactionsBinding.value.root.let { root ->

diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt
@@ -1686,12 +1686,21 @@ open class Storage(
         val timestamp = reaction.timestamp
         val localId = reaction.localId
         val isMms = reaction.isMms
+
         val messageId = if (localId != null && localId > 0 && isMms != null) {
+            // bail early is the message is marked as deleted
+            val messagingDatabase: MessagingDatabase = if (isMms == true) DatabaseComponent.get(context).mmsDatabase()
+            else DatabaseComponent.get(context).smsDatabase()
+            if(messagingDatabase.getMessageRecord(localId)?.isDeleted == true) return
+
             MessageId(localId, isMms)
         } else if (timestamp != null && timestamp > 0) {
             val messageRecord = DatabaseComponent.get(context).mmsSmsDatabase().getMessageForTimestamp(timestamp) ?: return
+            if (messageRecord.isDeleted) return
+
             MessageId(messageRecord.id, messageRecord.isMms)
         } else return
+
         DatabaseComponent.get(context).reactionDatabase().addReaction(
             messageId,
             ReactionRecord(

diff --git a/app/src/main/res/layout/view_deleted_message.xml b/app/src/main/res/layout/view_deleted_message.xml
@@ -6,14 +6,15 @@
     android:layout_height="wrap_content"
     xmlns:tools="http://schemas.android.com/tools"
     android:orientation="horizontal"
-    android:padding="@dimen/small_spacing">
+    android:gravity="center_vertical"
+    android:paddingVertical="@dimen/small_spacing">
 
     <ImageView
         android:id="@+id/deletedMessageViewIconImageView"
-        android:layout_width="16dp"
-        android:layout_height="16dp"
-        android:layout_marginStart="@dimen/small_spacing"
-        android:src="?menu_trash_icon"
+        android:layout_width="19dp"
+        android:layout_height="19dp"
+        android:layout_marginStart="18dp"
+        android:src="@drawable/ic_delete"
         app:tint="?android:textColorPrimary" />
 
     <TextView
@@ -22,7 +23,7 @@
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginStart="4dp"
-        android:layout_marginEnd="@dimen/small_spacing"
+        android:layout_marginEnd="@dimen/large_spacing"
         android:textSize="@dimen/very_small_font_size"
         android:textColor="?android:textColorPrimary"
         tools:text="This message has been deleted"

diff --git a/app/src/main/res/xml/preferences_privacy.xml b/app/src/main/res/xml/preferences_privacy.xml
@@ -11,12 +11,6 @@
             android:title="@string/lockApp"
             android:summary="@string/lockAppDescription" />
 
-        <org.thoughtcrime.securesms.components.SwitchPreferenceCompat
-            android:defaultValue="@bool/screen_security_default"
-            android:key="pref_screen_security"
-            android:title="@string/screenshotNotifications"
-            android:summary="@string/screenshotNotificationsDescription" />
-
     </PreferenceCategory>
 
     <PreferenceCategory