Merge pull request #1204 from RocketChat/fix/reply-with-username

[FIX] Use username when building replies always

app/build.gradle

@@ -104,7 +104,6 @@ dependencies {
     implementation libraries.frescoImageViewer
 
     implementation libraries.markwon
-    implementation libraries.markwonImageLoader
 
     implementation libraries.sheetMenu
 

app/src/main/java/chat/rocket/android/chatroom/adapter/ChatRoomAdapter.kt

@@ -14,11 +14,11 @@ import timber.log.Timber
 import java.security.InvalidParameterException
 
 class ChatRoomAdapter(
-        private val roomType: String,
-        private val roomName: String,
-        private val presenter: ChatRoomPresenter?,
-        private val enableActions: Boolean = true,
-        private val reactionListener: EmojiReactionListener? = null
+    private val roomType: String,
+    private val roomName: String,
+    private val presenter: ChatRoomPresenter?,
+    private val enableActions: Boolean = true,
+    private val reactionListener: EmojiReactionListener? = null
 ) : RecyclerView.Adapter<BaseViewHolder<*>>() {
 
     private val dataSet = ArrayList<BaseViewModel<*>>()
@@ -175,15 +175,15 @@ class ChatRoomAdapter(
         }
     }
 
-    val actionsListener = object : BaseViewHolder.ActionsListener {
+    private val actionsListener = object : BaseViewHolder.ActionsListener {
         override fun isActionsEnabled(): Boolean = enableActions
 
         override fun onActionSelected(item: MenuItem, message: Message) {
             message.apply {
                 when (item.itemId) {
                     R.id.action_menu_msg_delete -> presenter?.deleteMessage(roomId, id)
-                    R.id.action_menu_msg_quote -> presenter?.citeMessage(roomType, roomName, id, false)
-                    R.id.action_menu_msg_reply -> presenter?.citeMessage(roomType, roomName, id, true)
+                    R.id.action_menu_msg_quote -> presenter?.citeMessage(roomType, id, false)
+                    R.id.action_menu_msg_reply -> presenter?.citeMessage(roomType, id, true)
                     R.id.action_menu_msg_copy -> presenter?.copyMessage(id)
                     R.id.action_menu_msg_edit -> presenter?.editMessage(roomId, id, message.message)
                     R.id.action_menu_msg_pin_unpin -> {

app/src/main/java/chat/rocket/android/chatroom/presentation/ChatRoomPresenter.kt

@@ -31,7 +31,6 @@ import chat.rocket.core.internal.rest.*
 import chat.rocket.core.model.Command
 import chat.rocket.core.model.Message
 import chat.rocket.core.model.Myself
-import chat.rocket.core.model.Value
 import kotlinx.coroutines.experimental.CommonPool
 import kotlinx.coroutines.experimental.android.UI
 import kotlinx.coroutines.experimental.async
@@ -59,10 +58,11 @@ class ChatRoomPresenter @Inject constructor(
     private val mapper: ViewModelMapper,
     private val jobSchedulerInteractor: JobSchedulerInteractor
 ) {
+
     private val currentServer = serverInteractor.get()!!
     private val manager = factory.create(currentServer)
     private val client = manager.client
-    private var settings: Map<String, Value<Any>> = getSettingsInteractor.get(serverInteractor.get()!!)
+    private var settings: PublicSettings = getSettingsInteractor.get(serverInteractor.get()!!)
     private val messagesChannel = Channel<Message>()
 
     private var chatRoomId: String? = null
@@ -194,7 +194,7 @@ class ChatRoomPresenter @Inject constructor(
                 }
             } catch (ex: Exception) {
                 Timber.d(ex, "Error uploading file")
-                when(ex) {
+                when (ex) {
                     is RocketChatException -> view.showMessage(ex)
                     else -> view.showGenericErrorMessage()
                 }
@@ -279,7 +279,6 @@ class ChatRoomPresenter @Inject constructor(
                             // TODO - we need to better treat connection problems here, but no let gaps
                             // on the messages list
                             Timber.d(ex, "Error fetching channel history")
-                            ex.printStackTrace()
                         }
                     }
             }
@@ -326,43 +325,37 @@ class ChatRoomPresenter @Inject constructor(
      * Quote or reply a message.
      *
      * @param roomType The current room type.
-     * @param roomName The name of the current room.
      * @param messageId The id of the message to make citation for.
      * @param mentionAuthor true means the citation is a reply otherwise it's a quote.
      */
-    fun citeMessage(roomType: String, roomName: String, messageId: String, mentionAuthor: Boolean) {
+    fun citeMessage(roomType: String, messageId: String, mentionAuthor: Boolean) {
         launchUI(strategy) {
             val message = messagesRepository.getById(messageId)
             val me: Myself? = try {
                 retryIO("me()") { client.me() } //TODO: Cache this and use an interactor
             } catch (ex: Exception) {
-                Timber.d(ex, "Error getting myself info.")
-                ex.printStackTrace()
+                Timber.e(ex)
                 null
             }
-            message?.let { m ->
-                val id = m.id
-                val username = m.sender?.username
-                val user = "@" + if (settings.useRealName()) m.sender?.name
-                    ?: m.sender?.username else m.sender?.username
-                val mention = if (mentionAuthor && me?.username != username) user else ""
-                val type = roomTypeOf(roomType)
-                val room = when (type) {
-                    is RoomType.Channel -> "channel"
-                    is RoomType.DirectMessage -> "direct"
-                    is RoomType.PrivateGroup -> "group"
-                    is RoomType.Livechat -> "livechat"
-                    is RoomType.Custom -> "custom" //TODO: put appropriate callback string here.
-                }
+            message?.let { msg ->
+                val id = msg.id
+                val username = msg.sender?.username ?: ""
+                val mention = if (mentionAuthor && me?.username != username) "@$username" else ""
+                val room = if (roomTypeOf(roomType) is RoomType.DirectMessage) username else roomType
                 view.showReplyingAction(
-                    username = user,
-                    replyMarkdown = "[ ]($currentServer/$room/$roomName?msg=$id) $mention ",
+                    username = getDisplayName(msg.sender),
+                    replyMarkdown = "[ ]($currentServer/$roomType/$room?msg=$id) $mention ",
                     quotedMessage = mapper.map(message).last().preview?.message ?: ""
                 )
             }
         }
     }
 
+    private fun getDisplayName(user: SimpleUser?): String {
+        val username = user?.username ?: ""
+        return if (settings.useRealName()) user?.name ?: "@$username" else "@$username"
+    }
+
     /**
      * Copy message to clipboard.
      *

app/src/main/java/chat/rocket/android/dagger/module/AppModule.kt

@@ -48,10 +48,8 @@ import okhttp3.Interceptor
 import okhttp3.OkHttpClient
 import okhttp3.logging.HttpLoggingInterceptor
 import ru.noties.markwon.SpannableConfiguration
-import ru.noties.markwon.il.AsyncDrawableLoader
 import ru.noties.markwon.spans.SpannableTheme
 import timber.log.Timber
-import java.util.concurrent.Executors
 import java.util.concurrent.TimeUnit
 import javax.inject.Singleton
 
@@ -264,11 +262,6 @@ class AppModule {
     fun provideConfiguration(context: Application, client: OkHttpClient): SpannableConfiguration {
         val res = context.resources
         return SpannableConfiguration.builder(context)
-            .asyncDrawableLoader(AsyncDrawableLoader.builder()
-                .client(client)
-                .executorService(Executors.newCachedThreadPool())
-                .resources(res)
-                .build())
             .theme(SpannableTheme.builder()
                 .linkColor(res.getColor(R.color.colorAccent))
                 .build())
@@ -277,8 +270,9 @@ class AppModule {
 
     @Provides
     @Singleton
-    fun provideMessageParser(context: Application, configuration: SpannableConfiguration): MessageParser {
-        return MessageParser(context, configuration)
+    fun provideMessageParser(context: Application, configuration: SpannableConfiguration, serverInteractor: GetCurrentServerInteractor, settingsInteractor: GetSettingsInteractor): MessageParser {
+        val url = serverInteractor.get()!!
+        return MessageParser(context, configuration, settingsInteractor.get(url))
     }
 
     @Provides

app/src/main/java/chat/rocket/android/helper/MessageParser.kt

 package chat.rocket.android.helper
 
 import android.app.Application
-import android.content.ActivityNotFoundException
 import android.content.Context
-import android.content.Intent
 import android.graphics.Canvas
 import android.graphics.Paint
 import android.graphics.RectF
 import android.net.Uri
 import android.support.customtabs.CustomTabsIntent
-import android.provider.Browser
 import android.support.v4.content.res.ResourcesCompat
 import android.text.Spanned
 import android.text.style.ClickableSpan
@@ -17,6 +14,8 @@ import android.text.style.ReplacementSpan
 import android.util.Patterns
 import android.view.View
 import chat.rocket.android.R
+import chat.rocket.android.server.domain.PublicSettings
+import chat.rocket.android.server.domain.useRealName
 import chat.rocket.android.widget.emoji.EmojiParser
 import chat.rocket.android.widget.emoji.EmojiRepository
 import chat.rocket.android.widget.emoji.EmojiTypefaceSpan
@@ -29,10 +28,13 @@ import ru.noties.markwon.Markwon
 import ru.noties.markwon.SpannableBuilder
 import ru.noties.markwon.SpannableConfiguration
 import ru.noties.markwon.renderer.SpannableMarkdownVisitor
-import timber.log.Timber
 import javax.inject.Inject
 
-class MessageParser @Inject constructor(val context: Application, private val configuration: SpannableConfiguration) {
+class MessageParser @Inject constructor(
+    private val context: Application,
+    private val configuration: SpannableConfiguration,
+    private val settings: PublicSettings
+) {
 
     private val parser = Markwon.createParser()
 
@@ -53,7 +55,7 @@ class MessageParser @Inject constructor(val context: Application, private val co
         parentNode.accept(LinkVisitor(builder))
         parentNode.accept(EmojiVisitor(configuration, builder))
         message.mentions?.let {
-            parentNode.accept(MentionVisitor(context, builder, it, selfUsername))
+            parentNode.accept(MentionVisitor(context, builder, it, selfUsername, settings))
         }
 
         return builder.text()
@@ -62,23 +64,31 @@ class MessageParser @Inject constructor(val context: Application, private val co
     // Convert to a lenient markdown consistent with Rocket.Chat web markdown instead of the official specs.
     private fun toLenientMarkdown(text: String): String {
         return text.trim().replace("\\*(.+)\\*".toRegex()) { "**${it.groupValues[1].trim()}**" }
-                .replace("\\~(.+)\\~".toRegex()) { "~~${it.groupValues[1].trim()}~~" }
-                .replace("\\_(.+)\\_".toRegex()) { "_${it.groupValues[1].trim()}_" }
+            .replace("\\~(.+)\\~".toRegex()) { "~~${it.groupValues[1].trim()}~~" }
+            .replace("\\_(.+)\\_".toRegex()) { "_${it.groupValues[1].trim()}_" }
     }
 
-    class MentionVisitor(context: Context,
-                         private val builder: SpannableBuilder,
-                         private val mentions: List<SimpleUser>,
-                         private val currentUser: String?) : AbstractVisitor() {
+    class MentionVisitor(
+        context: Context,
+        private val builder: SpannableBuilder,
+        private val mentions: List<SimpleUser>,
+        private val currentUser: String?,
+        private val settings: PublicSettings
+    ) : AbstractVisitor() {
+
         private val othersTextColor = ResourcesCompat.getColor(context.resources, R.color.colorAccent, context.theme)
         private val othersBackgroundColor = ResourcesCompat.getColor(context.resources, android.R.color.transparent, context.theme)
         private val myselfTextColor = ResourcesCompat.getColor(context.resources, R.color.white, context.theme)
         private val myselfBackgroundColor = ResourcesCompat.getColor(context.resources, R.color.colorAccent, context.theme)
         private val mentionPadding = context.resources.getDimensionPixelSize(R.dimen.padding_mention).toFloat()
         private val mentionRadius = context.resources.getDimensionPixelSize(R.dimen.radius_mention).toFloat()
+
         override fun visit(t: Text) {
             val text = t.literal
-            val mentionsList = mentions.map { it.username }.toMutableList()
+            val mentionsList = mentions.map {
+                if (settings.useRealName()) it.name else it.username ?: ""
+            }.toMutableList()
+
             mentionsList.add("all")
             mentionsList.add("here")
 
@@ -90,7 +100,7 @@ class MessageParser @Inject constructor(val context: Application, private val co
                         val textColor = if (mentionMe) myselfTextColor else othersTextColor
                         val backgroundColor = if (mentionMe) myselfBackgroundColor else othersBackgroundColor
                         val usernameSpan = MentionSpan(backgroundColor, textColor, mentionRadius, mentionPadding,
-                                mentionMe)
+                            mentionMe)
                         // Add 1 to end offset to include the @.
                         val end = offset + it.length + 1
                         builder.setSpan(usernameSpan, offset, end, 0)
@@ -101,8 +111,11 @@ class MessageParser @Inject constructor(val context: Application, private val co
         }
     }
 
-    class EmojiVisitor(configuration: SpannableConfiguration, private val builder: SpannableBuilder)
-        : SpannableMarkdownVisitor(configuration, builder) {
+    class EmojiVisitor(
+        configuration: SpannableConfiguration,
+        private val builder: SpannableBuilder
+    ) : SpannableMarkdownVisitor(configuration, builder) {
+
         override fun visit(document: Document) {
             val spannable = EmojiParser.parse(builder.text())
             if (spannable is Spanned) {
@@ -127,7 +140,7 @@ class MessageParser @Inject constructor(val context: Application, private val co
                 if (!link.startsWith("@") && link !in consumed) {
                     builder.setSpan(object : ClickableSpan() {
                         override fun onClick(view: View) {
-                            with (view) {
+                            with(view) {
                                 val tabsbuilder = CustomTabsIntent.Builder()
                                 tabsbuilder.setToolbarColor(ResourcesCompat.getColor(context.resources, R.color.colorPrimary, context.theme))
                                 val customTabsIntent = tabsbuilder.build()
@@ -150,11 +163,14 @@ class MessageParser @Inject constructor(val context: Application, private val co
         }
     }
 
-    class MentionSpan(private val backgroundColor: Int,
-                      private val textColor: Int,
-                      private val radius: Float,
-                      padding: Float,
-                      referSelf: Boolean) : ReplacementSpan() {
+    class MentionSpan(
+        private val backgroundColor: Int,
+        private val textColor: Int,
+        private val radius: Float,
+        padding: Float,
+        referSelf: Boolean
+    ) : ReplacementSpan() {
+
         private val padding: Float = if (referSelf) padding else 0F
 
         override fun getSize(paint: Paint,

dependencies.gradle

@@ -91,7 +91,6 @@ ext {
             frescoImageViewer     : "com.github.luciofm:FrescoImageViewer:${versions.frescoImageViewer}",
 
             markwon               : "ru.noties:markwon:${versions.markwon}",
-            markwonImageLoader    : "ru.noties:markwon-image-loader:${versions.markwon}",
 
             sheetMenu             : "com.github.whalemare:sheetmenu:${versions.sheetMenu}",