Skip to content

Commit 43053de

Browse files
bmartybmarty
committed
Delete the temporary file only when the user explicitly cancel the upload.
1 parent 585b6a9 commit 43053de

File tree

7 files changed

+88
-38
lines changed

7 files changed

+88
-38
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@ import androidx.compose.runtime.Immutable
1212
@Immutable
1313
sealed interface AttachmentsPreviewEvents {
1414
data object SendAttachment : AttachmentsPreviewEvents
15+
data object Cancel : AttachmentsPreviewEvents
1516
data object ClearSendState : AttachmentsPreviewEvents
1617
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,21 @@ class AttachmentsPreviewNode @AssistedInject constructor(
3131

3232
private val inputs: Inputs = inputs()
3333

34-
private val presenter = presenterFactory.create(inputs.attachment)
34+
private val onDoneListener = OnDoneListener {
35+
navigateUp()
36+
}
37+
38+
private val presenter = presenterFactory.create(
39+
attachment = inputs.attachment,
40+
onDoneListener = onDoneListener,
41+
)
3542

3643
@Composable
3744
override fun View(modifier: Modifier) {
3845
ForcedDarkElementTheme {
3946
val state = presenter.present()
4047
AttachmentsPreviewView(
4148
state = state,
42-
onDismiss = this::navigateUp,
4349
modifier = modifier
4450
)
4551
}

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

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
package io.element.android.features.messages.impl.attachments.preview
99

1010
import androidx.compose.runtime.Composable
11-
import androidx.compose.runtime.DisposableEffect
1211
import androidx.compose.runtime.MutableState
1312
import androidx.compose.runtime.getValue
1413
import androidx.compose.runtime.mutableStateOf
@@ -36,13 +35,17 @@ import kotlin.coroutines.coroutineContext
3635

3736
class AttachmentsPreviewPresenter @AssistedInject constructor(
3837
@Assisted private val attachment: Attachment,
38+
@Assisted private val onDoneListener: OnDoneListener,
3939
private val mediaSender: MediaSender,
4040
private val permalinkBuilder: PermalinkBuilder,
4141
private val temporaryUriDeleter: TemporaryUriDeleter,
4242
) : Presenter<AttachmentsPreviewState> {
4343
@AssistedFactory
4444
interface Factory {
45-
fun create(attachment: Attachment): AttachmentsPreviewPresenter
45+
fun create(
46+
attachment: Attachment,
47+
onDoneListener: OnDoneListener,
48+
): AttachmentsPreviewPresenter
4649
}
4750

4851
@Composable
@@ -60,20 +63,6 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
6063

6164
val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }
6265

63-
DisposableEffect(Unit) {
64-
onDispose {
65-
// Delete the temporary file when the composable is disposed, in case it was not sent
66-
if (sendActionState.value == SendActionState.Idle) {
67-
// Attachment has not been sent, maybe delete it
68-
when (attachment) {
69-
is Attachment.Media -> {
70-
temporaryUriDeleter.delete(attachment.localMedia.uri)
71-
}
72-
}
73-
}
74-
}
75-
}
76-
7766
fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) {
7867
when (attachmentsPreviewEvents) {
7968
is AttachmentsPreviewEvents.SendAttachment -> {
@@ -85,6 +74,9 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
8574
sendActionState = sendActionState,
8675
)
8776
}
77+
AttachmentsPreviewEvents.Cancel -> {
78+
coroutineScope.cancel(attachment)
79+
}
8880
AttachmentsPreviewEvents.ClearSendState -> {
8981
ongoingSendAttachmentJob.value?.let {
9082
it.cancel()
@@ -119,6 +111,18 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
119111
}
120112
}
121113

114+
private fun CoroutineScope.cancel(
115+
attachment: Attachment,
116+
) = launch {
117+
// Delete the temporary file
118+
when (attachment) {
119+
is Attachment.Media -> {
120+
temporaryUriDeleter.delete(attachment.localMedia.uri)
121+
}
122+
}
123+
onDoneListener()
124+
}
125+
122126
private suspend fun sendMedia(
123127
mediaAttachment: Attachment.Media,
124128
caption: String?,
@@ -141,7 +145,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
141145
).getOrThrow()
142146
}.fold(
143147
onSuccess = {
144-
sendActionState.value = SendActionState.Done
148+
onDoneListener()
145149
},
146150
onFailure = { error ->
147151
Timber.e(error, "Failed to send attachment")

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,4 @@ sealed interface SendActionState {
3636
}
3737

3838
data class Failure(val error: Throwable) : SendActionState
39-
data object Done : SendActionState
4039
}

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package io.element.android.features.messages.impl.attachments.preview
99

10+
import androidx.activity.compose.BackHandler
1011
import androidx.compose.foundation.background
1112
import androidx.compose.foundation.layout.Box
1213
import androidx.compose.foundation.layout.IntrinsicSize
@@ -17,9 +18,6 @@ import androidx.compose.foundation.layout.imePadding
1718
import androidx.compose.foundation.layout.navigationBarsPadding
1819
import androidx.compose.material3.ExperimentalMaterial3Api
1920
import androidx.compose.runtime.Composable
20-
import androidx.compose.runtime.LaunchedEffect
21-
import androidx.compose.runtime.getValue
22-
import androidx.compose.runtime.rememberUpdatedState
2321
import androidx.compose.ui.Alignment
2422
import androidx.compose.ui.Modifier
2523
import androidx.compose.ui.res.stringResource
@@ -50,22 +48,22 @@ import me.saket.telephoto.zoomable.rememberZoomableState
5048
@Composable
5149
fun AttachmentsPreviewView(
5250
state: AttachmentsPreviewState,
53-
onDismiss: () -> Unit,
5451
modifier: Modifier = Modifier,
5552
) {
5653
fun postSendAttachment() {
5754
state.eventSink(AttachmentsPreviewEvents.SendAttachment)
5855
}
5956

57+
fun postCancel() {
58+
state.eventSink(AttachmentsPreviewEvents.Cancel)
59+
}
60+
6061
fun postClearSendState() {
6162
state.eventSink(AttachmentsPreviewEvents.ClearSendState)
6263
}
6364

64-
if (state.sendActionState is SendActionState.Done) {
65-
val latestOnDismiss by rememberUpdatedState(onDismiss)
66-
LaunchedEffect(state.sendActionState) {
67-
latestOnDismiss()
68-
}
65+
BackHandler(enabled = state.sendActionState !is SendActionState.Sending) {
66+
postCancel()
6967
}
7068

7169
Scaffold(
@@ -75,7 +73,7 @@ fun AttachmentsPreviewView(
7573
navigationIcon = {
7674
BackButton(
7775
imageVector = CompoundIcons.Close(),
78-
onClick = onDismiss,
76+
onClick = ::postCancel,
7977
)
8078
},
8179
title = {},
@@ -202,6 +200,5 @@ private fun AttachmentsPreviewBottomActions(
202200
internal fun AttachmentsPreviewViewPreview(@PreviewParameter(AttachmentsPreviewStateProvider::class) state: AttachmentsPreviewState) = ElementPreviewDark {
203201
AttachmentsPreviewView(
204202
state = state,
205-
onDismiss = {},
206203
)
207204
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright 2024 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only
5+
* Please see LICENSE in the repository root for full details.
6+
*/
7+
8+
package io.element.android.features.messages.impl.attachments.preview
9+
10+
fun interface OnDoneListener {
11+
operator fun invoke()
12+
}

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

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import app.cash.turbine.test
1616
import com.google.common.truth.Truth.assertThat
1717
import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewEvents
1818
import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewPresenter
19+
import io.element.android.features.messages.impl.attachments.preview.OnDoneListener
1920
import io.element.android.features.messages.impl.attachments.preview.SendActionState
2021
import io.element.android.features.messages.impl.fixtures.aMediaAttachment
2122
import io.element.android.libraries.androidutils.file.TemporaryUriDeleter
@@ -42,6 +43,7 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder
4243
import io.element.android.tests.testutils.lambda.value
4344
import io.mockk.mockk
4445
import kotlinx.coroutines.ExperimentalCoroutinesApi
46+
import kotlinx.coroutines.test.advanceUntilIdle
4547
import kotlinx.coroutines.test.runTest
4648
import org.junit.Rule
4749
import org.junit.Test
@@ -69,7 +71,11 @@ class AttachmentsPreviewPresenterTest {
6971
),
7072
sendFileResult = sendFileResult,
7173
)
72-
val presenter = createAttachmentsPreviewPresenter(room = room)
74+
val onDoneListener = lambdaRecorder<Unit> { }
75+
val presenter = createAttachmentsPreviewPresenter(
76+
room = room,
77+
onDoneListener = { onDoneListener() },
78+
)
7379
moleculeFlow(RecompositionMode.Immediate) {
7480
presenter.present()
7581
}.test {
@@ -80,9 +86,28 @@ class AttachmentsPreviewPresenterTest {
8086
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0f))
8187
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0.5f))
8288
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(1f))
83-
val successState = awaitItem()
84-
assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
89+
advanceUntilIdle()
8590
sendFileResult.assertions().isCalledOnce()
91+
onDoneListener.assertions().isCalledOnce()
92+
}
93+
}
94+
95+
@Test
96+
fun `present - cancel scenario`() = runTest {
97+
val onDoneListener = lambdaRecorder<Unit> { }
98+
val deleteCallback = lambdaRecorder<Uri?, Unit> {}
99+
val presenter = createAttachmentsPreviewPresenter(
100+
temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback),
101+
onDoneListener = { onDoneListener() },
102+
)
103+
moleculeFlow(RecompositionMode.Immediate) {
104+
presenter.present()
105+
}.test {
106+
val initialState = awaitItem()
107+
assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle)
108+
initialState.eventSink(AttachmentsPreviewEvents.Cancel)
109+
deleteCallback.assertions().isCalledOnce()
110+
onDoneListener.assertions().isCalledOnce()
86111
}
87112
}
88113

@@ -98,9 +123,11 @@ class AttachmentsPreviewPresenterTest {
98123
val room = FakeMatrixRoom(
99124
sendImageResult = sendImageResult,
100125
)
126+
val onDoneListener = lambdaRecorder<Unit> { }
101127
val presenter = createAttachmentsPreviewPresenter(
102128
room = room,
103129
mediaPreProcessor = mediaPreProcessor,
130+
onDoneListener = { onDoneListener() },
104131
)
105132
moleculeFlow(RecompositionMode.Immediate) {
106133
presenter.present()
@@ -110,8 +137,7 @@ class AttachmentsPreviewPresenterTest {
110137
initialState.textEditorState.setMarkdown(A_CAPTION)
111138
initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
112139
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing)
113-
val successState = awaitItem()
114-
assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
140+
advanceUntilIdle()
115141
sendImageResult.assertions().isCalledOnce().with(
116142
any(),
117143
any(),
@@ -120,6 +146,7 @@ class AttachmentsPreviewPresenterTest {
120146
any(),
121147
any(),
122148
)
149+
onDoneListener.assertions().isCalledOnce()
123150
}
124151
}
125152

@@ -135,9 +162,11 @@ class AttachmentsPreviewPresenterTest {
135162
val room = FakeMatrixRoom(
136163
sendVideoResult = sendVideoResult,
137164
)
165+
val onDoneListener = lambdaRecorder<Unit> { }
138166
val presenter = createAttachmentsPreviewPresenter(
139167
room = room,
140168
mediaPreProcessor = mediaPreProcessor,
169+
onDoneListener = { onDoneListener() },
141170
)
142171
moleculeFlow(RecompositionMode.Immediate) {
143172
presenter.present()
@@ -147,8 +176,7 @@ class AttachmentsPreviewPresenterTest {
147176
initialState.textEditorState.setMarkdown(A_CAPTION)
148177
initialState.eventSink(AttachmentsPreviewEvents.SendAttachment)
149178
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing)
150-
val successState = awaitItem()
151-
assertThat(successState.sendActionState).isEqualTo(SendActionState.Done)
179+
advanceUntilIdle()
152180
sendVideoResult.assertions().isCalledOnce().with(
153181
any(),
154182
any(),
@@ -157,6 +185,7 @@ class AttachmentsPreviewPresenterTest {
157185
any(),
158186
any(),
159187
)
188+
onDoneListener.assertions().isCalledOnce()
160189
}
161190
}
162191

@@ -210,9 +239,11 @@ class AttachmentsPreviewPresenterTest {
210239
permalinkBuilder: PermalinkBuilder = FakePermalinkBuilder(),
211240
mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(),
212241
temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(),
242+
onDoneListener: OnDoneListener = OnDoneListener {},
213243
): AttachmentsPreviewPresenter {
214244
return AttachmentsPreviewPresenter(
215245
attachment = aMediaAttachment(localMedia),
246+
onDoneListener = onDoneListener,
216247
mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()),
217248
permalinkBuilder = permalinkBuilder,
218249
temporaryUriDeleter = temporaryUriDeleter,

0 commit comments

Comments
 (0)