feat(security&privacy) : improve and document code after PR review.

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyPresenter.kt

@@ -96,7 +96,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor(
             address = savedSettings.address,
         )
 
-        var showEncryptionConfirmation by remember(savedSettings.isEncrypted) { mutableStateOf(false) }
+        var showEnableEncryptionConfirmation by remember(savedSettings.isEncrypted) { mutableStateOf(false) }
         val permissions by room.securityAndPrivacyPermissionsAsState(syncUpdateFlow.value)
 
         fun handleEvents(event: SecurityAndPrivacyEvents) {
@@ -116,7 +116,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor(
                     if (editedIsEncrypted) {
                         editedIsEncrypted = false
                     } else {
-                        showEncryptionConfirmation = true
+                        showEnableEncryptionConfirmation = true
                     }
                 }
                 is SecurityAndPrivacyEvents.ChangeHistoryVisibility -> {
@@ -130,10 +130,10 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor(
                 }
                 SecurityAndPrivacyEvents.EditRoomAddress -> navigator.openEditRoomAddress()
                 SecurityAndPrivacyEvents.CancelEnableEncryption -> {
-                    showEncryptionConfirmation = false
+                    showEnableEncryptionConfirmation = false
                 }
                 SecurityAndPrivacyEvents.ConfirmEnableEncryption -> {
-                    showEncryptionConfirmation = false
+                    showEnableEncryptionConfirmation = false
                     editedIsEncrypted = true
                 }
                 SecurityAndPrivacyEvents.DismissSaveError -> {
@@ -146,7 +146,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor(
             savedSettings = savedSettings,
             editedSettings = editedSettings,
             homeserverName = homeserverName,
-            showEncryptionConfirmation = showEncryptionConfirmation,
+            showEnableEncryptionConfirmation = showEnableEncryptionConfirmation,
             saveAction = saveAction.value,
             permissions = permissions,
             eventSink = ::handleEvents
@@ -189,8 +189,9 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor(
                 }
             }
             val updateJoinRule = async {
-                if (editedSettings.roomAccess != savedSettings.roomAccess) {
+                val joinRule = editedSettings.roomAccess.map()
-                    room.updateJoinRule(editedSettings.roomAccess.map())
+                if (editedSettings.roomAccess != savedSettings.roomAccess && joinRule != null) {
+                    room.updateJoinRule(joinRule)
                 } else {
                     Result.success(Unit)
                 }
@@ -239,19 +240,21 @@ private fun JoinRule?.map(): SecurityAndPrivacyRoomAccess {
         JoinRule.Public -> SecurityAndPrivacyRoomAccess.Anyone
         JoinRule.Knock, is JoinRule.KnockRestricted -> SecurityAndPrivacyRoomAccess.AskToJoin
         is JoinRule.Restricted -> SecurityAndPrivacyRoomAccess.SpaceMember
+        JoinRule.Invite -> SecurityAndPrivacyRoomAccess.InviteOnly
+        // All other cases are not supported so we default to InviteOnly
         is JoinRule.Custom,
-        JoinRule.Invite,
         JoinRule.Private,
         null -> SecurityAndPrivacyRoomAccess.InviteOnly
     }
 }
 
-private fun SecurityAndPrivacyRoomAccess.map(): JoinRule {
+private fun SecurityAndPrivacyRoomAccess.map(): JoinRule? {
     return when (this) {
         SecurityAndPrivacyRoomAccess.Anyone -> JoinRule.Public
         SecurityAndPrivacyRoomAccess.AskToJoin -> JoinRule.Knock
         SecurityAndPrivacyRoomAccess.InviteOnly -> JoinRule.Private
-        SecurityAndPrivacyRoomAccess.SpaceMember -> error("Unsupported")
+        // SpaceMember can't be selected in the ui
+        SecurityAndPrivacyRoomAccess.SpaceMember -> null
     }
 }
 
@@ -260,7 +263,8 @@ private fun RoomHistoryVisibility?.map(): SecurityAndPrivacyHistoryVisibility {
         RoomHistoryVisibility.WorldReadable -> SecurityAndPrivacyHistoryVisibility.Anyone
         RoomHistoryVisibility.Joined,
         RoomHistoryVisibility.Invited -> SecurityAndPrivacyHistoryVisibility.SinceInvite
-        RoomHistoryVisibility.Shared,
+        RoomHistoryVisibility.Shared -> SecurityAndPrivacyHistoryVisibility.SinceSelection
+        // All other cases are not supported so we default to SinceSelection
         is RoomHistoryVisibility.Custom,
         null -> SecurityAndPrivacyHistoryVisibility.SinceSelection
     }

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyState.kt

@@ -18,7 +18,7 @@ data class SecurityAndPrivacyState(
     // the settings the user wants to apply.
     val editedSettings: SecurityAndPrivacySettings,
     val homeserverName: String,
-    val showEncryptionConfirmation: Boolean,
+    val showEnableEncryptionConfirmation: Boolean,
     val saveAction: AsyncAction<Unit>,
     private val permissions: SecurityAndPrivacyPermissions,
     val eventSink: (SecurityAndPrivacyEvents) -> Unit

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyStateProvider.kt

@@ -79,7 +79,7 @@ fun aSecurityAndPrivacyState(
     editedSettings = editedSettings,
     savedSettings = savedSettings,
     homeserverName = homeserverName,
-    showEncryptionConfirmation = showEncryptionConfirmation,
+    showEnableEncryptionConfirmation = showEncryptionConfirmation,
     saveAction = saveAction,
     permissions = permissions,
     eventSink = eventSink

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyView.kt

@@ -100,9 +100,10 @@ fun SecurityAndPrivacyView(
             if (state.showEncryptionSection) {
                 EncryptionSection(
                     isRoomEncrypted = state.editedSettings.isEncrypted,
+                    // encryption can't be disabled once enabled
                     canToggleEncryption = !state.savedSettings.isEncrypted,
                     onToggleEncryption = { state.eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState) },
-                    showConfirmation = state.showEncryptionConfirmation,
+                    showConfirmation = state.showEnableEncryptionConfirmation,
                     onDismissConfirmation = { state.eventSink(SecurityAndPrivacyEvents.CancelEnableEncryption) },
                     onConfirmEncryption = { state.eventSink(SecurityAndPrivacyEvents.ConfirmEnableEncryption) },
                 )
@@ -206,6 +207,7 @@ private fun RoomAccessSection(
             trailingContent = ListItemContent.RadioButton(selected = edited == SecurityAndPrivacyRoomAccess.Anyone),
             onClick = { onSelectOption(SecurityAndPrivacyRoomAccess.Anyone) },
         )
+        // Show space member option, but disabled as we don't support this option for now.
         if (saved == SecurityAndPrivacyRoomAccess.SpaceMember) {
             ListItem(
                 headlineContent = { Text(text = stringResource(R.string.screen_security_and_privacy_room_access_space_members_option_title)) },
@@ -342,7 +344,6 @@ private fun HistoryVisibilitySection(
         title = stringResource(R.string.screen_security_and_privacy_room_history_section_header),
         modifier = modifier,
     ) {
-        Spacer(Modifier.height(16.dp))
         for (availableOption in availableOptions) {
             val isSelected = availableOption == editedOption
             HistoryVisibilityItem(
@@ -351,6 +352,7 @@ private fun HistoryVisibilitySection(
                 onSelectOption = onSelectOption,
             )
         }
+        // Also show the saved option if it's not in the available options, but disabled
         if (savedOptions != null && !availableOptions.contains(savedOptions)) {
             HistoryVisibilityItem(
                 option = savedOptions,

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/editroomaddress/EditRoomAddressPresenter.kt

@@ -118,12 +118,10 @@ class EditRoomAddressPresenter @AssistedInject constructor(
                 // Otherwise, only update the alternative aliases and keep the current canonical alias
                 else -> {
                     val newAlternativeAliases = buildList {
+                        // New alias is added first, so we make sure we pick it first
                         add(newRoomAlias)
-                        for (alias in room.alternativeAliases) {
+                        // Add all other aliases, except the one we just removed from the room directory
-                            if (alias != savedAliasFromHomeserver) {
+                        addAll(room.alternativeAliases.filter { it != savedAliasFromHomeserver })
-                                add(alias)
-                            }
-                        }
                     }
                     room.updateCanonicalAlias(savedCanonicalAlias, newAlternativeAliases).getOrThrow()
                 }
@@ -141,5 +139,5 @@ private fun MatrixRoom.firstAliasMatching(serverName: String): RoomAlias? {
     if (canonicalAlias?.matchesServer(serverName) == true) {
         return canonicalAlias
     }
-    return alternativeAliases.firstOrNull { it.value.contains(serverName) }
+    return alternativeAliases.firstOrNull { it.matchesServer(serverName) }
 }

features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/securityandprivacy/SecurityAndPrivacyPresenterTest.kt

@@ -37,7 +37,7 @@ class SecurityAndPrivacyPresenterTest {
             with(awaitItem()) {
                 assertThat(editedSettings).isEqualTo(savedSettings)
                 assertThat(canBeSaved).isFalse()
-                assertThat(showEncryptionConfirmation).isFalse()
+                assertThat(showEnableEncryptionConfirmation).isFalse()
                 assertThat(saveAction).isEqualTo(AsyncAction.Uninitialized)
                 assertThat(showRoomAccessSection).isFalse()
                 assertThat(showRoomVisibilitySections).isFalse()
@@ -47,7 +47,7 @@ class SecurityAndPrivacyPresenterTest {
             with(awaitItem()) {
                 assertThat(editedSettings).isEqualTo(savedSettings)
                 assertThat(canBeSaved).isFalse()
-                assertThat(showEncryptionConfirmation).isFalse()
+                assertThat(showEnableEncryptionConfirmation).isFalse()
                 assertThat(saveAction).isEqualTo(AsyncAction.Uninitialized)
                 assertThat(showRoomAccessSection).isTrue()
                 assertThat(showRoomVisibilitySections).isFalse()
@@ -138,21 +138,21 @@ class SecurityAndPrivacyPresenterTest {
                 eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState)
             }
             with(awaitItem()) {
-                assertThat(showEncryptionConfirmation).isTrue()
+                assertThat(showEnableEncryptionConfirmation).isTrue()
                 eventSink(SecurityAndPrivacyEvents.CancelEnableEncryption)
             }
             with(awaitItem()) {
-                assertThat(showEncryptionConfirmation).isFalse()
+                assertThat(showEnableEncryptionConfirmation).isFalse()
                 eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState)
             }
             with(awaitItem()) {
-                assertThat(showEncryptionConfirmation).isTrue()
+                assertThat(showEnableEncryptionConfirmation).isTrue()
                 eventSink(SecurityAndPrivacyEvents.ConfirmEnableEncryption)
             }
             skipItems(1)
             with(awaitItem()) {
                 assertThat(editedSettings.isEncrypted).isTrue()
-                assertThat(showEncryptionConfirmation).isFalse()
+                assertThat(showEnableEncryptionConfirmation).isFalse()
                 assertThat(canBeSaved).isTrue()
                 eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState)
             }

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt

@@ -428,9 +428,7 @@ interface MatrixRoom : Closeable {
 
     /**
      * Returns the visibility for this room in the room directory.
-     *
+     * If the room is not published, the result will be [RoomVisibility.Private].
-     * [Public](`RoomVisibility::Public`) rooms are listed in the room
-     * directory and can be found using it.
      */
     suspend fun getRoomVisibility(): Result<RoomVisibility>