Skip to content

Commit

Permalink
Unify the way we decide whether a room is a DM or a group room (#3100)
Browse files Browse the repository at this point in the history
* Add centralised 'room is DM' check

Also add extension functions for `MatrixRoom` and `MatrixRoomInfo`.

* Use the centralised method and extension functions through the app, including:

- Room list.
- Room details screen.
- Invites.
- Notifications.

Replace most `isDirect` usages with `isDm`.

* Update screenshots

---------

Co-authored-by: ElementBot <benoitm+elementbot@element.io>
  • Loading branch information
jmartinesp and ElementBot authored Jul 10, 2024
1 parent 1a03edb commit 0be7058
Show file tree
Hide file tree
Showing 47 changed files with 195 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ open class AcceptDeclineInviteStateProvider : PreviewParameterProvider<AcceptDec
anAcceptDeclineInviteState(),
anAcceptDeclineInviteState(
invite = Optional.of(
InviteData(RoomId("!room:matrix.org"), isDirect = true, roomName = "Alice"),
InviteData(RoomId("!room:matrix.org"), isDm = true, roomName = "Alice"),
),
declineAction = AsyncAction.Confirming,
),
anAcceptDeclineInviteState(
invite = Optional.of(
InviteData(RoomId("!room:matrix.org"), isDirect = false, roomName = "Some room"),
InviteData(RoomId("!room:matrix.org"), isDm = false, roomName = "Some room"),
),
declineAction = AsyncAction.Confirming,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ import io.element.android.libraries.matrix.api.core.RoomId
data class InviteData(
val roomId: RoomId,
val roomName: String,
val isDirect: Boolean,
val isDm: Boolean,
)
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ private fun DeclineConfirmationDialog(
onDismissClick: () -> Unit,
modifier: Modifier = Modifier
) {
val contentResource = if (invite.isDirect) {
val contentResource = if (invite.isDm) {
R.string.screen_invites_decline_direct_chat_message
} else {
R.string.screen_invites_decline_chat_message
}

val titleResource = if (invite.isDirect) {
val titleResource = if (invite.isDm) {
R.string.screen_invites_decline_direct_chat_title
} else {
R.string.screen_invites_decline_chat_title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,12 @@ class AcceptDeclineInvitePresenterTest {
private fun anInviteData(
roomId: RoomId = A_ROOM_ID,
name: String = A_ROOM_NAME,
isDirect: Boolean = false
isDm: Boolean = false
): InviteData {
return InviteData(
roomId = roomId,
roomName = name,
isDirect = isDirect
isDm = isDm
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
import io.element.android.libraries.matrix.api.room.CurrentUserMembership
import io.element.android.libraries.matrix.api.room.MatrixRoomInfo
import io.element.android.libraries.matrix.api.room.RoomType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.api.room.join.JoinRoom
import io.element.android.libraries.matrix.api.room.preview.RoomPreview
import io.element.android.libraries.matrix.ui.model.toInviteSender
Expand Down Expand Up @@ -173,7 +174,7 @@ private fun RoomPreview.toContentState(): ContentState {
topic = topic,
alias = canonicalAlias,
numberOfMembers = numberOfJoinedMembers,
isDirect = false,
isDm = false,
roomType = roomType,
roomAvatarUrl = avatarUrl,
joinAuthorisationStatus = when {
Expand All @@ -194,7 +195,7 @@ internal fun RoomDescription.toContentState(): ContentState {
topic = topic,
alias = alias,
numberOfMembers = numberOfMembers,
isDirect = false,
isDm = false,
roomType = RoomType.Room,
roomAvatarUrl = avatarUrl,
joinAuthorisationStatus = when (joinRule) {
Expand All @@ -213,7 +214,7 @@ internal fun MatrixRoomInfo.toContentState(): ContentState {
topic = topic,
alias = canonicalAlias,
numberOfMembers = activeMembersCount,
isDirect = isDirect,
isDm = isDm,
roomType = if (isSpace) RoomType.Space else RoomType.Room,
roomAvatarUrl = avatarUrl,
joinAuthorisationStatus = when {
Expand All @@ -233,7 +234,7 @@ internal fun ContentState.toInviteData(): InviteData? {
roomId = roomId,
// Note: name should not be null at this point, but use Id just in case...
roomName = name ?: roomId.value,
isDirect = isDirect
isDm = isDm
)
else -> null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ sealed interface ContentState {
val topic: String?,
val alias: RoomAlias?,
val numberOfMembers: Long?,
val isDirect: Boolean,
val isDm: Boolean,
val roomType: RoomType,
val roomAvatarUrl: String?,
val joinAuthorisationStatus: JoinAuthorisationStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
import io.element.android.libraries.matrix.api.room.RoomType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.ui.model.InviteSender

open class JoinRoomStateProvider : PreviewParameterProvider<JoinRoomState> {
Expand Down Expand Up @@ -84,6 +85,12 @@ open class JoinRoomStateProvider : PreviewParameterProvider<JoinRoomState> {
roomType = RoomType.Space,
)
),
aJoinRoomState(
contentState = aLoadedContentState(
name = "A DM",
isDm = true,
)
),
)
}

Expand All @@ -106,7 +113,7 @@ fun aLoadedContentState(
alias: RoomAlias? = RoomAlias("#exa:matrix.org"),
topic: String? = "Element X is a secure, private and decentralized messenger.",
numberOfMembers: Long? = null,
isDirect: Boolean = false,
isDm: Boolean = false,
roomType: RoomType = RoomType.Room,
roomAvatarUrl: String? = null,
joinAuthorisationStatus: JoinAuthorisationStatus = JoinAuthorisationStatus.Unknown
Expand All @@ -116,7 +123,7 @@ fun aLoadedContentState(
alias = alias,
topic = topic,
numberOfMembers = numberOfMembers,
isDirect = isDirect,
isDm = isDm,
roomType = roomType,
roomAvatarUrl = roomAvatarUrl,
joinAuthorisationStatus = joinAuthorisationStatus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class JoinRoomPresenterTest {
assertThat(contentState.topic).isEqualTo(roomInfo.topic)
assertThat(contentState.alias).isEqualTo(roomInfo.canonicalAlias)
assertThat(contentState.numberOfMembers).isEqualTo(roomInfo.activeMembersCount)
assertThat(contentState.isDirect).isEqualTo(roomInfo.isDirect)
assertThat(contentState.isDm).isEqualTo(roomInfo.isDirect)
assertThat(contentState.roomAvatarUrl).isEqualTo(roomInfo.avatarUrl)
}
}
Expand Down Expand Up @@ -283,7 +283,7 @@ class JoinRoomPresenterTest {
assertThat(contentState.topic).isEqualTo(roomDescription.topic)
assertThat(contentState.alias).isEqualTo(roomDescription.alias)
assertThat(contentState.numberOfMembers).isEqualTo(roomDescription.numberOfMembers)
assertThat(contentState.isDirect).isFalse()
assertThat(contentState.isDm).isFalse()
assertThat(contentState.roomAvatarUrl).isEqualTo(roomDescription.avatarUrl)
}
}
Expand Down Expand Up @@ -398,7 +398,7 @@ class JoinRoomPresenterTest {
topic = "Room topic",
alias = RoomAlias("#alias:matrix.org"),
numberOfMembers = 2,
isDirect = false,
isDm = false,
roomType = RoomType.Room,
roomAvatarUrl = "avatarUrl",
joinAuthorisationStatus = JoinAuthorisationStatus.CanJoin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
import io.element.android.libraries.matrix.api.room.isDm
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class DefaultLeaveRoomPresenterTest {
client = FakeMatrixClient().apply {
givenGetRoomResult(
roomId = A_ROOM_ID,
result = FakeMatrixRoom(activeMemberCount = 2, isDirect = true, isOneToOne = true),
result = FakeMatrixRoom(activeMemberCount = 2, isDirect = true),
)
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.MatrixRoomInfo
import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
import io.element.android.libraries.matrix.api.room.MessageEventType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.ui.messages.reply.map
import io.element.android.libraries.matrix.ui.model.getAvatarData
import io.element.android.libraries.matrix.ui.room.canCall
Expand Down Expand Up @@ -162,7 +163,7 @@ class MessagesPresenter @AssistedInject constructor(
var showReinvitePrompt by remember { mutableStateOf(false) }
LaunchedEffect(hasDismissedInviteDialog, composerState.textEditorState.hasFocus(), syncUpdateFlow.value) {
withContext(dispatchers.io) {
showReinvitePrompt = !hasDismissedInviteDialog && composerState.textEditorState.hasFocus() && room.isDirect && room.activeMemberCount == 1L
showReinvitePrompt = !hasDismissedInviteDialog && composerState.textEditorState.hasFocus() && room.isDm && room.activeMemberCount == 1L
}
}
val networkConnectionStatus by networkMonitor.connectivity.collectAsState()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.Mention
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
import io.element.android.libraries.matrix.api.room.draft.ComposerDraftType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.ui.messages.reply.InReplyToDetails
import io.element.android.libraries.matrix.ui.messages.reply.map
import io.element.android.libraries.mediapickers.api.PickerProvider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.MessageEventType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.api.room.roomMembers
import io.element.android.libraries.matrix.api.timeline.ReceiptType
import io.element.android.libraries.matrix.api.timeline.item.event.TimelineItemEventOrigin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,6 @@ class MessageComposerPresenterTest {
val david = aRoomMember(userId = A_USER_ID_4, displayName = "Dave", membership = RoomMembershipState.JOIN)
val room = FakeMatrixRoom(
isDirect = false,
isOneToOne = false,
).apply {
givenRoomMembersState(
MatrixRoomMembersState.Ready(
Expand Down Expand Up @@ -904,7 +903,8 @@ class MessageComposerPresenterTest {
val david = aRoomMember(userId = A_USER_ID_4, displayName = "Dave", membership = RoomMembershipState.JOIN)
val room = FakeMatrixRoom(
isDirect = true,
isOneToOne = true,
activeMemberCount = 2,
isEncrypted = true,
).apply {
givenRoomMembersState(
MatrixRoomMembersState.Ready(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.StateEventType
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.api.room.powerlevels.canInvite
import io.element.android.libraries.matrix.api.room.powerlevels.canSendState
import io.element.android.libraries.matrix.api.room.roomNotificationSettings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import io.element.android.libraries.matrix.api.core.UserId
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.RoomMembershipState
import io.element.android.libraries.matrix.api.room.isDm
import io.element.android.libraries.matrix.api.room.powerlevels.canBan
import io.element.android.libraries.matrix.api.room.powerlevels.canKick
import io.element.android.services.analytics.api.AnalyticsService
Expand All @@ -58,8 +59,7 @@ class DefaultRoomMembersModerationPresenter @Inject constructor(
private suspend fun canKick() = room.canKick().getOrDefault(false)

override suspend fun canDisplayModerationActions(): Boolean {
val isDm = room.isDm && room.isEncrypted
return !isDm && (canBan() || canKick())
return !room.isDm && (canBan() || canKick())
}

@Composable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import org.junit.Test
class DefaultRoomMembersModerationPresenterTest {
@Test
fun `canDisplayModerationActions - when room is DM is false`() = runTest {
val room = FakeMatrixRoom(isDirect = true, isPublic = true, isOneToOne = true).apply {
val room = FakeMatrixRoom(isDirect = true, isPublic = true, activeMemberCount = 2).apply {
givenRoomInfo(aRoomInfo(isDirect = true, isPublic = false, activeMembersCount = 2))
}
val presenter = createDefaultRoomMembersModerationPresenter(matrixRoom = room)
Expand All @@ -54,7 +54,7 @@ class DefaultRoomMembersModerationPresenterTest {

@Test
fun `canDisplayModerationActions - when user can kick other users, FF is enabled and room is not a DM returns true`() = runTest {
val room = FakeMatrixRoom(isDirect = false, isOneToOne = false).apply {
val room = FakeMatrixRoom(isDirect = false, activeMemberCount = 10).apply {
givenCanKickResult(Result.success(true))
}
val presenter = createDefaultRoomMembersModerationPresenter(matrixRoom = room)
Expand All @@ -63,7 +63,7 @@ class DefaultRoomMembersModerationPresenterTest {

@Test
fun `canDisplayModerationActions - when user can ban other users, FF is enabled and room is not a DM returns true`() = runTest {
val room = FakeMatrixRoom(isDirect = false, isOneToOne = false).apply {
val room = FakeMatrixRoom(isDirect = false, activeMemberCount = 10).apply {
givenCanBanResult(Result.success(true))
}
val presenter = createDefaultRoomMembersModerationPresenter(matrixRoom = room)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,5 +290,5 @@ internal fun RoomListRoomSummary.toInviteData() = InviteData(
roomId = roomId,
// Note: `name` should not be null at this point, but just in case, fallback to the roomId
roomName = name ?: roomId.value,
isDirect = isDirect,
isDm = isDm,
)
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ internal fun RoomSummaryRow(
modifier = modifier
) {
InviteNameAndIndicatorRow(name = room.name)
InviteSubtitle(isDirect = room.isDirect, inviteSender = room.inviteSender, canonicalAlias = room.canonicalAlias)
if (!room.isDirect && room.inviteSender != null) {
InviteSubtitle(isDm = room.isDm, inviteSender = room.inviteSender, canonicalAlias = room.canonicalAlias)
if (!room.isDm && room.inviteSender != null) {
Spacer(modifier = Modifier.height(4.dp))
InviteSenderView(
modifier = Modifier.fillMaxWidth(),
Expand Down Expand Up @@ -206,12 +206,12 @@ private fun NameAndTimestampRow(

@Composable
private fun InviteSubtitle(
isDirect: Boolean,
isDm: Boolean,
inviteSender: InviteSender?,
canonicalAlias: RoomAlias?,
modifier: Modifier = Modifier
) {
val subtitle = if (isDirect) {
val subtitle = if (isDm) {
inviteSender?.userId?.value
} else {
canonicalAlias?.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class RoomListRoomSummaryFactory @Inject constructor(
isMarkedUnread = details.isMarkedUnread,
timestamp = lastMessageTimestampFormatter.format(details.lastMessageTimestamp),
lastMessage = details.lastMessage?.let { message ->
roomLastMessageFormatter.format(message.event, details.isDirect)
roomLastMessageFormatter.format(message.event, details.isDm)
}.orEmpty(),
avatarData = avatarData,
userDefinedNotificationMode = details.userDefinedNotificationMode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ open class RoomListRoomSummaryProvider : PreviewParameterProvider<RoomListRoomSu
userId = UserId("@bob:matrix.org"),
displayName = "Bob",
),
isDirect = true,
isDm = true,
),
aRoomListRoomSummary(
name = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ data class NotificationData(
val roomAvatarUrl: String?,
val roomDisplayName: String?,
val isDirect: Boolean,
val isDm: Boolean,
val isEncrypted: Boolean,
val isNoisy: Boolean,
val timestamp: Long,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ interface MatrixRoom : Closeable {
val activeMemberCount: Long
val joinedMemberCount: Long

/** Whether the room is a direct message. */
val isDm: Boolean get() = isDirect && isOneToOne

val roomInfoFlow: Flow<MatrixRoomInfo>
val roomTypingMembersFlow: Flow<List<UserId>>

Expand Down
Loading

0 comments on commit 0be7058

Please sign in to comment.