Delete the temporary file only when the user explicitly cancel the upload.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewEvents.kt

@@ -12,5 +12,6 @@ import androidx.compose.runtime.Immutable
 @Immutable
 sealed interface AttachmentsPreviewEvents {
     data object SendAttachment : AttachmentsPreviewEvents
+    data object Cancel : AttachmentsPreviewEvents
     data object ClearSendState : AttachmentsPreviewEvents
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt

@@ -31,7 +31,14 @@ class AttachmentsPreviewNode @AssistedInject constructor(
 
     private val inputs: Inputs = inputs()
 
-    private val presenter = presenterFactory.create(inputs.attachment)
+    private val onDoneListener = OnDoneListener {
+        navigateUp()
+    }
+
+    private val presenter = presenterFactory.create(
+        attachment = inputs.attachment,
+        onDoneListener = onDoneListener,
+    )
 
     @Composable
     override fun View(modifier: Modifier) {
@@ -39,7 +46,6 @@ class AttachmentsPreviewNode @AssistedInject constructor(
             val state = presenter.present()
             AttachmentsPreviewView(
                 state = state,
-                onDismiss = this::navigateUp,
                 modifier = modifier
             )
         }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewPresenter.kt

@@ -8,7 +8,6 @@
 package io.element.android.features.messages.impl.attachments.preview
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
@@ -36,13 +35,17 @@ import kotlin.coroutines.coroutineContext
 
 class AttachmentsPreviewPresenter @AssistedInject constructor(
     @Assisted private val attachment: Attachment,
+    @Assisted private val onDoneListener: OnDoneListener,
     private val mediaSender: MediaSender,
     private val permalinkBuilder: PermalinkBuilder,
     private val temporaryUriDeleter: TemporaryUriDeleter,
 ) : Presenter<AttachmentsPreviewState> {
     @AssistedFactory
     interface Factory {
-        fun create(attachment: Attachment): AttachmentsPreviewPresenter
+        fun create(
+            attachment: Attachment,
+            onDoneListener: OnDoneListener,
+        ): AttachmentsPreviewPresenter
     }
 
     @Composable
@@ -60,20 +63,6 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
 
         val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }
 
-        DisposableEffect(Unit) {
-            onDispose {
-                // Delete the temporary file when the composable is disposed, in case it was not sent
-                if (sendActionState.value == SendActionState.Idle) {
-                    // Attachment has not been sent, maybe delete it
-                    when (attachment) {
-                        is Attachment.Media -> {
-                            temporaryUriDeleter.delete(attachment.localMedia.uri)
-                        }
-                    }
-                }
-            }
-        }
-
         fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) {
             when (attachmentsPreviewEvents) {
                 is AttachmentsPreviewEvents.SendAttachment -> {
@@ -85,6 +74,9 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
                         sendActionState = sendActionState,
                     )
                 }
+                AttachmentsPreviewEvents.Cancel -> {
+                    coroutineScope.cancel(attachment)
+                }
                 AttachmentsPreviewEvents.ClearSendState -> {
                     ongoingSendAttachmentJob.value?.let {
                         it.cancel()
@@ -119,6 +111,18 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
         }
     }
 
+    private fun CoroutineScope.cancel(
+        attachment: Attachment,
+    ) = launch {
+        // Delete the temporary file
+        when (attachment) {
+            is Attachment.Media -> {
+                temporaryUriDeleter.delete(attachment.localMedia.uri)
+            }
+        }
+        onDoneListener()
+    }
+
     private suspend fun sendMedia(
         mediaAttachment: Attachment.Media,
         caption: String?,
@@ -141,7 +145,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
         ).getOrThrow()
     }.fold(
         onSuccess = {
-            sendActionState.value = SendActionState.Done
+            onDoneListener()
         },
         onFailure = { error ->
             Timber.e(error, "Failed to send attachment")

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewState.kt

@@ -36,5 +36,4 @@ sealed interface SendActionState {
     }
 
     data class Failure(val error: Throwable) : SendActionState
-    data object Done : SendActionState
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt

@@ -7,6 +7,7 @@
 
 package io.element.android.features.messages.impl.attachments.preview
 
+import androidx.activity.compose.BackHandler
 import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.IntrinsicSize
@@ -17,9 +18,6 @@ import androidx.compose.foundation.layout.imePadding
 import androidx.compose.foundation.layout.navigationBarsPadding
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
@@ -50,22 +48,22 @@ import me.saket.telephoto.zoomable.rememberZoomableState
 @Composable
 fun AttachmentsPreviewView(
     state: AttachmentsPreviewState,
-    onDismiss: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
     fun postSendAttachment() {
         state.eventSink(AttachmentsPreviewEvents.SendAttachment)
     }
 
+    fun postCancel() {
+        state.eventSink(AttachmentsPreviewEvents.Cancel)
+    }
+
     fun postClearSendState() {
         state.eventSink(AttachmentsPreviewEvents.ClearSendState)
     }
 
-    if (state.sendActionState is SendActionState.Done) {
+    BackHandler(enabled = state.sendActionState !is SendActionState.Sending) {
-        val latestOnDismiss by rememberUpdatedState(onDismiss)
+        postCancel()
-        LaunchedEffect(state.sendActionState) {
-            latestOnDismiss()
-        }
     }
 
     Scaffold(
@@ -75,7 +73,7 @@ fun AttachmentsPreviewView(
                 navigationIcon = {
                     BackButton(
                         imageVector = CompoundIcons.Close(),
-                        onClick = onDismiss,
+                        onClick = ::postCancel,
                     )
                 },
                 title = {},
@@ -202,6 +200,5 @@ private fun AttachmentsPreviewBottomActions(
 internal fun AttachmentsPreviewViewPreview(@PreviewParameter(AttachmentsPreviewStateProvider::class) state: AttachmentsPreviewState) = ElementPreviewDark {
     AttachmentsPreviewView(
         state = state,
-        onDismiss = {},
     )
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/OnDoneListener.kt

@@ -0,0 +1,12 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.messages.impl.attachments.preview
+
+fun interface OnDoneListener {
+    operator fun invoke()
+}

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

@@ -16,6 +16,7 @@ import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewEvents
 import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewPresenter
+import io.element.android.features.messages.impl.attachments.preview.OnDoneListener
 import io.element.android.features.messages.impl.attachments.preview.SendActionState
 import io.element.android.features.messages.impl.fixtures.aMediaAttachment
 import io.element.android.libraries.androidutils.file.TemporaryUriDeleter
@@ -42,6 +43,7 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.lambda.value
 import io.mockk.mockk
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
@@ -69,7 +71,11 @@ class AttachmentsPreviewPresenterTest {
             ),
             sendFileResult = sendFileResult,
         )
-        val presenter = createAttachmentsPreviewPresenter(room = room)
+        val onDoneListener = lambdaRecorder<Unit> { }
+        val presenter = createAttachmentsPreviewPresenter(
+            room = room,
+            onDoneListener = { onDoneListener() },
+        )
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
@@ -80,9 +86,28 @@ class AttachmentsPreviewPresenterTest {
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0f))
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0.5f))
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(1f))
-            val successState = awaitItem()
+            advanceUntilIdle()
-            assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
             sendFileResult.assertions().isCalledOnce()
+            onDoneListener.assertions().isCalledOnce()
+        }
+    }
+
+    @Test
+    fun `present - cancel scenario`() = runTest {
+        val onDoneListener = lambdaRecorder<Unit> { }
+        val deleteCallback = lambdaRecorder<Uri?, Unit> {}
+        val presenter = createAttachmentsPreviewPresenter(
+            temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback),
+            onDoneListener = { onDoneListener() },
+        )
+        moleculeFlow(RecompositionMode.Immediate) {
+            presenter.present()
+        }.test {
+            val initialState = awaitItem()
+            assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
+            initialState.eventSink(AttachmentsPreviewEvents.Cancel)
+            deleteCallback.assertions().isCalledOnce()
+            onDoneListener.assertions().isCalledOnce()
         }
     }
 
@@ -98,9 +123,11 @@ class AttachmentsPreviewPresenterTest {
         val room = FakeMatrixRoom(
             sendImageResult = sendImageResult,
         )
+        val onDoneListener = lambdaRecorder<Unit> { }
         val presenter = createAttachmentsPreviewPresenter(
             room = room,
             mediaPreProcessor = mediaPreProcessor,
+            onDoneListener = { onDoneListener() },
         )
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
@@ -110,8 +137,7 @@ class AttachmentsPreviewPresenterTest {
             initialState.textEditorState.setMarkdown(A_CAPTION)
             initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing)
-            val successState = awaitItem()
+            advanceUntilIdle()
-            assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
             sendImageResult.assertions().isCalledOnce().with(
                 any(),
                 any(),
@@ -120,6 +146,7 @@ class AttachmentsPreviewPresenterTest {
                 any(),
                 any(),
             )
+            onDoneListener.assertions().isCalledOnce()
         }
     }
 
@@ -135,9 +162,11 @@ class AttachmentsPreviewPresenterTest {
         val room = FakeMatrixRoom(
             sendVideoResult = sendVideoResult,
         )
+        val onDoneListener = lambdaRecorder<Unit> { }
         val presenter = createAttachmentsPreviewPresenter(
             room = room,
             mediaPreProcessor = mediaPreProcessor,
+            onDoneListener = { onDoneListener() },
         )
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
@@ -147,8 +176,7 @@ class AttachmentsPreviewPresenterTest {
             initialState.textEditorState.setMarkdown(A_CAPTION)
             initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing)
-            val successState = awaitItem()
+            advanceUntilIdle()
-            assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
             sendVideoResult.assertions().isCalledOnce().with(
                 any(),
                 any(),
@@ -157,6 +185,7 @@ class AttachmentsPreviewPresenterTest {
                 any(),
                 any(),
             )
+            onDoneListener.assertions().isCalledOnce()
         }
     }
 
@@ -210,9 +239,11 @@ class AttachmentsPreviewPresenterTest {
         permalinkBuilder: PermalinkBuilder = FakePermalinkBuilder(),
         mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(),
         temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(),
+        onDoneListener: OnDoneListener = OnDoneListener {},
     ): AttachmentsPreviewPresenter {
         return AttachmentsPreviewPresenter(
             attachment = aMediaAttachment(localMedia),
+            onDoneListener = onDoneListener,
             mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()),
             permalinkBuilder = permalinkBuilder,
             temporaryUriDeleter = temporaryUriDeleter,