Merge pull request #5501 from seadowg/google-drive-fixes

Fix Google Drive banner and server switching

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 = "[email protected]"
+        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 = "[email protected]"

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);
         }
     }
 }

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

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<String, MutableLiveData<Boolean>>()
-    private val lastSyncFailure = mutableMapOf<String, MutableLiveData<FormSourceException?>>()
+class SyncStatusAppState(private val appState: AppState, private val context: Context) {
 
     fun isSyncing(projectId: String): LiveData<Boolean> {
         return getSyncingLiveData(projectId)
@@ -32,8 +28,18 @@ 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<FormSourceException>(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"
+    }
 }

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;
@@ -309,8 +310,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
@@ -349,9 +350,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

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<PropertyManager>()
     private val formUpdateScheduler = mock<FormUpdateScheduler>()
-    private var handler = CollectSettingsChangeHandler(propertyManager, formUpdateScheduler)
+    private val syncStatusAppState = mock<SyncStatusAppState>()
+    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")

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,15 +63,15 @@ 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));
     }
 
     @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,9 +80,22 @@ 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);
     }
+
+    @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()));
+    }
 }