mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-02 21:05:28 +00:00
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.
This commit is contained in:
+39
@@ -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
|
||||
}
|
||||
+2
@@ -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,
|
||||
|
||||
+3
-2
@@ -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()
|
||||
|
||||
+74
@@ -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}" }
|
||||
),
|
||||
)
|
||||
}
|
||||
+42
@@ -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")
|
||||
|
||||
+102
-14
@@ -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<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
|
||||
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<MatrixTimelineItem> = 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(
|
||||
|
||||
Reference in New Issue
Block a user