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

Remove unnecessary Room.updateMembers() calls. #2564

Merged
merged 2 commits into from
Mar 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,6 @@ class MessagesPresenter @AssistedInject constructor(
private fun CoroutineScope.reinviteOtherUser(inviteProgress: MutableState<AsyncData<Unit>>) = launch(dispatchers.io) {
inviteProgress.value = AsyncData.Loading()
runCatching {
room.updateMembers()

val memberList = when (val memberState = room.membersStateFlow.value) {
is MatrixRoomMembersState.Ready -> memberState.roomMembers
is MatrixRoomMembersState.Error -> memberState.prevRoomMembers.orEmpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,6 @@ class RoomDetailsPresenter @Inject constructor(
}
}

// Update room members only when first presenting the node
OnLifecycleEvent { _, event ->
if (event == Lifecycle.Event.ON_CREATE) {
scope.launch { room.updateMembers() }
}
}

val membersState by room.membersStateFlow.collectAsState()
val canInvite by getCanInvite(membersState)
val canEditName by getCanSendState(membersState, StateEventType.ROOM_NAME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ class RoomInviteMembersNode @AssistedInject constructor(
body = context.getString(CommonStrings.common_unable_to_invite_message),
)
}

room.updateMembers()
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
}
}
LaunchedEffect(Unit) {
room.updateMembers()
// Update room member info when opening this screen
// We don't need to assign the result as it will be automatically propagated by `room.getRoomMemberAsState`
room.getUpdatedMember(roomMemberId)
}

fun handleEvents(event: RoomMemberDetailsEvents) {
Expand Down Expand Up @@ -133,7 +135,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
.fold(
onSuccess = {
isBlockedState.value = AsyncData.Success(true)
room.updateMembers()
room.getUpdatedMember(userId)
},
onFailure = {
isBlockedState.value = AsyncData.Failure(it, false)
Expand All @@ -147,7 +149,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor(
.fold(
onSuccess = {
isBlockedState.value = AsyncData.Success(false)
room.updateMembers()
room.getUpdatedMember(userId)
},
onFailure = {
isBlockedState.value = AsyncData.Failure(it, true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ package io.element.android.features.roomdetails.impl.rolesandpermissions
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.ui.Modifier
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleEventObserver
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
Expand All @@ -33,10 +30,8 @@ import io.element.android.anvilannotations.ContributesNode
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.roomMembers
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.take
import kotlinx.coroutines.launch
Expand Down Expand Up @@ -69,22 +64,12 @@ class RolesAndPermissionsNode @AssistedInject constructor(
override fun onBuilt() {
super.onBuilt()

// Reload members when the user sees this screen
lifecycle.addObserver(object : LifecycleEventObserver {
override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) {
if (event == Lifecycle.Event.ON_RESUME) {
lifecycleScope.launch { room.updateMembers() }
}
}
})

// If the user is not an admin anymore, exit this section since they won't have permissions to use it
lifecycleScope.launch {
room.membersStateFlow
.map { state ->
state.roomMembers().orEmpty().find { it.userId == room.sessionId }
room.roomInfoFlow
.filter { info ->
info.userPowerLevels[room.sessionId] != RoomMember.Role.ADMIN.powerLevel
}
.filter { it?.role != RoomMember.Role.ADMIN }
.take(1)
.onEach { navigateUp() }
.collect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ interface MatrixRoom : Closeable {
*/
suspend fun updateMembers()

/**
* Will return an updated member or an error.
*/
suspend fun getUpdatedMember(userId: UserId): Result<RoomMember>

suspend fun updateRoomNotificationSettings(): Result<Unit>

val syncUpdateFlow: StateFlow<Long>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,12 @@ class RustMatrixRoom(
roomMemberListFetcher.fetchRoomMembers(source = source)
}

override suspend fun getUpdatedMember(userId: UserId): Result<RoomMember> = withContext(roomDispatcher) {
runCatching {
RoomMemberMapper.map(innerRoom.member(userId.value))
}
}

override suspend fun userDisplayName(userId: UserId): Result<String?> = withContext(roomDispatcher) {
runCatching {
innerRoom.memberDisplayName(userId.value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class FakeMatrixRoom(
private var userDisplayNameResult = Result.success<String?>(null)
private var userAvatarUrlResult = Result.success<String?>(null)
private var userRoleResult = Result.success(RoomMember.Role.USER)
private var updateMembersResult: Result<Unit> = Result.success(Unit)
private var getRoomMemberResult = Result.failure<RoomMember>(IllegalStateException("Member not found"))
private var joinRoomResult = Result.success(Unit)
private var inviteUserResult = Result.success(Unit)
private var canInviteResult = Result.success(true)
Expand Down Expand Up @@ -195,6 +195,10 @@ class FakeMatrixRoom(

override suspend fun updateMembers() = Unit

override suspend fun getUpdatedMember(userId: UserId): Result<RoomMember> {
return getRoomMemberResult
}

override suspend fun updateRoomNotificationSettings(): Result<Unit> = simulateLongTask {
val notificationSettings = notificationSettingsService.getRoomNotificationSettings(roomId, isEncrypted, isOneToOne).getOrThrow()
roomNotificationSettingsStateFlow.value = MatrixRoomNotificationSettingsState.Ready(notificationSettings)
Expand Down Expand Up @@ -532,8 +536,8 @@ class FakeMatrixRoom(
membersStateFlow.value = state
}

fun givenUpdateMembersResult(result: Result<Unit>) {
updateMembersResult = result
fun givenGetRoomMemberResult(result: Result<RoomMember>) {
getRoomMemberResult = result
}

fun givenUserDisplayNameResult(displayName: Result<String?>) {
Expand Down
Loading