ProtonMail
/
proton-mail-android
mirror of https://github.com/ProtonMail/proton-mail-android
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Separate logic to handle conversation / message loaded actions 

This solves a couple of UI issues:
- Error Toast being shown when opeining a conversation as the first
  returned conversation in the flow has no messages
- Pass conversation data to the view when a conversation is loaded
  (subject, labels etc)

MAILAND-1767
This commit is contained in:
Marino Meneghel 2021-05-21 12:42:25 +02:00
parent a2f08fcedc
commit 3ae34e3b32
3 changed files with 121 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
app/src/main/java/ch/protonmail/android/activities/messageDetails/viewmodel/MessageDetailsViewModel.kt
View File

@ -64,6 +64,7 @@ import ch.protonmail.android.events.DownloadEmbeddedImagesEvent
import ch.protonmail.android.events.Status
import ch.protonmail.android.jobs.helper.EmbeddedImage
import ch.protonmail.android.labels.domain.usecase.MoveMessagesToFolder
import ch.protonmail.android.mailbox.domain.Conversation
import ch.protonmail.android.mailbox.domain.ConversationsRepository
import ch.protonmail.android.mailbox.presentation.ConversationModeEnabled
import ch.protonmail.android.repository.MessageRepository
@ -289,7 +290,7 @@ internal class MessageDetailsViewModel @Inject constructor(
messageRepository.markUnRead(listOf(messageOrConversationId))
}
fun loadMessageDetails() {
fun loadMailboxItemDetails() {
viewModelScope.launch(dispatchers.Io) {
val userId = userManager.requireCurrentUserId()
@ -298,30 +299,53 @@ internal class MessageDetailsViewModel @Inject constructor(
.map { result ->
if (result is DataResult.Success) {
val conversation = result.value
val messages = conversation.messages?.map { message ->
messageRepository.findMessageOnce(userId, message.id)
if (conversation.messages?.isEmpty() == true) {
return@map null
}
Timber.v("Loaded conversation ${conversation.id} with ${messages?.size} messages")
onMessageLoaded(messages.orEmpty())
onConversationLoaded(conversation, userId)
return@map conversation
} else if (result is DataResult.Error) {
Timber.d("Error loading conversation $messageOrConversationId")
onMessageLoaded(emptyList())
_messageDetailsError.postValue(Event("Failed getting conversation details"))
}
}.first()
return@map null
}.first { it?.messages?.isNotEmpty() ?: false }
return@launch
}
val message = messageRepository.getMessage(userId, messageOrConversationId, true)
if (message == null) {
Timber.d("Failed fetching Message Details for message $messageOrConversationId")
_messageDetailsError.postValue(Event("Failed getting message details"))
return@launch
}
onMessageLoaded(listOf(message))
}
}
private suspend fun onMessageLoaded(messages: List<Message?>) {
if (messages.isEmpty() || messages.any { it == null }) {
private suspend fun onConversationLoaded(
conversation: Conversation, userId: Id
) {
val messages = conversation.messages?.mapNotNull { message ->
messageRepository.findMessageOnce(userId, message.id)?.let { localMessage ->
val contactEmail = contactsRepository.findContactEmailByEmail(localMessage.senderEmail)
localMessage.senderDisplayName = contactEmail?.name.orEmpty()
localMessage
}
}
Timber.v("Loaded conversation ${conversation.id} with ${messages?.size} messages")
if (messages.isNullOrEmpty()) {
Timber.d("Failed fetching Message Details for message $messageOrConversationId")
_messageDetailsError.postValue(Event("Failed getting message details"))
_messageDetailsError.postValue(Event("Failed getting conversation's messages"))
return
}
val conversationUiItem = conversationUiItemFrom(conversation, messages)
decryptLastMessageAndEmit(conversationUiItem)
}
private suspend fun onMessageLoaded(messages: List<Message?>) {
val validMessages = messages.filterNotNull()
validMessages.map {
@ -329,24 +353,24 @@ internal class MessageDetailsViewModel @Inject constructor(
it.senderDisplayName = contactEmail?.name.orEmpty()
}
refreshedKeys = true
decryptAndEmit(validMessages)
decryptLastMessageAndEmit(conversationUiItemFrom(validMessages))
}
private suspend fun decryptAndEmit(messages: List<Message>) {
val message = messages.last()
if (!message.isDownloaded) {
private suspend fun decryptLastMessageAndEmit(conversationUiModel: ConversationUiModel) {
refreshedKeys = true
val lastMessage = conversationUiModel.messages.last()
if (!lastMessage.isDownloaded) {
return
}
withContext(dispatchers.Comp) {
message.tryDecrypt(publicKeys) ?: false
lastMessage.tryDecrypt(publicKeys) ?: false
}
Timber.v("Emitting Message Detail = ${message.messageId} keys size: ${publicKeys?.size}")
_decryptedMessageLiveData.postValue(conversationFrom(messages))
Timber.v("Emitting ConversationUiItem Detail = ${lastMessage.messageId} keys size: ${publicKeys?.size}")
_decryptedMessageLiveData.postValue(conversationUiModel)
}
private fun conversationFrom(messages: List<Message?>): ConversationUiModel {
private fun conversationUiItemFrom(messages: List<Message?>): ConversationUiModel {
val message = messages.last()
return ConversationUiModel(
message?.isStarred ?: false,
@ -356,6 +380,14 @@ internal class MessageDetailsViewModel @Inject constructor(
)
}
private fun conversationUiItemFrom(conversation: Conversation, messages: List<Message?>) =
ConversationUiModel(
conversation.labels.any { it.id == STARRED_LABEL_ID },
conversation.subject,
conversation.labels.map { it.id },
messages.filterNotNull(),
)
private fun Message.tryDecrypt(verificationKeys: List<KeyInformation>?): Boolean? {
return try {
decrypt(userManager, userManager.requireCurrentUserId(), verificationKeys)
@ -579,7 +611,7 @@ internal class MessageDetailsViewModel @Inject constructor(
publicKeys = pubKeys
refreshedKeys = false
message?.let { decryptAndEmit(listOf(it)) }
message?.let { decryptLastMessageAndEmit(conversationUiItemFrom(listOf(it))) }
fetchingPubKeys = false
renderedFromCache = AtomicBoolean(false)

6
app/src/main/java/ch/protonmail/android/details/presentation/MessageDetailsActivity.kt
View File

@ -204,7 +204,7 @@ internal class MessageDetailsActivity : BaseStoragePermissionActivity() {
}
private fun continueSetup() {
viewModel.conversationUiModel.observe(this) { viewModel.loadMessageDetails() }
viewModel.conversationUiModel.observe(this) { viewModel.loadMailboxItemDetails() }
viewModel.decryptedMessageData.observe(this, DecryptedMessageObserver())
viewModel.labels
@ -315,7 +315,7 @@ internal class MessageDetailsActivity : BaseStoragePermissionActivity() {
}
private fun onConnectivityCheckRetry() {
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
networkSnackBarUtil.getCheckingConnectionSnackBar(
mSnackLayout,
R.id.messageDetailsActionsView
@ -335,7 +335,7 @@ internal class MessageDetailsActivity : BaseStoragePermissionActivity() {
Timber.v("isConnectionActive:${isConnectionActive.name}")
if (isConnectionActive == Constants.ConnectionState.CONNECTED) {
hideNoConnSnackExtended()
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
} else {
showNoConnSnackExtended(isConnectionActive)
}

85
app/src/test/java/ch/protonmail/android/activities/messageDetails/viewmodel/MessageDetailsViewModelTest.kt
View File

@ -156,14 +156,14 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
}
@Test
fun loadMessageDetailsInvokesMessageRepositoryWithMessageIdAndUserId() = runBlockingTest {
fun loadMailboxItemInvokesMessageRepositoryWithMessageIdAndUserId() = runBlockingTest {
// Given
val userId = Id("userId2")
every { userManager.requireCurrentUserId() } returns userId
coEvery { messageRepository.getMessage(any(), any(), any()) } returns Message()
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
coVerify { messageRepository.getMessage(userId, INPUT_ITEM_DETAIL_ID, true) }
@ -184,7 +184,7 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
coEvery { contactsRepository.findContactEmailByEmail(senderEmail) } returns senderContact
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
val expectedMessage = message.copy()
@ -207,7 +207,7 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
coEvery { messageRepository.getMessage(any(), any(), any()) } returns message
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
assertEquals(emptyList(), messageObserver.observedValues)
@ -220,14 +220,14 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
coEvery { messageRepository.getMessage(any(), any(), any()) } returns null
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
assertEquals("Failed getting message details", messageErrorObserver.observedValues[0]?.getContentIfNotHandled())
}
@Test
fun loadMessageDetailsInvokesConversationRepositoryWithConversationIdAndUserIdWhenConversationModeIsEnabled() =
fun loadMailboxItemInvokesConversationRepositoryWithConversationIdAndUserIdWhenConversationModeIsEnabled() =
runBlockingTest {
// Given
val inputMessageLocation = INBOX
@ -241,7 +241,7 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
inputMessageLocation.messageLocationTypeValue
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
coVerify(exactly = 0) { messageRepository.getMessage(any(), any()) }
@ -249,7 +249,7 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
}
@Test
fun loadMessageDetailsEmitsConversationUiItemWithConversationDataWhenRepositoryReturnsAConversation() =
fun loadMailboxItemEmitsConversationUiItemWithConversationDataWhenRepositoryReturnsAConversation() =
runBlockingTest {
// Given
val inputMessageLocation = INBOX
@ -283,21 +283,21 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
coEvery { conversationRepository.getConversation(inputConversationId, userId) } returns
flowOf(DataResult.Success(ResponseSource.Local, buildConversation(conversationId)))
// Return the same message from DB for simplicity
coEvery { messageRepository.getMessage(userId, "messageId4", true) } returns conversationMessage
coEvery { messageRepository.getMessage(userId, "messageId5", true) } returns conversationMessage
coEvery { messageRepository.findMessageOnce(userId, "messageId4") } returns conversationMessage
coEvery { messageRepository.findMessageOnce(userId, "messageId5") } returns conversationMessage
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
val conversationUiModel = ConversationUiModel(
false, "subject4", listOf("1", "2"), listOf(conversationMessage, conversationMessage)
false, "Conversation subject", listOf("0"), listOf(conversationMessage, conversationMessage)
)
assertEquals(conversationUiModel, conversationObserver.observedValues[0])
}
@Test
fun loadMessageDetailsEmitsErrorWhenConversationIsEnabledAndConversationRepositorySucceedsButMessageIsNotInDatabase() =
fun loadMailboxItemEmitsErrorWhenConversationIsEnabledAndConversationRepositorySucceedsButMessageIsNotInDatabase() =
runBlockingTest {
// Given
val inputMessageLocation = INBOX
@ -313,17 +313,19 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
inputMessageLocation.messageLocationTypeValue
coEvery { conversationRepository.getConversation(inputConversationId, userId) } returns
flowOf(DataResult.Success(ResponseSource.Local, buildConversation(conversationId)))
coEvery { messageRepository.getMessage(userId, any(), true) } returns null
coEvery { messageRepository.findMessageOnce(userId, any()) } returns null
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
assertEquals("Failed getting message details", errorObserver.observedValues[0]?.getContentIfNotHandled())
assertEquals(
"Failed getting conversation's messages", errorObserver.observedValues[0]?.getContentIfNotHandled()
)
}
@Test
fun loadMessageDetailsEmitsErrorWhenConversationIsEnabledAndConversationRepositoryFails() =
fun loadMailboxItemEmitsErrorWhenConversationIsEnabledAndConversationRepositoryFails() =
runBlockingTest {
// Given
val inputMessageLocation = INBOX
@ -340,12 +342,57 @@ class MessageDetailsViewModelTest : ArchTest, CoroutinesTest {
flowOf(DataResult.Error.Local("failed getting conversation", null))
// When
viewModel.loadMessageDetails()
viewModel.loadMailboxItemDetails()
// Then
assertEquals("Failed getting message details", errorObserver.observedValues[0]?.getContentIfNotHandled())
assertEquals(
"Failed getting conversation details", errorObserver.observedValues[0]?.getContentIfNotHandled()
)
}
@Test
fun loadMailboxItemIgnoresConversationsWithNoMessagesReturnedByTheRepository() =
runBlockingTest {
// Given
val inputMessageLocation = INBOX
// messageId is defined as a field as it's needed at VM's instantiation time.
val inputConversationId = INPUT_ITEM_DETAIL_ID
val userId = Id("userId4")
val errorObserver = viewModel.messageDetailsError.testObserver()
val conversationId = UUID.randomUUID().toString()
every { userManager.requireCurrentUserId() } returns userId
coEvery { conversationModeEnabled(inputMessageLocation) } returns true
every { savedStateHandle.get<String>(EXTRA_MESSAGE_OR_CONVERSATION_ID) } returns inputConversationId
every { savedStateHandle.get<Int>(EXTRA_MESSAGE_LOCATION_ID) } returns
inputMessageLocation.messageLocationTypeValue
coEvery { conversationRepository.getConversation(inputConversationId, userId) } returns flowOf(
DataResult.Success(ResponseSource.Local, buildEmptyConversation(conversationId)),
DataResult.Success(ResponseSource.Remote, buildConversation(conversationId))
)
coEvery { messageRepository.findMessageOnce(userId, any()) } returns Message()
// When
viewModel.loadMailboxItemDetails()
// Then
assertEquals(emptyList(), errorObserver.observedValues)
// Called twice as the second returned conversation has two messages
coVerify(exactly = 2) { messageRepository.findMessageOnce(userId, any()) }
}
private fun buildEmptyConversation(conversationId: String) = Conversation(
conversationId,
"Conversation with no messages subject",
emptyList(),
emptyList(),
0,
0,
0,
0,
emptyList(),
emptyList()
)
private fun buildConversation(conversationId: String): Conversation {
val messageId = "messageId4"
val secondMessageId = "messageId5"