Clear onboarding cards for restore/re-reg.

app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt

@@ -308,6 +308,11 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
       RestoreDecision.COMPLETED -> RestoreDecisionState.Completed
     }
 
+    if (decision == RestoreDecision.COMPLETED) {
+      Log.i(TAG, "[setRestoreDecision] Data was restored. Clearing onboarding state.")
+      SignalStore.onboarding.clearAll()
+    }
+
     RegistrationUtil.maybeMarkRegistrationComplete()
   }
 
@@ -807,6 +812,9 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo
       // Registering releases any username we previously held, so it has to be re-reserved once storage service tells us what it was.
       Log.i(TAG, "[applyAccountData] Re-registration. Marking that we need to reclaim our username and link.")
       SignalStore.misc.needsUsernameRestore = true
+
+      Log.i(TAG, "[applyAccountData] Re-registration. Clearing onboarding state.")
+      SignalStore.onboarding.clearAll()
     }
 
     accountData.authCredentialSalt?.let {

app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt

@@ -39,12 +39,14 @@ import org.signal.core.models.ServiceId.ACI
 import org.signal.core.models.ServiceId.PNI
 import org.signal.core.util.contentproviders.BlobProvider
 import org.signal.libsignal.protocol.IdentityKeyPair
+import org.signal.registration.RestoreDecision
 import org.signal.registration.proto.AccountData
 import org.signal.registration.proto.LinkedDeviceData
 import org.signal.registration.proto.RegistrationData
 import org.thoughtcrime.securesms.crypto.PreKeyUtil
 import org.thoughtcrime.securesms.database.RecipientTable
 import org.thoughtcrime.securesms.database.SignalDatabase
+import org.thoughtcrime.securesms.database.model.databaseprotos.RestoreDecisionState
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.jobmanager.runJobBlocking
 import org.thoughtcrime.securesms.jobs.DirectoryRefreshJob
@@ -52,7 +54,10 @@ import org.thoughtcrime.securesms.jobs.PreKeysSyncJob
 import org.thoughtcrime.securesms.jobs.ReclaimUsernameAndLinkJob
 import org.thoughtcrime.securesms.jobs.RefreshOwnProfileJob
 import org.thoughtcrime.securesms.jobs.RotateCertificateJob
+import org.thoughtcrime.securesms.keyvalue.Completed
+import org.thoughtcrime.securesms.keyvalue.NewAccount
 import org.thoughtcrime.securesms.keyvalue.SignalStore
+import org.thoughtcrime.securesms.keyvalue.Skipped
 import org.thoughtcrime.securesms.registration.util.RegistrationUtil
 import org.thoughtcrime.securesms.testutil.MockAppDependenciesRule
 import org.thoughtcrime.securesms.testutil.SignalDatabaseRule
@@ -327,6 +332,81 @@ class AppRegistrationStorageControllerTest {
     assertThat(SignalStore.misc.needsUsernameRestore).isFalse()
   }
 
+  @Test
+  fun `commit - re-registration - clears onboarding state`() = runBlocking<Unit> {
+    seedOnboardingState()
+    seedInProgressData(
+      RegistrationData(
+        accountData = accountData(reRegistration = true),
+        accountEntropyPool = aep.value
+      )
+    )
+
+    controller.commitRegistrationData()
+
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isFalse()
+  }
+
+  @Test
+  fun `commit - new account - leaves onboarding state alone`() = runBlocking<Unit> {
+    seedOnboardingState()
+    seedInProgressData(
+      RegistrationData(
+        accountData = accountData(reRegistration = false),
+        accountEntropyPool = aep.value
+      )
+    )
+
+    controller.commitRegistrationData()
+
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isTrue()
+  }
+
+  @Test
+  fun `setRestoreDecision - completed - clears onboarding state`() = runBlocking<Unit> {
+    SignalStore.registration.onFirstEverAppLaunch()
+    seedOnboardingState()
+
+    controller.setRestoreDecision(RestoreDecision.COMPLETED)
+
+    assertThat(SignalStore.registration.restoreDecisionState).isEqualTo(RestoreDecisionState.Completed)
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isFalse()
+  }
+
+  @Test
+  fun `setRestoreDecision - skipped - leaves onboarding state alone`() = runBlocking<Unit> {
+    SignalStore.registration.onFirstEverAppLaunch()
+    seedOnboardingState()
+
+    controller.setRestoreDecision(RestoreDecision.SKIPPED)
+
+    assertThat(SignalStore.registration.restoreDecisionState).isEqualTo(RestoreDecisionState.Skipped)
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isTrue()
+  }
+
+  @Test
+  fun `setRestoreDecision - new account - leaves onboarding state alone`() = runBlocking<Unit> {
+    SignalStore.registration.onFirstEverAppLaunch()
+    seedOnboardingState()
+
+    controller.setRestoreDecision(RestoreDecision.NEW_ACCOUNT)
+
+    assertThat(SignalStore.registration.restoreDecisionState).isEqualTo(RestoreDecisionState.NewAccount)
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isTrue()
+  }
+
+  @Test
+  fun `setRestoreDecision - decision already made - leaves onboarding state alone`() = runBlocking<Unit> {
+    SignalStore.registration.onFirstEverAppLaunch()
+    controller.setRestoreDecision(RestoreDecision.NEW_ACCOUNT)
+    seedOnboardingState()
+
+    controller.setRestoreDecision(RestoreDecision.COMPLETED)
+
+    assertThat(SignalStore.registration.restoreDecisionState).isEqualTo(RestoreDecisionState.NewAccount)
+    assertThat(SignalStore.onboarding.hasOnboarding(context)).isTrue()
+  }
+
   @Test
   fun `onRegistrationFlowFinished - username reclaim pending - enqueues reclaim job`() = runBlocking<Unit> {
     SignalStore.misc.needsUsernameRestore = true
@@ -518,6 +598,14 @@ class AppRegistrationStorageControllerTest {
     }
   }
 
+  /** Puts onboarding into the state a fresh install leaves it in, where the get-started megaphone would show. */
+  private fun seedOnboardingState() {
+    SignalStore.onboarding.setShowNewGroup(true)
+    SignalStore.onboarding.setShowInviteFriends(true)
+    SignalStore.onboarding.setShowAppearance(true)
+    SignalStore.onboarding.setShowAddPhoto(true)
+  }
+
   private fun seedInProgressData(data: RegistrationData) {
     val uri = Uri.parse("memoryblob://registration/seed-${blobCounter++}")
     blobData[uri] = RegistrationData.ADAPTER.encode(data)

feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt

@@ -589,6 +589,7 @@ class RegistrationEndToEndTest {
     assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
     assert(committed.accountEntropyPool == aep.value) { "Expected the committed AEP to be the one from the restored backup" }
     assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
+    assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" }
   }
 
   @Test
@@ -641,6 +642,7 @@ class RegistrationEndToEndTest {
     assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
     assert(committed.accountEntropyPool == aep.value) { "Expected the committed AEP to be the provisioned one" }
     assert(committed.pin == PIN) { "Expected the pin from the restored backup but was ${committed.pin}" }
+    assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" }
   }
 
   @Test
@@ -671,8 +673,10 @@ class RegistrationEndToEndTest {
     assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
     assert(committed.accountData?.reRegistration == true) { "Expected the committed account data to be flagged as a re-registration" }
 
-    // The re-registration flag is what tells the app to reclaim the username we just released, and the flow-finished
-    // hook is where it enqueues the job that does it. See AppRegistrationStorageController.
+    // The re-registration flag is what tells the app that this is an established account rather than someone new to
+    // Signal, which is what suppresses the get-started onboarding megaphone and drives the reclaim of the username we
+    // just released. The flow-finished hook is where it enqueues the job that does the reclaim.
+    // See AppRegistrationStorageController.
     assert(storageController.registrationFlowFinishedCount == 1) { "Expected the flow-finished hook to fire exactly once but fired ${storageController.registrationFlowFinishedCount} times" }
   }
 
@@ -808,6 +812,7 @@ class RegistrationEndToEndTest {
     assert(committed != null) { "Expected registration data to be committed" }
     assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
     assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
+    assert(committed.accountData?.reRegistration == true) { "Expected the committed account data to be flagged as a re-registration" }
   }
 
   @Test
@@ -955,6 +960,7 @@ class RegistrationEndToEndTest {
     assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
     assert(committed.accountEntropyPool == aep.value) { "Expected the committed AEP to be the one from the restored backup" }
     assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
+    assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" }
   }
 
   @Test
@@ -987,6 +993,9 @@ class RegistrationEndToEndTest {
     assert(committed != null) { "Expected registration data to be committed" }
     assert(committed!!.accountEntropyPool == aep.value) { "Expected the committed AEP to be the one the user entered" }
     assert(committed.pin == PIN) { "Expected the pin from the restored backup but was ${committed.pin}" }
+
+    // A COMPLETED decision is what tells the app the user brought their data with them, which is what suppresses the
+    // get-started onboarding megaphone. See AppRegistrationStorageController.setRestoreDecision.
     assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" }
   }
 
@@ -1475,6 +1484,7 @@ class RegistrationEndToEndTest {
     assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" }
     assert(committed.accountEntropyPool == aep.value) { "Expected the committed AEP to be the provisioned one" }
     assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" }
+    assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" }
   }
 
   @Test