From 6e722d6a541ad9831f8595ee9365f51c6fc4b986 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 15 Oct 2024 18:02:23 +0200 Subject: [PATCH 1/3] Add userId in identity change warning banner #3678 --- .../identity/IdentityChangeStatePresenter.kt | 4 ++-- .../identity/IdentityChangeStateProvider.kt | 24 +++++++++++++++---- .../identity/IdentityChangeStateView.kt | 20 +++++++++++++--- .../crypto/identity/IdentityRoomMember.kt | 2 +- .../libraries/matrix/api/core/UserId.kt | 5 ++++ .../libraries/matrix/api/room/RoomMember.kt | 6 +++++ .../src/main/res/values/localazy.xml | 2 ++ 7 files changed, 53 insertions(+), 10 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt index a4a13986e8..16bc39fac8 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt @@ -105,13 +105,13 @@ class IdentityChangeStatePresenter @Inject constructor( private fun RoomMember.toIdentityRoomMember() = IdentityRoomMember( userId = userId, - disambiguatedDisplayName = disambiguatedDisplayName, + displayNameOrDefault = displayNameOrDefault, avatarData = getAvatarData(AvatarSize.ComposerAlert), ) private fun createDefaultRoomMemberForIdentityChange(userId: UserId) = IdentityRoomMember( userId = userId, - disambiguatedDisplayName = userId.value, + displayNameOrDefault = userId.extractedDisplayName, avatarData = AvatarData( id = userId.value, name = null, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt index fa70bebe6a..2a36c5e23f 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt @@ -20,8 +20,16 @@ class IdentityChangeStateProvider : PreviewParameterProvider = emptyList(), ) = IdentityChangeState( @@ -38,7 +54,7 @@ internal fun anIdentityChangeState( internal fun anIdentityRoomMember( userId: UserId = UserId("@alice:example.com"), - disambiguatedDisplayName: String = userId.value, + displayNameOrDefault: String = userId.extractedDisplayName, avatarData: AvatarData = AvatarData( id = userId.value, name = null, @@ -47,6 +63,6 @@ internal fun anIdentityRoomMember( ), ) = IdentityRoomMember( userId = userId, - disambiguatedDisplayName = disambiguatedDisplayName, + displayNameOrDefault = displayNameOrDefault, avatarData = avatarData, ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt index 6ff167a8a7..aac05f8684 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt @@ -40,13 +40,27 @@ fun IdentityChangeStateView( avatar = pinViolationIdentityChange.identityRoomMember.avatarData, content = buildAnnotatedString { val learnMoreStr = stringResource(CommonStrings.action_learn_more) + val displayName = pinViolationIdentityChange.identityRoomMember.displayNameOrDefault + val userIdStr = stringResource( + CommonStrings.crypto_identity_change_pin_violation_new_user_id, + pinViolationIdentityChange.identityRoomMember.userId, + ) val fullText = stringResource( - id = CommonStrings.crypto_identity_change_pin_violation, - pinViolationIdentityChange.identityRoomMember.disambiguatedDisplayName, + id = CommonStrings.crypto_identity_change_pin_violation_new, + displayName, + userIdStr, learnMoreStr, ) - val learnMoreStartIndex = fullText.indexOf(learnMoreStr) append(fullText) + val userIdStartIndex = fullText.indexOf(userIdStr) + addStyle( + style = SpanStyle( + fontWeight = FontWeight.Bold, + ), + start = userIdStartIndex, + end = userIdStartIndex + userIdStr.length, + ) + val learnMoreStartIndex = fullText.lastIndexOf(learnMoreStr) addStyle( style = SpanStyle( textDecoration = TextDecoration.Underline, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt index f5ad9295b3..4acfbd5181 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt @@ -12,6 +12,6 @@ import io.element.android.libraries.matrix.api.core.UserId data class IdentityRoomMember( val userId: UserId, - val disambiguatedDisplayName: String, + val displayNameOrDefault: String, val avatarData: AvatarData, ) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt index e03069a243..ae6c3de5bd 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt @@ -24,4 +24,9 @@ value class UserId(val value: String) : Serializable { } override fun toString(): String = value + + val extractedDisplayName: String + get() = value + .removePrefix("@") + .substringBefore(":") } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt index 52d45bfc11..cabd222b72 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMember.kt @@ -51,6 +51,12 @@ data class RoomMember( isNameAmbiguous -> "$displayName ($userId)" else -> displayName } + + val displayNameOrDefault: String + get() = when { + displayName == null -> userId.extractedDisplayName + else -> displayName + } } enum class RoomMembershipState { diff --git a/libraries/ui-strings/src/main/res/values/localazy.xml b/libraries/ui-strings/src/main/res/values/localazy.xml index f3adc853f2..24e35e6e58 100644 --- a/libraries/ui-strings/src/main/res/values/localazy.xml +++ b/libraries/ui-strings/src/main/res/values/localazy.xml @@ -252,6 +252,8 @@ Reason: %1$s." "Waiting for this message" "You" "%1$s\'s identity appears to have changed. %2$s" + "%1$s’s %2$s identity appears to have changed. %3$s" + "(%1$s)" "Confirmation" "Error" "Success" From 0272788f3fba92a05a6b3bbd1c279a4d250000d9 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Tue, 15 Oct 2024 16:13:17 +0000 Subject: [PATCH 2/3] Update screenshots --- ....impl.crypto.identity_IdentityChangeStateView_Day_1_en.png | 4 ++-- ....impl.crypto.identity_IdentityChangeStateView_Day_2_en.png | 3 +++ ...mpl.crypto.identity_IdentityChangeStateView_Night_1_en.png | 4 ++-- ...mpl.crypto.identity_IdentityChangeStateView_Night_2_en.png | 3 +++ ...rypto.identity_MessagesViewWithIdentityChange_Day_1_en.png | 4 ++-- ...rypto.identity_MessagesViewWithIdentityChange_Day_2_en.png | 3 +++ ...pto.identity_MessagesViewWithIdentityChange_Night_1_en.png | 4 ++-- ...pto.identity_MessagesViewWithIdentityChange_Night_2_en.png | 3 +++ 8 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_2_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_2_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_2_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_2_en.png diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_1_en.png index 66783ec6cc..2103b4fffa 100644 --- a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:24481773bfccc5eb1ebf3f9955cdc77e8b3b5130d4fa56f96df732e3627ea3c6 -size 21018 +oid sha256:66419e3ac6da49bd535a005759cba38fe4b57d5307ad9ad3cb3e2fa4eed38890 +size 25136 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_2_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_2_en.png new file mode 100644 index 0000000000..c5e829c113 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Day_2_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:35651217adb52b19136f99922d7573071084ae7c4109feb525d182274ab4df78 +size 25131 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_1_en.png index 93bd368a94..9077b8eefe 100644 --- a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1421adb601d9a8050a5ed1b60aba8a05b8eab61aaf18d3936226efe891acd8b6 -size 23880 +oid sha256:d2831daee948b612f15623948ef49c497732a143fa50f331e82e689decd5a4c0 +size 27967 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_2_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_2_en.png new file mode 100644 index 0000000000..485eff2c4b --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_IdentityChangeStateView_Night_2_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3055ebf4ef01148506cac22bfd7c05aea5315d4a63952affc998c918cd6055b6 +size 27880 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_1_en.png index fec7097b6a..f69e6e4bc4 100644 --- a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8d37a9ffee50f8e9ea0c9fd50c61310969c4fbaa99cf858481b3f42f8db4467d -size 61102 +oid sha256:38b158a90790d6183df45914382ec16275ae8e4c5e7054c988d20d179cfebc44 +size 64996 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_2_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_2_en.png new file mode 100644 index 0000000000..60fddc6079 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Day_2_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b3b3a017453f60f98f245374089fa604848f5e764be20463d63b0ebc51eadd18 +size 65118 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_1_en.png index 2cbc735bd8..e944aae39d 100644 --- a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d0952d9cf3812ca419e144936faf523e0b865a22a61d2a55e07f089d9e6e9009 -size 64824 +oid sha256:5c4f814532fb387a3a38de6fbbf31563884de658ab7d930fe5e91b3fd299f355 +size 69088 diff --git a/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_2_en.png b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_2_en.png new file mode 100644 index 0000000000..9d4d8d2416 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.messages.impl.crypto.identity_MessagesViewWithIdentityChange_Night_2_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d9bfcbe306160d48ac80ad1a6c4e0d611da72a376bdfe3532ace022d9bbb5c9a +size 69181 From 6392404c3aedcda74ee58f3b1e32fca01c07f8ae Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 16 Oct 2024 10:01:04 +0200 Subject: [PATCH 3/3] Fix tests. --- .../crypto/identity/IdentityChangeStatePresenterTest.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt index 4ff0c8f340..92774f4feb 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt @@ -69,7 +69,7 @@ class IdentityChangeStatePresenterTest { } @Test - fun `present - when the room emits identity change, but the feature is disabled, the presenter emits new state`() = runTest { + fun `present - when the room emits identity change, but the feature is disabled, the presenter does not emit new state`() = runTest { val room = FakeMatrixRoom( isEncrypted = true, ) @@ -106,7 +106,7 @@ class IdentityChangeStatePresenterTest { } @Test - fun `present - when the clear room emits identity change, the presenter does not emits new state`() = runTest { + fun `present - when the clear room emits identity change, the presenter does not emit new state`() = runTest { val room = FakeMatrixRoom(isEncrypted = false) val presenter = createIdentityChangeStatePresenter(room) presenter.test { @@ -128,6 +128,7 @@ class IdentityChangeStatePresenterTest { assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1) val value = finalItem.roomMemberIdentityStateChanges.first() assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2) + assertThat(value.identityRoomMember.displayNameOrDefault).isEqualTo(A_USER_ID_2.extractedDisplayName) assertThat(value.identityState).isEqualTo(IdentityState.PinViolation) } } @@ -163,14 +164,14 @@ class IdentityChangeStatePresenterTest { assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1) val value = finalItem.roomMemberIdentityStateChanges.first() assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2) - assertThat(value.identityRoomMember.disambiguatedDisplayName).isEqualTo("Alice") + assertThat(value.identityRoomMember.displayNameOrDefault).isEqualTo("Alice") assertThat(value.identityRoomMember.avatarData.size).isEqualTo(AvatarSize.ComposerAlert) assertThat(value.identityState).isEqualTo(IdentityState.PinViolation) } } @Test - fun `present - when the user pin the identity, the presenter invokes the encryption service api`() = + fun `present - when the user pins the identity, the presenter invokes the encryption service api`() = runTest { val lambda = lambdaRecorder> { Result.success(Unit) } val encryptionService = FakeEncryptionService(