diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt index 89655f1b4e..ff52264188 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt +++ b/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 { diff --git a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt index 20fb3ed69e..cd7a4c096c 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt +++ b/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 { + 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 { + 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 { + 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 { + 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 { + 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 { + 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 { 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) diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index 25725595b9..b9aa331790 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/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