From fa25d667eeb259f3492e8f521460842634676d84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Tue, 17 Oct 2023 12:43:48 +0200 Subject: [PATCH] Fix remarks --- .../compose/screen/ConnectScreenTest.kt | 8 ++--- .../notificationbanner/NotificationBanner.kt | 6 ++-- .../compose/test/ComposeTestTagConstants.kt | 1 + .../constant/AccountExpiryConstant.kt | 1 + .../AccountExpiryNotificationUseCase.kt | 19 +++++------ .../usecase/TunnelStateNotificationUseCase.kt | 3 +- .../AccountExpiryNotificationUseCaseTest.kt | 32 +++++++------------ 7 files changed, 31 insertions(+), 39 deletions(-) diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt index 26dc17c4b45e..6756d196ce6f 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt @@ -16,7 +16,7 @@ import net.mullvad.mullvadvpn.compose.state.ConnectUiState import net.mullvad.mullvadvpn.compose.test.CIRCULAR_PROGRESS_INDICATOR import net.mullvad.mullvadvpn.compose.test.CONNECT_BUTTON_TEST_TAG import net.mullvad.mullvadvpn.compose.test.LOCATION_INFO_TEST_TAG -import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER +import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_ACTION import net.mullvad.mullvadvpn.compose.test.RECONNECT_BUTTON_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SCROLLABLE_COLUMN_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SELECT_LOCATION_BUTTON_TEST_TAG @@ -334,7 +334,7 @@ class ConnectScreenTest { daysLeftUntilExpiry = null, inAppNotification = InAppNotification.TunnelStateError( - ErrorState(ErrorStateCause.StartTunnelError, true) + ErrorState(ErrorStateCause.StartTunnelError, false) ) ), uiSideEffect = MutableSharedFlow().asSharedFlow() @@ -800,7 +800,7 @@ class ConnectScreenTest { } // Act - composeTestRule.onNodeWithTag(NOTIFICATION_BANNER).performClick() + composeTestRule.onNodeWithTag(NOTIFICATION_BANNER_ACTION).performClick() // Assert verify { mockedClickHandler.invoke() } @@ -833,7 +833,7 @@ class ConnectScreenTest { } // Act - composeTestRule.onNodeWithTag(NOTIFICATION_BANNER).performClick() + composeTestRule.onNodeWithTag(NOTIFICATION_BANNER_ACTION).performClick() // Assert verify { mockedClickHandler.invoke() } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt index a1103a94ac64..6078e4b39251 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/notificationbanner/NotificationBanner.kt @@ -27,10 +27,11 @@ import androidx.constraintlayout.compose.ConstraintLayout import androidx.constraintlayout.compose.Dimension import net.mullvad.mullvadvpn.compose.component.MullvadTopBar import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER +import net.mullvad.mullvadvpn.compose.test.NOTIFICATION_BANNER_ACTION import net.mullvad.mullvadvpn.compose.util.rememberPrevious -import net.mullvad.mullvadvpn.lib.theme.AlphaDescription import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens +import net.mullvad.mullvadvpn.lib.theme.color.AlphaDescription import net.mullvad.mullvadvpn.repository.InAppNotification import net.mullvad.mullvadvpn.ui.VersionInfo import net.mullvad.mullvadvpn.ui.notification.StatusLevel @@ -86,8 +87,8 @@ fun NotificationBanner( onClickShowAccount: () -> Unit, onClickDismissNewDevice: () -> Unit ) { + // Fix for animating to invisible state val previous = rememberPrevious(current = notification, shouldUpdate = { _, _ -> true }) - // Fix for animating t hide AnimatedVisibility( visible = notification != null, enter = slideInVertically(initialOffsetY = { -it }), @@ -183,6 +184,7 @@ private fun Notification(notificationBannerData: NotificationData) { end.linkTo(parent.end) bottom.linkTo(parent.bottom) } + .testTag(NOTIFICATION_BANNER_ACTION) .padding(all = Dimens.notificationEndIconPadding), onClick = it.onClick ) { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt index 3cf06f201bc3..dea9e12a3d82 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt @@ -24,5 +24,6 @@ const val LOCATION_INFO_TEST_TAG = "location_info_test_tag" // ConnectScreen - Notification banner const val NOTIFICATION_BANNER = "notification_banner" +const val NOTIFICATION_BANNER_ACTION = "notification_banner_action" const val LOGIN_TITLE_TEST_TAG = "login_title_test_tag" diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/constant/AccountExpiryConstant.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/constant/AccountExpiryConstant.kt index dff48b6228f8..7f7e4acf456f 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/constant/AccountExpiryConstant.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/constant/AccountExpiryConstant.kt @@ -1,3 +1,4 @@ package net.mullvad.mullvadvpn.constant const val ACCOUNT_EXPIRY_POLL_INTERVAL: Long = 15 /* s */ * 1000 /* ms */ +const val ACCOUNT_EXPIRY_CLOSE_TO_EXPIRY_THRESHOLD_DAYS = 3 diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCase.kt index 72ee15785499..a4961bafe7d0 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCase.kt @@ -2,24 +2,20 @@ package net.mullvad.mullvadvpn.usecase import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map +import net.mullvad.mullvadvpn.constant.ACCOUNT_EXPIRY_CLOSE_TO_EXPIRY_THRESHOLD_DAYS import net.mullvad.mullvadvpn.model.AccountExpiry +import net.mullvad.mullvadvpn.repository.AccountRepository import net.mullvad.mullvadvpn.repository.InAppNotification -import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager -import net.mullvad.mullvadvpn.util.flatMapReadyConnectionOrDefault import org.joda.time.DateTime class AccountExpiryNotificationUseCase( - private val serviceConnectionManager: ServiceConnectionManager, + private val accountRepository: AccountRepository, ) { fun notifications(): Flow> = - serviceConnectionManager.connectionState - .flatMapReadyConnectionOrDefault(flowOf(emptyList())) { - it.container.accountDataSource.accountExpiry - .map { accountExpiry -> accountExpiryNotification(accountExpiry) } - .map(::listOfNotNull) - } + accountRepository.accountExpiryState + .map(::accountExpiryNotification) + .map(::listOfNotNull) .distinctUntilChanged() private fun accountExpiryNotification(accountExpiry: AccountExpiry) = @@ -28,7 +24,8 @@ class AccountExpiryNotificationUseCase( } else null private fun AccountExpiry.isCloseToExpiring(): Boolean { - val threeDaysFromNow = DateTime.now().plusDays(3) + val threeDaysFromNow = + DateTime.now().plusDays(ACCOUNT_EXPIRY_CLOSE_TO_EXPIRY_THRESHOLD_DAYS) return this.date()?.isBefore(threeDaysFromNow) == true } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt index a795bf02c274..f228bd7dbecc 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/TunnelStateNotificationUseCase.kt @@ -38,7 +38,8 @@ class TunnelStateNotificationUseCase( } else null } is TunnelState.Error -> InAppNotification.TunnelStateError(tunnelUiState.errorState) - else -> null + is TunnelState.Connected, + TunnelState.Disconnected -> null } private fun ConnectionProxy.tunnelUiStateFlow(): Flow = diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCaseTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCaseTest.kt index 5827ca528700..5341708d3bde 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCaseTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/AccountExpiryNotificationUseCaseTest.kt @@ -11,11 +11,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule import net.mullvad.mullvadvpn.model.AccountExpiry +import net.mullvad.mullvadvpn.repository.AccountRepository import net.mullvad.mullvadvpn.repository.InAppNotification -import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionAccountDataSource -import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionContainer -import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager -import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionState import org.joda.time.DateTime import org.junit.After import org.junit.Before @@ -25,10 +22,6 @@ import org.junit.Test class AccountExpiryNotificationUseCaseTest { @get:Rule val testCoroutineRule = TestCoroutineRule() - private val mockServiceConnectionManager: ServiceConnectionManager = mockk() - private val mockServiceConnectionContainer: ServiceConnectionContainer = mockk() - private val serviceConnectionState = - MutableStateFlow(ServiceConnectionState.Disconnected) private val accountExpiry = MutableStateFlow(AccountExpiry.Missing) private lateinit var accountExpiryNotificationUseCase: AccountExpiryNotificationUseCase @@ -36,13 +29,10 @@ class AccountExpiryNotificationUseCaseTest { fun setup() { MockKAnnotations.init(this) - val accountDataSource = mockk() - every { mockServiceConnectionManager.connectionState } returns serviceConnectionState - every { mockServiceConnectionContainer.accountDataSource } returns accountDataSource - every { accountDataSource.accountExpiry } returns accountExpiry + val accountRepository = mockk() + every { accountRepository.accountExpiryState } returns accountExpiry - accountExpiryNotificationUseCase = - AccountExpiryNotificationUseCase(mockServiceConnectionManager) + accountExpiryNotificationUseCase = AccountExpiryNotificationUseCase(accountRepository) } @After @@ -63,11 +53,13 @@ class AccountExpiryNotificationUseCaseTest { // Arrange, Act, Assert accountExpiryNotificationUseCase.notifications().test { assertTrue { awaitItem().isEmpty() } - val expiryDate = DateTime.now().plusDays(2) - accountExpiry.value = AccountExpiry.Available(expiryDate) - serviceConnectionState.value = - ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) - assertEquals(listOf(InAppNotification.AccountExpiry(expiryDate)), awaitItem()) + val closeToExpiry = AccountExpiry.Available(DateTime.now().plusDays(2)) + accountExpiry.value = closeToExpiry + + assertEquals( + listOf(InAppNotification.AccountExpiry(closeToExpiry.expiryDateTime)), + awaitItem() + ) } } @@ -77,8 +69,6 @@ class AccountExpiryNotificationUseCaseTest { accountExpiryNotificationUseCase.notifications().test { assertTrue { awaitItem().isEmpty() } accountExpiry.value = AccountExpiry.Available(DateTime.now().plusDays(4)) - serviceConnectionState.value = - ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) expectNoEvents() } }