From b725d37b0e5830dca48f2241aea916ebda52fbd2 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:20:13 +0300 Subject: [PATCH] Do not display pinned events that the client cannot render The pinned events timeline is built with an exclusion filter rather than an allowlist, so an event of a type the app has no renderer for reaches both pinned surfaces. The banner falls through to the formatter catch-all and reads "Unsupported event", and the list renders an unknown content tile. Drop unknown content and message-like or state events that failed to parse from both surfaces, along with any day divider left without an event to introduce, and show the empty state when nothing renderable remains. The spec makes this the client's call. Such an event can still be unpinned from the room timeline, where it is still rendered. Undecryptable and redacted events are untouched: they carry information the user needs. --- .../impl/pinned/DisplayablePinnedEvents.kt | 39 ++++++ .../banner/PinnedMessagesBannerItemFactory.kt | 2 + .../list/PinnedMessagesListPresenter.kt | 5 +- .../PinnedMessagesBannerItemFactoryTest.kt | 74 +++++++++++ .../PinnedMessagesBannerPresenterTest.kt | 42 +++++++ .../list/PinnedMessagesListPresenterTest.kt | 116 +++++++++++++++--- 6 files changed, 262 insertions(+), 16 deletions(-) create mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/DisplayablePinnedEvents.kt create mode 100644 features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactoryTest.kt diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/DisplayablePinnedEvents.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/DisplayablePinnedEvents.kt new file mode 100644 index 0000000000..55c4b61ed1 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/DisplayablePinnedEvents.kt @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.messages.impl.pinned + +import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem +import io.element.android.libraries.matrix.api.timeline.item.event.EventContent +import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseMessageLikeContent +import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseStateContent +import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem + +internal fun List.keepDisplayablePinnedEvents(): List { + val items = filter { it.isDisplayableAsPinnedEvent() } + return items.filterIndexed { index, item -> + !item.isDayDivider() || items.getOrNull(index + 1) is MatrixTimelineItem.Event + } +} + +internal fun EventContent.isDisplayableAsPinnedEvent(): Boolean { + return when (this) { + UnknownContent, + is FailedToParseMessageLikeContent, + is FailedToParseStateContent -> false + else -> true + } +} + +private fun MatrixTimelineItem.isDisplayableAsPinnedEvent(): Boolean { + return this !is MatrixTimelineItem.Event || event.content.isDisplayableAsPinnedEvent() +} + +private fun MatrixTimelineItem.isDayDivider(): Boolean { + return this is MatrixTimelineItem.Virtual && virtual is VirtualTimelineItem.DayDivider +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactory.kt index d1d53b36b4..0420416bff 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactory.kt @@ -10,6 +10,7 @@ package io.element.android.features.messages.impl.pinned.banner import androidx.compose.ui.text.AnnotatedString import dev.zacsweers.metro.Inject +import io.element.android.features.messages.impl.pinned.isDisplayableAsPinnedEvent import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.eventformatter.api.PinnedMessagesBannerFormatter import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem @@ -24,6 +25,7 @@ class PinnedMessagesBannerItemFactory( when (timelineItem) { is MatrixTimelineItem.Event -> { val eventId = timelineItem.eventId ?: return@withContext null + if (!timelineItem.event.content.isDisplayableAsPinnedEvent()) return@withContext null val formatted = formatter.format(timelineItem.event) PinnedMessagesBannerItem( eventId = eventId, 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 6e8974b871..e1ee18e920 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 @@ -28,6 +28,7 @@ import io.element.android.features.messages.impl.actionlist.ActionListState import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction import io.element.android.features.messages.impl.link.LinkState import io.element.android.features.messages.impl.pinned.DefaultPinnedEventsTimelineProvider +import io.element.android.features.messages.impl.pinned.keepDisplayablePinnedEvents import io.element.android.features.messages.impl.timeline.TimelineRoomInfo import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig @@ -207,7 +208,7 @@ class PinnedMessagesListPresenter( val timelineItemsFlow = asyncTimeline.data.timelineItems combine(timelineItemsFlow, room.membersStateFlow) { items, membersState -> timelineItemsFactory.replaceWith( - timelineItems = items, + timelineItems = items.keepDisplayablePinnedEvents(), roomMembers = membersState.roomMembers().orEmpty(), renderReadReceipts = false, ) @@ -239,7 +240,7 @@ class PinnedMessagesListPresenter( AsyncData.Uninitialized, is AsyncData.Loading -> PinnedMessagesListState.Loading is AsyncData.Failure -> PinnedMessagesListState.Failed is AsyncData.Success -> { - if (timelineItems.data.isEmpty()) { + if (timelineItems.data.all { it is TimelineItem.Virtual }) { PinnedMessagesListState.Empty } else { val actionListState = actionListPresenter.present() diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactoryTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactoryTest.kt new file mode 100644 index 0000000000..114eed98d9 --- /dev/null +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerItemFactoryTest.kt @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.messages.impl.pinned.banner + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.eventformatter.test.FakePinnedMessagesBannerFormatter +import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem +import io.element.android.libraries.matrix.api.timeline.item.event.EventContent +import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseMessageLikeContent +import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseStateContent +import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem +import io.element.android.libraries.matrix.test.AN_EVENT_ID +import io.element.android.libraries.matrix.test.A_UNIQUE_ID +import io.element.android.libraries.matrix.test.timeline.aMessageContent +import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem +import io.element.android.tests.testutils.testCoroutineDispatchers +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class PinnedMessagesBannerItemFactoryTest { + @Test + fun `create - returns an item for an event which can be displayed`() = runTest { + val factory = createPinnedMessagesBannerItemFactory() + val item = factory.create(anEventTimelineItemOf(aMessageContent("A message"))) + assertThat(item?.eventId).isEqualTo(AN_EVENT_ID) + } + + @Test + fun `create - returns null for events which cannot be displayed`() = runTest { + val factory = createPinnedMessagesBannerItemFactory() + val contents = listOf( + UnknownContent, + FailedToParseMessageLikeContent(eventType = "m.room.message", error = "an error"), + FailedToParseStateContent(eventType = "m.room.topic", stateKey = "", error = "an error"), + ) + for (content in contents) { + assertThat(factory.create(anEventTimelineItemOf(content))).isNull() + } + } + + @Test + fun `create - returns null for virtual items`() = runTest { + val factory = createPinnedMessagesBannerItemFactory() + val item = factory.create( + MatrixTimelineItem.Virtual( + uniqueId = A_UNIQUE_ID, + virtual = VirtualTimelineItem.DayDivider(timestamp = 0L), + ) + ) + assertThat(item).isNull() + } + + private fun anEventTimelineItemOf(content: EventContent) = MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem( + eventId = AN_EVENT_ID, + content = content, + ), + ) + + private fun TestScope.createPinnedMessagesBannerItemFactory() = PinnedMessagesBannerItemFactory( + coroutineDispatchers = testCoroutineDispatchers(), + formatter = FakePinnedMessagesBannerFormatter( + formatLambda = { event -> "${event.content}" } + ), + ) +} diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt index 1f95baccc9..256837e5f7 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt @@ -14,6 +14,7 @@ import io.element.android.libraries.eventformatter.test.FakePinnedMessagesBanner import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.sync.SyncService import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem +import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EVENT_ID_2 import io.element.android.libraries.matrix.test.A_UNIQUE_ID @@ -24,6 +25,7 @@ import io.element.android.libraries.matrix.test.sync.FakeSyncService import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.matrix.test.timeline.aMessageContent import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem +import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.flow.flowOf @@ -90,6 +92,46 @@ class PinnedMessagesBannerPresenterTest { } } + @Test + fun `present - loaded state - events which cannot be displayed are not rendered`() = runTest { + val messageContent = aMessageContent("A message") + val pinnedEventsTimeline = FakeTimeline( + timelineItems = flowOf( + listOf( + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem( + eventId = AN_EVENT_ID, + content = messageContent, + ), + ), + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID_2, + event = anEventTimelineItem( + eventId = AN_EVENT_ID_2, + content = UnknownContent, + ), + ) + ) + ) + ) + val room = FakeJoinedRoom( + createTimelineResult = { Result.success(pinnedEventsTimeline) } + ).apply { + givenRoomInfo(aRoomInfo(pinnedEventIds = listOf(AN_EVENT_ID, AN_EVENT_ID_2))) + } + val presenter = createPinnedMessagesBannerPresenter(room = room) + presenter.test { + val loadedState = consumeItemsUntilPredicate { state -> + state is PinnedMessagesBannerState.Loaded + }.last() as PinnedMessagesBannerState.Loaded + assertThat(loadedState.loadedPinnedMessagesCount).isEqualTo(1) + assertThat(loadedState.currentPinnedMessageIndex).isEqualTo(0) + assertThat(loadedState.currentPinnedMessage.formatted.text).isEqualTo(messageContent.toString()) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - loaded state - multiple pinned messages`() = runTest { val messageContent1 = aMessageContent("A message") diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt index 8b2da2cefd..3efa707c3d 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListPresenterTest.kt @@ -21,13 +21,19 @@ import io.element.android.features.messages.test.timeline.FakeHtmlConverterProvi import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher 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.UniqueId import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.sync.SyncService import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo +import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseMessageLikeContent +import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem import io.element.android.libraries.matrix.test.AN_EVENT_ID +import io.element.android.libraries.matrix.test.AN_EVENT_ID_2 import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_UNIQUE_ID +import io.element.android.libraries.matrix.test.A_UNIQUE_ID_2 import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.matrix.test.room.aRoomInfo @@ -38,6 +44,7 @@ import io.element.android.libraries.matrix.test.timeline.aMessageContent import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem import io.element.android.services.analytics.api.AnalyticsService import io.element.android.services.analytics.test.FakeAnalyticsService +import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value @@ -131,6 +138,90 @@ class PinnedMessagesListPresenterTest { } } + @Test + fun `present - filled state - events which cannot be displayed are not rendered`() = runTest { + val pinnedEventsTimeline = createPinnedMessagesTimeline( + items = listOf( + MatrixTimelineItem.Virtual( + uniqueId = UniqueId("aDayDivider"), + virtual = VirtualTimelineItem.DayDivider(timestamp = 0L), + ), + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem( + eventId = AN_EVENT_ID, + content = aMessageContent("A message"), + ), + ), + MatrixTimelineItem.Virtual( + uniqueId = UniqueId("anotherDayDivider"), + virtual = VirtualTimelineItem.DayDivider(timestamp = 1L), + ), + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID_2, + event = anEventTimelineItem( + eventId = AN_EVENT_ID_2, + content = UnknownContent, + ), + ), + ), + ) + val room = FakeJoinedRoom( + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + ).apply { + givenRoomInfo(aRoomInfo(pinnedEventIds = listOf(AN_EVENT_ID, AN_EVENT_ID_2))) + }, + createTimelineResult = { Result.success(pinnedEventsTimeline) }, + ) + val presenter = createPinnedMessagesListPresenter(room = room) + presenter.test { + val filledState = consumeItemsUntilPredicate { state -> + state is PinnedMessagesListState.Filled + }.last() as PinnedMessagesListState.Filled + assertThat(filledState.loadedPinnedMessagesCount).isEqualTo(1) + assertThat(filledState.timelineItems).hasSize(2) + assertThat(filledState.timelineItems.filterIsInstance().map { it.eventId }).containsExactly(AN_EVENT_ID) + assertThat(filledState.timelineItems.filterIsInstance()).hasSize(1) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - empty state when no pinned event can be displayed`() = runTest { + val pinnedEventsTimeline = createPinnedMessagesTimeline( + items = listOf( + MatrixTimelineItem.Virtual( + uniqueId = A_UNIQUE_ID, + virtual = VirtualTimelineItem.DayDivider(timestamp = 0L), + ), + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID_2, + event = anEventTimelineItem( + eventId = AN_EVENT_ID, + content = FailedToParseMessageLikeContent(eventType = "m.room.message", error = "an error"), + ), + ), + ), + ) + val room = FakeJoinedRoom( + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + ).apply { + givenRoomInfo(aRoomInfo(pinnedEventIds = listOf(AN_EVENT_ID))) + }, + createTimelineResult = { Result.success(pinnedEventsTimeline) }, + ) + val presenter = createPinnedMessagesListPresenter(room = room) + presenter.test { + val emptyState = consumeItemsUntilPredicate { state -> + state is PinnedMessagesListState.Empty + }.last() + assertThat(emptyState).isEqualTo(PinnedMessagesListState.Empty) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - unpin event`() = runTest { val successUnpinEventLambda = lambdaRecorder { _: EventId? -> Result.success(true) } @@ -263,21 +354,18 @@ class PinnedMessagesListPresenterTest { } } - private fun createPinnedMessagesTimeline(): FakeTimeline { - val messageContent = aMessageContent("A message") - return FakeTimeline( - timelineItems = flowOf( - listOf( - MatrixTimelineItem.Event( - uniqueId = A_UNIQUE_ID, - event = anEventTimelineItem( - eventId = AN_EVENT_ID, - content = messageContent, - ), - ) - ) + private fun createPinnedMessagesTimeline( + items: List = listOf( + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem( + eventId = AN_EVENT_ID, + content = aMessageContent("A message"), + ), ) - ) + ), + ): FakeTimeline { + return FakeTimeline(timelineItems = flowOf(items)) } private fun roomPermissions(