Merge pull request #7379 from hayaksi1/fix/7314-invalid-pinned-events

Do not display pinned events that the client cannot render
This commit is contained in:
Benoit Marty
2026-08-12 13:44:48 +02:00
committed by GitHub
6 changed files with 262 additions and 16 deletions
@@ -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<MatrixTimelineItem>.keepDisplayablePinnedEvents(): List<MatrixTimelineItem> {
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
}
@@ -10,6 +10,7 @@ package io.element.android.features.messages.impl.pinned.banner
import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.AnnotatedString
import dev.zacsweers.metro.Inject 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.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.eventformatter.api.PinnedMessagesBannerFormatter import io.element.android.libraries.eventformatter.api.PinnedMessagesBannerFormatter
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
@@ -24,6 +25,7 @@ class PinnedMessagesBannerItemFactory(
when (timelineItem) { when (timelineItem) {
is MatrixTimelineItem.Event -> { is MatrixTimelineItem.Event -> {
val eventId = timelineItem.eventId ?: return@withContext null val eventId = timelineItem.eventId ?: return@withContext null
if (!timelineItem.event.content.isDisplayableAsPinnedEvent()) return@withContext null
val formatted = formatter.format(timelineItem.event) val formatted = formatter.format(timelineItem.event)
PinnedMessagesBannerItem( PinnedMessagesBannerItem(
eventId = eventId, eventId = eventId,
@@ -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.actionlist.model.TimelineItemAction
import io.element.android.features.messages.impl.link.LinkState 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.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.TimelineRoomInfo
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig
@@ -207,7 +208,7 @@ class PinnedMessagesListPresenter(
val timelineItemsFlow = asyncTimeline.data.timelineItems val timelineItemsFlow = asyncTimeline.data.timelineItems
combine(timelineItemsFlow, room.membersStateFlow) { items, membersState -> combine(timelineItemsFlow, room.membersStateFlow) { items, membersState ->
timelineItemsFactory.replaceWith( timelineItemsFactory.replaceWith(
timelineItems = items, timelineItems = items.keepDisplayablePinnedEvents(),
roomMembers = membersState.roomMembers().orEmpty(), roomMembers = membersState.roomMembers().orEmpty(),
renderReadReceipts = false, renderReadReceipts = false,
) )
@@ -239,7 +240,7 @@ class PinnedMessagesListPresenter(
AsyncData.Uninitialized, is AsyncData.Loading -> PinnedMessagesListState.Loading AsyncData.Uninitialized, is AsyncData.Loading -> PinnedMessagesListState.Loading
is AsyncData.Failure -> PinnedMessagesListState.Failed is AsyncData.Failure -> PinnedMessagesListState.Failed
is AsyncData.Success -> { is AsyncData.Success -> {
if (timelineItems.data.isEmpty()) { if (timelineItems.data.all { it is TimelineItem.Virtual }) {
PinnedMessagesListState.Empty PinnedMessagesListState.Empty
} else { } else {
val actionListState = actionListPresenter.present() val actionListState = actionListPresenter.present()
@@ -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}" }
),
)
}
@@ -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.room.JoinedRoom
import io.element.android.libraries.matrix.api.sync.SyncService 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.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
import io.element.android.libraries.matrix.test.AN_EVENT_ID_2 import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
import io.element.android.libraries.matrix.test.A_UNIQUE_ID 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.FakeTimeline
import io.element.android.libraries.matrix.test.timeline.aMessageContent import io.element.android.libraries.matrix.test.timeline.aMessageContent
import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem 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.test
import io.element.android.tests.testutils.testCoroutineDispatchers import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.flow.flowOf 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 @Test
fun `present - loaded state - multiple pinned messages`() = runTest { fun `present - loaded state - multiple pinned messages`() = runTest {
val messageContent1 = aMessageContent("A message") val messageContent1 = aMessageContent("A message")
@@ -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.designsystem.utils.snackbar.SnackbarDispatcher
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService 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.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.room.JoinedRoom
import io.element.android.libraries.matrix.api.sync.SyncService 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.MatrixTimelineItem
import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo 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
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.AN_EXCEPTION
import io.element.android.libraries.matrix.test.A_UNIQUE_ID 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.FakeBaseRoom
import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom
import io.element.android.libraries.matrix.test.room.aRoomInfo 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.libraries.matrix.test.timeline.anEventTimelineItem
import io.element.android.services.analytics.api.AnalyticsService import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.analytics.test.FakeAnalyticsService 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.assert
import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value 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<TimelineItem.Event>().map { it.eventId }).containsExactly(AN_EVENT_ID)
assertThat(filledState.timelineItems.filterIsInstance<TimelineItem.Virtual>()).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 @Test
fun `present - unpin event`() = runTest { fun `present - unpin event`() = runTest {
val successUnpinEventLambda = lambdaRecorder { _: EventId? -> Result.success(true) } val successUnpinEventLambda = lambdaRecorder { _: EventId? -> Result.success(true) }
@@ -263,21 +354,18 @@ class PinnedMessagesListPresenterTest {
} }
} }
private fun createPinnedMessagesTimeline(): FakeTimeline { private fun createPinnedMessagesTimeline(
val messageContent = aMessageContent("A message") items: List<MatrixTimelineItem> = listOf(
return FakeTimeline( MatrixTimelineItem.Event(
timelineItems = flowOf( uniqueId = A_UNIQUE_ID,
listOf( event = anEventTimelineItem(
MatrixTimelineItem.Event( eventId = AN_EVENT_ID,
uniqueId = A_UNIQUE_ID, content = aMessageContent("A message"),
event = anEventTimelineItem( ),
eventId = AN_EVENT_ID,
content = messageContent,
),
)
)
) )
) ),
): FakeTimeline {
return FakeTimeline(timelineItems = flowOf(items))
} }
private fun roomPermissions( private fun roomPermissions(