SES1718 - Message Sending Status (#1462)

* Investigation in progress

* Initial push for PR

* Fixes #1461

* Removed leftover debug comments

* Added minor optimisation to showMessageStatus method (bail early if the message isn't one we care about displaying details of to the user)

* Minor cleanup

* Tiny cleanup

* Addressed PR feedback

* Removed forgotten debug log line & forced delivery status elements to be removed on non-visible messages just in case

* Minor refactor to simplify 'VisibleMessageView.showStatusMessage'

---------

Co-authored-by: alansley <aclansley@gmail.com>

app/src/main/java/org/thoughtcrime/securesms/conversation/disappearingmessages/State.kt

@@ -68,7 +68,7 @@ enum class ExpiryType(
     AFTER_SEND(
         ExpiryMode::AfterSend,
         R.string.expiration_type_disappear_after_send,
-        R.string.expiration_type_disappear_after_read_description,
+        R.string.expiration_type_disappear_after_send_description,
         R.string.AccessibilityId_disappear_after_send_option
     );
 

app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageView.kt

@@ -37,6 +37,7 @@ import org.session.libsession.utilities.ViewUtil
 import org.session.libsession.utilities.getColorFromAttr
 import org.session.libsession.utilities.modifyLayoutParams
 import org.session.libsignal.utilities.IdPrefix
+import org.session.libsignal.utilities.Log
 import org.thoughtcrime.securesms.conversation.v2.ConversationActivityV2
 import org.thoughtcrime.securesms.database.LokiAPIDatabase
 import org.thoughtcrime.securesms.database.LokiThreadDatabase
@@ -206,7 +207,7 @@ class VisibleMessageView : LinearLayout {
         binding.dateBreakTextView.text = if (showDateBreak) DateUtils.getDisplayFormattedTimeSpanString(context, Locale.getDefault(), message.timestamp) else null
         binding.dateBreakTextView.isVisible = showDateBreak
 
-        // Message status indicator
+        // Update message status indicator
         showStatusMessage(message)
 
         // Emoji Reactions
@@ -243,44 +244,101 @@ class VisibleMessageView : LinearLayout {
         onDoubleTap = { binding.messageContentView.root.onContentDoubleTap?.invoke() }
     }
 
+    // Method to display or hide the status of a message.
+    // Note: Although most commonly used to display the delivery status of a message, we also use the
+    // message status area to display the disappearing messages state - so in this latter case we'll
+    // be displaying the "Sent" and the animating clock icon for outgoing messages or "Read" and the
+    // animated clock icon for incoming messages.
     private fun showStatusMessage(message: MessageRecord) {
+        // We'll start by hiding everything and then only make visible what we need
+        binding.messageStatusTextView.isVisible  = false
+        binding.messageStatusImageView.isVisible = false
+        binding.expirationTimerView.isVisible    = false
 
-        val scheduledToDisappear = message.expiresIn > 0
+        // Get details regarding how we should display the message (it's delivery icon, icon tint colour, and
+        // the resource string for what text to display (R.string.delivery_status_sent etc.).
+        val (iconID, iconColor, textId) = getMessageStatusInfo(message)
+
+        // If we get any nulls then a message isn't one with a state that we care about (i.e., control messages
+        // etc.) - so bail. See: `DisplayRecord.is<WHATEVER>` for the full suite of message state methods.
+        // Also: We set all delivery status elements visibility to false just to make sure we don't display any
+        // stale data.
+        if (textId == null) return
 
         binding.messageInnerLayout.modifyLayoutParams<FrameLayout.LayoutParams> {
             gravity = if (message.isOutgoing) Gravity.END else Gravity.START
         }
-
         binding.statusContainer.modifyLayoutParams<ConstraintLayout.LayoutParams> {
             horizontalBias = if (message.isOutgoing) 1f else 0f
         }
 
-        binding.expirationTimerView.isGone = true
+        // If the message is incoming AND it is not scheduled to disappear then don't show any status or timer details
+        val scheduledToDisappear = message.expiresIn > 0
+        if (message.isIncoming && !scheduledToDisappear) return
+
+        // Set text & icons as appropriate for the message state. Note: Possible message states we care
+        // about are: isFailed, isSyncFailed, isPending, isSyncing, isResyncing, isRead, and isSent.
+        textId.let(binding.messageStatusTextView::setText)
+        iconColor?.let(binding.messageStatusTextView::setTextColor)
+        iconID?.let { ContextCompat.getDrawable(context, it) }
+            ?.run { iconColor?.let { mutate().apply { setTint(it) } } ?: this }
+            ?.let(binding.messageStatusImageView::setImageDrawable)
+
+        // Potential options at this point are that the message is:
+        //   i.) incoming AND scheduled to disappear.
+        //   ii.) outgoing but NOT scheduled to disappear, or
+        //   iii.) outgoing AND scheduled to disappear.
+
+        // ----- Case i..) Message is incoming and scheduled to disappear -----
+        if (message.isIncoming && scheduledToDisappear) {
+            // Display the status ('Read') and the show the timer only (no delivery icon)
+            binding.messageStatusTextView.isVisible  = true
+            binding.expirationTimerView.isVisible    = true
+            binding.expirationTimerView.bringToFront()
+            updateExpirationTimer(message)
+            return
+        }
 
-        if (message.isOutgoing || scheduledToDisappear) {
-            val (iconID, iconColor, textId) = getMessageStatusImage(message)
-            textId?.let(binding.messageStatusTextView::setText)
-            iconColor?.let(binding.messageStatusTextView::setTextColor)
-            iconID?.let { ContextCompat.getDrawable(context, it) }
-                ?.run { iconColor?.let { mutate().apply { setTint(it) } } ?: this }
-                ?.let(binding.messageStatusImageView::setImageDrawable)
+        // --- If we got here then we know the message is outgoing ---
 
-            // Always show the delivery status of the last sent message
-            val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
-            val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId)
-            val isLastSentMessage = lastSentMessageId == message.id
+        val thisUsersSessionId = TextSecurePreferences.getLocalNumber(context)
+        val lastSentMessageId = mmsSmsDb.getLastSentMessageFromSender(message.threadId, thisUsersSessionId)
+        val isLastSentMessage = lastSentMessageId == message.id
 
-            binding.messageStatusTextView.isVisible = textId != null && (isLastSentMessage || scheduledToDisappear)
-            val showTimer = scheduledToDisappear && !message.isPending
-            binding.messageStatusImageView.isVisible = iconID != null && !showTimer && (!message.isSent || isLastSentMessage)
+        // ----- Case ii.) Message is outgoing but NOT scheduled to disappear -----
+        if (!scheduledToDisappear) {
+            // If this isn't a disappearing message then we never show the timer
 
-            binding.messageStatusImageView.bringToFront()
-            binding.expirationTimerView.bringToFront()
-            binding.expirationTimerView.isVisible = showTimer
-            if (showTimer) updateExpirationTimer(message)
-        } else {
-            binding.messageStatusTextView.isVisible = false
-            binding.messageStatusImageView.isVisible = false
+            // If the message has NOT been successfully sent then always show the delivery status text and icon..
+            val neitherSentNorRead = !(message.isSent || message.isRead)
+            if (neitherSentNorRead) {
+                binding.messageStatusTextView.isVisible = true
+                binding.messageStatusImageView.isVisible = true
+            } else {
+                // ..but if the message HAS been successfully sent or read then only display the delivery status
+                // text and image if this is the last sent message.
+                binding.messageStatusTextView.isVisible  = isLastSentMessage
+                binding.messageStatusImageView.isVisible = isLastSentMessage
+                if (isLastSentMessage) { binding.messageStatusImageView.bringToFront() }
+            }
+        }
+        else // ----- Case iii.) Message is outgoing AND scheduled to disappear -----
+        {
+            // Always display the delivery status text on all outgoing disappearing messages
+            binding.messageStatusTextView.isVisible = true
+
+            // If the message is sent or has been read..
+            val sentOrRead = message.isSent || message.isRead
+            if (sentOrRead) {
+                // ..then display the timer icon for this disappearing message (but keep the message status icon hidden)
+                binding.expirationTimerView.isVisible = true
+                binding.expirationTimerView.bringToFront()
+                updateExpirationTimer(message)
+            } else {
+                // If the message has NOT been sent or read (or it has failed) then show the delivery status icon rather than the timer icon
+                binding.messageStatusImageView.isVisible = true
+                binding.messageStatusImageView.bringToFront()
+            }
         }
     }
 
@@ -302,10 +360,9 @@ class VisibleMessageView : LinearLayout {
                                  @ColorInt val iconTint: Int?,
                                  @StringRes val messageText: Int?)
 
-    private fun getMessageStatusImage(message: MessageRecord): MessageStatusInfo = when {
+    private fun getMessageStatusInfo(message: MessageRecord): MessageStatusInfo = when {
         message.isFailed ->
-            MessageStatusInfo(
-                R.drawable.ic_delivery_status_failed,
+            MessageStatusInfo(R.drawable.ic_delivery_status_failed,
                 resources.getColor(R.color.destructive, context.theme),
                 R.string.delivery_status_failed
             )
@@ -318,24 +375,32 @@ class VisibleMessageView : LinearLayout {
         message.isPending ->
             MessageStatusInfo(
                 R.drawable.ic_delivery_status_sending,
-                context.getColorFromAttr(R.attr.message_status_color), R.string.delivery_status_sending
+                context.getColorFromAttr(R.attr.message_status_color),
+                R.string.delivery_status_sending
             )
-        message.isResyncing ->
+        message.isSyncing || message.isResyncing ->
             MessageStatusInfo(
                 R.drawable.ic_delivery_status_sending,
-                context.getColor(R.color.accent_orange), R.string.delivery_status_syncing
+                context.getColorFromAttr(R.attr.message_status_color),
+                R.string.delivery_status_sending // We COULD tell the user that we're `syncing` (R.string.delivery_status_syncing) but it will likely make more sense to them if we say "Sending"
             )
-        message.isRead || !message.isOutgoing ->
+        message.isRead || message.isIncoming ->
             MessageStatusInfo(
                 R.drawable.ic_delivery_status_read,
-                context.getColorFromAttr(R.attr.message_status_color), R.string.delivery_status_read
+                context.getColorFromAttr(R.attr.message_status_color),
+                R.string.delivery_status_read
             )
-        else ->
+        message.isSent ->
             MessageStatusInfo(
                 R.drawable.ic_delivery_status_sent,
                 context.getColorFromAttr(R.attr.message_status_color),
                 R.string.delivery_status_sent
             )
+        else -> {
+            // The message isn't one we care about for message statuses we display to the user (i.e.,
+            // control messages etc. - see the  `DisplayRecord.is<WHATEVER>` suite of methods for options).
+            MessageStatusInfo(null, null, null)
+        }
     }
 
     private fun updateExpirationTimer(message: MessageRecord) {

app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java

@@ -114,6 +114,11 @@ public abstract class DisplayRecord {
   public boolean isOutgoing() {
     return MmsSmsColumns.Types.isOutgoingMessageType(type);
   }
+
+  public boolean isIncoming() {
+    return !MmsSmsColumns.Types.isOutgoingMessageType(type);
+  }
+
   public boolean isGroupUpdateMessage() {
     return SmsDatabase.Types.isGroupUpdateMessage(type);
   }