Let the user give a reason when removing a message

The protocol carries a reason on a redaction and other clients let the sender
give one, but removing a message here sent the redaction straight away with no
reason and no confirmation.

Removing a message now opens a dialog with an optional reason, and the
redaction is only sent once it is confirmed. A blank reason is sent as none, so
confirming without typing anything behaves exactly as before. A message which
was never sent has nobody to give a reason to, so the send failure dialog's
Remove still redacts it immediately.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesEvent.kt

@@ -18,6 +18,8 @@ sealed interface MessagesEvent {
     data class ToggleReaction(val emoji: String, val eventOrTransactionId: EventOrTransactionId) : MessagesEvent
     data class InviteDialogDismissed(val action: InviteDialogAction) : MessagesEvent
     data class OnUserClicked(val user: MatrixUser) : MessagesEvent
+    data class ConfirmRedact(val reason: String?) : MessagesEvent
+    data object CancelRedact : MessagesEvent
     data object StopLiveLocationShare : MessagesEvent
     data object ShowLiveLocationShare : MessagesEvent
     data object MarkAsFullyReadAndExit : MessagesEvent

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt

@@ -240,6 +240,8 @@ class MessagesPresenter(
             onPauseOrDispose {}
         }
 
+        val eventToRedact = remember { mutableStateOf<TimelineItem.Event?>(null) }
+
         fun handleEvent(event: MessagesEvent) {
             when (event) {
                 is MessagesEvent.HandleAction -> {
@@ -250,8 +252,21 @@ class MessagesPresenter(
                         enableTextFormatting = composerState.showTextFormatting,
                         timelineState = timelineState,
                         timelineProtectionState = timelineProtectionState,
+                        eventToRedact = eventToRedact,
                     )
                 }
+                is MessagesEvent.ConfirmRedact -> {
+                    val target = eventToRedact.value
+                    eventToRedact.value = null
+                    if (target != null) {
+                        localCoroutineScope.launch {
+                            redact(target, event.reason?.takeIf { it.isNotBlank() })
+                        }
+                    }
+                }
+                MessagesEvent.CancelRedact -> {
+                    eventToRedact.value = null
+                }
                 is MessagesEvent.ToggleReaction -> {
                     localCoroutineScope.toggleReaction(event.emoji, event.eventOrTransactionId)
                 }
@@ -330,6 +345,7 @@ class MessagesPresenter(
                 hasUnreadThreads = false,
             ),
             showLiveLocationShareBanner = isCurrentlySharingLiveLocationInRoom && timelineState.timelineMode !is Timeline.Mode.Thread,
+            eventToRedact = eventToRedact.value,
             eventSink = ::handleEvent,
         )
     }
@@ -368,12 +384,13 @@ class MessagesPresenter(
         timelineProtectionState: TimelineProtectionState,
         enableTextFormatting: Boolean,
         timelineState: TimelineState,
+        eventToRedact: MutableState<TimelineItem.Event?>,
     ) = launch {
         when (action) {
             TimelineItemAction.CopyText -> handleCopyContents(targetEvent)
             TimelineItemAction.CopyCaption -> handleCopyCaption(targetEvent)
             TimelineItemAction.CopyLink -> handleCopyLink(targetEvent)
-            TimelineItemAction.Redact -> handleActionRedact(targetEvent)
+            TimelineItemAction.Redact -> handleActionRedact(targetEvent, eventToRedact)
             TimelineItemAction.Edit,
             TimelineItemAction.EditPoll -> handleActionEdit(targetEvent, composerState, enableTextFormatting)
             TimelineItemAction.AddCaption -> handleActionAddCaption(targetEvent, composerState)
@@ -495,9 +512,18 @@ class MessagesPresenter(
         )
     }
 
-    private suspend fun handleActionRedact(event: TimelineItem.Event) {
+    private suspend fun handleActionRedact(event: TimelineItem.Event, eventToRedact: MutableState<TimelineItem.Event?>) {
+        if (event.eventId == null) {
+            // The message was never sent, so there is nobody to give a reason to.
+            redact(event, reason = null)
+        } else {
+            eventToRedact.value = event
+        }
+    }
+
+    private suspend fun redact(event: TimelineItem.Event, reason: String?) {
         timelineController.invokeOnCurrentTimeline {
-            redactEvent(eventOrTransactionId = event.eventOrTransactionId, reason = null)
+            redactEvent(eventOrTransactionId = event.eventOrTransactionId, reason = reason)
                 .onFailure { Timber.e(it) }
         }
     }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesState.kt

@@ -18,6 +18,7 @@ import io.element.android.features.messages.impl.timeline.TimelineState
 import io.element.android.features.messages.impl.timeline.components.customreaction.CustomReactionState
 import io.element.android.features.messages.impl.timeline.components.reactionsummary.ReactionSummaryState
 import io.element.android.features.messages.impl.timeline.components.receipt.bottomsheet.ReadReceiptBottomSheetState
+import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.protection.TimelineProtectionState
 import io.element.android.features.roomcall.api.RoomCallState
 import io.element.android.features.roommembermoderation.api.RoomMemberModerationState
@@ -61,6 +62,8 @@ data class MessagesState(
     val successorRoom: SuccessorRoom?,
     val threads: Threads,
     val showLiveLocationShareBanner: Boolean,
+    /** The event the user asked to remove, waiting for them to confirm and optionally give a reason. */
+    val eventToRedact: TimelineItem.Event?,
     val eventSink: (MessagesEvent) -> Unit
 ) {
     val isTombstoned = successorRoom != null

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStatePreviewParam.kt

@@ -24,6 +24,7 @@ import io.element.android.features.messages.impl.messagecomposer.aMessageCompose
 import io.element.android.features.messages.impl.pinned.banner.PinnedMessagesBannerState
 import io.element.android.features.messages.impl.pinned.banner.aLoadedPinnedMessagesBannerState
 import io.element.android.features.messages.impl.timeline.TimelineState
+import io.element.android.features.messages.impl.timeline.aTimelineItemEvent
 import io.element.android.features.messages.impl.timeline.aTimelineItemList
 import io.element.android.features.messages.impl.timeline.aTimelineState
 import io.element.android.features.messages.impl.timeline.components.customreaction.CustomReactionEvent
@@ -92,6 +93,7 @@ open class MessagesStatePreviewParam : PreviewParameterProvider<MessagesState> {
                 composerState = aMessageComposerState(textEditorState = aTextEditorStateMarkdown()),
                 identityChangeState = anIdentityChangeState(listOf(aRoomMemberIdentityStateChange()))
             ),
+            aMessagesState(eventToRedact = aTimelineItemEvent()),
         )
 }
 
@@ -131,6 +133,7 @@ fun aMessagesState(
     isCurrentlySharingLiveLocationInRoom: Boolean = false,
     dmUserStatus: DisplayedStatus? = null,
     eventSink: (MessagesEvent) -> Unit = {},
+    eventToRedact: TimelineItem.Event? = null,
 ) = MessagesState(
     roomId = RoomId("!id:domain"),
     roomName = roomName,
@@ -161,6 +164,7 @@ fun aMessagesState(
     threads = threads,
     showLiveLocationShareBanner = isCurrentlySharingLiveLocationInRoom,
     dmUserStatus = dmUserStatus,
+    eventToRedact = eventToRedact,
     eventSink = eventSink,
 )
 

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt

@@ -104,6 +104,7 @@ import io.element.android.libraries.designsystem.atomic.molecules.ComposerAlertM
 import io.element.android.libraries.designsystem.components.ExpandableBottomSheetLayout
 import io.element.android.libraries.designsystem.components.ExpandableBottomSheetLayoutState
 import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
+import io.element.android.libraries.designsystem.components.dialogs.TextFieldDialog
 import io.element.android.libraries.designsystem.components.rememberExpandableBottomSheetLayoutState
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
@@ -455,6 +456,19 @@ fun MessagesView(
             state.timelineState.eventSink(TimelineEvent.HideSendFailureDialog)
         },
     )
+
+    if (state.eventToRedact != null) {
+        TextFieldDialog(
+            title = stringResource(CommonStrings.action_remove_message),
+            label = stringResource(CommonStrings.common_reason),
+            placeholder = null,
+            value = null,
+            submitText = stringResource(CommonStrings.action_remove),
+            destructiveSubmit = true,
+            onSubmit = { reason -> state.eventSink(MessagesEvent.ConfirmRedact(reason)) },
+            onDismissRequest = { state.eventSink(MessagesEvent.CancelRedact) },
+        )
+    }
 }
 
 @Composable

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt

@@ -78,10 +78,12 @@ import io.element.android.libraries.matrix.test.AN_AVATAR_URL
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.AN_EXCEPTION
 import io.element.android.libraries.matrix.test.A_CAPTION
+import io.element.android.libraries.matrix.test.A_REASON
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.A_SESSION_ID
 import io.element.android.libraries.matrix.test.A_SESSION_ID_2
 import io.element.android.libraries.matrix.test.A_THREAD_ID
+import io.element.android.libraries.matrix.test.A_TRANSACTION_ID
 import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.A_USER_ID_2
 import io.element.android.libraries.matrix.test.core.FakeSendHandle
@@ -557,13 +559,86 @@ class MessagesPresenterTest {
             val initialState = awaitItem()
             val messageEvent = aMessageEvent()
             initialState.eventSink(MessagesEvent.HandleAction(TimelineItemAction.Redact, messageEvent))
-            awaitItem()
+            advanceUntilIdle()
+            val confirmingState = expectMostRecentItem()
+            assertThat(confirmingState.eventToRedact).isEqualTo(messageEvent)
+            assert(redactEventLambda).isNeverCalled()
+            confirmingState.eventSink(MessagesEvent.ConfirmRedact(A_REASON))
+            advanceUntilIdle()
+            assertThat(expectMostRecentItem().eventToRedact).isNull()
+            assert(redactEventLambda)
+                .isCalledOnce()
+                .with(value(messageEvent.eventOrTransactionId), value(A_REASON))
+        }
+    }
+
+    @Test
+    fun `present - handle action redact - a blank reason is not sent`() = runTest {
+        val coroutineDispatchers = testCoroutineDispatchers(useUnconfinedTestDispatcher = true)
+        val liveTimeline = FakeTimeline()
+        val redactEventLambda = lambdaRecorder { _: EventOrTransactionId, _: String? -> Result.success(Unit) }
+        liveTimeline.redactEventLambda = redactEventLambda
+        val presenter = createMessagesPresenter(
+            timeline = liveTimeline,
+            coroutineDispatchers = coroutineDispatchers,
+        )
+        presenter.testWithLifecycleOwner {
+            val initialState = awaitItem()
+            val messageEvent = aMessageEvent()
+            initialState.eventSink(MessagesEvent.HandleAction(TimelineItemAction.Redact, messageEvent))
+            advanceUntilIdle()
+            expectMostRecentItem().eventSink(MessagesEvent.ConfirmRedact("  "))
+            advanceUntilIdle()
+            assertThat(expectMostRecentItem().eventToRedact).isNull()
             assert(redactEventLambda)
                 .isCalledOnce()
                 .with(value(messageEvent.eventOrTransactionId), value(null))
         }
     }
 
+    @Test
+    fun `present - handle action redact - cancelling does not redact`() = runTest {
+        val coroutineDispatchers = testCoroutineDispatchers(useUnconfinedTestDispatcher = true)
+        val liveTimeline = FakeTimeline()
+        val redactEventLambda = lambdaRecorder { _: EventOrTransactionId, _: String? -> Result.success(Unit) }
+        liveTimeline.redactEventLambda = redactEventLambda
+        val presenter = createMessagesPresenter(
+            timeline = liveTimeline,
+            coroutineDispatchers = coroutineDispatchers,
+        )
+        presenter.testWithLifecycleOwner {
+            val initialState = awaitItem()
+            initialState.eventSink(MessagesEvent.HandleAction(TimelineItemAction.Redact, aMessageEvent()))
+            advanceUntilIdle()
+            expectMostRecentItem().eventSink(MessagesEvent.CancelRedact)
+            advanceUntilIdle()
+            assertThat(expectMostRecentItem().eventToRedact).isNull()
+            assert(redactEventLambda).isNeverCalled()
+        }
+    }
+
+    @Test
+    fun `present - handle action redact - a message which was never sent is redacted without asking`() = runTest {
+        val coroutineDispatchers = testCoroutineDispatchers(useUnconfinedTestDispatcher = true)
+        val liveTimeline = FakeTimeline()
+        val redactEventLambda = lambdaRecorder { _: EventOrTransactionId, _: String? -> Result.success(Unit) }
+        liveTimeline.redactEventLambda = redactEventLambda
+        val presenter = createMessagesPresenter(
+            timeline = liveTimeline,
+            coroutineDispatchers = coroutineDispatchers,
+        )
+        presenter.testWithLifecycleOwner {
+            val initialState = awaitItem()
+            val localEcho = aMessageEvent(eventId = null, transactionId = A_TRANSACTION_ID)
+            initialState.eventSink(MessagesEvent.HandleAction(TimelineItemAction.Redact, localEcho))
+            advanceUntilIdle()
+            assertThat(expectMostRecentItem().eventToRedact).isNull()
+            assert(redactEventLambda)
+                .isCalledOnce()
+                .with(value(localEcho.eventOrTransactionId), value(null))
+        }
+    }
+
     @Test
     fun `present - handle action report content`() = runTest {
         val onReportContentClickLambda = lambdaRecorder { _: EventId, _: UserId -> }