From f55cc0fe01194e9eda06ab50a32d76c9dbdb11e0 Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 17 Mar 2023 11:55:52 +0000 Subject: [PATCH 1/3] Don't show Google Drive banner when switching away from it --- .../GoogleDriveDeprecationBannerTest.kt | 22 +++++++++++++++++++ .../android/activities/MainMenuActivity.java | 2 ++ 2 files changed, 24 insertions(+) diff --git a/collect_app/src/androidTest/java/org/odk/collect/android/feature/projects/GoogleDriveDeprecationBannerTest.kt b/collect_app/src/androidTest/java/org/odk/collect/android/feature/projects/GoogleDriveDeprecationBannerTest.kt index 38850bda0ed..9f8017abc77 100644 --- a/collect_app/src/androidTest/java/org/odk/collect/android/feature/projects/GoogleDriveDeprecationBannerTest.kt +++ b/collect_app/src/androidTest/java/org/odk/collect/android/feature/projects/GoogleDriveDeprecationBannerTest.kt @@ -11,6 +11,7 @@ import org.odk.collect.android.R import org.odk.collect.android.activities.WebViewActivity import org.odk.collect.android.support.TestDependencies import org.odk.collect.android.support.pages.MainMenuPage +import org.odk.collect.android.support.pages.ProjectSettingsPage import org.odk.collect.android.support.rules.CollectTestRule import org.odk.collect.android.support.rules.TestRuleChain import org.odk.collect.androidtest.RecordedIntentsRule @@ -44,6 +45,27 @@ class GoogleDriveDeprecationBannerTest { .assertText(R.string.google_drive_deprecation_message) } + @Test + fun bannerDisappearsAfterSwitchingFromGoogleDriveProjectToOdkServer() { + val googleAccount = "steph@curry.basket" + testDependencies.googleAccountPicker.setDeviceAccount(googleAccount) + + rule.startAtMainMenu() + .openProjectSettingsDialog() + .clickAddProject() + .switchToManualMode() + .openGooglePickerAndSelect(googleAccount) + .assertText(R.string.google_drive_deprecation_message) + .openProjectSettingsDialog() + .clickSettings() + .clickServerSettings() + .clickOnServerType() + .clickOnString(R.string.server_platform_odk) + .pressBack(ProjectSettingsPage()) + .pressBack(MainMenuPage()) + .assertTextDoesNotExist(R.string.google_drive_deprecation_message) + } + @Test fun forumThreadIsOpenedAfterClickingLearnMore() { val googleAccount = "steph@curry.basket" diff --git a/collect_app/src/main/java/org/odk/collect/android/activities/MainMenuActivity.java b/collect_app/src/main/java/org/odk/collect/android/activities/MainMenuActivity.java index b6b22408705..2e894bce537 100644 --- a/collect_app/src/main/java/org/odk/collect/android/activities/MainMenuActivity.java +++ b/collect_app/src/main/java/org/odk/collect/android/activities/MainMenuActivity.java @@ -328,6 +328,8 @@ private void manageGoogleDriveDeprecationBanner() { unprotectedSettings.save(ProjectKeys.GOOGLE_DRIVE_DEPRECATION_BANNER_DISMISSED, true); }); } + } else { + findViewById(R.id.google_drive_deprecation_banner).setVisibility(View.GONE); } } } From 67bc2d70bed6799507df9b3f23ce54f39b60952c Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 17 Mar 2023 15:57:32 +0000 Subject: [PATCH 2/3] Convert SyncStateAppState to use AppState --- .../matchexactly/SyncStatusAppState.kt | 17 +++++++++-------- .../injection/config/AppDependencyModule.java | 6 +++--- .../formmanagement/SyncStatusAppStateTest.java | 15 +++++++++------ 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt index 917bc90740d..abaa54398aa 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt @@ -4,14 +4,10 @@ import android.content.Context import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import org.odk.collect.android.external.FormsContract +import org.odk.collect.androidshared.data.AppState import org.odk.collect.forms.FormSourceException -import javax.inject.Singleton -@Singleton -class SyncStatusAppState(private val context: Context) { - - private val syncing = mutableMapOf>() - private val lastSyncFailure = mutableMapOf>() +class SyncStatusAppState(private val appState: AppState, private val context: Context) { fun isSyncing(projectId: String): LiveData { return getSyncingLiveData(projectId) @@ -32,8 +28,13 @@ class SyncStatusAppState(private val context: Context) { } private fun getSyncingLiveData(projectId: String) = - syncing.getOrPut(projectId) { MutableLiveData(false) } + appState.get("$KEY_PREFIX_SYNCING:$projectId", MutableLiveData(false)) private fun getSyncErrorLiveData(projectId: String) = - lastSyncFailure.getOrPut(projectId) { MutableLiveData(null) } + appState.get("$KEY_PREFIX_ERROR:$projectId", MutableLiveData(null)) + + companion object { + const val KEY_PREFIX_SYNCING = "syncStatusSyncing" + const val KEY_PREFIX_ERROR = "syncStatusError" + } } diff --git a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java index 8ed6f581fd5..31a06787eef 100644 --- a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java +++ b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java @@ -1,6 +1,7 @@ package org.odk.collect.android.injection.config; import static androidx.core.content.FileProvider.getUriForFile; +import static org.odk.collect.androidshared.data.AppStateKt.getState; import static org.odk.collect.settings.keys.MetaKeys.KEY_INSTALL_ID; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -353,9 +354,8 @@ public QRCodeDecoder providesQRCodeDecoder() { } @Provides - @Singleton - public SyncStatusAppState providesServerFormSyncRepository(Context context) { - return new SyncStatusAppState(context); + public SyncStatusAppState providesServerFormSyncRepository(Application application) { + return new SyncStatusAppState(getState(application), application); } @Provides diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java index a60fc2b5576..529f354fdba 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import org.odk.collect.android.formmanagement.matchexactly.SyncStatusAppState; import org.odk.collect.android.external.FormsContract; +import org.odk.collect.androidshared.data.AppState; import org.odk.collect.forms.FormSourceException; import static org.hamcrest.MatcherAssert.assertThat; @@ -25,6 +26,8 @@ public class SyncStatusAppStateTest { private final Context context = mock(Context.class); private final ContentResolver contentResolver = mock(ContentResolver.class); + private final AppState appState = new AppState(); + @Rule public InstantTaskExecutorRule instantTaskExecutorRule = new InstantTaskExecutorRule(); @@ -35,13 +38,13 @@ public void setup() { @Test public void getSyncError_isNullAtFirst() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); assertThat(syncStatusAppState.getSyncError("projectId").getValue(), is(nullValue())); } @Test public void getSyncError_whenFinishSyncWithException_isException() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); syncStatusAppState.startSync("projectId"); FormSourceException exception = new FormSourceException.FetchError(); syncStatusAppState.finishSync("projectId", exception); @@ -51,7 +54,7 @@ public void getSyncError_whenFinishSyncWithException_isException() { @Test public void getSyncError_whenFinishSyncWithNull_isNull() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); syncStatusAppState.startSync("projectId"); syncStatusAppState.finishSync("projectId", null); @@ -60,7 +63,7 @@ public void getSyncError_whenFinishSyncWithNull_isNull() { @Test public void isSyncing_isDifferentForDifferentProjects() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); syncStatusAppState.startSync("projectId"); assertThat(syncStatusAppState.isSyncing("projectId").getValue(), is(true)); assertThat(syncStatusAppState.isSyncing("otherProjectId").getValue(), is(false)); @@ -68,7 +71,7 @@ public void isSyncing_isDifferentForDifferentProjects() { @Test public void getSyncError_isDifferentForDifferentProjects() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); syncStatusAppState.startSync("projectId"); syncStatusAppState.finishSync("projectId", new FormSourceException.FetchError()); assertThat(syncStatusAppState.getSyncError("projectId").getValue(), is(notNullValue())); @@ -77,7 +80,7 @@ public void getSyncError_isDifferentForDifferentProjects() { @Test public void finishSync_updatesFormsContentObserver() { - SyncStatusAppState syncStatusAppState = new SyncStatusAppState(context); + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); syncStatusAppState.startSync("projectId"); syncStatusAppState.finishSync("projectId", null); verify(contentResolver).notifyChange(FormsContract.getUri("projectId"), null); From 60e7cf76bc25f0ee71b60cc3a1ae691b499009ee Mon Sep 17 00:00:00 2001 From: Callum Stott Date: Fri, 17 Mar 2023 16:33:37 +0000 Subject: [PATCH 3/3] Clear sync status when switching servers --- .../CollectSettingsChangeHandler.kt | 8 ++++++- .../matchexactly/SyncStatusAppState.kt | 5 ++++ .../injection/config/AppDependencyModule.java | 6 ++--- .../CollectSettingsChangeHandlerTest.kt | 23 ++++++++++++++++++- .../SyncStatusAppStateTest.java | 13 +++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/application/CollectSettingsChangeHandler.kt b/collect_app/src/main/java/org/odk/collect/android/application/CollectSettingsChangeHandler.kt index b187c6d6bee..e8c4170b3ae 100644 --- a/collect_app/src/main/java/org/odk/collect/android/application/CollectSettingsChangeHandler.kt +++ b/collect_app/src/main/java/org/odk/collect/android/application/CollectSettingsChangeHandler.kt @@ -2,18 +2,24 @@ package org.odk.collect.android.application import org.odk.collect.android.analytics.AnalyticsUtils import org.odk.collect.android.backgroundwork.FormUpdateScheduler +import org.odk.collect.android.formmanagement.matchexactly.SyncStatusAppState import org.odk.collect.android.logic.PropertyManager import org.odk.collect.settings.importing.SettingsChangeHandler import org.odk.collect.settings.keys.ProjectKeys class CollectSettingsChangeHandler( private val propertyManager: PropertyManager, - private val formUpdateScheduler: FormUpdateScheduler + private val formUpdateScheduler: FormUpdateScheduler, + private val syncStatusAppState: SyncStatusAppState ) : SettingsChangeHandler { override fun onSettingChanged(projectId: String, newValue: Any?, changedKey: String) { propertyManager.reload() + if (changedKey == ProjectKeys.KEY_SERVER_URL || changedKey == ProjectKeys.KEY_PROTOCOL) { + syncStatusAppState.clear(projectId) + } + if (changedKey == ProjectKeys.KEY_FORM_UPDATE_MODE || changedKey == ProjectKeys.KEY_PERIODIC_FORM_UPDATES_CHECK || changedKey == ProjectKeys.KEY_PROTOCOL diff --git a/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt b/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt index abaa54398aa..8db9eeea413 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formmanagement/matchexactly/SyncStatusAppState.kt @@ -33,6 +33,11 @@ class SyncStatusAppState(private val appState: AppState, private val context: Co private fun getSyncErrorLiveData(projectId: String) = appState.get("$KEY_PREFIX_ERROR:$projectId", MutableLiveData(null)) + fun clear(projectId: String) { + getSyncingLiveData(projectId).value = false + getSyncErrorLiveData(projectId).value = null + } + companion object { const val KEY_PREFIX_SYNCING = "syncStatusSyncing" const val KEY_PREFIX_ERROR = "syncStatusError" diff --git a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java index 31a06787eef..aae85c2c4c5 100644 --- a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java +++ b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java @@ -98,7 +98,6 @@ import org.odk.collect.android.utilities.FileProvider; import org.odk.collect.android.utilities.FormsDirDiskFormsSynchronizer; import org.odk.collect.android.utilities.FormsRepositoryProvider; -import org.odk.collect.androidshared.bitmap.ImageCompressor; import org.odk.collect.android.utilities.ImageCompressionController; import org.odk.collect.android.utilities.InstancesRepositoryProvider; import org.odk.collect.android.utilities.MediaUtils; @@ -108,6 +107,7 @@ import org.odk.collect.android.utilities.WebCredentialsUtils; import org.odk.collect.android.version.VersionInformation; import org.odk.collect.android.views.BarcodeViewDecoder; +import org.odk.collect.androidshared.bitmap.ImageCompressor; import org.odk.collect.androidshared.network.ConnectivityProvider; import org.odk.collect.androidshared.network.NetworkStateProvider; import org.odk.collect.androidshared.system.IntentLauncher; @@ -314,8 +314,8 @@ public PropertyManager providesPropertyManager(PermissionsProvider permissionsPr } @Provides - public SettingsChangeHandler providesSettingsChangeHandler(PropertyManager propertyManager, FormUpdateScheduler formUpdateScheduler) { - return new CollectSettingsChangeHandler(propertyManager, formUpdateScheduler); + public SettingsChangeHandler providesSettingsChangeHandler(PropertyManager propertyManager, FormUpdateScheduler formUpdateScheduler, SyncStatusAppState syncStatusAppState) { + return new CollectSettingsChangeHandler(propertyManager, formUpdateScheduler, syncStatusAppState); } @Provides diff --git a/collect_app/src/test/java/org/odk/collect/android/application/CollectSettingsChangeHandlerTest.kt b/collect_app/src/test/java/org/odk/collect/android/application/CollectSettingsChangeHandlerTest.kt index 97b715c2e5c..0600b660389 100644 --- a/collect_app/src/test/java/org/odk/collect/android/application/CollectSettingsChangeHandlerTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/application/CollectSettingsChangeHandlerTest.kt @@ -5,13 +5,16 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.odk.collect.android.backgroundwork.FormUpdateScheduler +import org.odk.collect.android.formmanagement.matchexactly.SyncStatusAppState import org.odk.collect.android.logic.PropertyManager import org.odk.collect.settings.keys.ProjectKeys class CollectSettingsChangeHandlerTest { + private val propertyManager = mock() private val formUpdateScheduler = mock() - private var handler = CollectSettingsChangeHandler(propertyManager, formUpdateScheduler) + private val syncStatusAppState = mock() + private val handler = CollectSettingsChangeHandler(propertyManager, formUpdateScheduler, syncStatusAppState) @Test fun `updates PropertyManager when a single setting is changed`() { @@ -41,12 +44,30 @@ class CollectSettingsChangeHandlerTest { verify(formUpdateScheduler).scheduleUpdates("projectId") } + @Test + fun `when changed key is PROTOCOL clears sync status`() { + handler.onSettingChanged("projectId", "anything", ProjectKeys.KEY_PROTOCOL) + verify(syncStatusAppState).clear("projectId") + } + + @Test + fun `when changed key is SERVER_URL clears sync status`() { + handler.onSettingChanged("projectId", "anything", ProjectKeys.KEY_SERVER_URL) + verify(syncStatusAppState).clear("projectId") + } + @Test fun `do not schedule updates if other single settings are changed`() { handler.onSettingChanged("projectId", "anything", "blah") verifyNoInteractions(formUpdateScheduler) } + @Test + fun `do not clear sync status if other single settings are changed`() { + handler.onSettingChanged("projectId", "anything", "blah") + verifyNoInteractions(syncStatusAppState) + } + @Test fun `updates PropertyManager when multiple settings are changed`() { handler.onSettingsChanged("projectId") diff --git a/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java b/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java index 529f354fdba..63630502553 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formmanagement/SyncStatusAppStateTest.java @@ -85,4 +85,17 @@ public void finishSync_updatesFormsContentObserver() { syncStatusAppState.finishSync("projectId", null); verify(contentResolver).notifyChange(FormsContract.getUri("projectId"), null); } + + @Test + public void clear_clearsSyncErrorAndIsSyncing() { + SyncStatusAppState syncStatusAppState = new SyncStatusAppState(appState, context); + + syncStatusAppState.startSync("projectId"); + syncStatusAppState.clear("projectId"); + assertThat(syncStatusAppState.isSyncing("projectId").getValue(), is(false)); + + syncStatusAppState.finishSync("projectId", new FormSourceException.FetchError()); + syncStatusAppState.clear("projectId"); + assertThat(syncStatusAppState.getSyncError("projectId").getValue(), is(nullValue())); + } }