Merge branch 'fix/3071-fix-ratings-flow-launch' into 'develop'

Call launch review flow to prompt user the rating dialog

See merge request android/mail/proton-mail-android!1257

app/src/main/java/ch/protonmail/android/di/ViewModelModule.kt

@@ -20,6 +20,7 @@
 
 package ch.protonmail.android.di
 
+import android.content.Context
 import androidx.lifecycle.ViewModelProvider
 import ch.protonmail.android.activities.messageDetails.repository.MessageDetailsRepository
 import ch.protonmail.android.activities.settings.NotificationSettingsViewModel
@@ -31,7 +32,7 @@ import ch.protonmail.android.contacts.groups.edit.chooser.AddressChooserViewMode
 import ch.protonmail.android.core.ProtonMailApplication
 import ch.protonmail.android.core.UserManager
 import ch.protonmail.android.drawer.presentation.mapper.DrawerFoldersAndLabelsSectionUiModelMapper
-import ch.protonmail.android.feature.rating.usecase.StartRateAppFlowIfNeeded
+import ch.protonmail.android.feature.rating.usecase.ShouldStartRateAppFlow
 import ch.protonmail.android.labels.domain.LabelRepository
 import ch.protonmail.android.labels.domain.usecase.ObserveLabels
 import ch.protonmail.android.labels.domain.usecase.ObserveLabelsAndFoldersWithChildren
@@ -54,9 +55,12 @@ import ch.protonmail.android.usecase.delete.DeleteMessage
 import ch.protonmail.android.usecase.delete.EmptyFolder
 import ch.protonmail.android.usecase.message.ChangeMessagesReadStatus
 import ch.protonmail.android.usecase.message.ChangeMessagesStarredStatus
+import com.google.android.play.core.review.ReviewManager
+import com.google.android.play.core.review.ReviewManagerFactory
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
+import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.components.SingletonComponent
 import me.proton.core.util.kotlin.DispatcherProvider
 
@@ -91,6 +95,11 @@ internal class ViewModelModule {
         pinFragmentViewModelFactory: PinFragmentViewModelFactory
     ): ViewModelProvider.NewInstanceFactory = pinFragmentViewModelFactory
 
+    @Provides
+    fun provideReviewManager(
+        @ApplicationContext context: Context
+    ): ReviewManager = ReviewManagerFactory.create(context)
+
     @Suppress("LongParameterList") // Every new parameter adds a new issue and breaks the build
     @Provides
     fun provideMailboxViewModel(
@@ -119,7 +128,7 @@ internal class ViewModelModule {
         mailboxItemUiModelMapper: MailboxItemUiModelMapper,
         fetchEventsAndReschedule: FetchEventsAndReschedule,
         clearNotificationsForUser: ClearNotificationsForUser,
-        startRateAppFlowIfNeeded: StartRateAppFlowIfNeeded
+        shouldStartRateAppFlow: ShouldStartRateAppFlow
     ) = MailboxViewModel(
         messageDetailsRepositoryFactory = messageDetailsRepositoryFactory,
         userManager = userManager,
@@ -146,6 +155,6 @@ internal class ViewModelModule {
         mailboxItemUiModelMapper = mailboxItemUiModelMapper,
         fetchEventsAndReschedule = fetchEventsAndReschedule,
         clearNotificationsForUser = clearNotificationsForUser,
-        startRateAppFlowIfNeeded = startRateAppFlowIfNeeded
+        shouldStartRateAppFlow = shouldStartRateAppFlow
     )
 }

app/src/main/java/ch/protonmail/android/feature/rating/StartRateAppFlow.kt

@@ -18,22 +18,22 @@
  */
 package ch.protonmail.android.feature.rating
 
-import android.content.Context
+import android.app.Activity
-import com.google.android.play.core.review.ReviewManagerFactory
+import com.google.android.play.core.review.ReviewManager
-import dagger.hilt.android.qualifiers.ApplicationContext
 import timber.log.Timber
 import javax.inject.Inject
 
 class StartRateAppFlow @Inject constructor(
-    @ApplicationContext
+    private val reviewManager: ReviewManager
-    private val context: Context
 ) {
 
-    operator fun invoke() {
+    operator fun invoke(activity: Activity) {
-        val manager = ReviewManagerFactory.create(context)
+        val request = reviewManager.requestReviewFlow()
-        manager.requestReviewFlow().addOnCompleteListener {
+        request.addOnSuccessListener { reviewInfo ->
-            Timber.d("App review finished. Success = ${it.isSuccessful}")
+            reviewManager.launchReviewFlow(activity, reviewInfo)
+        }
+        request.addOnFailureListener {
+            Timber.d("Rate app flow request failed ")
         }
-
     }
 }

app/src/main/java/ch/protonmail/android/feature/rating/usecase/ShouldStartRateAppFlow.kt

@@ -20,33 +20,31 @@
 package ch.protonmail.android.feature.rating.usecase
 
 import ch.protonmail.android.feature.rating.MailboxScreenViewInMemoryRepository
-import ch.protonmail.android.feature.rating.StartRateAppFlow
 import ch.protonmail.android.featureflags.MailFeatureFlags
 import me.proton.core.domain.entity.UserId
 import me.proton.core.featureflag.domain.FeatureFlagManager
 import me.proton.core.featureflag.domain.entity.FeatureFlag
 import javax.inject.Inject
 
-class StartRateAppFlowIfNeeded @Inject constructor(
+class ShouldStartRateAppFlow @Inject constructor(
     private val mailboxScreenViewsRepository: MailboxScreenViewInMemoryRepository,
-    private val featureFlagManager: FeatureFlagManager,
+    private val featureFlagManager: FeatureFlagManager
-    private val startRateAppFlow: StartRateAppFlow
 ) {
 
-    suspend operator fun invoke(userId: UserId) {
+    suspend operator fun invoke(userId: UserId) : Boolean {
         if (!isShowReviewFeatureFlagEnabled(userId)) {
-            return
+            return false
         }
         if (mailboxScreenViewsRepository.screenViewCount < MailboxScreenViewsThreshold) {
-            return
+            return false
         }
-        startRateAppFlow()
+        recordShowReviewFlowConditionsMet(userId)
-        recordReviewFlowStarted(userId)
+        return true
     }
 
-    private suspend fun recordReviewFlowStarted(userId: UserId) {
+    private suspend fun recordShowReviewFlowConditionsMet(userId: UserId) {
         val featureFlag = getShowReviewFeatureFlag(userId)
-        val offFeatureFlag = featureFlag.copy(defaultValue = false, value = false)
+        val offFeatureFlag = featureFlag.copy(value = false)
         featureFlagManager.update(offFeatureFlag)
     }
 
@@ -55,9 +53,9 @@ class StartRateAppFlowIfNeeded @Inject constructor(
     private suspend fun getShowReviewFeatureFlag(
         userId: UserId
     ) = featureFlagManager.getOrDefault(
-        userId,
+        userId = userId,
-        MailFeatureFlags.ShowReviewAppDialog.featureId,
+        featureId = MailFeatureFlags.ShowReviewAppDialog.featureId,
-        FeatureFlag.default(MailFeatureFlags.ShowReviewAppDialog.featureId.id, false)
+        default = FeatureFlag.default(MailFeatureFlags.ShowReviewAppDialog.featureId.id, false)
     )
 
     companion object {

app/src/main/java/ch/protonmail/android/mailbox/presentation/ui/MailboxActivity.kt

@@ -89,6 +89,7 @@ import ch.protonmail.android.events.MailboxNoMessagesEvent
 import ch.protonmail.android.events.SettingsChangedEvent
 import ch.protonmail.android.events.Status
 import ch.protonmail.android.feature.account.AccountStateManager
+import ch.protonmail.android.feature.rating.StartRateAppFlow
 import ch.protonmail.android.labels.domain.model.Label
 import ch.protonmail.android.labels.domain.model.LabelId
 import ch.protonmail.android.labels.domain.model.LabelType
@@ -189,6 +190,9 @@ internal class MailboxActivity :
     @Inject
     lateinit var isConversationModeEnabled: ConversationModeEnabled
 
+    @Inject
+    lateinit var startRateAppFlow: StartRateAppFlow
+
     @Inject
     @DefaultSharedPreferences
     lateinit var defaultSharedPreferences: SharedPreferences
@@ -413,6 +417,7 @@ internal class MailboxActivity :
             exitSelectionModeSharedFlow
                 .onEach { if (it) actionMode?.finish() }
                 .launchIn(lifecycleScope)
+
         }
 
         setUpMailboxActionsView()
@@ -436,6 +441,10 @@ internal class MailboxActivity :
                 )
             }.launchIn(lifecycleScope)
         }
+
+        mailboxViewModel.startRateAppFlow
+            .onEach { startRateAppFlow(this) }
+            .launchIn(lifecycleScope)
     }
 
     private fun startObserving() {

app/src/main/java/ch/protonmail/android/mailbox/presentation/viewmodel/MailboxViewModel.kt

@@ -41,7 +41,7 @@ import ch.protonmail.android.domain.loadMoreMap
 import ch.protonmail.android.drawer.presentation.mapper.DrawerFoldersAndLabelsSectionUiModelMapper
 import ch.protonmail.android.drawer.presentation.model.DrawerFoldersAndLabelsSectionUiModel
 import ch.protonmail.android.feature.NotLoggedIn
-import ch.protonmail.android.feature.rating.usecase.StartRateAppFlowIfNeeded
+import ch.protonmail.android.feature.rating.usecase.ShouldStartRateAppFlow
 import ch.protonmail.android.labels.domain.LabelRepository
 import ch.protonmail.android.labels.domain.model.Label
 import ch.protonmail.android.labels.domain.model.LabelId
@@ -138,7 +138,7 @@ internal class MailboxViewModel @Inject constructor(
     private val mailboxItemUiModelMapper: MailboxItemUiModelMapper,
     private val fetchEventsAndReschedule: FetchEventsAndReschedule,
     private val clearNotificationsForUser: ClearNotificationsForUser,
-    private val startRateAppFlowIfNeeded: StartRateAppFlowIfNeeded
+    private val shouldStartRateAppFlow: ShouldStartRateAppFlow
 ) : ConnectivityBaseViewModel(verifyConnection, networkConfigurator) {
 
     private val _manageLimitReachedWarning = MutableLiveData<Event<Boolean>>()
@@ -156,6 +156,7 @@ internal class MailboxViewModel @Inject constructor(
         onBufferOverflow = BufferOverflow.DROP_OLDEST
     )
     private val _exitSelectionModeSharedFlow = MutableSharedFlow<Boolean>()
+    private val _startRateAppFlow = MutableSharedFlow<Unit>()
 
     private val messageDetailsRepository: MessageDetailsRepository
         get() = messageDetailsRepositoryFactory.create(userManager.requireCurrentUserId())
@@ -188,6 +189,9 @@ internal class MailboxViewModel @Inject constructor(
     val exitSelectionModeSharedFlow: SharedFlow<Boolean>
         get() = _exitSelectionModeSharedFlow
 
+    val startRateAppFlow: SharedFlow<Unit>
+        get() = _startRateAppFlow
+
     val mailboxState = mutableMailboxState.asStateFlow()
     val mailboxLocation = mutableMailboxLocation.asStateFlow()
 
@@ -727,7 +731,11 @@ internal class MailboxViewModel @Inject constructor(
     fun startRateAppFlowIfNeeded() {
         val userId = userManager.currentUserId ?: return
         viewModelScope.launch {
-            startRateAppFlowIfNeeded.invoke(userId)
+            shouldStartRateAppFlow(userId).let { startFlow ->
+                if (startFlow) {
+                    _startRateAppFlow.emit(Unit)
+                }
+            }
         }
     }
 

app/src/test/java/ch/protonmail/android/feature/rating/StartRateAppFlowTest.kt

@@ -19,41 +19,69 @@
 
 package ch.protonmail.android.feature.rating
 
-import android.content.Context
+import android.app.Activity
+import com.google.android.gms.tasks.OnFailureListener
+import com.google.android.gms.tasks.OnSuccessListener
+import com.google.android.gms.tasks.Task
+import com.google.android.gms.tasks.Tasks
+import com.google.android.play.core.review.ReviewInfo
 import com.google.android.play.core.review.ReviewManager
-import com.google.android.play.core.review.ReviewManagerFactory
 import io.mockk.every
 import io.mockk.mockk
-import io.mockk.mockkStatic
-import io.mockk.unmockkStatic
 import io.mockk.verify
-import org.junit.After
 import org.junit.Test
 
 class StartRateAppFlowTest {
 
-    private val context: Context = mockk(relaxed = true)
+    private val activity: Activity = mockk(relaxed = true)
+    private val reviewInfo: ReviewInfo = mockk()
+    private val requestMock: Task<ReviewInfo> = mockk {
+        every { result } returns reviewInfo
+        every { addOnSuccessListener(any()) } answers {
+            if (this@mockk.isSuccessful) {
+                val successListener = firstArg<OnSuccessListener<ReviewInfo>>()
+                successListener.onSuccess(reviewInfo)
+            }
+            Tasks.forResult(reviewInfo)
+        }
+        every { addOnFailureListener(any()) } answers {
+            val exception = Exception("Failed")
+            if (!this@mockk.isSuccessful) {
+                val failureListener = firstArg<OnFailureListener>()
+                failureListener.onFailure(exception)
+            }
+            Tasks.forException(exception)
+        }
+    }
+    private val reviewManagerMock: ReviewManager = mockk {
+        every { requestReviewFlow() } returns requestMock
+    }
 
-    private val startRateAppFlow = StartRateAppFlow(context)
+    private val startRateAppFlow = StartRateAppFlow(reviewManagerMock)
 
-    @After
+    @Test
-    fun tearDown() {
+    fun `succeeds when review flow request is successful`() {
-        unmockkStatic(ReviewManagerFactory::class)
+        // given
+        every { requestMock.isSuccessful } returns true
+        every { reviewManagerMock.launchReviewFlow(activity, reviewInfo) } returns mockk()
+
+        // when
+        startRateAppFlow(activity)
+
+        // then
+        verify { reviewManagerMock.launchReviewFlow(activity, reviewInfo) }
     }
 
     @Test
-    fun `request review flow from google play review manager`() {
+    fun `fails when review flow request is not successful`() {
         // given
-        val reviewMangerMock = mockk<ReviewManager> {
+        every { requestMock.isSuccessful } returns false
-            every { this@mockk.requestReviewFlow() } returns mockk(relaxed = true)
+        every { reviewManagerMock.launchReviewFlow(activity, reviewInfo) } returns mockk()
-        }
-        mockkStatic(ReviewManagerFactory::class)
-        every { ReviewManagerFactory.create(context) } returns reviewMangerMock
 
         // when
-        startRateAppFlow()
+        startRateAppFlow(activity)
 
         // then
-        verify { reviewMangerMock.requestReviewFlow() }
+        verify(exactly = 0) { reviewManagerMock.launchReviewFlow(any(), any()) }
     }
 }

app/src/test/java/ch/protonmail/android/feature/rating/usecase/ShouldStartRateAppFlowTest.kt

@@ -20,47 +20,41 @@
 package ch.protonmail.android.feature.rating.usecase
 
 import ch.protonmail.android.feature.rating.MailboxScreenViewInMemoryRepository
-import ch.protonmail.android.feature.rating.StartRateAppFlow
 import ch.protonmail.android.featureflags.MailFeatureFlags
 import ch.protonmail.android.testdata.UserTestData
-import io.mockk.Called
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
-import io.mockk.verify
 import kotlinx.coroutines.test.runTest
 import me.proton.core.domain.entity.UserId
 import me.proton.core.featureflag.domain.FeatureFlagManager
 import me.proton.core.featureflag.domain.entity.FeatureFlag
 import me.proton.core.featureflag.domain.entity.Scope
 import org.junit.Test
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
 
-class StartRateAppFlowIfNeededTest {
+class ShouldStartRateAppFlowTest {
 
     private val userId = UserTestData.userId
 
     private val mailboxScreenViewsRepository: MailboxScreenViewInMemoryRepository = mockk()
     private val featureFlagManager: FeatureFlagManager = mockk(relaxUnitFun = true)
-    private val startRateAppFlow: StartRateAppFlow = mockk(relaxUnitFun = true)
 
-    private val startRateAppFlowIfNeeded = StartRateAppFlowIfNeeded(
+    private val shouldStartRateAppFlow = ShouldStartRateAppFlow(mailboxScreenViewsRepository, featureFlagManager)
-        mailboxScreenViewsRepository,
-        featureFlagManager,
-        startRateAppFlow
-    )
 
     @Test
-    fun `rate app flow is started when feature flag is true and mailbox screen views reached threshold`() = runTest {
+    fun `start rate app flow is true when feature flag is true and mailbox screen views reached threshold`() = runTest {
         // given
         mockFeatureFlagValue(userId, true)
         mockScreenViews(2)
 
         // when
-        startRateAppFlowIfNeeded(userId)
+        val result = shouldStartRateAppFlow(userId)
 
         // then
-        verify { startRateAppFlow() }
+        assertTrue(result)
     }
 
     @Test
@@ -70,10 +64,10 @@ class StartRateAppFlowIfNeededTest {
         mockScreenViews(2)
 
         // when
-        startRateAppFlowIfNeeded(userId)
+        val result = shouldStartRateAppFlow(userId)
 
         // then
-        verify { startRateAppFlow wasNot Called }
+        assertFalse(result)
     }
 
     @Test
@@ -83,10 +77,10 @@ class StartRateAppFlowIfNeededTest {
         mockScreenViews(1)
 
         // when
-        startRateAppFlowIfNeeded(userId)
+        val result = shouldStartRateAppFlow(userId)
 
         // then
-        verify { startRateAppFlow wasNot Called }
+        assertFalse(result)
     }
 
     @Test
@@ -96,7 +90,7 @@ class StartRateAppFlowIfNeededTest {
         mockScreenViews(2)
 
         // when
-        startRateAppFlowIfNeeded(userId)
+        shouldStartRateAppFlow(userId)
 
         // then
         val featureId = MailFeatureFlags.ShowReviewAppDialog.featureId
@@ -104,6 +98,24 @@ class StartRateAppFlowIfNeededTest {
         coVerify { featureFlagManager.update(featureFlag) }
     }
 
+    @Test
+    fun `rate app feature flag is not refreshed from network each time`() = runTest {
+        // This is explicitly checked as it'd be expensive due to high frequency of calls to this use case.
+        // Refresh from network happens at app launch through RefreshFeatureFlags use case
+        // given
+        mockFeatureFlagValue(userId, true)
+        mockScreenViews(2)
+
+        // when
+        shouldStartRateAppFlow(userId)
+
+        // then
+        val featureId = MailFeatureFlags.ShowReviewAppDialog.featureId
+        val default = FeatureFlag.default(featureId.id, false)
+        coVerify { featureFlagManager.getOrDefault(userId, featureId, default, false) }
+    }
+
+
     private suspend fun mockFeatureFlagValue(userId: UserId, isEnabled: Boolean) {
         val featureId = MailFeatureFlags.ShowReviewAppDialog.featureId
         coEvery {

app/src/test/java/ch/protonmail/android/mailbox/presentation/viewmodel/MailboxViewModelTest.kt

@@ -41,7 +41,7 @@ import ch.protonmail.android.di.JobEntryPoint
 import ch.protonmail.android.domain.loadMoreFlowOf
 import ch.protonmail.android.domain.withLoadMore
 import ch.protonmail.android.feature.NotLoggedIn
-import ch.protonmail.android.feature.rating.usecase.StartRateAppFlowIfNeeded
+import ch.protonmail.android.feature.rating.usecase.ShouldStartRateAppFlow
 import ch.protonmail.android.labels.domain.LabelRepository
 import ch.protonmail.android.labels.domain.model.Label
 import ch.protonmail.android.labels.domain.model.LabelId
@@ -181,7 +181,7 @@ class MailboxViewModelTest : ArchTest by ArchTest(),
     private val fetchEventsAndReschedule: FetchEventsAndReschedule = mockk {
         coEvery { this@mockk.invoke() } just runs
     }
-    private val startRateAppFlowIfNeeded: StartRateAppFlowIfNeeded = mockk()
+    private val shouldStartRateAppFlow: ShouldStartRateAppFlow = mockk()
 
     private lateinit var viewModel: MailboxViewModel
 
@@ -277,7 +277,7 @@ class MailboxViewModelTest : ArchTest by ArchTest(),
             mailboxItemUiModelMapper = mailboxItemUiModelMapper,
             fetchEventsAndReschedule = fetchEventsAndReschedule,
             clearNotificationsForUser = clearNotificationsForUser,
-            startRateAppFlowIfNeeded = startRateAppFlowIfNeeded
+            shouldStartRateAppFlow = shouldStartRateAppFlow
         )
     }
 
@@ -612,19 +612,35 @@ class MailboxViewModelTest : ArchTest by ArchTest(),
         }
 
     @Test
-    fun `calls to start rate app flow if needed delegates to use case`() = runTest {
+    fun `emit start rate app flow event when use case return true`() = runTest(dispatchers.Main) {
         // given
-        coEvery { startRateAppFlowIfNeeded.invoke(testUserId) } returns Unit
+        coEvery { shouldStartRateAppFlow.invoke(testUserId) } returns true
 
-        // when
+        viewModel.startRateAppFlow.test {
-        viewModel.startRateAppFlowIfNeeded()
+            // when
+            viewModel.startRateAppFlowIfNeeded()
 
-        // then
+            // then
-        coVerify { startRateAppFlowIfNeeded.invoke(testUserId) }
+            awaitItem()
+        }
     }
 
     @Test
-    fun `start rate app flow if needed is not called when current user id is invalid`() = runTest {
+    fun `does not emit start rate app flow event when use case returns false`() = runTest(dispatchers.Main) {
+        // given
+        coEvery { shouldStartRateAppFlow.invoke(testUserId) } returns false
+
+        viewModel.startRateAppFlow.test {
+            // when
+            viewModel.startRateAppFlowIfNeeded()
+
+            // then
+            ensureAllEventsConsumed()
+        }
+    }
+
+    @Test
+    fun `start rate app flow if needed is not called when current user id is invalid`() = runTest(dispatchers.Main) {
         // given
         every { userManager.currentUserId } returns null
 
@@ -632,7 +648,7 @@ class MailboxViewModelTest : ArchTest by ArchTest(),
         viewModel.startRateAppFlowIfNeeded()
 
         // then
-        verify { startRateAppFlowIfNeeded wasNot Called }
+        verify { shouldStartRateAppFlow wasNot Called }
     }
 
     private fun List<MailboxItemUiModel>.toMailboxState(): MailboxListState.Data =

buildSrc/src/main/kotlin/ProtonMail.kt

@@ -26,7 +26,7 @@ import org.gradle.api.JavaVersion
 object ProtonMail {
 
     const val versionName = "3.0.14"
-    const val versionCode = 935
+    const val versionCode = 936
 
     const val compileSdk = 33
     const val targetSdk = 31

buildSrc/src/main/kotlin/versionsConfig.kt

@@ -62,7 +62,7 @@ fun initVersions() {
 }
 
 // Proton Core
-const val `Proton-core version` =               "10.3.0"
+const val `Proton-core version` =               "10.4.0"
 
 // Test
 const val `aerogear version` =                  "1.0.0"         // Released: Mar 23, 2013