Code review changes

- Autoshielding information prompt will then automatically shield funds after display
- Delay autoshielding information prompt until after sync

.run/_app_androidTest.run.xml

@@ -0,0 +1,53 @@
+<component name="ProjectRunConfigurationManager">
+  <configuration default="false" name=":app:androidTest" type="AndroidTestRunConfigurationType" factoryName="Android Instrumented Tests">
+    <module name="Zcash_Wallet.app" />
+    <option name="TESTING_TYPE" value="0" />
+    <option name="METHOD_NAME" value="" />
+    <option name="CLASS_NAME" value="" />
+    <option name="PACKAGE_NAME" value="" />
+    <option name="INSTRUMENTATION_RUNNER_CLASS" value="" />
+    <option name="EXTRA_OPTIONS" value="" />
+    <option name="INCLUDE_GRADLE_EXTRA_OPTIONS" value="true" />
+    <option name="CLEAR_LOGCAT" value="false" />
+    <option name="SHOW_LOGCAT_AUTOMATICALLY" value="false" />
+    <option name="SKIP_NOOP_APK_INSTALLATIONS" value="true" />
+    <option name="FORCE_STOP_RUNNING_APP" value="true" />
+    <option name="TARGET_SELECTION_MODE" value="DEVICE_AND_SNAPSHOT_COMBO_BOX" />
+    <option name="SELECTED_CLOUD_MATRIX_CONFIGURATION_ID" value="2147483645" />
+    <option name="SELECTED_CLOUD_MATRIX_PROJECT_ID" value="api-9130115880275692386-873230" />
+    <option name="DEBUGGER_TYPE" value="Auto" />
+    <Auto>
+      <option name="USE_JAVA_AWARE_DEBUGGER" value="false" />
+      <option name="SHOW_STATIC_VARS" value="true" />
+      <option name="WORKING_DIR" value="" />
+      <option name="TARGET_LOGGING_CHANNELS" value="lldb process:gdb-remote packets" />
+      <option name="SHOW_OPTIMIZED_WARNING" value="true" />
+    </Auto>
+    <Hybrid>
+      <option name="USE_JAVA_AWARE_DEBUGGER" value="false" />
+      <option name="SHOW_STATIC_VARS" value="true" />
+      <option name="WORKING_DIR" value="" />
+      <option name="TARGET_LOGGING_CHANNELS" value="lldb process:gdb-remote packets" />
+      <option name="SHOW_OPTIMIZED_WARNING" value="true" />
+    </Hybrid>
+    <Java />
+    <Native>
+      <option name="USE_JAVA_AWARE_DEBUGGER" value="false" />
+      <option name="SHOW_STATIC_VARS" value="true" />
+      <option name="WORKING_DIR" value="" />
+      <option name="TARGET_LOGGING_CHANNELS" value="lldb process:gdb-remote packets" />
+      <option name="SHOW_OPTIMIZED_WARNING" value="true" />
+    </Native>
+    <Profilers>
+      <option name="ADVANCED_PROFILING_ENABLED" value="false" />
+      <option name="STARTUP_PROFILING_ENABLED" value="false" />
+      <option name="STARTUP_CPU_PROFILING_ENABLED" value="false" />
+      <option name="STARTUP_CPU_PROFILING_CONFIGURATION_NAME" value="Sample Java Methods" />
+      <option name="STARTUP_NATIVE_MEMORY_PROFILING_ENABLED" value="false" />
+      <option name="NATIVE_MEMORY_SAMPLE_RATE_BYTES" value="2048" />
+    </Profilers>
+    <method v="2">
+      <option name="Android.Gradle.BeforeRunTask" enabled="true" />
+    </method>
+  </configuration>
+</component>

app/src/androidTest/java/cash/z/ecc/android/ui/home/AutoshieldingInformationFragmentTest.kt

@@ -26,8 +26,8 @@ import org.junit.runner.RunWith
 class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
     @Test
     @MediumTest
-    fun dismiss_returns_home() {
-        val fragmentNavigationScenario = newScenario()
+    fun dismiss_returns_home_when_autoshield_not_available() {
+        val fragmentNavigationScenario = newScenario(isAutoshieldAvailable = false)
 
         onView(withId(cash.z.ecc.android.R.id.button_autoshield_dismiss)).also {
             it.perform(ViewActions.click())
@@ -41,23 +41,23 @@ class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
 
     @Test
     @MediumTest
-    fun dismiss_sets_preference() {
-        newScenario()
+    fun dismiss_starts_autoshield_when_autoshield_available() {
+        val fragmentNavigationScenario = newScenario(isAutoshieldAvailable = true)
 
         onView(withId(cash.z.ecc.android.R.id.button_autoshield_dismiss)).also {
             it.perform(ViewActions.click())
         }
 
         assertThat(
-            Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(ApplicationProvider.getApplicationContext<Context>()),
-            equalTo(true)
+            fragmentNavigationScenario.navigationController.currentDestination?.id,
+            equalTo(cash.z.ecc.android.R.id.nav_shield_final)
         )
     }
 
     @Test
     @MediumTest
     fun clicking_more_info_launches_browser() {
-        val fragmentNavigationScenario = newScenario()
+        val fragmentNavigationScenario = newScenario(isAutoshieldAvailable = false)
 
         onView(withId(cash.z.ecc.android.R.id.button_autoshield_more_info)).also {
             it.perform(ViewActions.click())
@@ -72,27 +72,12 @@ class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
         // navigation component works.
     }
 
-    @Test
-    @MediumTest
-    fun clicking_more_info_sets_preference() {
-        newScenario()
-
-        onView(withId(cash.z.ecc.android.R.id.button_autoshield_more_info)).also {
-            it.perform(ViewActions.click())
-        }
-
-        assertThat(
-            Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(ApplicationProvider.getApplicationContext<Context>()),
-            equalTo(true)
-        )
-    }
-
     @Test
     @MediumTest
     fun starting_fragment_does_not_launch_activities() {
         Intents.init()
         try {
-            val fragmentNavigationScenario = newScenario()
+            val fragmentNavigationScenario = newScenario(isAutoshieldAvailable = false)
 
             // The test framework launches an Activity to host the Fragment under test
             // Since the class name is not a public API, this could break in the future with newer
@@ -121,8 +106,19 @@ class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
 
     @Test
     @MediumTest
-    fun back_does_not_set_preference() {
-        val fragmentNavigationScenario = newScenario()
+    fun display_fragment_sets_preference() {
+        newScenario(isAutoshieldAvailable = false)
+
+        assertThat(
+            Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(ApplicationProvider.getApplicationContext<Context>()),
+            equalTo(true)
+        )
+    }
+
+    @Test
+    @MediumTest
+    fun back_navigates_home() {
+        val fragmentNavigationScenario = newScenario(isAutoshieldAvailable = false)
 
         fragmentNavigationScenario.fragmentScenario.onFragment {
             // Probably closest we can come to simulating back with the navigation test framework
@@ -133,15 +129,10 @@ class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
             fragmentNavigationScenario.navigationController.currentDestination?.id,
             equalTo(cash.z.ecc.android.R.id.nav_home)
         )
-
-        assertThat(
-            Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(ApplicationProvider.getApplicationContext<Context>()),
-            equalTo(false)
-        )
     }
 
     companion object {
-        private fun newScenario(): FragmentNavigationScenario<AutoshieldingInformationFragment> {
+        private fun newScenario(isAutoshieldAvailable: Boolean): FragmentNavigationScenario<AutoshieldingInformationFragment> {
             // Clear preferences for each scenario, as this most closely reflects how this fragment
             // is used in the app, as it is displayed usually on first launch
             SharedPreferenceFactory.getSharedPreferences(ApplicationProvider.getApplicationContext())
@@ -149,7 +140,7 @@ class AutoshieldingInformationFragmentTest : UiTestPrerequisites() {
 
             val scenario = FragmentScenario.launchInContainer(
                 AutoshieldingInformationFragment::class.java,
-                null,
+                HomeFragmentDirections.actionNavHomeToAutoshieldingInfo(isAutoshieldAvailable).arguments,
                 cash.z.ecc.android.R.style.ZcashTheme,
                 null
             )

app/src/main/java/cash/z/ecc/android/ui/MainActivity.kt

@@ -49,6 +49,7 @@ import androidx.core.content.getSystemService
 import androidx.fragment.app.Fragment
 import androidx.lifecycle.lifecycleScope
 import androidx.navigation.NavController
+import androidx.navigation.NavDirections
 import androidx.navigation.Navigator
 import androidx.navigation.findNavController
 import cash.z.ecc.android.R
@@ -220,11 +221,13 @@ class MainActivity : AppCompatActivity(R.layout.main_activity) {
         navController?.popBackStack(destination, inclusive)
     }
 
-    fun safeNavigate(@IdRes destination: Int, extras: Navigator.Extras? = null) {
+    fun safeNavigate(navDirections: NavDirections) = safeNavigate(navDirections.actionId, navDirections.arguments, null)
+
+    fun safeNavigate(@IdRes destination: Int, args: Bundle? = null, extras: Navigator.Extras? = null) {
         if (navController == null) {
             navInitListeners.add {
                 try {
-                    navController?.navigate(destination, null, null, extras)
+                    navController?.navigate(destination, args, null, extras)
                 } catch (t: Throwable) {
                     twig(
                         "WARNING: during callback, did not navigate to destination: R.id.${
@@ -237,7 +240,7 @@ class MainActivity : AppCompatActivity(R.layout.main_activity) {
             }
         } else {
             try {
-                navController?.navigate(destination, null, null, extras)
+                navController?.navigate(destination, args, null, extras)
             } catch (t: Throwable) {
                 twig(
                     "WARNING: did not immediately navigate to destination: R.id.${

app/src/main/java/cash/z/ecc/android/ui/home/AutoshieldingInformationFragment.kt

@@ -4,7 +4,7 @@ import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
 import androidx.navigation.fragment.findNavController
-import cash.z.ecc.android.R
+import androidx.navigation.fragment.navArgs
 import cash.z.ecc.android.databinding.FragmentAutoShieldInformationBinding
 import cash.z.ecc.android.ext.requireApplicationContext
 import cash.z.ecc.android.feedback.Report
@@ -12,33 +12,46 @@ import cash.z.ecc.android.preference.Preferences
 import cash.z.ecc.android.preference.model.put
 import cash.z.ecc.android.ui.base.BaseFragment
 
-/*
- * If the user presses the Android back button, the backstack will be popped and the user returns
- * to the app home screen.  The preference will not be set in that case, because it could be considered
- * that the user did not acknowledge this prompt.
- */
 class AutoshieldingInformationFragment : BaseFragment<FragmentAutoShieldInformationBinding>() {
     override val screen = Report.Screen.AUTO_SHIELD_INFORMATION
 
+    private val args: AutoshieldingInformationFragmentArgs by navArgs()
+
     override fun inflate(inflater: LayoutInflater): FragmentAutoShieldInformationBinding =
         FragmentAutoShieldInformationBinding.inflate(inflater)
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
+
+        /*
+         * Once the fragment is displayed, acknowledge it was presented to the user.  While it might
+         * be better to have explicit user interaction (positive/negative button or back),
+         * this implementation is simpler.  Hooking into the positive/negative button is easy, but
+         * hooking into the back button from a Fragment ends up being gross.
+         *
+         * Always acknowledging is necessary, because the HomeFragment will otherwise almost immediately
+         * re-launch this Fragment when it refreshes the UI (and therefore re-runs the
+         * check as to whether the preference to display this fragment has been set).
+         */
+        acknowledge()
+
         binding.buttonAutoshieldDismiss.setOnClickListener {
-            Preferences.isAcknowledgedAutoshieldingInformationPrompt.put(
-                requireApplicationContext(),
-                true
-            )
-            findNavController().navigate(R.id.action_nav_autoshielding_info_to_home)
+            if (args.isStartAutoshield) {
+                // TODO: Move the call to track last autoshield time to the AutoShieldFragment
+                // Dislike this call here; it would likely be better to track last autoshield
+                // time from the fragment that actually does the autoshielding.  Then the tracking
+                // is guaranteed to be called at the right time and we don't have this call scattered
+                // in several different places (e.g. here and in HomeFragment).
+                mainActivity?.lastAutoShieldTime = System.currentTimeMillis()
+
+                findNavController().navigate(AutoshieldingInformationFragmentDirections.actionNavAutoshieldingInfoToAutoshield())
+            } else {
+                findNavController().navigate(AutoshieldingInformationFragmentDirections.actionNavAutoshieldingInfoToHome())
+            }
         }
         binding.buttonAutoshieldMoreInfo.setOnClickListener {
-            Preferences.isAcknowledgedAutoshieldingInformationPrompt.put(
-                requireApplicationContext(),
-                true
-            )
             try {
-                findNavController().navigate(R.id.action_nav_autoshielding_info_to_browser)
+                findNavController().navigate(AutoshieldingInformationFragmentDirections.actionNavAutoshieldingInfoToBrowser())
             } catch (e: Exception) {
                 // ActivityNotFoundException could happen on certain devices, like Android TV, Android Things, etc.
 
@@ -49,8 +62,15 @@ class AutoshieldingInformationFragment : BaseFragment<FragmentAutoShieldInformat
                 // In the future, it might also be desirable to display a Toast or Snackbar indicating
                 // that the browser couldn't be launched
 
-                findNavController().navigate(R.id.action_nav_autoshielding_info_to_home)
+                findNavController().navigate(AutoshieldingInformationFragmentDirections.actionNavAutoshieldingInfoToHome())
             }
         }
     }
+
+    private fun acknowledge() {
+        Preferences.isAcknowledgedAutoshieldingInformationPrompt.put(
+            requireApplicationContext(),
+            true
+        )
+    }
 }

app/src/main/java/cash/z/ecc/android/ui/home/HomeFragment.kt

@@ -182,9 +182,7 @@ class HomeFragment : BaseFragment<FragmentHomeBinding>() {
         twig("Sync ready! Monitoring synchronizer state...")
         monitorUiModelChanges()
 
-        if (!Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(requireApplicationContext())) {
-            mainActivity?.safeNavigate(R.id.action_nav_home_to_autoshielding_info)
-        } else {
+        if (Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(requireApplicationContext())) {
             maybeInterruptUser()
         }
 
@@ -375,11 +373,23 @@ class HomeFragment : BaseFragment<FragmentHomeBinding>() {
     }
 
     private fun autoShield(uiModel: HomeViewModel.UiModel) {
+        // TODO: Move the preference read to a suspending function
+        // First time SharedPreferences are hit, it'll perform disk IO
+        val isAutoshieldingAcknowledged = Preferences.isAcknowledgedAutoshieldingInformationPrompt.get(requireApplicationContext())
+
         if (uiModel.hasAutoshieldFunds && canAutoshield()) {
-            twig("Autoshielding is available! Let's do this!!!")
-            mainActivity?.lastAutoShieldTime = System.currentTimeMillis()
-            mainActivity?.safeNavigate(R.id.action_nav_home_to_nav_funds_available)
+            if (!isAutoshieldingAcknowledged) {
+                mainActivity?.safeNavigate(HomeFragmentDirections.actionNavHomeToAutoshieldingInfo(true))
+            } else {
+                twig("Autoshielding is available! Let's do this!!!")
+                mainActivity?.lastAutoShieldTime = System.currentTimeMillis()
+                mainActivity?.safeNavigate(HomeFragmentDirections.actionNavHomeToNavFundsAvailable())
+            }
         } else {
+            if (!isAutoshieldingAcknowledged) {
+                mainActivity?.safeNavigate(HomeFragmentDirections.actionNavHomeToAutoshieldingInfo(false))
+            }
+
             // troubleshooting logs
             if (uiModel.transparentBalance.availableZatoshi > 0) {
                 twig("Transparent funds are available but not enough to autoshield. Available: ${uiModel.transparentBalance.availableZatoshi.convertZatoshiToZecString(10)}  Required: ${ZcashWalletApp.instance.autoshieldThreshold.convertZatoshiToZecString(8)}")

app/src/main/res/navigation/mobile_navigation.xml

@@ -122,12 +122,19 @@
     <fragment
         android:id="@+id/nav_autoshielding_info"
         android:name="cash.z.ecc.android.ui.home.AutoshieldingInformationFragment"
-        tools:layout="@layout/fragment_auto_shield_information" >
+        tools:layout="@layout/fragment_auto_shield_information"
+        >
+        <argument android:name="isStartAutoshield"
+            app:argType="boolean"
+            android:defaultValue="false"/>
         <action
             android:id="@+id/action_nav_autoshielding_info_to_home"
             app:destination="@id/nav_home"
             app:popUpTo="@id/nav_home"
             app:popUpToInclusive="true" />
+        <action
+            android:id="@+id/action_nav_autoshielding_info_to_autoshield"
+            app:destination="@id/nav_shield_final" />
         <action
             android:id="@+id/action_nav_autoshielding_info_to_browser"
             app:destination="@id/nav_autoshielding_info_details"