Skip to content

Commit f427c0e

Browse files
ganfraganfra
authored
Merge pull request #3239 from element-hq/feature/fga/use_room_edit
Edit : fallback to room.edit when timeline item is not found.
2 parents 32052a0 + a23e5e2 commit f427c0e

File tree

8 files changed

+148
-47
lines changed

8 files changed

+148
-47
lines changed

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import io.element.android.libraries.matrix.api.room.Mention
6060
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
6161
import io.element.android.libraries.matrix.api.room.draft.ComposerDraftType
6262
import io.element.android.libraries.matrix.api.room.isDm
63+
import io.element.android.libraries.matrix.api.timeline.TimelineException
6364
import io.element.android.libraries.matrix.ui.messages.RoomMemberProfilesCache
6465
import io.element.android.libraries.matrix.ui.messages.reply.InReplyToDetails
6566
import io.element.android.libraries.matrix.ui.messages.reply.map
@@ -436,7 +437,14 @@ class MessageComposerPresenter @Inject constructor(
436437
val eventId = capturedMode.eventId
437438
val transactionId = capturedMode.transactionId
438439
timelineController.invokeOnCurrentTimeline {
440+
// First try to edit the message in the current timeline
439441
editMessage(eventId, transactionId, message.markdown, message.html, message.mentions)
442+
.onFailure { cause ->
443+
if (cause is TimelineException.EventNotFound && eventId != null) {
444+
// if the event is not found in the timeline, try to edit the message directly
445+
room.editMessage(eventId, message.markdown, message.html, message.mentions)
446+
}
447+
}
440448
}
441449
}
442450

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/textcomposer/MessageComposerPresenterTest.kt‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import io.element.android.libraries.matrix.api.room.Mention
5656
import io.element.android.libraries.matrix.api.room.RoomMembershipState
5757
import io.element.android.libraries.matrix.api.room.draft.ComposerDraft
5858
import io.element.android.libraries.matrix.api.room.draft.ComposerDraftType
59+
import io.element.android.libraries.matrix.api.timeline.TimelineException
5960
import io.element.android.libraries.matrix.api.timeline.item.event.InReplyTo
6061
import io.element.android.libraries.matrix.test.ANOTHER_MESSAGE
6162
import io.element.android.libraries.matrix.test.AN_EVENT_ID
@@ -417,6 +418,67 @@ class MessageComposerPresenterTest {
417418
}
418419
}
419420

421+
@Test
422+
fun `present - edit sent message event not found`() = runTest {
423+
val timelineEditMessageLambda = lambdaRecorder { _: EventId?, _: TransactionId?, _: String, _: String?, _: List<Mention> ->
424+
Result.failure<Unit>(TimelineException.EventNotFound)
425+
}
426+
val timeline = FakeTimeline().apply {
427+
this.editMessageLambda = timelineEditMessageLambda
428+
}
429+
val roomEditMessageLambda = lambdaRecorder { _: EventId?, _: String, _: String?, _: List<Mention> ->
430+
Result.success(Unit)
431+
}
432+
val fakeMatrixRoom = FakeMatrixRoom(
433+
liveTimeline = timeline,
434+
typingNoticeResult = { Result.success(Unit) }
435+
).apply {
436+
this.editMessageLambda = roomEditMessageLambda
437+
}
438+
val presenter = createPresenter(
439+
this,
440+
fakeMatrixRoom,
441+
)
442+
moleculeFlow(RecompositionMode.Immediate) {
443+
val state = presenter.present()
444+
remember(state, state.textEditorState.messageHtml()) { state }
445+
}.test {
446+
val initialState = awaitFirstItem()
447+
assertThat(initialState.textEditorState.messageHtml()).isEqualTo("")
448+
val mode = anEditMode()
449+
initialState.eventSink.invoke(MessageComposerEvents.SetMode(mode))
450+
val withMessageState = awaitItem()
451+
assertThat(withMessageState.mode).isEqualTo(mode)
452+
assertThat(withMessageState.textEditorState.messageHtml()).isEqualTo(A_MESSAGE)
453+
withMessageState.textEditorState.setHtml(ANOTHER_MESSAGE)
454+
val withEditedMessageState = awaitItem()
455+
assertThat(withEditedMessageState.textEditorState.messageHtml()).isEqualTo(ANOTHER_MESSAGE)
456+
withEditedMessageState.eventSink.invoke(MessageComposerEvents.SendMessage)
457+
skipItems(1)
458+
val messageSentState = awaitItem()
459+
assertThat(messageSentState.textEditorState.messageHtml()).isEqualTo("")
460+
461+
advanceUntilIdle()
462+
463+
assert(timelineEditMessageLambda)
464+
.isCalledOnce()
465+
.with(value(AN_EVENT_ID), value(null), value(ANOTHER_MESSAGE), value(ANOTHER_MESSAGE), any())
466+
467+
assert(roomEditMessageLambda)
468+
.isCalledOnce()
469+
.with(value(AN_EVENT_ID), value(ANOTHER_MESSAGE), value(ANOTHER_MESSAGE), any())
470+
471+
assertThat(analyticsService.capturedEvents).containsExactly(
472+
Composer(
473+
inThread = false,
474+
isEditing = true,
475+
isReply = false,
476+
messageType = Composer.MessageType.Text,
477+
)
478+
)
479+
}
480+
}
481+
420482
@Test
421483
fun `present - edit not sent message`() = runTest {
422484
val editMessageLambda = lambdaRecorder { _: EventId?, _: TransactionId?, _: String, _: String?, _: List<Mention> ->

‎libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ interface MatrixRoom : Closeable {
126126

127127
suspend fun sendMessage(body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit>
128128

129+
suspend fun editMessage(eventId: EventId, body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit>
130+
129131
suspend fun sendImage(
130132
file: File,
131133
thumbnailFile: File?,

‎libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/TimelineException.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ package io.element.android.libraries.matrix.api.timeline
1818

1919
sealed class TimelineException : Exception() {
2020
data object CannotPaginate : TimelineException()
21+
data object EventNotFound : TimelineException()
2122
}

‎libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import io.element.android.libraries.matrix.impl.room.member.RoomMemberMapper
5656
import io.element.android.libraries.matrix.impl.room.powerlevels.RoomPowerLevelsMapper
5757
import io.element.android.libraries.matrix.impl.timeline.RustTimeline
5858
import io.element.android.libraries.matrix.impl.timeline.toRustReceiptType
59+
import io.element.android.libraries.matrix.impl.util.MessageEventContent
5960
import io.element.android.libraries.matrix.impl.util.mxCallbackFlow
6061
import io.element.android.libraries.matrix.impl.widget.RustWidgetDriver
6162
import io.element.android.libraries.matrix.impl.widget.generateWidgetWebViewUrl
@@ -324,6 +325,14 @@ class RustMatrixRoom(
324325
}
325326
}
326327

328+
override suspend fun editMessage(eventId: EventId, body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit> = withContext(roomDispatcher) {
329+
runCatching {
330+
MessageEventContent.from(body, htmlBody, mentions).use { newContent ->
331+
innerRoom.edit(eventId.value, newContent)
332+
}
333+
}
334+
}
335+
327336
override suspend fun sendMessage(body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit> {
328337
return liveTimeline.sendMessage(body, htmlBody, mentions)
329338
}

‎libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt‎

Lines changed: 25 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import io.element.android.libraries.matrix.impl.media.toMSC3246range
4242
import io.element.android.libraries.matrix.impl.poll.toInner
4343
import io.element.android.libraries.matrix.impl.room.RoomContentForwarder
4444
import io.element.android.libraries.matrix.impl.room.location.toInner
45-
import io.element.android.libraries.matrix.impl.room.map
4645
import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper
4746
import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper
4847
import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper
@@ -51,6 +50,7 @@ import io.element.android.libraries.matrix.impl.timeline.postprocessor.LoadingIn
5150
import io.element.android.libraries.matrix.impl.timeline.postprocessor.RoomBeginningPostProcessor
5251
import io.element.android.libraries.matrix.impl.timeline.postprocessor.TimelineEncryptedHistoryPostProcessor
5352
import io.element.android.libraries.matrix.impl.timeline.reply.InReplyToMapper
53+
import io.element.android.libraries.matrix.impl.util.MessageEventContent
5454
import io.element.android.services.toolbox.api.systemclock.SystemClock
5555
import kotlinx.coroutines.CompletableDeferred
5656
import kotlinx.coroutines.CoroutineDispatcher
@@ -70,12 +70,10 @@ import kotlinx.coroutines.flow.onEach
7070
import kotlinx.coroutines.flow.onStart
7171
import kotlinx.coroutines.launch
7272
import kotlinx.coroutines.withContext
73+
import org.matrix.rustcomponents.sdk.EventTimelineItem
7374
import org.matrix.rustcomponents.sdk.FormattedBody
7475
import org.matrix.rustcomponents.sdk.MessageFormat
75-
import org.matrix.rustcomponents.sdk.RoomMessageEventContentWithoutRelation
7676
import org.matrix.rustcomponents.sdk.SendAttachmentJoinHandle
77-
import org.matrix.rustcomponents.sdk.messageEventContentFromHtml
78-
import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown
7977
import org.matrix.rustcomponents.sdk.use
8078
import timber.log.Timber
8179
import uniffi.matrix_sdk_ui.LiveBackPaginationStatus
@@ -266,7 +264,7 @@ class RustTimeline(
266264
}
267265

268266
override suspend fun sendMessage(body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit> = withContext(dispatcher) {
269-
messageEventContentFromParts(body, htmlBody).withMentions(mentions.map()).use { content ->
267+
MessageEventContent.from(body, htmlBody, mentions).use { content ->
270268
runCatching<Unit> {
271269
inner.send(content)
272270
}
@@ -275,20 +273,8 @@ class RustTimeline(
275273

276274
override suspend fun redactEvent(eventId: EventId?, transactionId: TransactionId?, reason: String?): Result<Boolean> = withContext(dispatcher) {
277275
runCatching {
278-
when {
279-
eventId != null -> {
280-
inner.getEventTimelineItemByEventId(eventId.value).use {
281-
inner.redactEvent(item = it, reason = reason)
282-
}
283-
}
284-
transactionId != null -> {
285-
inner.getEventTimelineItemByTransactionId(transactionId.value).use {
286-
inner.redactEvent(item = it, reason = reason)
287-
}
288-
}
289-
else -> {
290-
error("Either eventId or transactionId must be non-null")
291-
}
276+
getEventTimelineItem(eventId, transactionId).use { item ->
277+
inner.redactEvent(item = item, reason = reason)
292278
}
293279
}
294280
}
@@ -302,26 +288,11 @@ class RustTimeline(
302288
): Result<Unit> =
303289
withContext(dispatcher) {
304290
runCatching<Unit> {
305-
when {
306-
originalEventId != null -> {
307-
inner.getEventTimelineItemByEventId(originalEventId.value).use {
308-
inner.edit(
309-
newContent = messageEventContentFromParts(body, htmlBody).withMentions(mentions.map()),
310-
item = it,
311-
)
312-
}
313-
}
314-
transactionId != null -> {
315-
inner.getEventTimelineItemByTransactionId(transactionId.value).use {
316-
inner.edit(
317-
newContent = messageEventContentFromParts(body, htmlBody).withMentions(mentions.map()),
318-
item = it,
319-
)
320-
}
321-
}
322-
else -> {
323-
error("Either originalEventId or transactionId must be non null")
324-
}
291+
getEventTimelineItem(originalEventId, transactionId).use { item ->
292+
inner.edit(
293+
newContent = MessageEventContent.from(body, htmlBody, mentions),
294+
item = item,
295+
)
325296
}
326297
}
327298
}
@@ -334,7 +305,7 @@ class RustTimeline(
334305
fromNotification: Boolean,
335306
): Result<Unit> = withContext(dispatcher) {
336307
runCatching {
337-
val msg = messageEventContentFromParts(body, htmlBody).withMentions(mentions.map())
308+
val msg = MessageEventContent.from(body, htmlBody, mentions)
338309
inner.sendReply(msg, eventId.value)
339310
}
340311
}
@@ -361,6 +332,20 @@ class RustTimeline(
361332
}
362333
}
363334

335+
@Throws
336+
private suspend fun getEventTimelineItem(eventId: EventId?, transactionId: TransactionId?): EventTimelineItem {
337+
return try {
338+
when {
339+
eventId != null -> inner.getEventTimelineItemByEventId(eventId.value)
340+
transactionId != null -> inner.getEventTimelineItemByTransactionId(transactionId.value)
341+
else -> error("Either eventId or transactionId must be non-null")
342+
}
343+
} catch (e: Exception) {
344+
Timber.e(e, "Failed to get event timeline item")
345+
throw TimelineException.EventNotFound
346+
}
347+
}
348+
364349
override suspend fun sendVideo(
365350
file: File,
366351
thumbnailFile: File?,
@@ -517,13 +502,6 @@ class RustTimeline(
517502
)
518503
}
519504

520-
private fun messageEventContentFromParts(body: String, htmlBody: String?): RoomMessageEventContentWithoutRelation =
521-
if (htmlBody != null) {
522-
messageEventContentFromHtml(body, htmlBody)
523-
} else {
524-
messageEventContentFromMarkdown(body)
525-
}
526-
527505
private fun sendAttachment(files: List<File>, handle: () -> SendAttachmentJoinHandle): Result<MediaUploadHandler> {
528506
return runCatching {
529507
MediaUploadHandlerImpl(files, handle())

‎libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/util/MessageEventContent.kt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright (c) 2024 New Vector Ltd
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.element.android.libraries.matrix.impl.util
18+
19+
import io.element.android.libraries.matrix.api.room.Mention
20+
import io.element.android.libraries.matrix.impl.room.map
21+
import org.matrix.rustcomponents.sdk.RoomMessageEventContentWithoutRelation
22+
import org.matrix.rustcomponents.sdk.messageEventContentFromHtml
23+
import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown
24+
25+
/**
26+
* Creates a [RoomMessageEventContentWithoutRelation] from a body, an html body and a list of mentions.
27+
*/
28+
object MessageEventContent {
29+
fun from(body: String, htmlBody: String?, mentions: List<Mention>): RoomMessageEventContentWithoutRelation {
30+
return if (htmlBody != null) {
31+
messageEventContentFromHtml(body, htmlBody)
32+
} else {
33+
messageEventContentFromMarkdown(body)
34+
}.withMentions(mentions.map())
35+
}
36+
}

‎libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ class FakeMatrixRoom(
212212
return updateUserRoleResult()
213213
}
214214

215+
var editMessageLambda: (EventId, String, String?, List<Mention>) -> Result<Unit> = { _, _, _, _ -> lambdaError() }
216+
override suspend fun editMessage(eventId: EventId, body: String, htmlBody: String?, mentions: List<Mention>): Result<Unit> {
217+
return editMessageLambda(eventId, body, htmlBody, mentions)
218+
}
219+
215220
override suspend fun sendMessage(body: String, htmlBody: String?, mentions: List<Mention>) = simulateLongTask {
216221
sendMessageResult(body, htmlBody, mentions)
217222
}

0 commit comments

Comments
 (0)