From 4ac35f2be3ca75ed0129ab5a61b1c3ae7699d9c5 Mon Sep 17 00:00:00 2001 From: ThomasSession Date: Mon, 17 Mar 2025 08:48:04 +1030 Subject: [PATCH] QA Fix ups (#1020) * Making sure we do not crash when trying to open a document that hasn't yet been downloaded * Do not attempt to open a doc marked as "in progress" of download * SES-3510 - Removed debug toasts * SES-3516 - show 'failed to send status' in message details * SES-3515 getting current user for outgoing message details --- .../conversation/v2/MessageDetailActivity.kt | 84 +++++++++++--- .../v2/MessageDetailsViewModel.kt | 107 ++++++++++++------ .../v2/messages/VisibleMessageContentView.kt | 38 ++++--- .../securesms/ui/theme/Dimensions.kt | 1 + .../securesms/webrtc/WebRtcCallBridge.kt | 12 +- 5 files changed, 162 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailActivity.kt index 6b46a1dc29..afbfa6770b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailActivity.kt @@ -7,7 +7,10 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.MotionEvent.ACTION_UP import androidx.activity.viewModels +import androidx.annotation.DrawableRes +import androidx.appcompat.content.res.AppCompatResources import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.Image import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -20,6 +23,7 @@ import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.aspectRatio import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width @@ -41,6 +45,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource @@ -52,6 +57,7 @@ import androidx.compose.ui.viewinterop.AndroidView import androidx.lifecycle.lifecycleScope import com.bumptech.glide.integration.compose.ExperimentalGlideComposeApi import com.bumptech.glide.integration.compose.GlideImage +import com.google.accompanist.drawablepainter.rememberDrawablePainter import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject import kotlinx.coroutines.launch @@ -170,24 +176,35 @@ fun MessageDetails( verticalArrangement = Arrangement.spacedBy(LocalDimensions.current.smallSpacing) ) { state.record?.let { message -> - AndroidView( - modifier = Modifier.padding(horizontal = LocalDimensions.current.spacing), - factory = { - ViewVisibleMessageContentBinding.inflate(LayoutInflater.from(it)).mainContainerConstraint.apply { - bind( - message, - thread = state.thread!!, - onAttachmentNeedsDownload = onAttachmentNeedsDownload, - suppressThumbnails = true - ) - - setOnTouchListener { _, event -> - if (event.actionMasked == ACTION_UP) onContentClick(event) - true + Column( + modifier = Modifier.padding(horizontal = LocalDimensions.current.spacing) + ) { + AndroidView( + factory = { + ViewVisibleMessageContentBinding.inflate(LayoutInflater.from(it)).mainContainerConstraint.apply { + bind( + message, + thread = state.thread!!, + onAttachmentNeedsDownload = onAttachmentNeedsDownload, + suppressThumbnails = true + ) + + setOnTouchListener { _, event -> + if (event.actionMasked == ACTION_UP) onContentClick(event) + true + } } } + ) + + state.status?.let { + Spacer(modifier = Modifier.height(LocalDimensions.current.xxsSpacing)) + MessageStatus( + modifier = Modifier.padding(horizontal = 2.dp), + status = it + ) } - ) + } } Carousel(state.imageAttachments) { onClickImage(it) } state.nonImageAttachmentFileDetails?.let { FileDetails(it) } @@ -202,6 +219,43 @@ fun MessageDetails( } } +@Composable +fun MessageStatus( + status: MessageStatus, + modifier: Modifier = Modifier +){ + val color = if(status.errorStatus) LocalColors.current.danger else LocalColors.current.text + Row( + modifier = modifier, + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(LocalDimensions.current.xxsSpacing) + ) { + Image( + modifier = Modifier.size(LocalDimensions.current.iconXSmall), + painter = painterResource(id = status.icon), + colorFilter = ColorFilter.tint(color), + contentDescription = null, + ) + + Text( + text = status.title, + style = LocalType.current.extraSmall.copy(color = color) + ) + } +} + +@Preview +@Composable +fun PreviewStatus(){ + PreviewTheme { + MessageStatus( + "Failed to send", + R.drawable.ic_triangle_alert, + errorStatus = true + ) + } +} + @Composable fun CellMetadata( state: MessageDetailsState, diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt index acf12a869e..9f4a500e7b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/MessageDetailsViewModel.kt @@ -1,6 +1,7 @@ package org.thoughtcrime.securesms.conversation.v2 import android.net.Uri +import androidx.annotation.DrawableRes import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -22,6 +23,8 @@ import org.session.libsession.messaging.jobs.AttachmentDownloadJob import org.session.libsession.messaging.jobs.JobQueue import org.session.libsession.messaging.sending_receiving.attachments.AttachmentTransferProgress import org.session.libsession.messaging.sending_receiving.attachments.DatabaseAttachment +import org.session.libsession.utilities.Address +import org.session.libsession.utilities.TextSecurePreferences import org.session.libsession.utilities.Util import org.session.libsession.utilities.recipients.Recipient import org.thoughtcrime.securesms.ApplicationContext @@ -40,6 +43,7 @@ import org.thoughtcrime.securesms.ui.TitledText @HiltViewModel class MessageDetailsViewModel @Inject constructor( + private val prefs: TextSecurePreferences, private val attachmentDb: AttachmentDatabase, private val lokiMessageDatabase: LokiMessageDatabase, private val mmsSmsDatabase: MmsSmsDatabase, @@ -77,43 +81,65 @@ class MessageDetailsViewModel @Inject constructor( .collect { event.send(Event.Finish) } } - state.value = messageRecord.run { - val slides = mmsRecord?.slideDeck?.slides ?: emptyList() - - val recipient = threadDb.getRecipientForThreadId(threadId)!! - val isDeprecatedLegacyGroup = recipient.isLegacyGroupRecipient && - deprecationManager.isDeprecated - - - val errorString = lokiMessageDatabase.getErrorMessage(id) - - MessageDetailsState( - attachments = slides.map(::Attachment), - record = messageRecord, - - // Set the "Sent" message info TitledText appropriately - sent = if (messageRecord.isSending && errorString == null) { - val sendingWithEllipsisString = context.getString(R.string.sending) + ellipsis // e.g., "Sending…" - TitledText(sendingWithEllipsisString, null) - } else if (messageRecord.isSent && errorString == null) { - dateReceived.let(::Date).toString().let { TitledText(R.string.sent, it) } - } else { - null // Not sending or sent? Don't display anything for the "Sent" element. - }, - - // Set the "Received" message info TitledText appropriately - received = if (messageRecord.isIncoming && errorString == null) { - dateReceived.let(::Date).toString().let { TitledText(R.string.received, it) } - } else { - null // Not incoming? Then don't display anything for the "Received" element. - }, - - error = errorString?.let { TitledText(context.getString(R.string.theError) + ":", it) }, - senderInfo = individualRecipient.run { TitledText(name, address.toString()) }, - sender = individualRecipient, - thread = recipient, - readOnly = isDeprecatedLegacyGroup - ) + viewModelScope.launch { + state.value = messageRecord.run { + val slides = mmsRecord?.slideDeck?.slides ?: emptyList() + + val conversation = threadDb.getRecipientForThreadId(threadId)!! + val isDeprecatedLegacyGroup = conversation.isLegacyGroupRecipient && + deprecationManager.isDeprecated + + + val errorString = lokiMessageDatabase.getErrorMessage(id) + + var status: MessageStatus? = null + // create a 'failed to send' status if appropriate + if(messageRecord.isFailed){ + status = MessageStatus( + title = context.getString(R.string.messageStatusFailedToSend), + icon = R.drawable.ic_triangle_alert, + errorStatus = true + ) + } + + val sender = if(messageRecord.isOutgoing){ + Recipient.from(context, Address.fromSerialized(prefs.getLocalNumber() ?: ""), false) + } else individualRecipient + + MessageDetailsState( + attachments = slides.map(::Attachment), + record = messageRecord, + + // Set the "Sent" message info TitledText appropriately + sent = if (messageRecord.isSending && errorString == null) { + val sendingWithEllipsisString = context.getString(R.string.sending) + ellipsis // e.g., "Sending…" + TitledText(sendingWithEllipsisString, null) + } else if (messageRecord.isSent && errorString == null) { + dateReceived.let(::Date).toString().let { TitledText(R.string.sent, it) } + } else { + null // Not sending or sent? Don't display anything for the "Sent" element. + }, + + // Set the "Received" message info TitledText appropriately + received = if (messageRecord.isIncoming && errorString == null) { + dateReceived.let(::Date).toString().let { TitledText(R.string.received, it) } + } else { + null // Not incoming? Then don't display anything for the "Received" element. + }, + + error = errorString?.let { TitledText(context.getString(R.string.theError) + ":", it) }, + status = status, + senderInfo = sender.run { + TitledText( + if(messageRecord.isOutgoing) context.getString(R.string.you) else name, + address.toString() + ) + }, + sender = sender, + thread = conversation, + readOnly = isDeprecatedLegacyGroup + ) + } } } @@ -181,6 +207,7 @@ data class MessageDetailsState( val sent: TitledText? = null, val received: TitledText? = null, val error: TitledText? = null, + val status: MessageStatus? = null, val senderInfo: TitledText? = null, val sender: Recipient? = null, val thread: Recipient? = null, @@ -198,6 +225,12 @@ data class Attachment( val hasImage: Boolean ) +data class MessageStatus( + val title: String, + @DrawableRes val icon: Int, + val errorStatus: Boolean +) + sealed class Event { object Finish: Event() data class StartMediaPreview(val args: MediaPreviewArgs): Event() diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageContentView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageContentView.kt index f9b6ca31c9..3f336a9be1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageContentView.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/messages/VisibleMessageContentView.kt @@ -191,24 +191,26 @@ class VisibleMessageContentView : ConstraintLayout { if (mediaDownloaded || mediaInProgress || message.isOutgoing) { binding.documentView.root.bind(message, getTextColor(context, message)) message.slideDeck.documentSlide?.let { slide -> - onContentClick.add { - // open the document when tapping it - val intent = Intent(Intent.ACTION_VIEW) - intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) - intent.setDataAndType( - PartAuthority.getAttachmentPublicUri(slide.uri), - slide.contentType - ) - - try { - context.startActivity(intent) - } catch (e: ActivityNotFoundException) { - Log.e("VisibleMessageContentView", "Error opening document", e) - Toast.makeText( - context, - R.string.attachmentsErrorOpen, - Toast.LENGTH_LONG - ).show() + if(!mediaInProgress) { // do not attempt to open a doc in progress of downloading + onContentClick.add { + // open the document when tapping it + try { + val intent = Intent(Intent.ACTION_VIEW) + intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION) + intent.setDataAndType( + PartAuthority.getAttachmentPublicUri(slide.uri), + slide.contentType + ) + + context.startActivity(intent) + } catch (e: ActivityNotFoundException) { + Log.e("VisibleMessageContentView", "Error opening document", e) + Toast.makeText( + context, + R.string.attachmentsErrorOpen, + Toast.LENGTH_LONG + ).show() + } } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/ui/theme/Dimensions.kt b/app/src/main/java/org/thoughtcrime/securesms/ui/theme/Dimensions.kt index 57e5eb0945..f342bc7d9e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ui/theme/Dimensions.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ui/theme/Dimensions.kt @@ -24,6 +24,7 @@ data class Dimensions( val borderStroke: Dp = 1.dp, val badgeSize: Dp = 20.dp, + val iconXSmall: Dp = 14.dp, val iconMedium: Dp = 24.dp, val iconLarge: Dp = 46.dp, val iconXLarge: Dp = 60.dp, diff --git a/app/src/main/java/org/thoughtcrime/securesms/webrtc/WebRtcCallBridge.kt b/app/src/main/java/org/thoughtcrime/securesms/webrtc/WebRtcCallBridge.kt index f34a1eee86..255cda2cdd 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/webrtc/WebRtcCallBridge.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/webrtc/WebRtcCallBridge.kt @@ -190,7 +190,7 @@ class WebRtcCallBridge @Inject constructor( val recipient = getRecipientFromAddress(address) if (isIncomingMessageExpired(callTime)) { - debugToast("Pre offer expired - message timestamp was deemed expired: ${System.currentTimeMillis() - callTime}s") + Log.d(TAG, "Pre offer expired - message timestamp was deemed expired: ${System.currentTimeMillis() - callTime}s") insertMissedCall(recipient, true) terminate() return@execute @@ -211,14 +211,6 @@ class WebRtcCallBridge @Inject constructor( } } - fun debugToast(message: String) { - if (BuildConfig.BUILD_TYPE != "release") { - ContextCompat.getMainExecutor(context).execute { - Toast.makeText(context, message, Toast.LENGTH_LONG).show() - } - } - } - private fun handleIncomingPreOffer(address: Address, sdp: String, callId: UUID, callTime: Long) { serviceExecutor.execute { val recipient = getRecipientFromAddress(address) @@ -327,7 +319,7 @@ class WebRtcCallBridge @Inject constructor( if (isIncomingMessageExpired(timestamp)) { val didHangup = callManager.postConnectionEvent(Event.TimeOut) { - debugToast("Answer expired - message timestamp was deemed expired: ${System.currentTimeMillis() - timestamp}s") + Log.d(TAG, "Answer expired - message timestamp was deemed expired: ${System.currentTimeMillis() - timestamp}s") insertMissedCall( recipient, true