From d2b6014b5e5ecaaff3b9280afa3348bb8ab23ae7 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 26 Mar 2025 21:46:23 +0100 Subject: [PATCH] change (composer suggestions) : remove feature flag --- .../MessageComposerPresenter.kt | 81 ++++++++++--------- .../MessageComposerPresenterTest.kt | 33 +++----- .../suggestions/SuggestionsProcessorTest.kt | 53 +++++++++++- .../DeveloperSettingsPresenterTest.kt | 2 +- .../libraries/featureflag/api/FeatureFlags.kt | 14 ---- 5 files changed, 106 insertions(+), 77 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 679589f797..8489139bb1 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -23,6 +23,7 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.media3.common.MimeTypes import androidx.media3.common.util.UnstableApi import dagger.assisted.Assisted @@ -84,11 +85,13 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.merge +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import timber.log.Timber import kotlin.time.Duration.Companion.seconds @@ -135,7 +138,6 @@ class MessageComposerPresenter @AssistedInject constructor( @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal var showTextFormatting: Boolean by mutableStateOf(false) - @OptIn(FlowPreview::class) @SuppressLint("UnsafeOptInUsageError") @Composable override fun present(): MessageComposerState { @@ -148,12 +150,6 @@ class MessageComposerPresenter @AssistedInject constructor( richTextEditorState.isReadyToProcessActions = true } val markdownTextEditorState = rememberMarkdownTextEditorState(initialText = null, initialFocus = false) - var isMentionsEnabled by remember { mutableStateOf(false) } - var isRoomAliasSuggestionsEnabled by remember { mutableStateOf(false) } - LaunchedEffect(Unit) { - isMentionsEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.Mentions) - isRoomAliasSuggestionsEnabled = featureFlagService.isFeatureEnabled(FeatureFlags.RoomAliasSuggestions) - } val cameraPermissionState = cameraPermissionPresenter.present() @@ -187,8 +183,6 @@ class MessageComposerPresenter @AssistedInject constructor( val sendTypingNotifications by sessionPreferencesStore.isSendTypingNotificationsEnabled().collectAsState(initial = true) - val roomAliasSuggestions by roomAliasSuggestionsDataSource.getAllRoomAliasSuggestions().collectAsState(initial = emptyList()) - LaunchedEffect(cameraPermissionState.permissionGranted) { if (cameraPermissionState.permissionGranted) { when (pendingEvent) { @@ -201,35 +195,7 @@ class MessageComposerPresenter @AssistedInject constructor( } val suggestions = remember { mutableStateListOf() } - LaunchedEffect(isMentionsEnabled) { - if (!isMentionsEnabled) return@LaunchedEffect - val currentUserId = room.sessionId - - suspend fun canSendRoomMention(): Boolean { - val userCanSendAtRoom = room.canUserTriggerRoomNotification(currentUserId).getOrDefault(false) - return !room.isDm() && userCanSendAtRoom - } - - // This will trigger a search immediately when `@` is typed - val mentionStartTrigger = suggestionSearchTrigger.filter { it?.text.isNullOrEmpty() } - // This will start a search when the user changes the text after the `@` with a debounce to prevent too much wasted work - val mentionCompletionTrigger = suggestionSearchTrigger.debounce(0.3.seconds).filter { !it?.text.isNullOrEmpty() } - merge(mentionStartTrigger, mentionCompletionTrigger) - .combine(room.membersStateFlow) { suggestion, roomMembersState -> - suggestions.clear() - val result = suggestionsProcessor.process( - suggestion = suggestion, - roomMembersState = roomMembersState, - roomAliasSuggestions = if (isRoomAliasSuggestionsEnabled) roomAliasSuggestions else emptyList(), - currentUserId = currentUserId, - canSendRoomMention = ::canSendRoomMention, - ) - if (result.isNotEmpty()) { - suggestions.addAll(result) - } - } - .collect() - } + ResolveSuggestionsEffect(suggestions) DisposableEffect(Unit) { // Declare that the user is not typing anymore when the composer is disposed @@ -409,6 +375,45 @@ class MessageComposerPresenter @AssistedInject constructor( ) } + @OptIn(FlowPreview::class) + @Composable + private fun ResolveSuggestionsEffect( + suggestions: SnapshotStateList, + ) { + LaunchedEffect(Unit) { + val currentUserId = room.sessionId + + suspend fun canSendRoomMention(): Boolean { + val userCanSendAtRoom = room.canUserTriggerRoomNotification(currentUserId).getOrDefault(false) + return !room.isDm() && userCanSendAtRoom + } + + // This will trigger a search immediately when `@` is typed + val mentionStartTrigger = suggestionSearchTrigger.filter { it?.text.isNullOrEmpty() } + // This will start a search when the user changes the text after the `@` with a debounce to prevent too much wasted work + val mentionCompletionTrigger = suggestionSearchTrigger.debounce(0.3.seconds).filter { !it?.text.isNullOrEmpty() } + + val mentionTriggerFlow = merge(mentionStartTrigger, mentionCompletionTrigger) + + val roomAliasSuggestionsFlow = roomAliasSuggestionsDataSource + .getAllRoomAliasSuggestions() + .stateIn(this, SharingStarted.Lazily, emptyList()) + + combine(mentionTriggerFlow, room.membersStateFlow, roomAliasSuggestionsFlow) { suggestion, roomMembersState, roomAliasSuggestions -> + val result = suggestionsProcessor.process( + suggestion = suggestion, + roomMembersState = roomMembersState, + roomAliasSuggestions = roomAliasSuggestions, + currentUserId = currentUserId, + canSendRoomMention = ::canSendRoomMention, + ) + suggestions.clear() + suggestions.addAll(result) + } + .collect() + } + } + private fun CoroutineScope.sendMessage( markdownTextEditorState: MarkdownTextEditorState, richTextEditorState: RichTextEditorState, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt index 79f8221b46..8328624bf6 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt @@ -61,7 +61,6 @@ import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.A_USER_ID_2 import io.element.android.libraries.matrix.test.A_USER_ID_3 import io.element.android.libraries.matrix.test.A_USER_ID_4 -import io.element.android.libraries.matrix.test.core.aBuildMeta import io.element.android.libraries.matrix.test.permalink.FakePermalinkBuilder import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser import io.element.android.libraries.matrix.test.room.FakeMatrixRoom @@ -1011,16 +1010,10 @@ class MessageComposerPresenterTest { ) givenRoomInfo(aRoomInfo(isDirect = false)) } - val flagsService = FakeFeatureFlagService( - mapOf( - FeatureFlags.Mentions.key to true, - ) - ) - val presenter = createPresenter(this, room, featureFlagService = flagsService) + val presenter = createPresenter(this, room) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - skipItems(1) val initialState = awaitItem() // A null suggestion (no suggestion was received) returns nothing @@ -1078,16 +1071,10 @@ class MessageComposerPresenterTest { ) ) } - val flagsService = FakeFeatureFlagService( - mapOf( - FeatureFlags.Mentions.key to true, - ) - ) - val presenter = createPresenter(this, room, featureFlagService = flagsService) + val presenter = createPresenter(this, room) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - skipItems(1) val initialState = awaitItem() // An empty suggestion returns the joined members that are not the current user, but not the room @@ -1293,11 +1280,11 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - awaitFirstItem().also { state -> + skipItems(2) + awaitItem().also { state -> assertThat(state.textEditorState.messageMarkdown(permalinkBuilder)).isEqualTo(A_MESSAGE) assertThat(state.textEditorState.messageHtml()).isNull() } - assert(loadDraftLambda) .isCalledOnce() .with(value(A_ROOM_ID), value(false)) @@ -1327,7 +1314,8 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - awaitFirstItem().also { state -> + skipItems(1) + awaitItem().also { state -> assertThat(state.showTextFormatting).isTrue() assertThat(state.textEditorState.messageMarkdown(permalinkBuilder)).isEqualTo(A_MESSAGE) assertThat(state.textEditorState.messageHtml()).isEqualTo(A_MESSAGE) @@ -1360,7 +1348,8 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - awaitFirstItem().also { state -> + skipItems(2) + awaitItem().also { state -> assertThat(state.showTextFormatting).isFalse() assertThat(state.mode).isEqualTo(anEditMode()) assertThat(state.textEditorState.messageMarkdown(permalinkBuilder)).isEqualTo(A_MESSAGE) @@ -1406,7 +1395,8 @@ class MessageComposerPresenterTest { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - awaitFirstItem().also { state -> + skipItems(2) + awaitItem().also { state -> assertThat(state.showTextFormatting).isFalse() assertThat(state.mode).isEqualTo(aReplyMode()) assertThat(state.textEditorState.messageMarkdown(permalinkBuilder)).isEqualTo(A_MESSAGE) @@ -1580,8 +1570,7 @@ class MessageComposerPresenterTest { } private suspend fun ReceiveTurbine.awaitFirstItem(): T { - // Skip 2 item if Mentions feature is enabled, else 1 - skipItems(if (FeatureFlags.Mentions.defaultValue(aBuildMeta())) 2 else 1) + skipItems(1) return awaitItem() } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/suggestions/SuggestionsProcessorTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/suggestions/SuggestionsProcessorTest.kt index 20581ddab5..66b8aeb152 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/suggestions/SuggestionsProcessorTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/suggestions/SuggestionsProcessorTest.kt @@ -133,7 +133,7 @@ class SuggestionsProcessorTest { } @Test - fun `processing Room suggestion with aliases will return a suggestion`() = runTest { + fun `processing Room suggestion with aliases will return a suggestion when matching on alias`() = runTest { val aRoomSummary = aRoomSummary(canonicalAlias = A_ROOM_ALIAS) val result = suggestionsProcessor.process( suggestion = aRoomSuggestion("ali"), @@ -171,7 +171,56 @@ class SuggestionsProcessorTest { RoomAliasSuggestion( roomAlias = A_ROOM_ALIAS, roomId = aRoomSummary.roomId, - roomName = aRoomSummary.info.name, + roomName = "Element", + roomAvatarUrl = aRoomSummary.info.avatarUrl, + ) + ), + currentUserId = A_USER_ID, + canSendRoomMention = { true }, + ) + assertThat(result).isEmpty() + } + + @Test + fun `processing Room suggestion will return a suggestion when matching on room name`() = runTest { + val aRoomSummary = aRoomSummary(canonicalAlias = A_ROOM_ALIAS) + val result = suggestionsProcessor.process( + suggestion = aRoomSuggestion("lement"), + roomMembersState = MatrixRoomMembersState.Ready(persistentListOf()), + roomAliasSuggestions = listOf( + RoomAliasSuggestion( + roomAlias = A_ROOM_ALIAS, + roomId = aRoomSummary.roomId, + roomName = "Element", + roomAvatarUrl = aRoomSummary.info.avatarUrl, + ) + ), + currentUserId = A_USER_ID, + canSendRoomMention = { true }, + ) + assertThat(result).isEqualTo( + listOf( + ResolvedSuggestion.Alias( + roomAlias = A_ROOM_ALIAS, + roomId = aRoomSummary.roomId, + roomName = "Element", + roomAvatarUrl = aRoomSummary.info.avatarUrl, + ) + ) + ) + } + + @Test + fun `processing Room suggestion will not return a suggestion when room has no name`() = runTest { + val aRoomSummary = aRoomSummary(canonicalAlias = A_ROOM_ALIAS) + val result = suggestionsProcessor.process( + suggestion = aRoomSuggestion("lement"), + roomMembersState = MatrixRoomMembersState.Ready(persistentListOf()), + roomAliasSuggestions = listOf( + RoomAliasSuggestion( + roomAlias = A_ROOM_ALIAS, + roomId = aRoomSummary.roomId, + roomName = null, roomAvatarUrl = aRoomSummary.info.avatarUrl, ) ), diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt index a940e2a3a2..c2b4672878 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt @@ -80,7 +80,7 @@ class DeveloperSettingsPresenterTest { presenter.test { skipItems(2) awaitItem().also { state -> - val feature = state.features.first() + val feature = state.features.first { !it.isEnabled } state.eventSink(DeveloperSettingsEvents.UpdateEnabledFeature(feature, !feature.isEnabled)) } awaitItem().also { state -> diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt index f8e0d3405a..6e8fba8bd1 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt @@ -54,20 +54,6 @@ enum class FeatureFlags( defaultValue = { true }, isFinished = true, ), - Mentions( - key = "feature.mentions", - title = "Mentions", - description = "Type `@` to get mention suggestions and insert them", - defaultValue = { true }, - isFinished = false, - ), - RoomAliasSuggestions( - key = "feature.roomAliasSuggestions", - title = "Room alias suggestions", - description = "Type `#` to get room alias suggestions and insert them", - defaultValue = { false }, - isFinished = false, - ), MarkAsUnread( key = "feature.markAsUnread", title = "Mark as unread",