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 1d6f4e3c68
commit 72497af09a
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.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<Result<Unit>>()
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<Unit> {
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()
}
}
}

View file

@ -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 = "",
)
)))
}