From 6910984588d71cde14b5593ce5f8587a736e621d Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 7 Jun 2023 11:10:29 +0100 Subject: [PATCH] Fix changing room avatar from details screen The presenter was expecting the MediaProcessor to return a MediaUploadInfo.Image, but it actually returns MediaUploadInfo.AnyFile because we're not compressing avatars (so it doesn't process the file and return more detailed info). This check/cast was entirely pointless, so change to just working on whatever we're given. The pickers constrain which types of file the user select, so we should be reasonably happy the files are images. Also actually log error details when updating the details, so we know what's going wrong. Closes #550 --- .../impl/edit/RoomDetailsEditPresenter.kt | 26 +++++++++++-------- .../edit/RoomDetailsEditPresenterTest.kt | 10 ++----- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt index a6726f9105..615541a3de 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt @@ -29,19 +29,19 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.core.net.toUri -import io.element.android.libraries.matrix.ui.media.AvatarAction import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.execute import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.StateEventType +import io.element.android.libraries.matrix.ui.media.AvatarAction import io.element.android.libraries.mediapickers.api.PickerProvider import io.element.android.libraries.mediaupload.api.MediaPreProcessor -import io.element.android.libraries.mediaupload.api.MediaUploadInfo import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import timber.log.Timber import javax.inject.Inject class RoomDetailsEditPresenter @Inject constructor( @@ -139,13 +139,19 @@ class RoomDetailsEditPresenter @Inject constructor( val results = mutableListOf>() suspend { if (topic.orEmpty().trim() != room.topic.orEmpty().trim()) { - results.add(room.setTopic(topic.orEmpty())) + results.add(room.setTopic(topic.orEmpty()).onFailure { + Timber.e(it, "Failed to set room topic") + }) } if (name.isNotEmpty() && name.trim() != room.name.orEmpty().trim()) { - results.add(room.setName(name)) + results.add(room.setName(name).onFailure { + Timber.e(it, "Failed to set room name") + }) } if (avatarUri?.toString()?.trim() != room.avatarUrl?.trim()) { - results.add(updateAvatar(avatarUri)) + results.add(updateAvatar(avatarUri).onFailure { + Timber.e(it, "Failed to update avatar") + }) } if (results.all { it.isSuccess }) Unit else results.first { it.isFailure }.getOrThrow() }.execute(action) @@ -153,14 +159,12 @@ class RoomDetailsEditPresenter @Inject constructor( private suspend fun updateAvatar(avatarUri: Uri?): Result { return runCatching { - val result = if (avatarUri != null) { - val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() as? MediaUploadInfo.Image - val byteArray = preprocessed?.file?.readBytes() - byteArray?.let { room.updateAvatar(MimeTypes.Jpeg, it) } ?: error("Could not process the given uri ($avatarUri)") + if (avatarUri != null) { + val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() + room.updateAvatar(MimeTypes.Jpeg, preprocessed.file.readBytes()).getOrThrow() } else { - room.removeAvatar() + room.removeAvatar().getOrThrow() } - result.getOrThrow() } } } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt index adbec76ae7..c1d098dab5 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt @@ -21,7 +21,6 @@ import app.cash.molecule.RecompositionClock import app.cash.molecule.moleculeFlow import app.cash.turbine.test import com.google.common.truth.Truth.assertThat -import io.element.android.libraries.matrix.ui.media.AvatarAction import io.element.android.features.roomdetails.aMatrixRoom import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditEvents import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditPresenter @@ -29,9 +28,9 @@ import io.element.android.libraries.architecture.Async import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.StateEventType import io.element.android.libraries.matrix.test.AN_AVATAR_URL +import io.element.android.libraries.matrix.ui.media.AvatarAction import io.element.android.libraries.mediapickers.test.FakePickerProvider import io.element.android.libraries.mediaupload.api.MediaUploadInfo -import io.element.android.libraries.mediaupload.api.ThumbnailProcessingInfo import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor import io.mockk.every import io.mockk.mockk @@ -600,14 +599,9 @@ class RoomDetailsEditPresenterTest { } fakePickerProvider.givenResult(anotherAvatarUri) - fakeMediaPreProcessor.givenResult(Result.success(MediaUploadInfo.Image( + fakeMediaPreProcessor.givenResult(Result.success(MediaUploadInfo.AnyFile( file = processedFile, info = mockk(), - thumbnailInfo = ThumbnailProcessingInfo( - file = processedFile, - info = mockk(), - blurhash = "", - ) ))) }