Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Create private room] Picture doesn't not displayed #5405

Merged
merged 6 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5402.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[Create room] Picture doesn't not displayed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[Create room] Picture doesn't not displayed
[Create room] Setting an avatar when creating a room had no effect

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.matrix.android.sdk.internal.session.room.create

import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.EventType
Expand All @@ -42,6 +44,7 @@ internal class CreateRoomBodyBuilder @Inject constructor(
private val deviceListManager: DeviceListManager,
private val identityStore: IdentityStore,
private val fileUploader: FileUploader,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
@UserId
private val userId: String,
@AuthenticatedIdentity
Expand Down Expand Up @@ -112,19 +115,20 @@ internal class CreateRoomBodyBuilder @Inject constructor(
private suspend fun buildAvatarEvent(params: CreateRoomParams): Event? {
return params.avatarUri?.let { avatarUri ->
// First upload the image, ignoring any error
tryOrNull {
fileUploader.uploadFromUri(
uri = avatarUri,
filename = UUID.randomUUID().toString(),
mimeType = MimeTypes.Jpeg)
tryOrNull("Failed to load image") {
Claire1817 marked this conversation as resolved.
Show resolved Hide resolved
withContext(coroutineDispatchers.io) {
fileUploader.uploadFromUri(
uri = avatarUri,
filename = UUID.randomUUID().toString(),
mimeType = MimeTypes.Jpeg)
}
}?.let { response ->
Event(
type = EventType.STATE_ROOM_AVATAR,
stateKey = "",
content = mapOf("url" to response.contentUri)
)
}
?.let { response ->
Event(
type = EventType.STATE_ROOM_AVATAR,
stateKey = "",
content = mapOf("url" to response.contentUri)
)
}
}
}

Expand Down Expand Up @@ -180,19 +184,19 @@ internal class CreateRoomBodyBuilder @Inject constructor(
params.invite3pids.isEmpty() &&
params.invitedUserIds.isNotEmpty() &&
params.invitedUserIds.let { userIds ->
val keys = deviceListManager.downloadKeys(userIds, forceDownload = false)

userIds.all { userId ->
keys.map[userId].let { deviceMap ->
if (deviceMap.isNullOrEmpty()) {
// A user has no device, so do not enable encryption
false
} else {
// Check that every user's device have at least one key
deviceMap.values.all { !it.keys.isNullOrEmpty() }
val keys = deviceListManager.downloadKeys(userIds, forceDownload = false)

userIds.all { userId ->
keys.map[userId].let { deviceMap ->
if (deviceMap.isNullOrEmpty()) {
// A user has no device, so do not enable encryption
false
} else {
// Check that every user's device have at least one key
deviceMap.values.all { !it.keys.isNullOrEmpty() }
}
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import androidx.lifecycle.LiveData
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
import org.matrix.android.sdk.api.query.QueryStringValue
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.EventType
Expand All @@ -42,8 +44,9 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
private val stateEventDataSource: StateEventDataSource,
private val sendStateTask: SendStateTask,
private val fileUploader: FileUploader,
private val viaParameterFinder: ViaParameterFinder
) : StateService {
private val viaParameterFinder: ViaParameterFinder,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
) : StateService {

@AssistedFactory
interface Factory {
Expand Down Expand Up @@ -155,12 +158,14 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
}

override suspend fun updateAvatar(avatarUri: Uri, fileName: String) {
val response = fileUploader.uploadFromUri(avatarUri, fileName, MimeTypes.Jpeg)
sendStateEvent(
eventType = EventType.STATE_ROOM_AVATAR,
body = mapOf("url" to response.contentUri),
stateKey = ""
)
withContext(coroutineDispatchers.io) {
val response = fileUploader.uploadFromUri(avatarUri, fileName, MimeTypes.Jpeg)
sendStateEvent(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could stay outside of the withContext(coroutineDispatchers.io) block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used when we update the picture in the room settings.
It's not working for me in release (i have a popup : "Sorry, an error occurred")but with this fix it's working.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i put the with(coroutineDispatchers.io) in here it's working too.
I think it's a better solution to avoid code duplication and always do this action on the io thread.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so please do it, it will be cleaner. Thanks!

eventType = EventType.STATE_ROOM_AVATAR,
body = mapOf("url" to response.contentUri),
stateKey = ""
)
}
}

override suspend fun deleteAvatar() {
Expand Down