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
This commit is contained in:
Chris Smith 2023-06-07 11:10:29 +01:00
parent bbef43751b
commit 6910984588
2 changed files with 17 additions and 19 deletions

View file

@ -29,19 +29,19 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue import androidx.compose.runtime.setValue
import androidx.core.net.toUri 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.Async
import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.execute import io.element.android.libraries.architecture.execute
import io.element.android.libraries.core.mimetype.MimeTypes 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.MatrixRoom
import io.element.android.libraries.matrix.api.room.StateEventType 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.mediapickers.api.PickerProvider
import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.api.MediaPreProcessor
import io.element.android.libraries.mediaupload.api.MediaUploadInfo
import kotlinx.collections.immutable.toImmutableList import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
class RoomDetailsEditPresenter @Inject constructor( class RoomDetailsEditPresenter @Inject constructor(
@ -139,13 +139,19 @@ class RoomDetailsEditPresenter @Inject constructor(
val results = mutableListOf<Result<Unit>>() val results = mutableListOf<Result<Unit>>()
suspend { suspend {
if (topic.orEmpty().trim() != room.topic.orEmpty().trim()) { 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()) { 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()) { 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() if (results.all { it.isSuccess }) Unit else results.first { it.isFailure }.getOrThrow()
}.execute(action) }.execute(action)
@ -153,14 +159,12 @@ class RoomDetailsEditPresenter @Inject constructor(
private suspend fun updateAvatar(avatarUri: Uri?): Result<Unit> { private suspend fun updateAvatar(avatarUri: Uri?): Result<Unit> {
return runCatching { return runCatching {
val result = if (avatarUri != null) { if (avatarUri != null) {
val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() as? MediaUploadInfo.Image val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow()
val byteArray = preprocessed?.file?.readBytes() room.updateAvatar(MimeTypes.Jpeg, preprocessed.file.readBytes()).getOrThrow()
byteArray?.let { room.updateAvatar(MimeTypes.Jpeg, it) } ?: error("Could not process the given uri ($avatarUri)")
} else { } else {
room.removeAvatar() room.removeAvatar().getOrThrow()
} }
result.getOrThrow()
} }
} }
} }

View file

@ -21,7 +21,6 @@ import app.cash.molecule.RecompositionClock
import app.cash.molecule.moleculeFlow import app.cash.molecule.moleculeFlow
import app.cash.turbine.test import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat 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.aMatrixRoom
import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditEvents import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditEvents
import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditPresenter 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.MatrixRoom
import io.element.android.libraries.matrix.api.room.StateEventType 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.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.mediapickers.test.FakePickerProvider
import io.element.android.libraries.mediaupload.api.MediaUploadInfo 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.element.android.libraries.mediaupload.test.FakeMediaPreProcessor
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
@ -600,14 +599,9 @@ class RoomDetailsEditPresenterTest {
} }
fakePickerProvider.givenResult(anotherAvatarUri) fakePickerProvider.givenResult(anotherAvatarUri)
fakeMediaPreProcessor.givenResult(Result.success(MediaUploadInfo.Image( fakeMediaPreProcessor.givenResult(Result.success(MediaUploadInfo.AnyFile(
file = processedFile, file = processedFile,
info = mockk(), info = mockk(),
thumbnailInfo = ThumbnailProcessingInfo(
file = processedFile,
info = mockk(),
blurhash = "",
)
))) )))
} }