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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e056fe9
Update replied event when it's updated
Claire1817 Jun 28, 2022
12a1124
add changelog
Claire1817 Jun 28, 2022
5442e70
fix linter
Claire1817 Jun 28, 2022
8b041c8
improve updateRepliedEvent
Claire1817 Jun 29, 2022
3fbbdcd
Update check for uncrypted messages
Claire1817 Jun 29, 2022
019ffc6
update changelog
Claire1817 Jul 6, 2022
6e73cda
Fix PR comments
Claire1817 Jul 6, 2022
3aa0ce7
Use same eventId than before
Claire1817 Jul 6, 2022
9029dc1
clean
Claire1817 Jul 6, 2022
dfc066b
clean
Claire1817 Jul 6, 2022
5f2c3d7
Fix PR comments
Claire1817 Jul 7, 2022
9042ba3
Fix PR comments
Claire1817 Jul 8, 2022
7f4cfb9
Fix PR comments
Claire1817 Jul 18, 2022
573948f
Fix build
Claire1817 Jul 18, 2022
14b8c37
Fix PR comments
Claire1817 Jul 19, 2022
d32b9fa
Merge branch 'develop' into cgizard/ISSUE-5546
Claire1817 Jul 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5546.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If you reply to a redaction, the redacted text still appears
Claire1817 marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion ...k-android/src/main/java/org/matrix/android/sdk/api/session/room/timeline/TimelineEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ data class TimelineEvent(

init {
if (BuildConfig.DEBUG) {
assert(eventId == root.eventId)
// assert(eventId == root.eventId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why did you add this line @bmarty ? If i keep it, it's not working

Copy link
Member

Choose a reason for hiding this comment

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

this.eventId should always have the same value than this.root.eventId. In which case this was not true on your side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to use some code from LocalEventFactory to avoid the code duplication. In createEvent, a new eventId is created for the event.
In my case i don't know if i should use the past id even if i updated the relatedTo part ? or should i create a new id ? 🤔
Knowing i never update the DB, i juste update the object before print it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ganfra told me that i should keep the id.

}
}

Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoEventFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,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
3 changes: 3 additions & 0 deletions ...id/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt
View file
Open in desktop
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
3 changes: 3 additions & 0 deletions ...main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimelineService.kt
View file
Open in desktop
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
49 changes: 47 additions & 2 deletions ...c/main/java/org/matrix/android/sdk/internal/session/room/timeline/LoadTimelineStrategy.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,28 @@ import org.matrix.android.sdk.api.MatrixCoroutineDispatchers
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.failure.Failure
import org.matrix.android.sdk.api.failure.MatrixError
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.content.EncryptedEventContent
import org.matrix.android.sdk.api.session.events.model.toModel
import org.matrix.android.sdk.api.session.room.send.SendState
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.helper.addIfNecessary
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.RoomEntity
import org.matrix.android.sdk.internal.database.model.TimelineEventEntity
import org.matrix.android.sdk.internal.database.model.deleteAndClearThreadEvents
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 +112,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 @@ -240,10 +248,47 @@ internal class LoadTimelineStrategy constructor(

fun buildSnapshot(): List<TimelineEvent> {
val events = buildSendingEvents() + timelineChunk?.builtItems(includesNext = true, includesPrev = true).orEmpty()
val eventsUpdated = updateRepliedEventIfNeeded(events)
Claire1817 marked this conversation as resolved.
Show resolved Hide resolved
return if (dependencies.timelineSettings.useLiveSenderInfo) {
events.map(this::applyLiveRoomState)
eventsUpdated.map(this::applyLiveRoomState)
} else {
events
eventsUpdated
}
}

private fun updateRepliedEventIfNeeded(events: List<TimelineEvent>): List<TimelineEvent> {
return events.mapNotNull {
if (it.isReply()) {
if (it.isEncrypted()) {
createNewRepliedEvent(it)?.let { newEvent ->
it.copy(root = newEvent)
} ?: it
Claire1817 marked this conversation as resolved.
Show resolved Hide resolved
} else it
} else it
}
}

private fun createNewRepliedEvent(currentTimelineEvent: TimelineEvent): Event? {
return currentTimelineEvent.root.content.toModel<EncryptedEventContent>()?.relatesTo?.inReplyTo?.eventId?.let { eventId ->
val timeLineEventEntity = TimelineEventEntity.where(
dependencies.realm.get(),
roomId,
eventId
).findFirst()

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

timeLineEventEntity?.let { timelineEventEntity ->
dependencies.localEchoEventFactory.createReplyTextEvent(
roomId,
dependencies.timelineEventMapper.map(timelineEventEntity),
replyText,
false,
showInThread = false
)
}
}
}

Expand Down