SES-1156 - Ban and delete functionality fix (#1428)

* WIP * Investigation in progress * End of day push * WIP * Fixes signalapp#1416 * Cleanup * Added code to remove zombie messages caught in limbo during a ban & delete - still chock full o' debug while finding root cause * Root cause debug WIP * Push prior to cleanup * Cleaned up for PR * fix: mms delete, remove unnecessary values from sms * Addressed PR feedback * fix: fix unit tests * Added '.run' folder with test setup * Update README.md Test commit for CI * Re-added accidentally removed closing brace --------- Co-authored-by: alansley <[email protected]> Co-authored-by: Al Lansley <[email protected]> Co-authored-by: 0x330a <[email protected]>
oxen-io · Apr 2, 2024 · a8a257a · a8a257a
1 parent 9ad5bd2
commit a8a257a
Show file tree

Hide file tree

Showing 44 changed files with 350 additions and 142 deletions.
diff --git a/.run/Run Tests.run.xml b/.run/Run Tests.run.xml
@@ -0,0 +1,24 @@
+<component name="ProjectRunConfigurationManager">
+  <configuration default="false" name="Run Tests" type="GradleRunConfiguration" factoryName="Gradle">
+    <ExternalSystemSettings>
+      <option name="executionName" />
+      <option name="externalProjectPath" value="$PROJECT_DIR$" />
+      <option name="externalSystemIdString" value="GRADLE" />
+      <option name="scriptParameters" value="" />
+      <option name="taskDescriptions">
+        <list />
+      </option>
+      <option name="taskNames">
+        <list>
+          <option value="testPlayDebugUnitTestCoverageReport" />
+        </list>
+      </option>
+      <option name="vmOptions" />
+    </ExternalSystemSettings>
+    <ExternalSystemDebugServerProcess>true</ExternalSystemDebugServerProcess>
+    <ExternalSystemReattachDebugProcess>true</ExternalSystemReattachDebugProcess>
+    <DebugAllEnabled>false</DebugAllEnabled>
+    <RunAsTest>false</RunAsTest>
+    <method v="2" />
+  </configuration>
+</component>
diff --git a/README.md b/README.md
@@ -1,4 +1,4 @@
-# Session Android
+# Session Android 
 
 [Download on the Google Play Store](https://getsession.org/android)
 

diff --git a/app/src/main/java/org/thoughtcrime/securesms/attachments/DatabaseAttachmentProvider.kt b/app/src/main/java/org/thoughtcrime/securesms/attachments/DatabaseAttachmentProvider.kt
@@ -184,16 +184,21 @@ class DatabaseAttachmentProvider(context: Context, helper: SQLCipherOpenHelper)
     override fun deleteMessage(messageID: Long, isSms: Boolean) {
         val messagingDatabase: MessagingDatabase = if (isSms)  DatabaseComponent.get(context).smsDatabase()
                                                    else DatabaseComponent.get(context).mmsDatabase()
+
         messagingDatabase.deleteMessage(messageID)
         DatabaseComponent.get(context).lokiMessageDatabase().deleteMessage(messageID, isSms)
         DatabaseComponent.get(context).lokiMessageDatabase().deleteMessageServerHash(messageID, mms = !isSms)
     }
 
     override fun deleteMessages(messageIDs: List<Long>, threadId: Long, isSms: Boolean) {
+
         val messagingDatabase: MessagingDatabase = if (isSms)  DatabaseComponent.get(context).smsDatabase()
                                                    else DatabaseComponent.get(context).mmsDatabase()
 
+        // Perform local delete
         messagingDatabase.deleteMessages(messageIDs.toLongArray(), threadId)
+
+        // Perform online delete
         DatabaseComponent.get(context).lokiMessageDatabase().deleteMessages(messageIDs)
         DatabaseComponent.get(context).lokiMessageDatabase().deleteMessageServerHashes(messageIDs, mms = !isSms)
     }

diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/ProfilePictureView.kt b/app/src/main/java/org/thoughtcrime/securesms/components/ProfilePictureView.kt
@@ -132,7 +132,8 @@ class ProfilePictureView @JvmOverloads constructor(
                     .diskCacheStrategy(DiskCacheStrategy.NONE)
                     .circleCrop()
                     .into(imageView)
-            } else if (recipient.isOpenGroupRecipient && recipient.groupAvatarId == null) {
+            } else if (recipient.isCommunityRecipient && recipient.groupAvatarId == null) {
+                glide.clear(imageView)
                 glide.load(unknownOpenGroupDrawable)
                     .centerCrop()
                     .circleCrop()

diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/ContactSelectionListLoader.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/ContactSelectionListLoader.kt
@@ -58,7 +58,7 @@ class ContactSelectionListLoader(context: Context, val mode: Int, val filter: St
 
     private fun getOpenGroups(contacts: List<Recipient>): List<ContactSelectionListItem> {
         return getItems(contacts, context.getString(R.string.fragment_contact_selection_open_groups_title)) {
-            it.address.isOpenGroup
+            it.address.isCommunity
         }
     }
 

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActionBarView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActionBarView.kt
@@ -116,7 +116,7 @@ class ConversationActionBarView @JvmOverloads constructor(
             )
         }
         if (recipient.isGroupRecipient) {
-            val title = if (recipient.isOpenGroupRecipient) {
+            val title = if (recipient.isCommunityRecipient) {
                 val userCount = openGroup?.let { lokiApiDb.getUserCount(it.room, it.server) } ?: 0
                 context.getString(R.string.ConversationActivity_active_member_count, userCount)
             } else {

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt
@@ -395,7 +395,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         messageToScrollAuthor.set(intent.getParcelableExtra(SCROLL_MESSAGE_AUTHOR))
         val recipient = viewModel.recipient
         val openGroup = recipient.let { viewModel.openGroup }
-        if (recipient == null || (recipient.isOpenGroupRecipient && openGroup == null)) {
+        if (recipient == null || (recipient.isCommunityRecipient && openGroup == null)) {
             Toast.makeText(this, "This thread has been deleted.", Toast.LENGTH_LONG).show()
             return finish()
         }
@@ -976,11 +976,11 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
             view.glide = glide
             view.onCandidateSelected = { handleMentionSelected(it) }
             additionalContentContainer.addView(view)
-            val candidates = MentionsManager.getMentionCandidates(query, viewModel.threadId, recipient.isOpenGroupRecipient)
+            val candidates = MentionsManager.getMentionCandidates(query, viewModel.threadId, recipient.isCommunityRecipient)
             this.mentionCandidatesView = view
             view.show(candidates, viewModel.threadId)
         } else {
-            val candidates = MentionsManager.getMentionCandidates(query, viewModel.threadId, recipient.isOpenGroupRecipient)
+            val candidates = MentionsManager.getMentionCandidates(query, viewModel.threadId, recipient.isCommunityRecipient)
             this.mentionCandidatesView!!.setMentionCandidates(candidates)
         }
         isShowingMentionCandidatesView = true
@@ -1197,7 +1197,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
     }
 
     override fun copyOpenGroupUrl(thread: Recipient) {
-        if (!thread.isOpenGroupRecipient) { return }
+        if (!thread.isCommunityRecipient) { return }
 
         val threadId = threadDb.getThreadIdIfExistsFor(thread) ?: return
         val openGroup = lokiThreadDb.getOpenGroupChat(threadId) ?: return
@@ -1361,7 +1361,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         } else originalMessage.individualRecipient.address
         // Send it
         reactionMessage.reaction = Reaction.from(originalMessage.timestamp, originalAuthor.serialize(), emoji, true)
-        if (recipient.isOpenGroupRecipient) {
+        if (recipient.isCommunityRecipient) {
             val messageServerId = lokiMessageDb.getServerID(originalMessage.id, !originalMessage.isMms) ?: return
             viewModel.openGroup?.let {
                 OpenGroupApi.addReaction(it.room, it.server, messageServerId, emoji)
@@ -1385,7 +1385,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         } else originalMessage.individualRecipient.address
 
         message.reaction = Reaction.from(originalMessage.timestamp, originalAuthor.serialize(), emoji, false)
-        if (recipient.isOpenGroupRecipient) {
+        if (recipient.isCommunityRecipient) {
             val messageServerId = lokiMessageDb.getServerID(originalMessage.id, !originalMessage.isMms) ?: return
             viewModel.openGroup?.let {
                 OpenGroupApi.deleteReaction(it.room, it.server, messageServerId, emoji)
@@ -1782,7 +1782,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
                 sendAttachments(slideDeck.asAttachments(), body)
             }
             INVITE_CONTACTS -> {
-                if (viewModel.recipient?.isOpenGroupRecipient != true) { return }
+                if (viewModel.recipient?.isCommunityRecipient != true) { return }
                 val extras = intent?.extras ?: return
                 if (!intent.hasExtra(selectedContactsKey)) { return }
                 val selectedContacts = extras.getStringArray(selectedContactsKey)!!
@@ -1848,19 +1848,62 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         handleLongPress(messages.first(), 0) //TODO: begin selection mode
     }
 
+    // The option to "Delete just for me" or "Delete for everyone"
+    private fun showDeleteOrDeleteForEveryoneInCommunityUI(messages: Set<MessageRecord>) {
+        val bottomSheet = DeleteOptionsBottomSheet()
+        bottomSheet.recipient = viewModel.recipient!!
+        bottomSheet.onDeleteForMeTapped = {
+            messages.forEach(viewModel::deleteLocally)
+            bottomSheet.dismiss()
+            endActionMode()
+        }
+        bottomSheet.onDeleteForEveryoneTapped = {
+            messages.forEach(viewModel::deleteForEveryone)
+            bottomSheet.dismiss()
+            endActionMode()
+        }
+        bottomSheet.onCancelTapped = {
+            bottomSheet.dismiss()
+            endActionMode()
+        }
+        bottomSheet.show(supportFragmentManager, bottomSheet.tag)
+    }
+
+    private fun showDeleteLocallyUI(messages: Set<MessageRecord>) {
+        val messageCount = 1
+        showSessionDialog {
+            title(resources.getQuantityString(R.plurals.ConversationFragment_delete_selected_messages, messageCount, messageCount))
+            text(resources.getQuantityString(R.plurals.ConversationFragment_this_will_permanently_delete_all_n_selected_messages, messageCount, messageCount))
+            button(R.string.delete) { messages.forEach(viewModel::deleteLocally); endActionMode() }
+            cancelButton(::endActionMode)
+        }
+    }
+
+    // Note: The messages in the provided set may be a single message, or multiple if there are a
+    // group of selected messages.
     override fun deleteMessages(messages: Set<MessageRecord>) {
-        val recipient = viewModel.recipient ?: return
+        val recipient = viewModel.recipient
+        if (recipient == null) {
+            Log.w("ConversationActivityV2", "Asked to delete messages but could not obtain viewModel recipient - aborting.")
+            return
+        }
+
         val allSentByCurrentUser = messages.all { it.isOutgoing }
         val allHasHash = messages.all { lokiMessageDb.getMessageServerHash(it.id, it.isMms) != null }
-        if (recipient.isOpenGroupRecipient) {
-            val messageCount = 1
+
+        // If the recipient is a community then we delete the message for everyone
+        if (recipient.isCommunityRecipient) {
+            val messageCount = 1 // Only used for plurals string
 
             showSessionDialog {
                 title(resources.getQuantityString(R.plurals.ConversationFragment_delete_selected_messages, messageCount, messageCount))
                 text(resources.getQuantityString(R.plurals.ConversationFragment_this_will_permanently_delete_all_n_selected_messages, messageCount, messageCount))
-                button(R.string.delete) { messages.forEach(viewModel::deleteForEveryone); endActionMode() }
+                button(R.string.delete) {
+                    messages.forEach(viewModel::deleteForEveryone); endActionMode()
+                }
                 cancelButton { endActionMode() }
             }
+        // Otherwise if this is a 1-on-1 conversation we may decided to delete just for ourselves or delete for everyone
         } else if (allSentByCurrentUser && allHasHash) {
             val bottomSheet = DeleteOptionsBottomSheet()
             bottomSheet.recipient = recipient
@@ -1879,13 +1922,17 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
                 endActionMode()
             }
             bottomSheet.show(supportFragmentManager, bottomSheet.tag)
-        } else {
+        }
+        else // Finally, if this is a closed group and you are deleting someone else's message(s)
+        // then we can only delete locally.
+        {
             val messageCount = 1
-
             showSessionDialog {
                 title(resources.getQuantityString(R.plurals.ConversationFragment_delete_selected_messages, messageCount, messageCount))
                 text(resources.getQuantityString(R.plurals.ConversationFragment_this_will_permanently_delete_all_n_selected_messages, messageCount, messageCount))
-                button(R.string.delete) { messages.forEach(viewModel::deleteLocally); endActionMode() }
+                button(R.string.delete) {
+                    messages.forEach(viewModel::deleteLocally); endActionMode()
+                }
                 cancelButton(::endActionMode)
             }
         }
@@ -1904,7 +1951,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe
         showSessionDialog {
             title(R.string.ConversationFragment_ban_selected_user)
             text("This will ban the selected user from this room and delete all messages sent by them. It won't ban them from other rooms or delete the messages they sent there.")
-            button(R.string.ban) { viewModel.banAndDeleteAll(messages.first().individualRecipient); endActionMode() }
+            button(R.string.ban) { viewModel.banAndDeleteAll(messages.first()); endActionMode() }
             cancelButton(::endActionMode)
         }
     }

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
@@ -541,7 +541,7 @@ class ConversationReactionOverlay : FrameLayout {
             items += ActionItem(R.attr.menu_copy_icon, R.string.copy, { handleActionItemClicked(Action.COPY_MESSAGE) })
         }
         // Copy Session ID
-        if (recipient.isGroupRecipient && !recipient.isOpenGroupRecipient && message.recipient.address.toString() != userPublicKey) {
+        if (recipient.isGroupRecipient && !recipient.isCommunityRecipient && message.recipient.address.toString() != userPublicKey) {
             items += ActionItem(R.attr.menu_copy_icon, R.string.activity_conversation_menu_copy_session_id, { handleActionItemClicked(Action.COPY_SESSION_ID) })
         }
         // Delete message

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
@@ -1,18 +1,23 @@
 package org.thoughtcrime.securesms.conversation.v2
 
 import android.content.Context
+
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.viewModelScope
+
 import com.goterl.lazysodium.utils.KeyPair
+
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
+
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
+
 import org.session.libsession.messaging.messages.ExpirationConfiguration
 import org.session.libsession.messaging.open_groups.OpenGroup
 import org.session.libsession.messaging.open_groups.OpenGroupApi
@@ -22,9 +27,12 @@ import org.session.libsession.utilities.Address
 import org.session.libsession.utilities.recipients.Recipient
 import org.session.libsignal.utilities.IdPrefix
 import org.session.libsignal.utilities.Log
+import org.thoughtcrime.securesms.database.MmsSmsDatabase
+
 import org.thoughtcrime.securesms.database.Storage
 import org.thoughtcrime.securesms.database.model.MessageRecord
 import org.thoughtcrime.securesms.repository.ConversationRepository
+
 import java.util.UUID
 
 class ConversationViewModel(
@@ -144,9 +152,14 @@ class ConversationViewModel(
     }
 
     fun deleteForEveryone(message: MessageRecord) = viewModelScope.launch {
-        val recipient = recipient ?: return@launch
+        val recipient = recipient ?: return@launch Log.w("Loki", "Recipient was null for delete for everyone - aborting delete operation.")
+
         repository.deleteForEveryone(threadId, recipient, message)
+            .onSuccess {
+                Log.d("Loki", "Deleted message ${message.id} ")
+            }
             .onFailure {
+                Log.w("Loki", "FAILED TO delete message ${message.id} ")
                 showMessage("Couldn't delete message due to error: $it")
             }
     }
@@ -168,10 +181,15 @@ class ConversationViewModel(
             }
     }
 
-    fun banAndDeleteAll(recipient: Recipient) = viewModelScope.launch {
-        repository.banAndDeleteAll(threadId, recipient)
+    fun banAndDeleteAll(messageRecord: MessageRecord) = viewModelScope.launch {
+
+        repository.banAndDeleteAll(threadId, messageRecord.individualRecipient)
             .onSuccess {
+                // At this point the server side messages have been successfully deleted..
                 showMessage("Successfully banned user and deleted all their messages")
+
+                // ..so we can now delete all their messages in this thread from local storage & remove the views.
+                repository.deleteAllLocalMessagesInThreadFromSenderOfMessage(messageRecord)
             }
             .onFailure {
                 showMessage("Couldn't execute request due to error: $it")

diff --git a/...n/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationActionModeCallback.kt b/...n/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationActionModeCallback.kt
@@ -65,7 +65,7 @@ class ConversationActionModeCallback(private val adapter: ConversationAdapter, p
         menu.findItem(R.id.menu_context_copy).isVisible = !containsControlMessage && hasText
         // Copy Session ID
         menu.findItem(R.id.menu_context_copy_public_key).isVisible =
-             (thread.isGroupRecipient && !thread.isOpenGroupRecipient && selectedItems.size == 1 && firstMessage.individualRecipient.address.toString() != userPublicKey)
+             (thread.isGroupRecipient && !thread.isCommunityRecipient && selectedItems.size == 1 && firstMessage.individualRecipient.address.toString() != userPublicKey)
         // Message detail
         menu.findItem(R.id.menu_message_details).isVisible = selectedItems.size == 1
         // Resend

diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuHelper.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/menus/ConversationMenuHelper.kt
@@ -50,7 +50,7 @@ object ConversationMenuHelper {
     ) {
         // Prepare
         menu.clear()
-        val isOpenGroup = thread.isOpenGroupRecipient
+        val isOpenGroup = thread.isCommunityRecipient
         // Base menu (options that should always be present)
         inflater.inflate(R.menu.menu_conversation, menu)
         // Expiring messages
@@ -253,7 +253,7 @@ object ConversationMenuHelper {
     }
 
     private fun copyOpenGroupUrl(context: Context, thread: Recipient) {
-        if (!thread.isOpenGroupRecipient) { return }
+        if (!thread.isCommunityRecipient) { return }
         val listener = context as? ConversationMenuListener ?: return
         listener.copyOpenGroupUrl(thread)
     }
@@ -300,7 +300,7 @@ object ConversationMenuHelper {
     }
 
     private fun inviteContacts(context: Context, thread: Recipient) {
-        if (!thread.isOpenGroupRecipient) { return }
+        if (!thread.isCommunityRecipient) { return }
         val intent = Intent(context, SelectContactsActivity::class.java)
         val activity = context as AppCompatActivity
         activity.startActivityForResult(intent, ConversationActivityV2.INVITE_CONTACTS)

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
@@ -166,7 +166,7 @@ class VisibleMessageView : LinearLayout {
                 binding.profilePictureView.publicKey = senderSessionID
                 binding.profilePictureView.update(message.individualRecipient)
                 binding.profilePictureView.setOnClickListener {
-                    if (thread.isOpenGroupRecipient) {
+                    if (thread.isCommunityRecipient) {
                         val openGroup = lokiThreadDb.getOpenGroupChat(threadID)
                         if (IdPrefix.fromValue(senderSessionID) == IdPrefix.BLINDED && openGroup?.canWrite == true) {
                             // TODO: support v2 soon
@@ -179,7 +179,7 @@ class VisibleMessageView : LinearLayout {
                         maybeShowUserDetails(senderSessionID, threadID)
                     }
                 }
-                if (thread.isOpenGroupRecipient) {
+                if (thread.isCommunityRecipient) {
                     val openGroup = lokiThreadDb.getOpenGroupChat(threadID) ?: return
                     var standardPublicKey = ""
                     var blindedPublicKey: String? = null
@@ -195,7 +195,7 @@ class VisibleMessageView : LinearLayout {
         }
         binding.senderNameTextView.isVisible = !message.isOutgoing && (isStartOfMessageCluster && (isGroupThread || snIsSelected))
         val contactContext =
-            if (thread.isOpenGroupRecipient) ContactContext.OPEN_GROUP else ContactContext.REGULAR
+            if (thread.isCommunityRecipient) ContactContext.OPEN_GROUP else ContactContext.REGULAR
         binding.senderNameTextView.text = contact?.displayName(contactContext) ?: senderSessionID
 
         // Unread marker