From 28478dcc62259bbdf016f4b208f2e6d35af06937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 21 Aug 2025 16:13:27 +0200 Subject: [PATCH 1/4] Allow replying to any remote message in a thread. This will open the thread screen based on the selected event: - If it was already part of a thread, it will open that thread. - Otherwise, it'll open the thread timeline screen so you can start a thread from the event. --- .../messages/impl/MessagesPresenter.kt | 9 +++- .../impl/actionlist/ActionListPresenter.kt | 6 +-- .../messages/impl/MessagesPresenterTest.kt | 42 +++++++++++++++++++ .../actionlist/ActionListPresenterTest.kt | 17 ++++++++ 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index 380a868aec..0976072ed8 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -63,6 +63,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage import io.element.android.libraries.designsystem.utils.snackbar.collectSnackbarMessageAsState +import io.element.android.libraries.matrix.api.core.toThreadId import io.element.android.libraries.matrix.api.encryption.EncryptionService import io.element.android.libraries.matrix.api.encryption.identity.IdentityState import io.element.android.libraries.matrix.api.permalink.PermalinkParser @@ -318,8 +319,12 @@ class MessagesPresenter @AssistedInject constructor( TimelineItemAction.AddCaption -> handleActionAddCaption(targetEvent, composerState) TimelineItemAction.EditCaption -> handleActionEditCaption(targetEvent, composerState) TimelineItemAction.RemoveCaption -> handleRemoveCaption(targetEvent) - TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread -> handleActionReply(targetEvent, composerState, timelineProtectionState) + TimelineItemAction.Reply -> handleActionReply(targetEvent, composerState, timelineProtectionState) + TimelineItemAction.ReplyInThread -> { + // Get either the thread id this event is in, or the event id if it's not in a thread so we can start one + val threadId = targetEvent.threadInfo.threadRootId ?: targetEvent.eventId!!.toThreadId() + navigator.onOpenThread(threadId, null) + } TimelineItemAction.ViewSource -> handleShowDebugInfoAction(targetEvent) TimelineItemAction.Forward -> handleForwardAction(targetEvent) TimelineItemAction.ReportContent -> handleReportAction(targetEvent) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt index fb2cd915bc..9825b04643 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt @@ -159,11 +159,11 @@ class DefaultActionListPresenter @AssistedInject constructor( val canRedact = timelineItem.isMine && usersEventPermissions.canRedactOwn || !timelineItem.isMine && usersEventPermissions.canRedactOther return buildSet { if (timelineItem.canBeRepliedTo && usersEventPermissions.canSendMessage) { - if (timelineMode !is Timeline.Mode.Thread && timelineItem.threadInfo.threadRootId != null) { + if (timelineMode !is Timeline.Mode.Thread && timelineItem.isRemote) { add(TimelineItemAction.ReplyInThread) - } else { - add(TimelineItemAction.Reply) } + + add(TimelineItemAction.Reply) } if (timelineItem.isRemote && timelineItem.content.canBeForwarded()) { add(TimelineItemAction.Forward) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt index b16398635d..e8624d9cf6 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt @@ -48,7 +48,9 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.core.ThreadId import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.api.core.toThreadId import io.element.android.libraries.matrix.api.encryption.identity.IdentityState import io.element.android.libraries.matrix.api.media.MediaSource import io.element.android.libraries.matrix.api.permalink.PermalinkParser @@ -57,6 +59,7 @@ import io.element.android.libraries.matrix.api.room.RoomMembersState import io.element.android.libraries.matrix.api.room.RoomMembershipState import io.element.android.libraries.matrix.api.room.tombstone.SuccessorRoom import io.element.android.libraries.matrix.api.timeline.Timeline +import io.element.android.libraries.matrix.api.timeline.item.EventThreadInfo import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo import io.element.android.libraries.matrix.api.timeline.item.event.EventOrTransactionId import io.element.android.libraries.matrix.api.timeline.item.event.toEventOrTransactionId @@ -67,6 +70,7 @@ import io.element.android.libraries.matrix.test.A_CAPTION import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_SESSION_ID_2 +import io.element.android.libraries.matrix.test.A_THREAD_ID 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.core.aBuildMeta @@ -1158,6 +1162,44 @@ class MessagesPresenterTest { } } + @Test + fun `present - handle action reply in thread for an event in a thread`() = runTest { + val openThreadLambda = lambdaRecorder { _: ThreadId, _: EventId? -> } + val presenter = createMessagesPresenter( + navigator = FakeMessagesNavigator(onOpenThreadLambda = openThreadLambda), + ) + presenter.testWithLifecycleOwner { + val initialState = awaitItem() + initialState.eventSink(MessagesEvents.HandleAction( + action = TimelineItemAction.ReplyInThread, + event = aMessageEvent(threadInfo = EventThreadInfo(A_THREAD_ID, null)) + )) + awaitItem() + openThreadLambda.assertions().isCalledOnce().with(value(A_THREAD_ID), value(null)) + } + } + + @Test + fun `present - handle action reply in thread to start a new thread`() = runTest { + val openThreadLambda = lambdaRecorder { _: ThreadId, _: EventId? -> } + val presenter = createMessagesPresenter( + navigator = FakeMessagesNavigator(onOpenThreadLambda = openThreadLambda), + ) + presenter.testWithLifecycleOwner { + val initialState = awaitItem() + initialState.eventSink(MessagesEvents.HandleAction( + action = TimelineItemAction.ReplyInThread, + event = aMessageEvent( + // The event id will be used as the thread id instead + eventId = AN_EVENT_ID, + threadInfo = EventThreadInfo(null, null), + ) + )) + awaitItem() + openThreadLambda.assertions().isCalledOnce().with(value(AN_EVENT_ID.toThreadId()), value(null)) + } + } + private fun TestScope.createMessagesPresenter( coroutineDispatchers: CoroutineDispatchers = testCoroutineDispatchers(), joinedRoom: FakeJoinedRoom = FakeJoinedRoom( diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt index d2d187188c..31c6b92f8b 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt @@ -173,6 +173,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -218,6 +219,7 @@ class ActionListPresenterTest { displayEmojiReactions = true, verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( + TimelineItemAction.Reply, TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, @@ -312,6 +314,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -360,6 +363,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -407,6 +411,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -452,6 +457,7 @@ class ActionListPresenterTest { displayEmojiReactions = true, verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( + TimelineItemAction.Reply, TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, @@ -500,6 +506,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -590,6 +597,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.AddCaption, TimelineItemAction.CopyLink, @@ -639,6 +647,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.EditCaption, TimelineItemAction.CopyLink, @@ -690,6 +699,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -803,6 +813,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -849,6 +860,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -901,6 +913,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -1041,6 +1054,7 @@ class ActionListPresenterTest { actions = persistentListOf( TimelineItemAction.EndPoll, TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.EditPoll, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -1085,6 +1099,7 @@ class ActionListPresenterTest { actions = persistentListOf( TimelineItemAction.EndPoll, TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.CopyLink, TimelineItemAction.Pin, TimelineItemAction.Redact, @@ -1127,6 +1142,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.CopyLink, TimelineItemAction.Pin, TimelineItemAction.Redact, @@ -1171,6 +1187,7 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, From 2fa402f25da3de8afbf62f04ee62911eeddd005d Mon Sep 17 00:00:00 2001 From: ElementBot Date: Thu, 21 Aug 2025 14:25:35 +0000 Subject: [PATCH 2/4] Update screenshots --- ...s.matrix.ui.components_EditableOrgAvatarRtl_Night_0_en.png | 4 ++-- ...ries.matrix.ui.components_EditableOrgAvatar_Night_0_en.png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatarRtl_Night_0_en.png b/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatarRtl_Night_0_en.png index 437a7026aa..8b36a5e259 100644 --- a/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatarRtl_Night_0_en.png +++ b/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatarRtl_Night_0_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:6e9f6f4fe633ca50c2a27cc2e2a0157880f698f91cf759a35fa3f4d49b805905 -size 31332 +oid sha256:8da76de0773fc62f7eb73dd16756d6ccad8229db121eb39d43f7c27d4ab8b48b +size 31339 diff --git a/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatar_Night_0_en.png b/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatar_Night_0_en.png index 1f367b38b8..5b5950e4c3 100644 --- a/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatar_Night_0_en.png +++ b/tests/uitests/src/test/snapshots/images/libraries.matrix.ui.components_EditableOrgAvatar_Night_0_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3f837b094f91617dba15cbaab3336aa501b0a834299a07fc6673c9b6b89b8905 -size 30702 +oid sha256:6b5b92e446d5968fb115e49b2b2337a46a9a7f43577413924e7a1b1f3b752646 +size 30692 From 19141990785154864f6091516ffd7dcdfe3fa28a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 21 Aug 2025 16:33:45 +0200 Subject: [PATCH 3/4] Add the feature flag to decide which action to perform. Also, rename the feature flag to something easier to understand. --- .../messages/impl/MessagesPresenter.kt | 14 ++++++-- .../list/PinnedMessagesListPresenter.kt | 2 +- .../impl/timeline/TimelinePresenter.kt | 2 +- .../messages/impl/MessagesPresenterTest.kt | 34 +++++++++++++++++++ .../libraries/featureflag/api/FeatureFlags.kt | 2 +- .../matrix/impl/RustMatrixClientFactory.kt | 2 +- .../matrix/impl/room/JoinedRustRoom.kt | 2 +- .../matrix/impl/room/RustRoomFactory.kt | 2 +- 8 files changed, 51 insertions(+), 9 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index 0976072ed8..40a8de1276 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -63,6 +63,8 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage import io.element.android.libraries.designsystem.utils.snackbar.collectSnackbarMessageAsState +import io.element.android.libraries.featureflag.api.FeatureFlagService +import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.core.toThreadId import io.element.android.libraries.matrix.api.encryption.EncryptionService import io.element.android.libraries.matrix.api.encryption.identity.IdentityState @@ -116,6 +118,7 @@ class MessagesPresenter @AssistedInject constructor( private val permalinkParser: PermalinkParser, private val analyticsService: AnalyticsService, private val encryptionService: EncryptionService, + private val featureFlagService: FeatureFlagService, ) : Presenter { @AssistedFactory interface Factory { @@ -321,9 +324,14 @@ class MessagesPresenter @AssistedInject constructor( TimelineItemAction.RemoveCaption -> handleRemoveCaption(targetEvent) TimelineItemAction.Reply -> handleActionReply(targetEvent, composerState, timelineProtectionState) TimelineItemAction.ReplyInThread -> { - // Get either the thread id this event is in, or the event id if it's not in a thread so we can start one - val threadId = targetEvent.threadInfo.threadRootId ?: targetEvent.eventId!!.toThreadId() - navigator.onOpenThread(threadId, null) + val displayThreads = featureFlagService.isFeatureEnabled(FeatureFlags.Threads) + if (displayThreads) { + // Get either the thread id this event is in, or the event id if it's not in a thread so we can start one + val threadId = targetEvent.threadInfo.threadRootId ?: targetEvent.eventId!!.toThreadId() + navigator.onOpenThread(threadId, null) + } else { + handleActionReply(targetEvent, composerState, timelineProtectionState) + } } TimelineItemAction.ViewSource -> handleShowDebugInfoAction(targetEvent) TimelineItemAction.Forward -> handleForwardAction(targetEvent) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt index 68d2e26109..523c371747 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenter.kt @@ -118,7 +118,7 @@ class PinnedMessagesListPresenter @AssistedInject constructor( val syncUpdateFlow = room.syncUpdateFlow.collectAsState() val userEventPermissions by userEventPermissions(syncUpdateFlow.value) - val displayThreadSummaries by featureFlagService.isFeatureEnabledFlow(FeatureFlags.HideThreadedEvents).collectAsState(false) + val displayThreadSummaries by featureFlagService.isFeatureEnabledFlow(FeatureFlags.Threads).collectAsState(false) var pinnedMessageItems by remember { mutableStateOf>>(AsyncData.Uninitialized) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt index 32556eae7d..ef740e3a8b 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt @@ -136,7 +136,7 @@ class TimelinePresenter @AssistedInject constructor( }.collectAsState(initial = true) val displayThreadSummaries by produceState(false) { - value = featureFlagService.isFeatureEnabled(FeatureFlags.HideThreadedEvents) + value = featureFlagService.isFeatureEnabled(FeatureFlags.Threads) } fun handleEvents(event: TimelineEvents) { diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt index e8624d9cf6..6a1abd4b3f 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt @@ -46,6 +46,8 @@ import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.designsystem.components.avatar.AvatarData import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher +import io.element.android.libraries.featureflag.api.FeatureFlags +import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.ThreadId @@ -1167,6 +1169,9 @@ class MessagesPresenterTest { val openThreadLambda = lambdaRecorder { _: ThreadId, _: EventId? -> } val presenter = createMessagesPresenter( navigator = FakeMessagesNavigator(onOpenThreadLambda = openThreadLambda), + featureFlagService = FakeFeatureFlagService( + initialState = mapOf(FeatureFlags.Threads.key to true) + ), ) presenter.testWithLifecycleOwner { val initialState = awaitItem() @@ -1184,6 +1189,9 @@ class MessagesPresenterTest { val openThreadLambda = lambdaRecorder { _: ThreadId, _: EventId? -> } val presenter = createMessagesPresenter( navigator = FakeMessagesNavigator(onOpenThreadLambda = openThreadLambda), + featureFlagService = FakeFeatureFlagService( + initialState = mapOf(FeatureFlags.Threads.key to true) + ), ) presenter.testWithLifecycleOwner { val initialState = awaitItem() @@ -1200,6 +1208,30 @@ class MessagesPresenterTest { } } + @Test + fun `present - handle action reply in a thread with threads disabled`() = runTest { + val composerRecorder = EventsRecorder() + val presenter = createMessagesPresenter( + featureFlagService = FakeFeatureFlagService( + initialState = mapOf(FeatureFlags.Threads.key to false) + ), + messageComposerPresenter = { aMessageComposerState(eventSink = composerRecorder) }, + ) + presenter.testWithLifecycleOwner { + val initialState = awaitItem() + initialState.eventSink(MessagesEvents.HandleAction(TimelineItemAction.ReplyInThread, aMessageEvent())) + awaitItem() + composerRecorder.assertSingle( + MessageComposerEvents.SetMode( + composerMode = MessageComposerMode.Reply( + replyToDetails = InReplyToDetails.Loading(AN_EVENT_ID), + hideImage = false, + ) + ) + ) + } + } + private fun TestScope.createMessagesPresenter( coroutineDispatchers: CoroutineDispatchers = testCoroutineDispatchers(), joinedRoom: FakeJoinedRoom = FakeJoinedRoom( @@ -1231,6 +1263,7 @@ class MessagesPresenterTest { aRoomMemberModerationState() }, encryptionService: FakeEncryptionService = FakeEncryptionService(), + featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(), actionListEventSink: (ActionListEvents) -> Unit = {}, ): MessagesPresenter { return MessagesPresenter( @@ -1259,6 +1292,7 @@ class MessagesPresenterTest { permalinkParser = permalinkParser, encryptionService = encryptionService, analyticsService = analyticsService, + featureFlagService = featureFlagService, ) } } 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 7c87a6e034..f420e6179f 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 @@ -93,7 +93,7 @@ enum class FeatureFlags( // False so it's displayed in the developer options screen isFinished = false, ), - HideThreadedEvents( + Threads( key = "feature.thread_timeline", title = "Threads", description = "Renders thread messages as a dedicated timeline. Restarting the app is required for this setting to fully take effect.", diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt index 4ad26800cd..08ef1a9f4b 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt @@ -132,7 +132,7 @@ class RustMatrixClientFactory @Inject constructor( ) ) .enableShareHistoryOnInvite(featureFlagService.isFeatureEnabled(FeatureFlags.EnableKeyShareOnInvite)) - .threadsEnabled(featureFlagService.isFeatureEnabled(FeatureFlags.HideThreadedEvents), threadSubscriptions = false) + .threadsEnabled(featureFlagService.isFeatureEnabled(FeatureFlags.Threads), threadSubscriptions = false) .run { // Apply sliding sync version settings when (slidingSyncType) { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt index c476c34069..baa9f85906 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/JoinedRustRoom.kt @@ -156,7 +156,7 @@ class JoinedRustRoom( override suspend fun createTimeline( createTimelineParams: CreateTimelineParams, ): Result = withContext(roomDispatcher) { - val hideThreadedEvents = featureFlagService.isFeatureEnabled(FeatureFlags.HideThreadedEvents) + val hideThreadedEvents = featureFlagService.isFeatureEnabled(FeatureFlags.Threads) val focus = when (createTimelineParams) { is CreateTimelineParams.PinnedOnly -> TimelineFocus.PinnedEvents( maxEventsToLoad = 100u, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt index db681416de..ee9e7bfe52 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomFactory.kt @@ -108,7 +108,7 @@ class RustRoomFactory( val sdkRoom = awaitRoomInRoomList(roomId) ?: return@withContext null if (sdkRoom.membership() == Membership.JOINED) { - val hideThreadedEvents = featureFlagService.isFeatureEnabled(FeatureFlags.HideThreadedEvents) + val hideThreadedEvents = featureFlagService.isFeatureEnabled(FeatureFlags.Threads) // Init the live timeline in the SDK from the Room val timeline = sdkRoom.timelineWithConfiguration( TimelineConfiguration( From dc38254f379470bd02ac4fd034f5206bbca2c434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 21 Aug 2025 16:57:27 +0200 Subject: [PATCH 4/4] Display the reply in thread action based on the feature flag too --- .../impl/actionlist/ActionListPresenter.kt | 25 ++- .../actionlist/ActionListPresenterTest.kt | 188 ++++++++++++++++-- 2 files changed, 190 insertions(+), 23 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt index 9825b04643..ca7a6fa4a3 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt @@ -39,6 +39,8 @@ import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.dateformatter.api.DateFormatter import io.element.android.libraries.dateformatter.api.DateFormatterMode import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.featureflag.api.FeatureFlagService +import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.room.BaseRoom import io.element.android.libraries.matrix.api.timeline.Timeline @@ -68,6 +70,7 @@ class DefaultActionListPresenter @AssistedInject constructor( private val room: BaseRoom, private val userSendFailureFactory: VerifiedUserSendFailureFactory, private val dateFormatter: DateFormatter, + private val featureFlagService: FeatureFlagService, ) : ActionListPresenter { @AssistedFactory @ContributesBinding(RoomScope::class) @@ -95,6 +98,8 @@ class DefaultActionListPresenter @AssistedInject constructor( room.roomInfoFlow.map { it.pinnedEventIds } }.collectAsState(initial = persistentListOf()) + val isThreadsEnabled = featureFlagService.isFeatureEnabledFlow(FeatureFlags.Threads).collectAsState(false) + fun handleEvents(event: ActionListEvents) { when (event) { ActionListEvents.Clear -> target.value = ActionListState.Target.None @@ -104,6 +109,7 @@ class DefaultActionListPresenter @AssistedInject constructor( isDeveloperModeEnabled = isDeveloperModeEnabled, pinnedEventIds = pinnedEventIds, target = target, + isThreadsEnabled = isThreadsEnabled.value, ) } } @@ -119,7 +125,8 @@ class DefaultActionListPresenter @AssistedInject constructor( usersEventPermissions: UserEventPermissions, isDeveloperModeEnabled: Boolean, pinnedEventIds: ImmutableList, - target: MutableState + target: MutableState, + isThreadsEnabled: Boolean, ) = launch { target.value = ActionListState.Target.Loading(timelineItem) @@ -128,6 +135,7 @@ class DefaultActionListPresenter @AssistedInject constructor( usersEventPermissions = usersEventPermissions, isDeveloperModeEnabled = isDeveloperModeEnabled, isEventPinned = pinnedEventIds.contains(timelineItem.eventId), + isThreadsEnabled = isThreadsEnabled, ) val verifiedUserSendFailure = userSendFailureFactory.create(timelineItem.localSendState) @@ -155,15 +163,24 @@ class DefaultActionListPresenter @AssistedInject constructor( usersEventPermissions: UserEventPermissions, isDeveloperModeEnabled: Boolean, isEventPinned: Boolean, + isThreadsEnabled: Boolean, ): List { val canRedact = timelineItem.isMine && usersEventPermissions.canRedactOwn || !timelineItem.isMine && usersEventPermissions.canRedactOther return buildSet { if (timelineItem.canBeRepliedTo && usersEventPermissions.canSendMessage) { - if (timelineMode !is Timeline.Mode.Thread && timelineItem.isRemote) { + if (isThreadsEnabled && timelineMode !is Timeline.Mode.Thread && timelineItem.isRemote) { + // If threads are enabled, we can reply in thread if the item is remote add(TimelineItemAction.ReplyInThread) + add(TimelineItemAction.Reply) + } else { + if (!isThreadsEnabled && timelineItem.threadInfo.threadRootId != null) { + // If threads are not enabled, we can reply in a thread if the item is already in the thread + add(TimelineItemAction.ReplyInThread) + } else { + // Otherwise, we can only reply in the room + add(TimelineItemAction.Reply) + } } - - add(TimelineItemAction.Reply) } if (timelineItem.isRemote && timelineItem.content.canBeForwarded()) { add(TimelineItemAction.Forward) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt index 31c6b92f8b..964143ac78 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt @@ -27,6 +27,8 @@ import io.element.android.features.messages.impl.timeline.model.event.aTimelineI import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemVoiceContent import io.element.android.features.poll.api.pollcontent.aPollAnswerItemList import io.element.android.libraries.dateformatter.test.FakeDateFormatter +import io.element.android.libraries.featureflag.api.FeatureFlags +import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.room.BaseRoom import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.timeline.item.EventThreadInfo @@ -35,6 +37,7 @@ import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.A_CAPTION import io.element.android.libraries.matrix.test.A_MESSAGE import io.element.android.libraries.matrix.test.A_THREAD_ID +import io.element.android.libraries.matrix.test.A_TRANSACTION_ID import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.aRoomInfo @@ -173,7 +176,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -219,7 +221,6 @@ class ActionListPresenterTest { displayEmojiReactions = true, verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( - TimelineItemAction.Reply, TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, @@ -314,7 +315,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -363,7 +363,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -411,7 +410,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -457,7 +455,6 @@ class ActionListPresenterTest { displayEmojiReactions = true, verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( - TimelineItemAction.Reply, TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, @@ -506,7 +503,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -597,7 +593,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.AddCaption, TimelineItemAction.CopyLink, @@ -647,7 +642,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.EditCaption, TimelineItemAction.CopyLink, @@ -699,7 +693,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -813,7 +806,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -860,7 +852,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -913,7 +904,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.Edit, TimelineItemAction.CopyLink, @@ -1054,7 +1044,6 @@ class ActionListPresenterTest { actions = persistentListOf( TimelineItemAction.EndPoll, TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.EditPoll, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -1099,7 +1088,6 @@ class ActionListPresenterTest { actions = persistentListOf( TimelineItemAction.EndPoll, TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.CopyLink, TimelineItemAction.Pin, TimelineItemAction.Redact, @@ -1142,7 +1130,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.CopyLink, TimelineItemAction.Pin, TimelineItemAction.Redact, @@ -1187,7 +1174,6 @@ class ActionListPresenterTest { verifiedUserSendFailure = VerifiedUserSendFailure.None, actions = persistentListOf( TimelineItemAction.Reply, - TimelineItemAction.ReplyInThread, TimelineItemAction.Forward, TimelineItemAction.CopyLink, TimelineItemAction.Pin, @@ -1262,8 +1248,12 @@ class ActionListPresenterTest { } @Test - fun `present - compute for threaded timeline`() = runTest { - val presenter = createActionListPresenter(isDeveloperModeEnabled = false, timelineMode = Timeline.Mode.Thread(A_THREAD_ID)) + fun `present - compute for threaded timeline with threads enabled`() = runTest { + val presenter = createActionListPresenter( + isDeveloperModeEnabled = false, + timelineMode = Timeline.Mode.Thread(A_THREAD_ID), + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.Threads.key to true)), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -1307,12 +1297,171 @@ class ActionListPresenterTest { ) } } + + @Test + fun `present - compute for remote timeline item with threads enabled`() = runTest { + val presenter = createActionListPresenter( + isDeveloperModeEnabled = false, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.Threads.key to true)), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + val messageEvent = aMessageEvent( + eventId = AN_EVENT_ID, + isMine = true, + isEditable = false, + content = aTimelineItemVoiceContent( + caption = null, + ), + ) + + assertThat(messageEvent.isRemote).isTrue() + + initialState.eventSink.invoke( + ActionListEvents.ComputeForMessage( + event = messageEvent, + userEventPermissions = aUserEventPermissions( + canRedactOwn = true, + canRedactOther = false, + canSendMessage = true, + canSendReaction = true, + canPinUnpin = true + ) + ) + ) + val successState = awaitItem() + assertThat(successState.target).isEqualTo( + ActionListState.Target.Success( + event = messageEvent, + sentTimeFull = "0 Full true", + displayEmojiReactions = true, + verifiedUserSendFailure = VerifiedUserSendFailure.None, + actions = persistentListOf( + TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, + TimelineItemAction.Forward, + TimelineItemAction.CopyLink, + TimelineItemAction.Pin, + TimelineItemAction.Redact, + ) + ) + ) + } + } + + @Test + fun `present - compute for remote timeline item already in thread with threads enabled`() = runTest { + val presenter = createActionListPresenter( + isDeveloperModeEnabled = false, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.Threads.key to true)), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + val messageEvent = aMessageEvent( + eventId = AN_EVENT_ID, + isMine = true, + isEditable = false, + content = aTimelineItemVoiceContent( + caption = null, + ), + threadInfo = EventThreadInfo(A_THREAD_ID, null), + ) + + assertThat(messageEvent.isRemote).isTrue() + + initialState.eventSink.invoke( + ActionListEvents.ComputeForMessage( + event = messageEvent, + userEventPermissions = aUserEventPermissions( + canRedactOwn = true, + canRedactOther = false, + canSendMessage = true, + canSendReaction = true, + canPinUnpin = true + ) + ) + ) + val successState = awaitItem() + assertThat(successState.target).isEqualTo( + ActionListState.Target.Success( + event = messageEvent, + sentTimeFull = "0 Full true", + displayEmojiReactions = true, + verifiedUserSendFailure = VerifiedUserSendFailure.None, + actions = persistentListOf( + TimelineItemAction.Reply, + TimelineItemAction.ReplyInThread, + TimelineItemAction.Forward, + TimelineItemAction.CopyLink, + TimelineItemAction.Pin, + TimelineItemAction.Redact, + ) + ) + ) + } + } + + @Test + fun `present - compute for local timeline item with threads enabled`() = runTest { + val presenter = createActionListPresenter( + isDeveloperModeEnabled = false, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.Threads.key to true)), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + val messageEvent = aMessageEvent( + eventId = null, + transactionId = A_TRANSACTION_ID, + isMine = true, + isEditable = false, + content = aTimelineItemVoiceContent( + caption = null, + ), + ) + + assertThat(messageEvent.isRemote).isFalse() + + initialState.eventSink.invoke( + ActionListEvents.ComputeForMessage( + event = messageEvent, + userEventPermissions = aUserEventPermissions( + canRedactOwn = true, + canRedactOther = false, + canSendMessage = true, + canSendReaction = true, + canPinUnpin = true + ) + ) + ) + val successState = awaitItem() + assertThat(successState.target).isEqualTo( + ActionListState.Target.Success( + event = messageEvent, + sentTimeFull = "0 Full true", + displayEmojiReactions = true, + verifiedUserSendFailure = VerifiedUserSendFailure.None, + actions = persistentListOf( + // Can't reply in thread for local events + TimelineItemAction.Reply, + TimelineItemAction.Redact, + ) + ) + ) + } + } } private fun createActionListPresenter( isDeveloperModeEnabled: Boolean, room: BaseRoom = FakeBaseRoom(), timelineMode: Timeline.Mode = Timeline.Mode.Live, + featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(), ): ActionListPresenter { val preferencesStore = InMemoryAppPreferencesStore(isDeveloperModeEnabled = isDeveloperModeEnabled) return DefaultActionListPresenter( @@ -1322,5 +1471,6 @@ private fun createActionListPresenter( userSendFailureFactory = VerifiedUserSendFailureFactory(room), dateFormatter = FakeDateFormatter(), timelineMode = timelineMode, + featureFlagService = featureFlagService, ) }