From 41fbdbd9abb4c3edc2bdbb054152053ff55a650a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Forlini?= Date: Mon, 24 Nov 2025 16:30:15 +0100 Subject: [PATCH] Do not instantiate observers to resolve message/conversation subject When displaying the "More" actions bottomsheet, we show the subject at the top of the component. The information is currently retrieved by observing and capturing the first data event emitted by observeMessage/observeConversation, which instantiate a whole watcher and subsequent callbacks. ET-5458 --- .../MessageDetailHeaderUiModelMapper.kt | 3 +- .../model/MessageDetailHeaderUiModel.kt | 1 + .../MessageDetailHeaderPreviewData.kt | 3 +- .../MessageDetailHeaderUiModelSample.kt | 3 +- .../usecase/GetMoreActionsBottomSheetData.kt | 78 ++++++------- .../viewmodel/ConversationDetailViewModel.kt | 26 +++-- .../MessageDetailHeaderUiModelMapperTest.kt | 11 +- .../reducer/BottomSheetReducerTest.kt | 1 - .../GetMoreActionsBottomSheetDataTest.kt | 108 ++++++++---------- ...versationDetailViewModelIntegrationTest.kt | 52 +-------- .../model/bottomsheet/BottomSheetStates.kt | 1 - ...DetailMoreActionsBottomSheetReducerTest.kt | 7 -- .../MessageDetailHeaderUiModelTestData.kt | 3 +- 13 files changed, 123 insertions(+), 174 deletions(-) diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapper.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapper.kt index 2b3d9ad489..7f90feaff7 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapper.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapper.kt @@ -94,7 +94,8 @@ class MessageDetailHeaderUiModelMapper @Inject constructor( messageIdUiModel = toMessageUiModel(message.messageId), themeOverride = viewModePreference.toThemeOverride(), shouldShowQuickReply = message.isReplyAllowed, - hasMoreActions = hasMoreActionsEnabled + hasMoreActions = hasMoreActionsEnabled, + subject = message.subject ) } diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/model/MessageDetailHeaderUiModel.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/model/MessageDetailHeaderUiModel.kt index 125458856c..3cc9ef9ff7 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/model/MessageDetailHeaderUiModel.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/model/MessageDetailHeaderUiModel.kt @@ -32,6 +32,7 @@ data class MessageDetailHeaderUiModel( val avatar: AvatarUiModel, val avatarImage: AvatarImageUiModel, val sender: ParticipantUiModel, + val subject: String, val shouldShowTrackerProtectionIcon: Boolean, val shouldShowAttachmentIcon: Boolean, val shouldShowStar: Boolean, diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/previewdata/MessageDetailHeaderPreviewData.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/previewdata/MessageDetailHeaderPreviewData.kt index 932d3ecf69..7cd52d3bd4 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/previewdata/MessageDetailHeaderPreviewData.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/previewdata/MessageDetailHeaderPreviewData.kt @@ -93,7 +93,8 @@ object MessageDetailHeaderPreviewData { messageIdUiModel = MessageIdUiModel("string"), themeOverride = null, shouldShowQuickReply = true, - hasMoreActions = true + hasMoreActions = true, + subject = "A subject" ) val WithLabels = WithoutLabels.copy( diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/sample/MessageDetailHeaderUiModelSample.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/sample/MessageDetailHeaderUiModelSample.kt index 13dff790f4..3c16d02b09 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/sample/MessageDetailHeaderUiModelSample.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/sample/MessageDetailHeaderUiModelSample.kt @@ -65,6 +65,7 @@ object MessageDetailHeaderUiModelSample { messageIdUiModel = MessageIdUiModel("id"), themeOverride = null, shouldShowQuickReply = true, - hasMoreActions = true + hasMoreActions = true, + subject = "A subject" ) } diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetData.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetData.kt index 414de78829..04701f6dd3 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetData.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetData.kt @@ -20,71 +20,65 @@ package ch.protonmail.android.maildetail.presentation.usecase import ch.protonmail.android.mailcommon.domain.model.Action import ch.protonmail.android.mailcommon.domain.model.ConversationId -import ch.protonmail.android.mailconversation.domain.entity.ConversationDetailEntryPoint import ch.protonmail.android.mailconversation.domain.usecase.GetConversationAvailableActions -import ch.protonmail.android.mailconversation.domain.usecase.ObserveConversation import ch.protonmail.android.maildetail.presentation.model.MoreActionsBottomSheetEntryPoint import ch.protonmail.android.maillabel.domain.model.LabelId import ch.protonmail.android.mailmessage.domain.model.MessageId import ch.protonmail.android.mailmessage.domain.model.MessageThemeOptions import ch.protonmail.android.mailmessage.domain.usecase.GetMessageAvailableActions -import ch.protonmail.android.mailmessage.domain.usecase.ObserveMessage import ch.protonmail.android.mailmessage.presentation.model.bottomsheet.DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent -import kotlinx.coroutines.flow.firstOrNull import me.proton.core.domain.entity.UserId import javax.inject.Inject class GetMoreActionsBottomSheetData @Inject constructor( private val getMessageAvailableActions: GetMessageAvailableActions, - private val getConversationAvailableActions: GetConversationAvailableActions, - private val observeMessage: ObserveMessage, - private val observeConversation: ObserveConversation + private val getConversationAvailableActions: GetConversationAvailableActions ) { - suspend fun forMessage( - userId: UserId, - labelId: LabelId, - messageId: MessageId, - messageThemeOptions: MessageThemeOptions, - entryPoint: MoreActionsBottomSheetEntryPoint + internal suspend fun forMessage( + payload: MoreMessageActionsBottomSheetDataPayload ): DetailMoreActionsBottomSheetEvent.DataLoaded? = getMessageAvailableActions( - userId, labelId, messageId, messageThemeOptions + payload.userId, payload.labelId, payload.messageId, payload.messageThemeOptions ).map { availableActions -> - val message = observeMessage(userId, messageId).firstOrNull()?.getOrNull() - ?: return null - val requestIsFromBottomBar = entryPoint is MoreActionsBottomSheetEntryPoint.BottomBar + val requestIsFromBottomBar = payload.entryPoint is MoreActionsBottomSheetEntryPoint.BottomBar return DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = message.sender.name, - messageSubject = message.subject, - messageIdInConversation = message.messageId.id, + messageSubject = payload.subject, + messageIdInConversation = payload.messageId.id, availableActions = availableActions, customizeToolbarAction = if (requestIsFromBottomBar) Action.CustomizeToolbar else null ) }.getOrNull() - suspend fun forConversation( - userId: UserId, - labelId: LabelId, - conversationId: ConversationId, - entryPoint: ConversationDetailEntryPoint, - showAllMessages: Boolean - ): DetailMoreActionsBottomSheetEvent.DataLoaded? = - getConversationAvailableActions(userId, labelId, conversationId).map { availableActions -> - val conversation = observeConversation(userId, conversationId, labelId, entryPoint, showAllMessages) - .firstOrNull() - ?.getOrNull() - ?: return null - - return DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = conversation.senders.first().name, - messageSubject = conversation.subject, - messageIdInConversation = null, - availableActions = availableActions, - customizeToolbarAction = Action.CustomizeToolbar - ) - }.getOrNull() - + internal suspend fun forConversation( + payload: MoreConversationActionsBottomSheetDataPayload + ): DetailMoreActionsBottomSheetEvent.DataLoaded? = getConversationAvailableActions( + payload.userId, + payload.labelId, + payload.conversationId + ).map { availableActions -> + return DetailMoreActionsBottomSheetEvent.DataLoaded( + messageSubject = payload.subject, + messageIdInConversation = null, + availableActions = availableActions, + customizeToolbarAction = Action.CustomizeToolbar + ) + }.getOrNull() } +internal data class MoreMessageActionsBottomSheetDataPayload( + val userId: UserId, + val labelId: LabelId, + val messageId: MessageId, + val messageThemeOptions: MessageThemeOptions, + val entryPoint: MoreActionsBottomSheetEntryPoint, + val subject: String +) + +internal data class MoreConversationActionsBottomSheetDataPayload( + val userId: UserId, + val labelId: LabelId, + val conversationId: ConversationId, + val subject: String +) diff --git a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModel.kt b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModel.kt index 126946301c..23ccf1caf4 100644 --- a/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModel.kt +++ b/mail-detail/presentation/src/main/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModel.kt @@ -96,6 +96,8 @@ import ch.protonmail.android.maildetail.presentation.reducer.ConversationDetailR import ch.protonmail.android.maildetail.presentation.usecase.GetMessagesInSameExclusiveLocation import ch.protonmail.android.maildetail.presentation.usecase.GetMoreActionsBottomSheetData import ch.protonmail.android.maildetail.presentation.usecase.LoadImageAvoidDuplicatedExecution +import ch.protonmail.android.maildetail.presentation.usecase.MoreConversationActionsBottomSheetDataPayload +import ch.protonmail.android.maildetail.presentation.usecase.MoreMessageActionsBottomSheetDataPayload import ch.protonmail.android.maildetail.presentation.usecase.ObservePrimaryUserAddress import ch.protonmail.android.maildetail.presentation.usecase.print.PrintConfiguration import ch.protonmail.android.maildetail.presentation.usecase.print.PrintMessage @@ -535,6 +537,7 @@ class ConversationDetailViewModel @AssistedInject constructor( emitNewStateFrom(ConversationDetailEvent.ErrorLoadingImageProxyFailed(messageId)) null } + is AttachmentDataError.Other -> null } }, @@ -1041,14 +1044,16 @@ class ConversationDetailViewModel @AssistedInject constructor( val userId = primaryUserId.first() val labelId = openedFromLocation - val moreActions = getMoreActionsBottomSheetData.forMessage( + val bottomSheetDataPayload = MoreMessageActionsBottomSheetDataPayload( userId, labelId, initialEvent.messageId, initialEvent.themeOptions, - initialEvent.entryPoint + initialEvent.entryPoint, + resolveSubject() ?: "" ) - ?: return + + val moreActions = getMoreActionsBottomSheetData.forMessage(bottomSheetDataPayload) ?: return emitNewStateFrom(ConversationDetailEvent.ConversationBottomSheetEvent(moreActions)) } @@ -1076,14 +1081,15 @@ class ConversationDetailViewModel @AssistedInject constructor( val userId = primaryUserId.first() val labelId = openedFromLocation - val moreActions = getMoreActionsBottomSheetData.forConversation( + + val bottomSheetDataPayload = MoreConversationActionsBottomSheetDataPayload( userId, labelId, conversationId, - conversationEntryPoint, - showAllMessages.value - ) ?: return + resolveSubject() ?: "" + ) + val moreActions = getMoreActionsBottomSheetData.forConversation(bottomSheetDataPayload) ?: return emitNewStateFrom(ConversationDetailEvent.ConversationBottomSheetEvent(moreActions)) } @@ -1729,6 +1735,12 @@ class ConversationDetailViewModel @AssistedInject constructor( return label == SystemLabelId.AllMail } + private fun resolveSubject(): String? { + return (state.value.conversationState as? ConversationDetailMetadataState.Data) + ?.conversationUiModel + ?.subject + } + private suspend fun withUserId(block: suspend (userId: UserId) -> Either<*, *>) { val userId = primaryUserId.first() block(userId) diff --git a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapperTest.kt b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapperTest.kt index 8b0fc20e89..219529cbda 100644 --- a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapperTest.kt +++ b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/mapper/MessageDetailHeaderUiModelMapperTest.kt @@ -106,7 +106,8 @@ class MessageDetailHeaderUiModelMapperTest { messageIdUiModel = MessageIdUiModel(message.messageId.id), themeOverride = null, shouldShowQuickReply = true, - hasMoreActions = true + hasMoreActions = true, + subject = "Subject" ) private val colorMapper: ColorMapper = mockk { @@ -165,6 +166,9 @@ class MessageDetailHeaderUiModelMapperTest { @Test fun `map to ui model returns a correct model`() = runTest { + // Given + val expectedResult = expectedResult.copy(subject = message.subject) + // When val result = messageDetailHeaderUiModelMapper.toUiModel( message = message, @@ -184,7 +188,7 @@ class MessageDetailHeaderUiModelMapperTest { numAttachments = 1, attachmentCount = AttachmentCount(1) ) - val expectedResult = expectedResult.copy(shouldShowAttachmentIcon = false) + val expectedResult = expectedResult.copy(shouldShowAttachmentIcon = false, subject = message.subject) // When val result = messageDetailHeaderUiModelMapper.toUiModel( message = message, @@ -201,7 +205,7 @@ class MessageDetailHeaderUiModelMapperTest { fun `when the message is not starred, don't show star icon`() = runTest { // Given val message = message.copy(isStarred = false) - val expectedResult = expectedResult.copy(shouldShowStar = false) + val expectedResult = expectedResult.copy(shouldShowStar = false, subject = message.subject) // When val result = messageDetailHeaderUiModelMapper.toUiModel( message = message, @@ -223,6 +227,7 @@ class MessageDetailHeaderUiModelMapperTest { ) val expectedResult = expectedResult.copy( shouldShowUndisclosedRecipients = true, + subject = message.subject, allRecipients = emptyList().toImmutableList(), toRecipients = emptyList().toImmutableList(), ccRecipients = emptyList().toImmutableList() diff --git a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/reducer/BottomSheetReducerTest.kt b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/reducer/BottomSheetReducerTest.kt index 012dddcb0a..09a15d9ded 100644 --- a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/reducer/BottomSheetReducerTest.kt +++ b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/reducer/BottomSheetReducerTest.kt @@ -162,7 +162,6 @@ internal class BottomSheetReducerTest( TestInput( currentState = BottomSheetState(null, Effect.empty()), operation = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = "Sender", messageSubject = "Subject", messageIdInConversation = "messageId", availableActions = AvailableActionsTestData.replyActionsOnly, diff --git a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetDataTest.kt b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetDataTest.kt index 637832df3b..5b1df8f902 100644 --- a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetDataTest.kt +++ b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/usecase/GetMoreActionsBottomSheetDataTest.kt @@ -6,23 +6,19 @@ import ch.protonmail.android.mailcommon.domain.model.Action import ch.protonmail.android.mailcommon.domain.model.DataError import ch.protonmail.android.mailcommon.domain.sample.ConversationIdSample import ch.protonmail.android.mailcommon.domain.sample.UserIdSample -import ch.protonmail.android.mailconversation.domain.entity.ConversationDetailEntryPoint import ch.protonmail.android.mailconversation.domain.sample.ConversationSample import ch.protonmail.android.mailconversation.domain.usecase.GetConversationAvailableActions -import ch.protonmail.android.mailconversation.domain.usecase.ObserveConversation import ch.protonmail.android.maildetail.presentation.model.MoreActionsBottomSheetEntryPoint import ch.protonmail.android.maillabel.domain.model.SystemLabelId import ch.protonmail.android.mailmessage.domain.sample.MessageIdSample import ch.protonmail.android.mailmessage.domain.sample.MessageSample import ch.protonmail.android.mailmessage.domain.usecase.GetMessageAvailableActions -import ch.protonmail.android.mailmessage.domain.usecase.ObserveMessage import ch.protonmail.android.mailmessage.presentation.model.bottomsheet.DetailMoreActionsBottomSheetState import ch.protonmail.android.testdata.action.AvailableActionsTestData import ch.protonmail.android.testdata.conversation.ConversationTestData.conversation import ch.protonmail.android.testdata.message.MessageThemeOptionsTestData import io.mockk.coEvery import io.mockk.mockk -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals @@ -30,16 +26,12 @@ import kotlin.test.assertNull class GetMoreActionsBottomSheetDataTest { - private val observeMessage = mockk() - private val observeConversation = mockk() private val getConversationAvailableActions = mockk() private val getMessageAvailableActions = mockk() private val getMoreBottomSheetData = GetMoreActionsBottomSheetData( getMessageAvailableActions, - getConversationAvailableActions, - observeMessage, - observeConversation + getConversationAvailableActions ) @Test @@ -48,27 +40,27 @@ class GetMoreActionsBottomSheetDataTest { val themeOptions = MessageThemeOptionsTestData.darkOverrideLight val userId = UserIdSample.Primary val labelId = SystemLabelId.Archive.labelId - val messageId = MessageIdSample.PlainTextMessage + val messageId = MessageSample.Invoice.messageId val message = MessageSample.Invoice - val entryPoint = MoreActionsBottomSheetEntryPoint.MessageHeader val availableActions = AvailableActionsTestData.replyActionsOnly + + val payload = MoreMessageActionsBottomSheetDataPayload( + userId = userId, + labelId = labelId, + messageId = messageId, + messageThemeOptions = themeOptions, + entryPoint = MoreActionsBottomSheetEntryPoint.MessageHeader, + subject = message.subject + ) coEvery { getMessageAvailableActions(userId, labelId, messageId, themeOptions) } returns availableActions.right() - coEvery { observeMessage(userId, messageId) } returns flowOf(message.right()) // When - val actual = getMoreBottomSheetData.forMessage( - userId, - labelId, - messageId, - themeOptions, - entryPoint - ) + val actual = getMoreBottomSheetData.forMessage(payload) // Then val expected = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = message.sender.name, messageSubject = message.subject, messageIdInConversation = message.messageId.id, availableActions = availableActions, @@ -84,27 +76,27 @@ class GetMoreActionsBottomSheetDataTest { val themeOptions = MessageThemeOptionsTestData.darkOverrideLight val userId = UserIdSample.Primary val labelId = SystemLabelId.Archive.labelId - val messageId = MessageIdSample.PlainTextMessage val message = MessageSample.Invoice - val entryPoint = MoreActionsBottomSheetEntryPoint.BottomBar + val messageId = message.messageId val availableActions = AvailableActionsTestData.replyActionsOnly + + val payload = MoreMessageActionsBottomSheetDataPayload( + userId = userId, + labelId = labelId, + messageId = messageId, + messageThemeOptions = themeOptions, + entryPoint = MoreActionsBottomSheetEntryPoint.BottomBar, + subject = message.subject + ) coEvery { getMessageAvailableActions(userId, labelId, messageId, themeOptions) } returns availableActions.right() - coEvery { observeMessage(userId, messageId) } returns flowOf(message.right()) // When - val actual = getMoreBottomSheetData.forMessage( - userId, - labelId, - messageId, - themeOptions, - entryPoint - ) + val actual = getMoreBottomSheetData.forMessage(payload) // Then val expected = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = message.sender.name, messageSubject = message.subject, messageIdInConversation = message.messageId.id, availableActions = availableActions, @@ -122,13 +114,22 @@ class GetMoreActionsBottomSheetDataTest { val messageId = MessageIdSample.PlainTextMessage val message = MessageSample.Invoice val entryPoint = MoreActionsBottomSheetEntryPoint.MessageHeader + + val payload = MoreMessageActionsBottomSheetDataPayload( + userId = userId, + labelId = labelId, + messageId = messageId, + messageThemeOptions = themeOptions, + entryPoint = entryPoint, + subject = message.subject + ) + coEvery { getMessageAvailableActions(userId, labelId, messageId, themeOptions) } returns DataError.Local.NoDataCached.left() - coEvery { observeMessage(userId, messageId) } returns flowOf(message.right()) // When - val actual = getMoreBottomSheetData.forMessage(userId, labelId, messageId, themeOptions, entryPoint) + val actual = getMoreBottomSheetData.forMessage(payload) // Then assertNull(actual) @@ -142,27 +143,22 @@ class GetMoreActionsBottomSheetDataTest { val conversationId = ConversationIdSample.WeatherForecast val conversation = ConversationSample.WeatherForecast val availableActions = AvailableActionsTestData.replyActionsOnly - val entryPoint = ConversationDetailEntryPoint.Mailbox - val showAll = false coEvery { getConversationAvailableActions(userId, labelId, conversationId) } returns availableActions.right() - coEvery { - observeConversation( - userId, - conversationId, - labelId, - entryPoint, - showAll - ) - } returns flowOf(conversation.right()) + + val payload = MoreConversationActionsBottomSheetDataPayload( + userId = userId, + labelId = labelId, + subject = conversation.subject, + conversationId = conversationId + ) // When - val actual = getMoreBottomSheetData.forConversation(userId, labelId, conversationId, entryPoint, showAll) + val actual = getMoreBottomSheetData.forConversation(payload) // Then val expected = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = conversation.senders.first().name, messageSubject = conversation.subject, messageIdInConversation = null, availableActions = availableActions, @@ -177,23 +173,19 @@ class GetMoreActionsBottomSheetDataTest { val userId = UserIdSample.Primary val labelId = SystemLabelId.Archive.labelId val conversationId = ConversationIdSample.WeatherForecast - val entryPoint = ConversationDetailEntryPoint.Mailbox - val showAll = false coEvery { getConversationAvailableActions(userId, labelId, conversationId) } returns DataError.Local.NoDataCached.left() - coEvery { - observeConversation( - userId, - conversationId, - labelId, - entryPoint, - showAll - ) - } returns flowOf(conversation.right()) + + val payload = MoreConversationActionsBottomSheetDataPayload( + userId = userId, + labelId = labelId, + subject = conversation.subject, + conversationId = conversationId + ) // When - val actual = getMoreBottomSheetData.forConversation(userId, labelId, conversationId, entryPoint, showAll) + val actual = getMoreBottomSheetData.forConversation(payload) // Then assertNull(actual) diff --git a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModelIntegrationTest.kt b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModelIntegrationTest.kt index 4aa48a9d89..2da072d8d0 100644 --- a/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModelIntegrationTest.kt +++ b/mail-detail/presentation/src/test/kotlin/ch/protonmail/android/maildetail/presentation/viewmodel/ConversationDetailViewModelIntegrationTest.kt @@ -63,15 +63,12 @@ import ch.protonmail.android.mailcommon.presentation.sample.ParticipantAvatarSam import ch.protonmail.android.mailcommon.presentation.ui.delete.DeleteDialogState import ch.protonmail.android.mailcommon.presentation.usecase.FormatExtendedTime import ch.protonmail.android.mailcommon.presentation.usecase.FormatShortTime -import ch.protonmail.android.mailcontact.domain.model.ContactMetadata import ch.protonmail.android.mailcontact.domain.usecase.FindContactByEmail -import ch.protonmail.android.mailcontact.domain.usecase.ObserveContacts import ch.protonmail.android.mailconversation.domain.entity.ConversationDetailEntryPoint import ch.protonmail.android.mailconversation.domain.entity.ConversationWithMessages import ch.protonmail.android.mailconversation.domain.sample.ConversationSample import ch.protonmail.android.mailconversation.domain.usecase.DeleteConversations import ch.protonmail.android.mailconversation.domain.usecase.GetConversationAvailableActions -import ch.protonmail.android.mailconversation.domain.usecase.ObserveConversation import ch.protonmail.android.mailconversation.domain.usecase.ObserveConversationMessages import ch.protonmail.android.mailconversation.domain.usecase.ObserveConversationWithMessages import ch.protonmail.android.mailconversation.domain.usecase.StarConversations @@ -247,16 +244,6 @@ internal class ConversationDetailViewModelIntegrationTest { private val showAllMessages = false // region mock observe use cases - private val observeContacts: ObserveContacts = mockk { - coEvery { - this@mockk(userId = UserIdSample.Primary) - } returns flowOf(emptyList().right()) - } - - private val observeConversation = mockk { - coEvery { this@mockk(UserIdSample.Primary, ConversationIdSample.WeatherForecast, any(), any(), any()) } returns - flowOf(ConversationSample.WeatherForecast.right()) - } private val observeConversationMessages = mockk { coEvery { this@mockk(UserIdSample.Primary, ConversationIdSample.WeatherForecast, any(), any(), any()) } returns @@ -391,9 +378,7 @@ internal class ConversationDetailViewModelIntegrationTest { private val attachmentGroupUiModelMapper = AttachmentGroupUiModelMapper(attachmentMetadataUiModelMapper) private val getMoreActionsBottomSheetData = GetMoreActionsBottomSheetData( getMessageAvailableActions, - getConversationAvailableActions, - observeMessage, - observeConversation + getConversationAvailableActions ) private val getMessagesInSameExclusiveLocation = mockk() @@ -1384,41 +1369,6 @@ internal class ConversationDetailViewModelIntegrationTest { } } - @Test - fun `verify no bottom sheet data is emitted when more actions bottom sheet is requested and loading fails`() = - runTest { - // Given - val messageId = MessageId("messageId") - val labelId = SystemLabelId.Archive.labelId - val themeOptions = MessageThemeOptionsTestData.darkNoOverride - - coEvery { - observeMessage( - userId = userId, - messageId = messageId - ) - } returns flowOf(DataError.Local.NoDataCached.left()) - coEvery { - getMessageAvailableActions(userId, labelId, messageId, themeOptions) - } returns AvailableActionsTestData.replyActionsOnly.right() - - // When - val viewModel = buildConversationDetailViewModel() - viewModel.state.test { - viewModel.submit( - ConversationDetailViewAction.RequestMessageMoreActionsBottomSheet( - messageId, - themeOptions, - MoreActionsBottomSheetEntryPoint.BottomBar - ) - ) - advanceUntilIdle() - - // Then - assertNull(lastEmittedItem().bottomSheetState?.contentState) - } - } - @Test fun `when user clicks report phishing then confirm dialog is shown`() = runTest { // Given diff --git a/mail-message/presentation/src/main/kotlin/ch/protonmail/android/mailmessage/presentation/model/bottomsheet/BottomSheetStates.kt b/mail-message/presentation/src/main/kotlin/ch/protonmail/android/mailmessage/presentation/model/bottomsheet/BottomSheetStates.kt index 0d96ffd4bf..4243911950 100644 --- a/mail-message/presentation/src/main/kotlin/ch/protonmail/android/mailmessage/presentation/model/bottomsheet/BottomSheetStates.kt +++ b/mail-message/presentation/src/main/kotlin/ch/protonmail/android/mailmessage/presentation/model/bottomsheet/BottomSheetStates.kt @@ -116,7 +116,6 @@ sealed interface DetailMoreActionsBottomSheetState : BottomSheetContentState { sealed interface DetailMoreActionsBottomSheetOperation : BottomSheetOperation sealed interface DetailMoreActionsBottomSheetEvent : DetailMoreActionsBottomSheetOperation { data class DataLoaded( - val messageSender: String, val messageSubject: String, val messageIdInConversation: String?, val availableActions: AvailableActions, diff --git a/mail-message/presentation/src/test/kotlin/ch/protonmail/android/mailmessage/presentation/reducer/DetailMoreActionsBottomSheetReducerTest.kt b/mail-message/presentation/src/test/kotlin/ch/protonmail/android/mailmessage/presentation/reducer/DetailMoreActionsBottomSheetReducerTest.kt index 9a2eec2ef0..5ed6cec7a3 100644 --- a/mail-message/presentation/src/test/kotlin/ch/protonmail/android/mailmessage/presentation/reducer/DetailMoreActionsBottomSheetReducerTest.kt +++ b/mail-message/presentation/src/test/kotlin/ch/protonmail/android/mailmessage/presentation/reducer/DetailMoreActionsBottomSheetReducerTest.kt @@ -68,12 +68,8 @@ internal class DetailMoreActionsBottomSheetReducerTest( companion object { - private const val ExpectedSender = "Sender" private const val ExpectedSubject = "Subject" private const val ExpectedMessageId = "messageId" - private const val SingleParticipantCount = 1 - private const val MultipleParticipantsCount = 10 - private val expectedUiModel = DetailMoreActionsBottomSheetState.DetailDataUiModel( headerSubjectText = TextUiModel(ExpectedSubject), messageIdInConversation = ExpectedMessageId @@ -82,7 +78,6 @@ internal class DetailMoreActionsBottomSheetReducerTest( TestInput( currentState = BottomSheetState(DetailMoreActionsBottomSheetState.Loading), operation = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = ExpectedSender, messageSubject = ExpectedSubject, messageIdInConversation = ExpectedMessageId, availableActions = AvailableActionsTestData.replyReportPhishing, @@ -102,7 +97,6 @@ internal class DetailMoreActionsBottomSheetReducerTest( TestInput( currentState = BottomSheetState(DetailMoreActionsBottomSheetState.Loading), operation = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = ExpectedSender, messageSubject = ExpectedSubject, messageIdInConversation = ExpectedMessageId, availableActions = AvailableActionsTestData.forwardReportPhishingActions, @@ -122,7 +116,6 @@ internal class DetailMoreActionsBottomSheetReducerTest( TestInput( currentState = BottomSheetState(DetailMoreActionsBottomSheetState.Loading), operation = DetailMoreActionsBottomSheetState.DetailMoreActionsBottomSheetEvent.DataLoaded( - messageSender = ExpectedSender, messageSubject = ExpectedSubject, messageIdInConversation = ExpectedMessageId, availableActions = AvailableActionsTestData.fullAvailableActions, diff --git a/test/test-data/src/main/kotlin/ch/protonmail/android/testdata/maildetail/MessageDetailHeaderUiModelTestData.kt b/test/test-data/src/main/kotlin/ch/protonmail/android/testdata/maildetail/MessageDetailHeaderUiModelTestData.kt index 4f17f09f69..606e2c96f2 100644 --- a/test/test-data/src/main/kotlin/ch/protonmail/android/testdata/maildetail/MessageDetailHeaderUiModelTestData.kt +++ b/test/test-data/src/main/kotlin/ch/protonmail/android/testdata/maildetail/MessageDetailHeaderUiModelTestData.kt @@ -87,7 +87,8 @@ object MessageDetailHeaderUiModelTestData { messageIdUiModel = MessageIdUiModel("id"), themeOverride = null, shouldShowQuickReply = true, - hasMoreActions = true + hasMoreActions = true, + subject = "A subject" ) fun buildParticipant(