From c5ee5e12bf79d942bcdb661a5616502af4774bb7 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Mon, 16 Jan 2023 14:36:00 +1000 Subject: [PATCH 1/5] Rename blogging reminders resolver to helper --- ...ggingRemindersResolver.kt => BloggingRemindersHelper.kt} | 2 +- .../java/org/wordpress/android/ui/main/WPMainActivity.java | 6 +++--- ...indersResolverTest.kt => BloggingRemindersHelperTest.kt} | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) rename WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/{BloggingRemindersResolver.kt => BloggingRemindersHelper.kt} (98%) rename WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/{BloggingRemindersResolverTest.kt => BloggingRemindersHelperTest.kt} (99%) diff --git a/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolver.kt b/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt similarity index 98% rename from WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolver.kt rename to WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt index ffe8d8306781..25d4a255ccb1 100644 --- a/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolver.kt +++ b/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt @@ -22,7 +22,7 @@ import org.wordpress.android.workers.reminder.ReminderScheduler import javax.inject.Inject import javax.inject.Named -class BloggingRemindersResolver @Inject constructor( +class BloggingRemindersHelper @Inject constructor( private val jetpackBloggingRemindersSyncFlag: JetpackBloggingRemindersSyncFlag, private val appPrefsWrapper: AppPrefsWrapper, private val bloggingRemindersSyncAnalyticsTracker: BloggingRemindersSyncAnalyticsTracker, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java index db35bc4e18f4..c39b3b22f819 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java @@ -36,7 +36,7 @@ import org.wordpress.android.WordPress; import org.wordpress.android.analytics.AnalyticsTracker; import org.wordpress.android.analytics.AnalyticsTracker.Stat; -import org.wordpress.android.bloggingreminders.resolver.BloggingRemindersResolver; +import org.wordpress.android.bloggingreminders.resolver.BloggingRemindersHelper; import org.wordpress.android.fluxc.Dispatcher; import org.wordpress.android.fluxc.generated.AccountActionBuilder; import org.wordpress.android.fluxc.generated.SiteActionBuilder; @@ -264,7 +264,7 @@ public class WPMainActivity extends LocaleAwareActivity implements @Inject WeeklyRoundupScheduler mWeeklyRoundupScheduler; @Inject MySiteDashboardTodaysStatsCardFeatureConfig mTodaysStatsCardFeatureConfig; @Inject QuickStartTracker mQuickStartTracker; - @Inject BloggingRemindersResolver mBloggingRemindersResolver; + @Inject BloggingRemindersHelper mBloggingRemindersHelper; @Inject JetpackAppMigrationFlowUtils mJetpackAppMigrationFlowUtils; @Inject DeepLinkOpenWebLinksWithJetpackHelper mDeepLinkOpenWebLinksWithJetpackHelper; @Inject OpenWebLinksWithJetpackFlowFeatureConfig mOpenWebLinksWithJetpackFlowFeatureConfig; @@ -1657,7 +1657,7 @@ public void onSiteChanged(OnSiteChanged event) { mSelectedSiteRepository.updateSite(site); } } - mBloggingRemindersResolver.trySyncBloggingReminders( + mBloggingRemindersHelper.trySyncBloggingReminders( () -> Unit.INSTANCE, () -> Unit.INSTANCE ); } diff --git a/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolverTest.kt b/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt similarity index 99% rename from WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolverTest.kt rename to WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt index 1bc3d7493e3b..e99ae8f98150 100644 --- a/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersResolverTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt @@ -37,7 +37,7 @@ import java.time.DayOfWeek // TODO: adapt these tests to the unified provider / orchestrator approach @Ignore("Disabled for now: will refactor in another PR after unification.") @ExperimentalCoroutinesApi -class BloggingRemindersResolverTest : BaseUnitTest() { +class BloggingRemindersHelperTest : BaseUnitTest() { private val jetpackBloggingRemindersSyncFlag: JetpackBloggingRemindersSyncFlag = mock() private val contextProvider: ContextProvider = mock() private val wordPressPublicData: WordPressPublicData = mock() @@ -49,7 +49,7 @@ class BloggingRemindersResolverTest : BaseUnitTest() { private val reminderScheduler: ReminderScheduler = mock() private val bloggingRemindersModelMapper: BloggingRemindersModelMapper = mock() private val localMigrationContentResolver: LocalMigrationContentResolver = mock() - private val classToTest = BloggingRemindersResolver( + private val classToTest = BloggingRemindersHelper( jetpackBloggingRemindersSyncFlag, appPrefsWrapper, bloggingRemindersSyncAnalyticsTracker, From bf6790a58c22320759909c1790610cd9224549ce Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 17 Jan 2023 15:51:35 +1000 Subject: [PATCH 2/5] Add generic type parameter for orElse method of LocalMigrationResult This is useful to provide continuity of type inference through the usage of the orElse method, allowing it to be used in the middle of a result chain without obscuring the passed data subtype to downstream success handlers. --- .../android/localcontentmigration/LocalMigrationResult.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationResult.kt b/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationResult.kt index 144e38d942e8..335df661b44a 100644 --- a/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationResult.kt +++ b/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationResult.kt @@ -32,8 +32,8 @@ fun LocalMigrationResult this } -fun LocalMigrationResult.orElse( - handleError: (E) -> LocalMigrationResult +fun LocalMigrationResult.orElse( + handleError: (E) -> LocalMigrationResult ) = when (this) { is Success -> this is Failure -> handleError(this.error) From db57027ef0ed284bb5aa4a284890bc9d8d288ba7 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Tue, 17 Jan 2023 15:54:54 +1000 Subject: [PATCH 3/5] Refactor and standardize blogging reminder helper This also moves the blogging reminder migration step to be during the migration flow, instead of afterwards. --- .../resolver/BloggingRemindersHelper.kt | 94 ++++++++----------- .../LocalMigrationError.kt | 3 + .../resolver/LocalMigrationOrchestrator.kt | 3 + .../android/ui/main/WPMainActivity.java | 4 - .../resolver/BloggingRemindersHelperTest.kt | 37 ++++---- 5 files changed, 62 insertions(+), 79 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt b/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt index 25d4a255ccb1..fe8045e2116f 100644 --- a/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelper.kt @@ -1,8 +1,7 @@ package org.wordpress.android.bloggingreminders.resolver -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.first -import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.wordpress.android.bloggingreminders.BloggingRemindersSyncAnalyticsTracker import org.wordpress.android.bloggingreminders.BloggingRemindersSyncAnalyticsTracker.ErrorType import org.wordpress.android.bloggingreminders.JetpackBloggingRemindersSyncFlag @@ -12,15 +11,17 @@ import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.localcontentmigration.LocalContentEntity.BloggingReminders import org.wordpress.android.localcontentmigration.LocalContentEntityData.BloggingRemindersData import org.wordpress.android.localcontentmigration.LocalMigrationContentResolver -import org.wordpress.android.localcontentmigration.LocalMigrationResult.Companion.EmptyResult -import org.wordpress.android.localcontentmigration.otherwise +import org.wordpress.android.localcontentmigration.LocalMigrationError.FeatureDisabled.BloggingRemindersSyncDisabled +import org.wordpress.android.localcontentmigration.LocalMigrationError.MigrationAlreadyAttempted.BloggingRemindersSyncAlreadyAttempted +import org.wordpress.android.localcontentmigration.LocalMigrationError.PersistenceError.FailedToSaveBloggingRemindersWithException +import org.wordpress.android.localcontentmigration.LocalMigrationResult.Failure +import org.wordpress.android.localcontentmigration.LocalMigrationResult.Success +import org.wordpress.android.localcontentmigration.orElse import org.wordpress.android.localcontentmigration.thenWith -import org.wordpress.android.modules.APPLICATION_SCOPE import org.wordpress.android.ui.bloggingreminders.BloggingRemindersModelMapper import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.workers.reminder.ReminderScheduler import javax.inject.Inject -import javax.inject.Named class BloggingRemindersHelper @Inject constructor( private val jetpackBloggingRemindersSyncFlag: JetpackBloggingRemindersSyncFlag, @@ -28,69 +29,50 @@ class BloggingRemindersHelper @Inject constructor( private val bloggingRemindersSyncAnalyticsTracker: BloggingRemindersSyncAnalyticsTracker, private val siteStore: SiteStore, private val bloggingRemindersStore: BloggingRemindersStore, - @Named(APPLICATION_SCOPE) private val coroutineScope: CoroutineScope, private val reminderScheduler: ReminderScheduler, private val bloggingRemindersModelMapper: BloggingRemindersModelMapper, private val localMigrationContentResolver: LocalMigrationContentResolver, ) { - fun trySyncBloggingReminders(onSuccess: () -> Unit, onFailure: () -> Unit) { - if (!shouldTrySyncBloggingReminders()) { - onFailure() - return - } + fun migrateBloggingReminders() = if (!jetpackBloggingRemindersSyncFlag.isEnabled()) { + Failure(BloggingRemindersSyncDisabled) + } else if (!appPrefsWrapper.getIsFirstTryBloggingRemindersSyncJetpack()) { + Failure(BloggingRemindersSyncAlreadyAttempted) + } else { + bloggingRemindersSyncAnalyticsTracker.trackStart() + appPrefsWrapper.saveIsFirstTryBloggingRemindersSyncJetpack(false) localMigrationContentResolver.getResultForEntityType(BloggingReminders) - .thenWith { (reminders) -> - if (reminders.isNotEmpty()) { - val success = setBloggingReminders(reminders) - if (success) onSuccess() else onFailure() + .orElse { + bloggingRemindersSyncAnalyticsTracker.trackFailed(ErrorType.QueryBloggingRemindersError) + Failure(it) + } + }.thenWith(::setBloggingReminders) + + private fun setBloggingReminders(bloggingRemindersData: BloggingRemindersData) = runCatching { + bloggingRemindersData.reminders.count { bloggingReminder -> + siteStore.getSiteByLocalId(bloggingReminder.siteId)?.let { _ -> + if (!isBloggingReminderAlreadySet(bloggingReminder.siteId)) { + updateBloggingReminders(bloggingReminder) + setLocalReminderNotification(bloggingReminder) + true } else { - bloggingRemindersSyncAnalyticsTracker.trackSuccess(0) - onSuccess() + false } - EmptyResult - }.otherwise { onFailure() } + } ?: false + }.let { bloggingRemindersSyncAnalyticsTracker.trackSuccess(it) } + Success(bloggingRemindersData) + }.getOrElse { throwable -> + bloggingRemindersSyncAnalyticsTracker.trackFailed(ErrorType.UpdateBloggingRemindersError) + Failure(FailedToSaveBloggingRemindersWithException(throwable)) } - @Suppress("ReturnCount") - private fun shouldTrySyncBloggingReminders(): Boolean { - val isFeatureFlagEnabled = jetpackBloggingRemindersSyncFlag.isEnabled() - if (!isFeatureFlagEnabled) { - return false - } - val isFirstTry = appPrefsWrapper.getIsFirstTryBloggingRemindersSyncJetpack() - if (!isFirstTry) { - return false - } - bloggingRemindersSyncAnalyticsTracker.trackStart() - appPrefsWrapper.saveIsFirstTryBloggingRemindersSyncJetpack(false) - return true + private fun isBloggingReminderAlreadySet(siteLocalId: Int) = runBlocking { + bloggingRemindersStore.bloggingRemindersModel(siteLocalId).first().enabledDays.isNotEmpty() } - @Suppress("TooGenericExceptionCaught", "SwallowedException") - private fun setBloggingReminders(reminders: List): Boolean { - try { - coroutineScope.launch { - var syncCount = 0 - for (bloggingReminder in reminders) { - val site = siteStore.getSiteByLocalId(bloggingReminder.siteId) - if (site != null && !isBloggingReminderAlreadySet(bloggingReminder.siteId)) { - bloggingRemindersStore.updateBloggingReminders(bloggingReminder) - setLocalReminderNotification(bloggingReminder) - syncCount = syncCount.inc() - } - } - bloggingRemindersSyncAnalyticsTracker.trackSuccess(syncCount) - } - return true - } catch (exception: Exception) { - bloggingRemindersSyncAnalyticsTracker.trackFailed(ErrorType.UpdateBloggingRemindersError) - return false - } + private fun updateBloggingReminders(bloggingReminder: BloggingRemindersModel) = runBlocking { + bloggingRemindersStore.updateBloggingReminders(bloggingReminder) } - private suspend fun isBloggingReminderAlreadySet(siteLocalId: Int) = - bloggingRemindersStore.bloggingRemindersModel(siteLocalId).first().enabledDays.isNotEmpty() - private fun setLocalReminderNotification(bloggingRemindersModel: BloggingRemindersModel) { val bloggingRemindersUiModel = bloggingRemindersModelMapper.toUiModel(bloggingRemindersModel) reminderScheduler.schedule( diff --git a/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationError.kt b/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationError.kt index c17ca0fe8274..d930e247b786 100644 --- a/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationError.kt +++ b/WordPress/src/main/java/org/wordpress/android/localcontentmigration/LocalMigrationError.kt @@ -18,12 +18,14 @@ sealed class LocalMigrationError { object SharedLoginDisabled : FeatureDisabled() object UserFlagsDisabled : FeatureDisabled() object ReaderSavedPostsDisabled : FeatureDisabled() + object BloggingRemindersSyncDisabled : FeatureDisabled() } sealed class MigrationAlreadyAttempted : LocalMigrationError() { object SharedLoginAlreadyAttempted : MigrationAlreadyAttempted() object UserFlagsAlreadyAttempted : MigrationAlreadyAttempted() object ReaderSavedPostsAlreadyAttempted : MigrationAlreadyAttempted() + object BloggingRemindersSyncAlreadyAttempted : MigrationAlreadyAttempted() } sealed class PersistenceError : LocalMigrationError() { @@ -31,6 +33,7 @@ sealed class LocalMigrationError { object FailedToSaveUserFlags : PersistenceError() data class FailedToSaveUserFlagsWithException(val throwable: Throwable) : PersistenceError() object FailedToSaveReaderSavedPosts : PersistenceError() + data class FailedToSaveBloggingRemindersWithException(val throwable: Throwable) : PersistenceError() sealed class LocalPostsPersistenceError : PersistenceError() { data class FailedToResetSequenceForPosts(val throwable: Throwable) : LocalPostsPersistenceError() data class FailedToInsertLocalPost(val post: PostModel) : LocalPostsPersistenceError() diff --git a/WordPress/src/main/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestrator.kt b/WordPress/src/main/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestrator.kt index 8812ca059aa7..93e3d4858ce7 100644 --- a/WordPress/src/main/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestrator.kt +++ b/WordPress/src/main/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestrator.kt @@ -1,6 +1,7 @@ package org.wordpress.android.sharedlogin.resolver import kotlinx.coroutines.flow.MutableStateFlow +import org.wordpress.android.bloggingreminders.resolver.BloggingRemindersHelper import org.wordpress.android.localcontentmigration.ContentMigrationAnalyticsTracker import org.wordpress.android.localcontentmigration.ContentMigrationAnalyticsTracker.ErrorType.LocalDraftContent import org.wordpress.android.localcontentmigration.EligibilityHelper @@ -42,6 +43,7 @@ class LocalMigrationOrchestrator @Inject constructor( private val sitesMigrationHelper: SitesMigrationHelper, private val localPostsHelper: LocalPostsHelper, private val eligibilityHelper: EligibilityHelper, + private val bloggingRemindersHelper: BloggingRemindersHelper, ) { fun tryLocalMigration(migrationStateFlow: MutableStateFlow) { eligibilityHelper.validate() @@ -50,6 +52,7 @@ class LocalMigrationOrchestrator @Inject constructor( .then(userFlagsHelper::migrateUserFlags) .then(readerSavedPostsHelper::migrateReaderSavedPosts) .then(localPostsHelper::migratePosts) + .then(bloggingRemindersHelper::migrateBloggingReminders) .orElse { error -> migrationStateFlow.value = when (error) { is Ineligibility -> Ineligible diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java index c39b3b22f819..31e2d66e331b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java @@ -264,7 +264,6 @@ public class WPMainActivity extends LocaleAwareActivity implements @Inject WeeklyRoundupScheduler mWeeklyRoundupScheduler; @Inject MySiteDashboardTodaysStatsCardFeatureConfig mTodaysStatsCardFeatureConfig; @Inject QuickStartTracker mQuickStartTracker; - @Inject BloggingRemindersHelper mBloggingRemindersHelper; @Inject JetpackAppMigrationFlowUtils mJetpackAppMigrationFlowUtils; @Inject DeepLinkOpenWebLinksWithJetpackHelper mDeepLinkOpenWebLinksWithJetpackHelper; @Inject OpenWebLinksWithJetpackFlowFeatureConfig mOpenWebLinksWithJetpackFlowFeatureConfig; @@ -1657,9 +1656,6 @@ public void onSiteChanged(OnSiteChanged event) { mSelectedSiteRepository.updateSite(site); } } - mBloggingRemindersHelper.trySyncBloggingReminders( - () -> Unit.INSTANCE, () -> Unit.INSTANCE - ); } @SuppressWarnings("unused") diff --git a/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt index e99ae8f98150..c3072d12a684 100644 --- a/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/bloggingreminders/resolver/BloggingRemindersHelperTest.kt @@ -55,7 +55,6 @@ class BloggingRemindersHelperTest : BaseUnitTest() { bloggingRemindersSyncAnalyticsTracker, siteStore, bloggingRemindersStore, - testScope(), reminderScheduler, bloggingRemindersModelMapper, localMigrationContentResolver, @@ -85,7 +84,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { @Test fun `Should track start if feature flag is ENABLED and IS first try`() { featureEnabled() - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersSyncAnalyticsTracker).trackStart() } @@ -93,7 +92,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { fun `Should trigger failure callback if feature flag is DISABLED`() { whenever(jetpackBloggingRemindersSyncFlag.isEnabled()).thenReturn(false) val onFailure: () -> Unit = mock() - classToTest.trySyncBloggingReminders({}, onFailure) + classToTest.migrateBloggingReminders() verify(onFailure).invoke() } @@ -102,21 +101,21 @@ class BloggingRemindersHelperTest : BaseUnitTest() { whenever(appPrefsWrapper.getIsFirstTryBloggingRemindersSyncJetpack()).thenReturn(false) whenever(jetpackBloggingRemindersSyncFlag.isEnabled()).thenReturn(true) val onFailure: () -> Unit = mock() - classToTest.trySyncBloggingReminders({}, onFailure) + classToTest.migrateBloggingReminders() verify(onFailure).invoke() } @Test fun `Should save IS NOT first try sync blogging reminders as FALSE if feature flag is ENABLED and IS first try`() { featureEnabled() - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(appPrefsWrapper).saveIsFirstTryBloggingRemindersSyncJetpack(false) } @Test fun `Should NOT query ContentResolver if feature flag is DISABLED`() { whenever(jetpackBloggingRemindersSyncFlag.isEnabled()).thenReturn(false) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(contentResolverWrapper, never()).queryUri(any(), any()) } @@ -124,14 +123,14 @@ class BloggingRemindersHelperTest : BaseUnitTest() { fun `Should NOT query ContentResolver if IS NOT the first try`() { whenever(appPrefsWrapper.getIsFirstTryBloggingRemindersSyncJetpack()).thenReturn(false) whenever(jetpackBloggingRemindersSyncFlag.isEnabled()).thenReturn(true) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(contentResolverWrapper, never()).queryUri(any(), any()) } @Test fun `Should query ContentResolver if feature flag is ENABLED and IS first try`() { featureEnabled() - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() // verify(contentResolverWrapper).queryUri(contentResolver, uriValue) } @@ -139,7 +138,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { fun `Should track failed with error QueryBloggingRemindersError if cursor is null`() { featureEnabled() // whenever(contentResolverWrapper.queryUri(contentResolver, uriValue)).thenReturn(null) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersSyncAnalyticsTracker).trackFailed(QueryBloggingRemindersError) } @@ -148,14 +147,14 @@ class BloggingRemindersHelperTest : BaseUnitTest() { featureEnabled() // whenever(contentResolverWrapper.queryUri(contentResolver, uriValue)).thenReturn(null) val onFailure: () -> Unit = mock() - classToTest.trySyncBloggingReminders({}, onFailure) + classToTest.migrateBloggingReminders() verify(onFailure).invoke() } @Test fun `Should track success with reminders synced count 0 if result map is empty`() { featureEnabled() - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersSyncAnalyticsTracker).trackSuccess(0) } @@ -163,7 +162,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { fun `Should trigger failure callback if result map is empty`() { featureEnabled() val onSuccess: () -> Unit = mock() - classToTest.trySyncBloggingReminders(onSuccess) {} + classToTest.migrateBloggingReminders() verify(onSuccess).invoke() } @@ -177,7 +176,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[\"MONDAY\"],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersSyncAnalyticsTracker).trackSuccess(1) } @@ -191,7 +190,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) val onSuccess: () -> Unit = mock() - classToTest.trySyncBloggingReminders(onSuccess) {} + classToTest.migrateBloggingReminders() verify(onSuccess).invoke() } @@ -205,7 +204,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[\"MONDAY\"],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersStore, times(1)).updateBloggingReminders( BloggingRemindersModel( siteId = validLocalId, @@ -227,7 +226,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[\"MONDAY\"],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersModelMapper).toUiModel(userSetBloggingRemindersModel) } @@ -241,7 +240,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[\"MONDAY\"],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(reminderScheduler).schedule( validLocalId, bloggingRemindersUiModel.hour, @@ -258,7 +257,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[\"MONDAY\"],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":$invalidLocalId}}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersStore, times(0)).updateBloggingReminders(any()) } @@ -271,7 +270,7 @@ class BloggingRemindersHelperTest : BaseUnitTest() { "[{\"enabledDays\":[],\"hour\":5" + ",\"isPromptIncluded\":false,\"minute\":43,\"siteId\":123}]" ) - classToTest.trySyncBloggingReminders({}, {}) + classToTest.migrateBloggingReminders() verify(bloggingRemindersStore, times(0)).updateBloggingReminders(any()) } From efe543358c5b5479c8e2fb7c979617512ead95ec Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 18 Jan 2023 12:38:27 +1000 Subject: [PATCH 4/5] Update migration orchestrator test --- .../sharedlogin/resolver/LocalMigrationOrchestratorTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/WordPress/src/test/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestratorTest.kt b/WordPress/src/test/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestratorTest.kt index 81ac689c7bcf..43c107dfad39 100644 --- a/WordPress/src/test/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestratorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/sharedlogin/resolver/LocalMigrationOrchestratorTest.kt @@ -8,6 +8,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest +import org.wordpress.android.bloggingreminders.resolver.BloggingRemindersHelper import org.wordpress.android.fluxc.model.PostModel import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.localcontentmigration.ContentMigrationAnalyticsTracker @@ -19,6 +20,7 @@ import org.wordpress.android.localcontentmigration.LocalContentEntity.ReaderPost import org.wordpress.android.localcontentmigration.LocalContentEntity.Sites import org.wordpress.android.localcontentmigration.LocalContentEntity.UserFlags import org.wordpress.android.localcontentmigration.LocalContentEntityData.AccessTokenData +import org.wordpress.android.localcontentmigration.LocalContentEntityData.BloggingRemindersData import org.wordpress.android.localcontentmigration.LocalContentEntityData.Companion.IneligibleReason.LocalDraftContentIsPresent import org.wordpress.android.localcontentmigration.LocalContentEntityData.Companion.IneligibleReason.WPNotLoggedIn import org.wordpress.android.localcontentmigration.LocalContentEntityData.EligibilityStatusData @@ -59,6 +61,7 @@ class LocalMigrationOrchestratorTest : BaseUnitTest() { private val sitesMigrationHelper: SitesMigrationHelper = mock() private val localPostsHelper: LocalPostsHelper = mock() private val eligibilityHelper: EligibilityHelper = mock() + private val bloggingRemindersHelper: BloggingRemindersHelper = mock() private val classToTest = LocalMigrationOrchestrator( sharedLoginAnalyticsTracker, migrationAnalyticsTracker, @@ -68,6 +71,7 @@ class LocalMigrationOrchestratorTest : BaseUnitTest() { sitesMigrationHelper, localPostsHelper, eligibilityHelper, + bloggingRemindersHelper, ) private val avatarUrl = "avatarUrl" private val sites = listOf(SiteModel(), SiteModel()) @@ -211,6 +215,8 @@ class LocalMigrationOrchestratorTest : BaseUnitTest() { whenever(readerSavedPostsHelper.migrateReaderSavedPosts()) .thenReturn(Success(ReaderPostsData(ReaderPostList()))) whenever(localPostsHelper.migratePosts()).thenReturn(Success(PostData(PostModel()))) + whenever(bloggingRemindersHelper.migrateBloggingReminders()) + .thenReturn(Success(BloggingRemindersData(listOf()))) } private fun assertFailure(error: ProviderError) { From 0d751e389b56a91f76095a92d0deea583fd87253 Mon Sep 17 00:00:00 2001 From: Matthew Kevins Date: Wed, 18 Jan 2023 14:23:36 +1000 Subject: [PATCH 5/5] Remove unused imports --- .../main/java/org/wordpress/android/ui/main/WPMainActivity.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java index 31e2d66e331b..3343da7a965a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java @@ -36,7 +36,6 @@ import org.wordpress.android.WordPress; import org.wordpress.android.analytics.AnalyticsTracker; import org.wordpress.android.analytics.AnalyticsTracker.Stat; -import org.wordpress.android.bloggingreminders.resolver.BloggingRemindersHelper; import org.wordpress.android.fluxc.Dispatcher; import org.wordpress.android.fluxc.generated.AccountActionBuilder; import org.wordpress.android.fluxc.generated.SiteActionBuilder; @@ -172,7 +171,6 @@ import static org.wordpress.android.ui.JetpackConnectionSource.NOTIFICATIONS; import dagger.hilt.android.AndroidEntryPoint; -import kotlin.Unit; /** * Main activity which hosts sites, reader, me and notifications pages