Skip to content

Let messages and conversations be deleted without a SIM #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
RankoR wants to merge 1 commit into
base: main
Choose a base branch
from
+159 −21
Draft

Let messages and conversations be deleted without a SIM #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions ...ssaging/domain/conversation/usecase/action/CheckConversationActionRequirementsImplTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class CheckConversationActionRequirementsImplTest {

assertEquals(
ConversationActionRequirementsResult.SmsNotCapable,
useCase(),
useCase(requiresPreferredSmsSim = true),
)
}

Expand All @@ -49,7 +49,7 @@ class CheckConversationActionRequirementsImplTest {

assertEquals(
ConversationActionRequirementsResult.NoPreferredSmsSim,
useCase(),
useCase(requiresPreferredSmsSim = true),
)
}

Expand All @@ -64,7 +64,7 @@ class CheckConversationActionRequirementsImplTest {

assertEquals(
ConversationActionRequirementsResult.MissingDefaultSmsRole,
useCase(),
useCase(requiresPreferredSmsSim = true),
)
}

Expand All @@ -82,7 +82,43 @@ class CheckConversationActionRequirementsImplTest {

assertEquals(
ConversationActionRequirementsResult.MissingDefaultSmsRole,
useCase(),
useCase(requiresPreferredSmsSim = true),
)
}

@Test
fun invoke_whenPreferredSmsSimIsNotRequired_ignoresMissingPreferredSmsSim() {
every { phoneUtils.isSmsCapable } returns true
every { phoneUtils.hasPreferredSmsSim } returns false
every {
roleManager.isRoleAvailable(RoleManager.ROLE_SMS)
} returns true
every {
roleManager.isRoleHeld(RoleManager.ROLE_SMS)
} returns true
val useCase = CheckConversationActionRequirementsImpl(roleManager = roleManager)

assertEquals(
ConversationActionRequirementsResult.Ready,
useCase(requiresPreferredSmsSim = false),
)
}

@Test
fun invoke_whenSimNotRequiredAndSmsRoleNotHeld_returnsMissingDefaultSmsRole() {
every { phoneUtils.isSmsCapable } returns true
every { phoneUtils.hasPreferredSmsSim } returns false
every {
roleManager.isRoleAvailable(RoleManager.ROLE_SMS)
} returns true
every {
roleManager.isRoleHeld(RoleManager.ROLE_SMS)
} returns false
val useCase = CheckConversationActionRequirementsImpl(roleManager = roleManager)

assertEquals(
ConversationActionRequirementsResult.MissingDefaultSmsRole,
useCase(requiresPreferredSmsSim = false),
)
}

Expand All @@ -100,7 +136,7 @@ class CheckConversationActionRequirementsImplTest {

assertEquals(
ConversationActionRequirementsResult.Ready,
useCase(),
useCase(requiresPreferredSmsSim = true),
)
}
}
2 changes: 1 addition & 1 deletion ...id/messaging/ui/conversation/composer/delegate/draft/BaseConversationDraftDelegateTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ internal abstract class BaseConversationDraftDelegateTest {
results: List<ConversationActionRequirementsResult>,
): CheckConversationActionRequirements {
val mock = mockk<CheckConversationActionRequirements>()
every { mock.invoke() } returnsMany results
every { mock.invoke(requiresPreferredSmsSim = true) } returnsMany results
return mock
}

Expand Down
2 changes: 1 addition & 1 deletion ...ging/ui/conversation/composer/delegate/draft/ConversationDraftDelegateSimSelectionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal class ConversationDraftDelegateSimSelectionTest {
@Before
fun setUp() {
every {
checkConversationActionRequirements()
checkConversationActionRequirements(requiresPreferredSmsSim = true)
} returns ConversationActionRequirementsResult.Ready

every {
Expand Down
11 changes: 10 additions & 1 deletion .../conversation/messages/delegate/selection/BaseConversationMessageSelectionDelegateTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,19 @@ internal abstract class BaseConversationMessageSelectionDelegateTest {
results: List<ConversationActionRequirementsResult>,
): CheckConversationActionRequirements {
val mock = mockk<CheckConversationActionRequirements>()
every { mock.invoke() } returnsMany results
every { mock.invoke(requiresPreferredSmsSim = any()) } returnsMany results
return mock
}

protected fun createNoPreferredSmsSimRequirements(): CheckConversationActionRequirements {
return CheckConversationActionRequirements { requiresPreferredSmsSim ->
when {
requiresPreferredSmsSim -> ConversationActionRequirementsResult.NoPreferredSmsSim
else -> ConversationActionRequirementsResult.Ready
}
}
}

protected fun createAttachmentPart(): ConversationMessagePartUiModel.Attachment {
return ConversationMessagePartUiModel.Attachment.Image(
text = null,
Expand Down
45 changes: 45 additions & 0 deletions ...onversation/messages/delegate/selection/ConversationMessageSelectionDelegateDeleteTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.android.messaging.ui.conversation.messages.delegate.selection

import app.cash.turbine.test
import com.android.messaging.data.conversation.model.MessageId
import com.android.messaging.ui.conversation.screen.model.ConversationMessageAction
import com.android.messaging.ui.conversation.screen.model.ConversationMessageSelectionUiState
Expand Down Expand Up @@ -99,4 +100,48 @@ internal class ConversationMessageSelectionDelegateDeleteTest :
}
}
}

@Test
fun deleteAction_whenPreferredSmsSimIsMissing_deletesSelectedMessage() {
runTest(context = mainDispatcherRule.testDispatcher) {
val harness = createHarness(
actionRequirements = createNoPreferredSmsSimRequirements(),
)

try {
harness.messagesStateFlow.value = createMessagesUiState(
createMessageUiModel(messageId = "message-1"),
)
advanceUntilIdle()
harness.delegate.onMessageLongClick(messageId = MessageId("message-1"))
advanceUntilIdle()

harness.delegate.effects.test {
harness.delegate.onMessageSelectionActionClick(
action = ConversationMessageAction.Delete,
)
advanceUntilIdle()

assertEquals(
persistentSetOf(MessageId("message-1")),
harness.delegate.state.value.deleteConfirmation?.messageIds,
)

harness.delegate.confirmDeleteSelectedMessages()
advanceUntilIdle()

expectNoEvents()
cancelAndIgnoreRemainingEvents()
}

verify(exactly = 1) {
harness.conversationsRepository.deleteMessages(
messageIds = persistentSetOf(MessageId("message-1")),
)
}
} finally {
harness.cancel()
}
}
}
}
4 changes: 1 addition & 3 deletions ...onversation/messages/delegate/selection/ConversationMessageSelectionDelegateResendTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ internal class ConversationMessageSelectionDelegateResendTest :
fun resendAction_whenPreferredSmsSimIsMissing_emitsNoPreferredSimMessage() {
runTest(context = mainDispatcherRule.testDispatcher) {
val harness = createHarness(
actionRequirements = createActionRequirementsMock(
initialResult = ConversationActionRequirementsResult.NoPreferredSmsSim,
),
actionRequirements = createNoPreferredSmsSimRequirements(),
)

try {
Expand Down
48 changes: 44 additions & 4 deletions ...droid/messaging/ui/conversation/metadata/delegate/ConversationMetadataDelegateImplTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.android.messaging.data.conversation.model.metadata.ConversationCompos
import com.android.messaging.data.conversation.model.metadata.ConversationMetadata
import com.android.messaging.data.conversation.repository.ConversationsRepository
import com.android.messaging.domain.conversation.usecase.action.ArchiveConversation
import com.android.messaging.domain.conversation.usecase.action.CheckConversationActionRequirements
import com.android.messaging.domain.conversation.usecase.action.ConversationActionRequirementsResult
import com.android.messaging.testutil.MainDispatcherRule
import com.android.messaging.testutil.TEST_CALL_ACTION_PHONE_NUMBER
Expand Down Expand Up @@ -230,6 +231,43 @@ class ConversationMetadataDelegateImplTest {
}
}

@Test
fun onDeleteConversationClick_whenPreferredSmsSimIsMissing_showsConfirmation() {
runTest(context = mainDispatcherRule.testDispatcher) {
val harness = createHarness(
conversationId = ConversationId("conversation-42"),
checkConversationActionRequirements = { requiresPreferredSmsSim ->
when {
requiresPreferredSmsSim -> {
ConversationActionRequirementsResult.NoPreferredSmsSim
}

else -> ConversationActionRequirementsResult.Ready
}
},
)

try {
advanceUntilIdle()

harness.delegate.effects.test {
harness.delegate.onDeleteConversationClick()
advanceUntilIdle()

expectNoEvents()
cancelAndIgnoreRemainingEvents()
}

assertEquals(
true,
harness.delegate.isDeleteConversationConfirmationVisible.value,
)
} finally {
harness.cancel()
}
}
}

@Test
fun confirmDeleteConversation_deletesViaRepositoryAndEmitsCloseConversation() {
runTest(context = mainDispatcherRule.testDispatcher) {
Expand Down Expand Up @@ -308,7 +346,11 @@ class ConversationMetadataDelegateImplTest {
}
}

private fun createHarness(conversationId: ConversationId?): DelegateHarness {
private fun createHarness(
conversationId: ConversationId?,
checkConversationActionRequirements: CheckConversationActionRequirements =
CheckConversationActionRequirements { ConversationActionRequirementsResult.Ready },
): DelegateHarness {
val dispatcher = mainDispatcherRule.testDispatcher
val scope = TestScope(dispatcher)
val conversationsRepository = mockk<ConversationsRepository>(relaxed = true)
Expand Down Expand Up @@ -342,9 +384,7 @@ class ConversationMetadataDelegateImplTest {
}

val delegate = ConversationMetadataDelegateImpl(
checkConversationActionRequirements = {
ConversationActionRequirementsResult.Ready
},
checkConversationActionRequirements = checkConversationActionRequirements,
conversationsRepository = conversationsRepository,
conversationMetadataUiStateMapper = mapper,
blockedParticipantsRepository = mockk<BlockedParticipantsRepository>(relaxed = true),
Expand Down
8 changes: 5 additions & 3 deletions ...droid/messaging/domain/conversation/usecase/action/CheckConversationActionRequirements.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import com.android.messaging.util.PhoneUtils
import javax.inject.Inject

internal fun interface CheckConversationActionRequirements {
operator fun invoke(): ConversationActionRequirementsResult
operator fun invoke(requiresPreferredSmsSim: Boolean): ConversationActionRequirementsResult
}

internal class CheckConversationActionRequirementsImpl @Inject constructor(
Expand All @@ -14,11 +14,13 @@ internal class CheckConversationActionRequirementsImpl @Inject constructor(

private val phoneUtils by lazy { PhoneUtils.getDefault() }

override operator fun invoke(): ConversationActionRequirementsResult {
override operator fun invoke(
requiresPreferredSmsSim: Boolean,
): ConversationActionRequirementsResult {
return when {
!phoneUtils.isSmsCapable -> ConversationActionRequirementsResult.SmsNotCapable

!phoneUtils.hasPreferredSmsSim -> {
requiresPreferredSmsSim && !phoneUtils.hasPreferredSmsSim -> {
ConversationActionRequirementsResult.NoPreferredSmsSim
}

Expand Down
2 changes: 1 addition & 1 deletion src/com/android/messaging/ui/conversation/composer/delegate/ConversationDraftDelegate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ internal class ConversationDraftDelegateImpl @Inject constructor(
}

private fun sendDraftWhenActionRequirementsSatisfied(sendRequest: DraftSendRequest) {
when (checkConversationActionRequirements()) {
when (checkConversationActionRequirements(requiresPreferredSmsSim = true)) {
ConversationActionRequirementsResult.Ready -> {
sendDraft(sendRequest = sendRequest)
}
Expand Down
10 changes: 9 additions & 1 deletion ...droid/messaging/ui/conversation/messages/delegate/ConversationMessageSelectionDelegate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ internal class ConversationMessageSelectionDelegateImpl @Inject constructor(
) {
runWhenConversationActionRequirementsSatisfied(
isSending = false,
requiresPreferredSmsSim = false,
onReady = {
messageSelectionState.update { currentState ->
currentState.copy(
Expand All @@ -322,6 +323,7 @@ internal class ConversationMessageSelectionDelegateImpl @Inject constructor(
) {
runWhenConversationActionRequirementsSatisfied(
isSending = false,
requiresPreferredSmsSim = false,
onReady = {
clearMessageSelection()
conversationsRepository.deleteMessages(messageIds = messageIds)
Expand All @@ -343,6 +345,7 @@ internal class ConversationMessageSelectionDelegateImpl @Inject constructor(
) {
runWhenConversationActionRequirementsSatisfied(
isSending = messageAction.isSending,
requiresPreferredSmsSim = true,
onReady = {
runMessageAction(messageAction = messageAction)
},
Expand All @@ -354,11 +357,16 @@ internal class ConversationMessageSelectionDelegateImpl @Inject constructor(

private fun runWhenConversationActionRequirementsSatisfied(
isSending: Boolean,
requiresPreferredSmsSim: Boolean,
onReady: () -> Unit,
onBlocked: () -> Unit = {},
onMissingDefaultSmsRole: () -> Unit = {},
) {
when (checkConversationActionRequirements()) {
val requirementsResult = checkConversationActionRequirements(
requiresPreferredSmsSim = requiresPreferredSmsSim,
)

when (requirementsResult) {
ConversationActionRequirementsResult.Ready -> {
onReady()
}
Expand Down
2 changes: 1 addition & 1 deletion src/com/android/messaging/ui/conversation/metadata/delegate/ConversationMetadataDelegate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ internal class ConversationMetadataDelegateImpl @Inject constructor(
return
}

when (checkConversationActionRequirements()) {
when (checkConversationActionRequirements(requiresPreferredSmsSim = false)) {
ConversationActionRequirementsResult.Ready -> {
_isDeleteConversationConfirmationVisible.value = true
}
Expand Down
Loading