Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replyTo are not updated if the original message is edited #6404

Merged
merged 16 commits into from
Jul 22, 2022
1 change: 1 addition & 0 deletions changelog.d/5546.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ReplyTo are not updated if the original message is edited
Original file line number Diff line number Diff line change
Expand Up @@ -573,17 +573,13 @@ internal class LocalEchoEventFactory @Inject constructor(
return clock.epochMillis()
}

/**
* Creates a reply to a regular timeline Event or a thread Event if needed.
*/
fun createReplyTextEvent(
roomId: String,
fun createReplyTextContent(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use this in createReplyTextEvent

eventReplied: TimelineEvent,
replyText: CharSequence,
autoMarkdown: Boolean,
rootThreadEventId: String? = null,
showInThread: Boolean
): Event? {
): MessageContent? {
// Fallbacks and event representation
// TODO Add error/warning logs when any of this is null
val permalink = permalinkFactory.createPermalink(eventReplied.root, false) ?: return null
Expand All @@ -609,7 +605,7 @@ internal class LocalEchoEventFactory @Inject constructor(
val replyFallback = buildReplyFallback(body, userId, replyText.toString())

val eventId = eventReplied.root.eventId ?: return null
val content = MessageTextContent(
return MessageTextContent(
msgType = MessageType.MSGTYPE_TEXT,
format = MessageFormat.FORMAT_MATRIX_HTML,
body = replyFallback,
Expand All @@ -620,7 +616,23 @@ internal class LocalEchoEventFactory @Inject constructor(
showInThread = showInThread
)
)
return createMessageEvent(roomId, content)
}

/**
* Creates a reply to a regular timeline Event or a thread Event if needed.
*/
fun createReplyTextEvent(
roomId: String,
eventReplied: TimelineEvent,
replyText: CharSequence,
autoMarkdown: Boolean,
rootThreadEventId: String? = null,
showInThread: Boolean,
): Event? {
val content = createReplyTextContent(eventReplied, replyText, autoMarkdown, rootThreadEventId, showInThread)
return content?.let {
createMessageEvent(roomId, it)
}
}

/**
Expand Down Expand Up @@ -681,7 +693,7 @@ internal class LocalEchoEventFactory @Inject constructor(
* In case of an edit of a reply the last content is not
* himself a reply, but it will contain the fallbacks, so we have to trim them.
*/
private fun bodyForReply(content: MessageContent?, isReply: Boolean): TextContent {
fun bodyForReply(content: MessageContent?, isReply: Boolean): TextContent {
when (content?.msgType) {
MessageType.MSGTYPE_EMOTE,
MessageType.MSGTYPE_TEXT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import org.matrix.android.sdk.api.settings.LightweightSettingsStorage
import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask
import org.matrix.android.sdk.internal.session.room.relation.threads.FetchThreadTimelineTask
import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
import org.matrix.android.sdk.internal.session.room.state.StateEventDataSource
import org.matrix.android.sdk.internal.session.sync.handler.room.ReadReceiptHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
Expand All @@ -61,6 +62,7 @@ internal class DefaultTimeline(
private val settings: TimelineSettings,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val clock: Clock,
localEchoEventFactory: LocalEchoEventFactory,
stateEventDataSource: StateEventDataSource,
paginationTask: PaginationTask,
getEventTask: GetContextOfEventTask,
Expand Down Expand Up @@ -112,6 +114,7 @@ internal class DefaultTimeline(
onNewTimelineEvents = this::onNewTimelineEvents,
stateEventDataSource = stateEventDataSource,
matrixCoroutineDispatchers = coroutineDispatchers,
localEchoEventFactory = localEchoEventFactory
)

private var strategy: LoadTimelineStrategy = buildStrategy(LoadTimelineStrategy.Mode.Live)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask
import org.matrix.android.sdk.internal.session.room.relation.threads.FetchThreadTimelineTask
import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
import org.matrix.android.sdk.internal.session.room.state.StateEventDataSource
import org.matrix.android.sdk.internal.session.sync.handler.room.ReadReceiptHandler
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
Expand All @@ -55,6 +56,7 @@ internal class DefaultTimelineService @AssistedInject constructor(
private val timelineEventDataSource: TimelineEventDataSource,
private val clock: Clock,
private val stateEventDataSource: StateEventDataSource,
private val localEchoEventFactory: LocalEchoEventFactory
) : TimelineService {

@AssistedFactory
Expand Down Expand Up @@ -82,6 +84,7 @@ internal class DefaultTimelineService @AssistedInject constructor(
lightweightSettingsStorage = lightweightSettingsStorage,
clock = clock,
stateEventDataSource = stateEventDataSource,
localEchoEventFactory = localEchoEventFactory
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import org.matrix.android.sdk.internal.database.query.findAllIncludingEvents
import org.matrix.android.sdk.internal.database.query.findLastForwardChunkOfThread
import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.session.room.relation.threads.FetchThreadTimelineTask
import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
import org.matrix.android.sdk.internal.session.room.state.StateEventDataSource
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import org.matrix.android.sdk.internal.util.time.Clock
Expand Down Expand Up @@ -105,6 +106,7 @@ internal class LoadTimelineStrategy constructor(
val onNewTimelineEvents: (List<String>) -> Unit,
val stateEventDataSource: StateEventDataSource,
val matrixCoroutineDispatchers: MatrixCoroutineDispatchers,
val localEchoEventFactory: LocalEchoEventFactory
)

private var getContextLatch: CompletableDeferred<Unit>? = null
Expand Down Expand Up @@ -340,6 +342,8 @@ internal class LoadTimelineStrategy constructor(
initialEventId = mode.originEventId(),
onBuiltEvents = dependencies.onEventsUpdated,
onEventsDeleted = dependencies.onEventsDeleted,
realm = dependencies.realm,
localEchoEventFactory = dependencies.localEchoEventFactory
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.session.room.timeline

import io.realm.OrderedCollectionChangeSet
import io.realm.OrderedRealmCollectionChangeListener
import io.realm.Realm
import io.realm.RealmConfiguration
import io.realm.RealmObjectChangeListener
import io.realm.RealmQuery
Expand All @@ -26,23 +27,34 @@ import io.realm.Sort
import kotlinx.coroutines.CompletableDeferred
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.events.model.UnsignedData
import org.matrix.android.sdk.api.session.events.model.content.EncryptedEventContent
import org.matrix.android.sdk.api.session.events.model.toContent
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.session.room.model.message.MessageContent
import org.matrix.android.sdk.api.session.room.timeline.Timeline
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings
import org.matrix.android.sdk.api.session.room.timeline.getLastMessageContent
import org.matrix.android.sdk.api.session.room.timeline.isReply
import org.matrix.android.sdk.api.settings.LightweightSettingsStorage
import org.matrix.android.sdk.internal.database.mapper.EventMapper
import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
import org.matrix.android.sdk.internal.database.model.ChunkEntity
import org.matrix.android.sdk.internal.database.model.ChunkEntityFields
import org.matrix.android.sdk.internal.database.model.TimelineEventEntity
import org.matrix.android.sdk.internal.database.model.TimelineEventEntityFields
import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.session.room.relation.threads.DefaultFetchThreadTimelineTask
import org.matrix.android.sdk.internal.session.room.relation.threads.FetchThreadTimelineTask
import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
import org.matrix.android.sdk.internal.session.sync.handler.room.ThreadsAwarenessHandler
import timber.log.Timber
import java.util.Collections
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicReference

/**
* This is a wrapper around a ChunkEntity in the database.
Expand All @@ -66,6 +78,8 @@ internal class TimelineChunk(
private val initialEventId: String?,
private val onBuiltEvents: (Boolean) -> Unit,
private val onEventsDeleted: () -> Unit,
private val realm: AtomicReference<Realm>,
val localEchoEventFactory: LocalEchoEventFactory,
) {

private val isLastForward = AtomicBoolean(chunkEntity.isLastForward)
Expand Down Expand Up @@ -411,9 +425,56 @@ internal class TimelineChunk(
private fun buildTimelineEvent(eventEntity: TimelineEventEntity) = timelineEventMapper.map(
timelineEventEntity = eventEntity,
buildReadReceipts = timelineSettings.buildReadReceipts
).let {
).let { timelineEvent ->
// eventually enhance with ui echo?
(uiEchoManager?.decorateEventWithReactionUiEcho(it) ?: it)
uiEchoManager?.decorateEventWithReactionUiEcho(timelineEvent)

if (timelineEvent.isReply()) {
createNewEncryptedRepliedEvent(timelineEvent)?.let { newEvent ->
timelineEvent.copy(root = newEvent)
} ?: timelineEvent
} else timelineEvent
}

private fun createNewEncryptedRepliedEvent(currentTimelineEvent: TimelineEvent): Event? {
val relatesEventId = if (currentTimelineEvent.isEncrypted()) {
currentTimelineEvent.root.content.toModel<EncryptedEventContent>()?.relatesTo?.inReplyTo?.eventId
} else {
currentTimelineEvent.root.content.toModel<MessageContent>()?.relatesTo?.inReplyTo?.eventId
}
return relatesEventId?.let { eventId ->
val timeLineEventEntity = TimelineEventEntity.where(
realm.get(),
roomId,
eventId
).findFirst()

val replyText = localEchoEventFactory
.bodyForReply(currentTimelineEvent.getLastMessageContent(), true).formattedText ?: ""

timeLineEventEntity?.let { timelineEventEntity ->
val newContent = localEchoEventFactory.createReplyTextContent(
timelineEventMapper.map(timelineEventEntity),
replyText,
false,
showInThread = false
).toContent()
val event = currentTimelineEvent.root
Event(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing the transient fields here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i keep the transient fileds here, my message is not updated: the old version is keep.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you are also copying the decrypted response :-)

roomId = event.roomId,
originServerTs = event.originServerTs,
senderId = event.senderId,
eventId = currentTimelineEvent.eventId,
type = EventType.MESSAGE,
content = newContent,
unsignedData = UnsignedData(age = null, transactionId = currentTimelineEvent.eventId),
).apply {
this.sendState = event.sendState
this.ageLocalTs = event.ageLocalTs
this.threadDetails = event.threadDetails
}
}
}
}

/**
Expand Down Expand Up @@ -595,7 +656,9 @@ internal class TimelineChunk(
lightweightSettingsStorage = lightweightSettingsStorage,
initialEventId = null,
onBuiltEvents = this.onBuiltEvents,
onEventsDeleted = this.onEventsDeleted
onEventsDeleted = this.onEventsDeleted,
realm = realm,
localEchoEventFactory = localEchoEventFactory
)
}

Expand Down