From 74939da01f0546e1643dab977a07c8cd4bda800d Mon Sep 17 00:00:00 2001 From: ThomasSession Date: Mon, 21 Oct 2024 07:46:41 +1100 Subject: [PATCH] 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 711e31a43a889187ec3be189ad4aa78f18c217d7. * Avoiding adding reactions if the message is marked as deleted --- .../securesms/BaseActionBarActivity.java | 6 +- .../v2/ConversationReactionOverlay.kt | 7 +- .../conversation/v2/ConversationV2Dialogs.kt | 95 +++---------------- .../conversation/v2/ConversationViewModel.kt | 39 ++++---- .../v2/menus/ConversationMenuItemHelper.kt | 7 -- .../v2/messages/ControlMessageView.kt | 6 +- .../v2/messages/VisibleMessageView.kt | 2 +- .../securesms/database/Storage.kt | 9 ++ .../main/res/layout/view_deleted_message.xml | 13 +-- app/src/main/res/xml/preferences_privacy.xml | 6 -- .../utilities/TextSecurePreferences.kt | 11 --- 11 files changed, 57 insertions(+), 144 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java b/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java index c3321504ea..4e385cfe2b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/BaseActionBarActivity.java @@ -127,11 +127,7 @@ public abstract class BaseActionBarActivity extends AppCompatActivity { 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 index a76dead344..3aa19af994 100644 --- 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 index b9756a13be..a958f9d84d 100644 --- 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 index 29a9b9d79d..5ecddd2147 100644 --- 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,8 +816,7 @@ 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( @@ -827,6 +824,7 @@ class ConversationViewModel( 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): Commands() diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuItemHelper.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuItemHelper.kt index ffc36fddf6..3356453596 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuItemHelper.kt +++ b/app/src/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 index 89591777be..08a06407c5 100644 --- 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 index f4ace02e1a..98e864e472 100644 --- 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 index a1903cc891..d8460159fe 100644 --- 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 index 719783a0d9..f52f7b7a1b 100644 --- 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"> - -