Merge pull request #371 from vector-im/feature/composer_fix_edit_reply

Feature/composer fix edit reply

CHANGES.md

@@ -2,11 +2,12 @@ Changes in RiotX 0.2.1 (2019-XX-XX)
 ===================================================
 
 Features:
- - Message Editing: View edit history
+ - Message Editing: View edit history (#121)
  - Rooms filtering (#304)
 
 Improvements:
  - Handle click on redacted events: view source and create permalink
+ - Improve edit of replies
 
 Other changes:
  -

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/message/MessageContent.kt

@@ -25,4 +25,9 @@ interface MessageContent {
     val body: String
     val relatesTo: RelationDefaultContent?
     val newContent: Content?
+}
+
+
+fun MessageContent?.isReply(): Boolean {
+    return this?.relatesTo?.inReplyTo != null
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/relation/RelationService.kt

@@ -80,6 +80,22 @@ interface RelationService {
                         newBodyAutoMarkdown: Boolean,
                         compatibilityBodyText: String = "* $newBodyText"): Cancelable
 
+
+    /**
+     * Edit a reply. This is a special case because replies contains fallback text as a prefix.
+     * This method will take the new body (stripped from fallbacks) and re-add them before sending.
+     * @param replyToEdit The event to edit
+     * @param originalSenderId the sender of the message that this reply (being edited) is relating to
+     * @param originalEventId the event id that this reply (being edited) is relating to
+     * @param newBodyText The edited body (stripped from in reply to content)
+     * @param compatibilityBodyText The text that will appear on clients that don't support yet edition
+     */
+    fun editReply(replyToEdit: TimelineEvent,
+                  originalSenderId: String?,
+                  originalEventId : String,
+                  newBodyText: String,
+                  compatibilityBodyText: String = "* $newBodyText"): Cancelable
+
     /**
      * Get's the edit history of the given event
      */

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/timeline/TimelineEvent.kt

@@ -21,7 +21,9 @@ import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.api.session.events.model.toModel
 import im.vector.matrix.android.api.session.room.model.EventAnnotationsSummary
 import im.vector.matrix.android.api.session.room.model.message.MessageContent
+import im.vector.matrix.android.api.session.room.model.message.isReply
 import im.vector.matrix.android.api.session.room.send.SendState
+import im.vector.matrix.android.api.util.ContentUtils.extractUsefulTextFromReply
 
 /**
  * This data class is a wrapper around an Event. It allows to get useful data in the context of a timeline.
@@ -88,3 +90,15 @@ data class TimelineEvent(
  */
 fun TimelineEvent.getLastMessageContent(): MessageContent? = annotations?.editSummary?.aggregatedContent?.toModel()
         ?: root.getClearContent().toModel()
+
+
+fun TimelineEvent.getTextEditableContent(): String? {
+    val originalContent = root.getClearContent().toModel<MessageContent>() ?: return null
+    val isReply = originalContent.isReply()
+    val lastContent = getLastMessageContent()
+    return if (isReply) {
+        return extractUsefulTextFromReply(lastContent?.body ?: "")
+    } else {
+        lastContent?.body ?: ""
+    }
+}

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/util/ContentUtils.kt

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2019 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package im.vector.matrix.android.api.util
+
+
+object ContentUtils {
+    fun extractUsefulTextFromReply(repliedBody: String): String {
+        val lines = repliedBody.lines()
+        var wellFormed = repliedBody.startsWith(">")
+        var endOfPreviousFound = false
+        val usefullines = ArrayList<String>()
+        lines.forEach {
+            if (it == "") {
+                endOfPreviousFound = true
+                return@forEach
+            }
+            if (!endOfPreviousFound) {
+                wellFormed = wellFormed && it.startsWith(">")
+            } else {
+                usefullines.add(it)
+            }
+        }
+        return usefullines.joinToString("\n").takeIf { wellFormed } ?: repliedBody
+    }
+
+    fun extractUsefulTextFromHtmlReply(repliedBody: String): String {
+        if (repliedBody.startsWith("<mx-reply>")) {
+            val closingTagIndex = repliedBody.lastIndexOf("</mx-reply>")
+            if (closingTagIndex != -1)
+                return repliedBody.substring(closingTagIndex + "</mx-reply>".length).trim()
+        }
+        return repliedBody
+    }
+}

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/relation/DefaultRelationService.kt

@@ -25,6 +25,7 @@ import im.vector.matrix.android.api.auth.data.Credentials
 import im.vector.matrix.android.api.session.crypto.CryptoService
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.room.model.EventAnnotationsSummary
+import im.vector.matrix.android.api.session.room.model.message.MessageType
 import im.vector.matrix.android.api.session.room.model.relation.RelationService
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.api.util.Cancelable
@@ -132,6 +133,24 @@ internal class DefaultRelationService @Inject constructor(private val context: C
 
     }
 
+    override fun editReply(replyToEdit: TimelineEvent,
+                           originalSenderId: String?,
+                           originalEventId: String,
+                           newBodyText: String,
+                           compatibilityBodyText: String): Cancelable {
+        val event = eventFactory
+                .createReplaceTextOfReply(roomId,
+                        replyToEdit,
+                        originalSenderId, originalEventId,
+                        newBodyText, true, MessageType.MSGTYPE_TEXT, compatibilityBodyText)
+                .also {
+                    saveLocalEcho(it)
+                }
+        val workRequest = createSendEventWork(event)
+        TimelineSendEventWorkCommon.postWork(context, roomId, workRequest)
+        return CancelableWork(context, workRequest.id)
+    }
+
     override fun fetchEditHistory(eventId: String, callback: MatrixCallback<List<Event>>) {
         val params = FetchEditHistoryTask.Params(roomId, eventId)
         fetchEditHistoryTask.configureWith(params)

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt

@@ -104,6 +104,45 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
                 ))
     }
 
+    fun createReplaceTextOfReply(roomId: String, eventReplaced: TimelineEvent,
+                                 originalSenderId: String?,
+                                 originalEventId: String,
+                                 newBodyText: String,
+                                 newBodyAutoMarkdown: Boolean,
+                                 msgType: String,
+                                 compatibilityText: String): Event {
+        val permalink = PermalinkFactory.createPermalink(roomId, originalEventId)
+        val userLink = originalSenderId?.let { PermalinkFactory.createPermalink(it) } ?: ""
+
+        val body = bodyForReply(eventReplaced.getLastMessageContent(), eventReplaced.root.getClearContent().toModel())
+        val replyFormatted = REPLY_PATTERN.format(
+                permalink,
+                stringProvider.getString(R.string.message_reply_to_prefix),
+                userLink,
+                originalSenderId,
+                body.takeFormatted(),
+                createTextContent(newBodyText, newBodyAutoMarkdown).takeFormatted()
+        )
+        //
+        // > <@alice:example.org> This is the original body
+        //
+        val replyFallback = buildReplyFallback(body, originalSenderId, newBodyText)
+
+        return createEvent(roomId,
+                MessageTextContent(
+                        type = msgType,
+                        body = compatibilityText,
+                        relatesTo = RelationDefaultContent(RelationType.REPLACE, eventReplaced.root.eventId),
+                        newContent = MessageTextContent(
+                                type = msgType,
+                                format = MessageType.FORMAT_MATRIX_HTML,
+                                body = replyFallback,
+                                formattedBody = replyFormatted
+                        )
+                                .toContent()
+                ))
+    }
+
     fun createMediaEvent(roomId: String, attachment: ContentAttachmentData): Event {
         return when (attachment.type) {
             ContentAttachmentData.Type.IMAGE -> createImageEvent(roomId, attachment)
@@ -239,16 +278,8 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
         val permalink = PermalinkFactory.createPermalink(eventReplied.root) ?: return null
         val userId = eventReplied.root.senderId ?: return null
         val userLink = PermalinkFactory.createPermalink(userId) ?: return null
-        // <mx-reply>
-        //     <blockquote>
-        //         <a href="https://matrix.to/#/!somewhere:domain.com/$event:domain.com">In reply to</a>
-        //         <a href="https://matrix.to/#/@alice:example.org">@alice:example.org</a>
-        //         <br />
-        //         <!-- This is where the related event's HTML would be. -->
-        //     </blockquote>
-        // </mx-reply>
-        // This is where the reply goes.
-        val body = bodyForReply(eventReplied.getLastMessageContent())
+
+        val body = bodyForReply(eventReplied.getLastMessageContent(), eventReplied.root.getClearContent().toModel())
         val replyFormatted = REPLY_PATTERN.format(
                 permalink,
                 stringProvider.getString(R.string.message_reply_to_prefix),
@@ -260,8 +291,22 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
         //
         // > <@alice:example.org> This is the original body
         //
+        val replyFallback = buildReplyFallback(body, userId, replyText)
+
+        val eventId = eventReplied.root.eventId ?: return null
+        val content = MessageTextContent(
+                type = MessageType.MSGTYPE_TEXT,
+                format = MessageType.FORMAT_MATRIX_HTML,
+                body = replyFallback,
+                formattedBody = replyFormatted,
+                relatesTo = RelationDefaultContent(null, null, ReplyToContent(eventId))
+        )
+        return createEvent(roomId, content)
+    }
+
+    private fun buildReplyFallback(body: TextContent, originalSenderId: String?, newBodyText: String): String {
         val lines = body.text.split("\n")
-        val replyFallback = StringBuffer("><$userId>")
+        val replyFallback = StringBuffer("><$originalSenderId>")
         lines.forEachIndexed { index, s ->
             if (index == 0) {
                 replyFallback.append(" $s")
@@ -269,23 +314,16 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
                 replyFallback.append("\n>$s")
             }
         }
-        replyFallback.append("\n\n").append(replyText)
-
-        val eventId = eventReplied.root.eventId ?: return null
-        val content = MessageTextContent(
-                type = MessageType.MSGTYPE_TEXT,
-                format = MessageType.FORMAT_MATRIX_HTML,
-                body = replyFallback.toString(),
-                formattedBody = replyFormatted,
-                relatesTo = RelationDefaultContent(null, null, ReplyToContent(eventId))
-        )
-        return createEvent(roomId, content)
+        replyFallback.append("\n\n").append(newBodyText)
+        return replyFallback.toString()
     }
 
     /**
      * Returns a TextContent used for the fallback event representation in a reply message.
+     * We also pass the original content, because 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?): TextContent {
+    private fun bodyForReply(content: MessageContent?, originalContent: MessageContent?): TextContent {
         when (content?.type) {
             MessageType.MSGTYPE_EMOTE,
             MessageType.MSGTYPE_TEXT,
@@ -296,7 +334,7 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
                         formattedText = content.formattedBody
                     }
                 }
-                val isReply = content.relatesTo?.inReplyTo?.eventId != null
+                val isReply = content.isReply() || originalContent.isReply()
                 return if (isReply)
                     TextContent(content.body, formattedText).removeInReplyFallbacks()
                 else
@@ -353,7 +391,16 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials
     companion object {
         const val LOCAL_ID_PREFIX = "local."
 
-        // No whitespace
+
+        // <mx-reply>
+        //     <blockquote>
+        //         <a href="https://matrix.to/#/!somewhere:domain.com/$event:domain.com">In reply to</a>
+        //         <a href="https://matrix.to/#/@alice:example.org">@alice:example.org</a>
+        //         <br />
+        //         <!-- This is where the related event's HTML would be. -->
+        //     </blockquote>
+        // </mx-reply>
+        // No whitespace because currently breaks temporary formatted text to Span
         const val REPLY_PATTERN = """<mx-reply><blockquote><a href="%s">%s</a><a href="%s">%s</a><br />%s</blockquote></mx-reply>%s"""
 
         fun isLocalEchoId(eventId: String): Boolean = eventId.startsWith(LOCAL_ID_PREFIX)

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/TextContent.kt

@@ -18,6 +18,8 @@ package im.vector.matrix.android.internal.session.room.send
 
 import im.vector.matrix.android.api.session.room.model.message.MessageTextContent
 import im.vector.matrix.android.api.session.room.model.message.MessageType
+import im.vector.matrix.android.api.util.ContentUtils.extractUsefulTextFromHtmlReply
+import im.vector.matrix.android.api.util.ContentUtils.extractUsefulTextFromReply
 
 /**
  * Contains a text and eventually a formatted text
@@ -47,28 +49,4 @@ fun TextContent.removeInReplyFallbacks(): TextContent {
     )
 }
 
-private fun extractUsefulTextFromReply(repliedBody: String): String {
-    val lines = repliedBody.lines()
-    var wellFormed = repliedBody.startsWith(">")
-    var endOfPreviousFound = false
-    val usefullines = ArrayList<String>()
-    lines.forEach {
-        if (it == "") {
-            endOfPreviousFound = true
-            return@forEach
-        }
-        if (!endOfPreviousFound) {
-            wellFormed = wellFormed && it.startsWith(">")
-        } else {
-            usefullines.add(it)
-        }
-    }
-    return usefullines.joinToString("\n").takeIf { wellFormed } ?: repliedBody
-}
 
-private fun extractUsefulTextFromHtmlReply(repliedBody: String): String {
-    if (repliedBody.startsWith("<mx-reply>")) {
-        return repliedBody.substring(repliedBody.lastIndexOf("</mx-reply>") + "</mx-reply>".length).trim()
-    }
-    return repliedBody
-}

vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt

@@ -59,6 +59,7 @@ import im.vector.matrix.android.api.session.room.model.Membership
 import im.vector.matrix.android.api.session.room.model.message.*
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.api.session.room.timeline.getLastMessageContent
+import im.vector.matrix.android.api.session.room.timeline.getTextEditableContent
 import im.vector.matrix.android.api.session.user.model.User
 import im.vector.riotx.R
 import im.vector.riotx.core.di.ScreenComponent
@@ -258,7 +259,7 @@ class RoomDetailFragment :
         composerLayout.composerRelatedMessageContent.text = formattedBody
                 ?: nonFormattedBody
 
-        composerLayout.composerEditText.setText(if (useText) nonFormattedBody else "")
+        composerLayout.composerEditText.setText(if (useText) event.getTextEditableContent() else "")
         composerLayout.composerRelatedMessageActionIcon.setImageDrawable(ContextCompat.getDrawable(requireContext(), iconRes))
 
         avatarRenderer.render(event.senderAvatar, event.root.senderId

vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt

@@ -39,7 +39,6 @@ import im.vector.matrix.android.api.session.room.model.message.getFileUrl
 import im.vector.matrix.android.api.session.room.timeline.TimelineEvent
 import im.vector.matrix.android.internal.crypto.attachments.toElementToDecrypt
 import im.vector.matrix.rx.rx
-import im.vector.riotx.R
 import im.vector.riotx.core.intent.getFilenameFromUri
 import im.vector.riotx.core.platform.VectorViewModel
 import im.vector.riotx.core.resources.UserPreferencesProvider
@@ -52,8 +51,6 @@ import org.commonmark.parser.Parser
 import org.commonmark.renderer.html.HtmlRenderer
 import timber.log.Timber
 import java.io.File
-import java.text.SimpleDateFormat
-import java.util.*
 import java.util.concurrent.TimeUnit
 
 
@@ -229,16 +226,24 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro
                     }
                 }
                 is SendMode.EDIT  -> {
-                    val messageContent: MessageContent? =
-                            state.sendMode.timelineEvent.annotations?.editSummary?.aggregatedContent.toModel()
-                                    ?: state.sendMode.timelineEvent.root.getClearContent().toModel()
-                    val nonFormattedBody = messageContent?.body ?: ""
 
-                    if (nonFormattedBody != action.text) {
-                        room.editTextMessage(state.sendMode.timelineEvent.root.eventId
-                                ?: "", messageContent?.type ?: MessageType.MSGTYPE_TEXT, action.text, action.autoMarkdown)
+                    //is original event a reply?
+                    val inReplyTo = state.sendMode.timelineEvent.root.getClearContent().toModel<MessageContent>()?.relatesTo?.inReplyTo?.eventId
+                    if (inReplyTo != null) {
+                        //TODO check if same content?
+                        room.editReply(state.sendMode.timelineEvent, room.getTimeLineEvent(inReplyTo)?.root?.senderId, inReplyTo, action.text)
                     } else {
-                        Timber.w("Same message content, do not send edition")
+                        val messageContent: MessageContent? =
+                                state.sendMode.timelineEvent.annotations?.editSummary?.aggregatedContent.toModel()
+                                        ?: state.sendMode.timelineEvent.root.getClearContent().toModel()
+                        val existingBody = messageContent?.body ?: ""
+                        if (existingBody != action.text) {
+                            room.editTextMessage(state.sendMode.timelineEvent.root.eventId
+                                    ?: "", messageContent?.type
+                                    ?: MessageType.MSGTYPE_TEXT, action.text, action.autoMarkdown)
+                        } else {
+                            Timber.w("Same message content, do not send edition")
+                        }
                     }
                     setState {
                         copy(

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/ViewEditHistoryEpoxyController.kt

@@ -26,6 +26,7 @@ import com.airbnb.mvrx.Success
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.session.events.model.toModel
 import im.vector.matrix.android.api.session.room.model.message.MessageTextContent
+import im.vector.matrix.android.api.util.ContentUtils.extractUsefulTextFromReply
 import im.vector.riotx.R
 import im.vector.riotx.core.extensions.localDateTime
 import im.vector.riotx.core.ui.list.genericFooterItem
@@ -60,13 +61,13 @@ class ViewEditHistoryEpoxyController(private val context: Context,
                 }
             }
             is Success    -> {
-                state.editList()?.let { renderEvents(it) }
+                state.editList()?.let { renderEvents(it, state.isOriginalAReply) }
             }
 
         }
     }
 
-    private fun renderEvents(sourceEvents: List<Event>) {
+    private fun renderEvents(sourceEvents: List<Event>, isOriginalReply: Boolean) {
         if (sourceEvents.isEmpty()) {
             genericItem {
                 id("footer")
@@ -92,7 +93,7 @@ class ViewEditHistoryEpoxyController(private val context: Context,
                     }
                 }
                 lastDate = evDate
-                val cContent = getCorrectContent(timelineEvent)
+                val cContent = getCorrectContent(timelineEvent, isOriginalReply)
                 val body = cContent.second?.let { eventHtmlRenderer.render(it) }
                         ?: cContent.first
 
@@ -101,7 +102,7 @@ class ViewEditHistoryEpoxyController(private val context: Context,
                 var spannedDiff: Spannable? = null
                 if (nextEvent != null && cContent.second == null /*No diff for html*/) {
                     //compares the body
-                    val nContent = getCorrectContent(nextEvent)
+                    val nContent = getCorrectContent(nextEvent, isOriginalReply)
                     val nextBody = nContent.second?.let { eventHtmlRenderer.render(it) }
                             ?: nContent.first
                     val dmp = diff_match_patch()
@@ -144,11 +145,14 @@ class ViewEditHistoryEpoxyController(private val context: Context,
         }
     }
 
-    private fun getCorrectContent(event: Event): Pair<String, String?> {
+    private fun getCorrectContent(event: Event, isOriginalReply: Boolean): Pair<String, String?> {
         val clearContent = event.getClearContent().toModel<MessageTextContent>()
         val newContent = clearContent
                 ?.newContent
                 ?.toModel<MessageTextContent>()
+        if (isOriginalReply) {
+            return extractUsefulTextFromReply(newContent?.body ?: clearContent?.body ?: "") to null
+        }
         return (newContent?.body ?: clearContent?.body ?: "") to (newContent?.formattedBody
                 ?: clearContent?.formattedBody)
     }

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/ViewEditHistoryViewModel.kt

@@ -21,6 +21,9 @@ import com.squareup.inject.assisted.AssistedInject
 import im.vector.matrix.android.api.MatrixCallback
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.session.events.model.Event
+import im.vector.matrix.android.api.session.events.model.toModel
+import im.vector.matrix.android.api.session.room.model.message.MessageContent
+import im.vector.matrix.android.api.session.room.model.message.isReply
 import im.vector.riotx.core.platform.VectorViewModel
 import im.vector.riotx.features.home.room.detail.timeline.helper.TimelineDateFormatter
 
@@ -28,6 +31,7 @@ import im.vector.riotx.features.home.room.detail.timeline.helper.TimelineDateFor
 data class ViewEditHistoryViewState(
         val eventId: String,
         val roomId: String,
+        val isOriginalAReply: Boolean = false,
         val editList: Async<List<Event>> = Uninitialized)
     : MvRxState {
 
@@ -77,11 +81,16 @@ class ViewEditHistoryViewModel @AssistedInject constructor(@Assisted
             override fun onSuccess(data: List<Event>) {
                 //TODO until supported by API Add original event manually
                 val withOriginal = data.toMutableList()
+                var originalIsReply = false
                 room.getTimeLineEvent(eventId)?.let {
                     withOriginal.add(it.root)
+                    originalIsReply = it.root.getClearContent().toModel<MessageContent>().isReply()
                 }
                 setState {
-                    copy(editList = Success(withOriginal))
+                    copy(
+                            editList = Success(withOriginal),
+                            isOriginalAReply = originalIsReply
+                    )
                 }
             }
         })