mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
Replace AppMigration11 by a dismissal when entering the login flow
AppMigration11 was run once, in the first release containing it, whatever the state of the MultiAccount feature flag. Users installing the app while the feature flag was disabled were considered as new users and would never see the announcement once the feature flag got enabled. The multi-account announcement is now pending by default (NeverShown), and is marked as shown when the user enters the login flow while the feature is enabled, either for the first time or to add another account. Users who were already logged in before the feature became available will see the announcement. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -13,6 +13,7 @@ package io.element.android.appnav
|
||||
import android.os.Parcelable
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.lifecycle.lifecycleScope
|
||||
import coil3.SingletonImageLoader
|
||||
import coil3.annotation.DelicateCoilApi
|
||||
import com.bumble.appyx.core.lifecycle.subscribe
|
||||
@@ -24,6 +25,8 @@ import dev.zacsweers.metro.AppScope
|
||||
import dev.zacsweers.metro.Assisted
|
||||
import dev.zacsweers.metro.AssistedInject
|
||||
import io.element.android.annotations.ContributesNode
|
||||
import io.element.android.features.announcement.api.Announcement
|
||||
import io.element.android.features.announcement.api.AnnouncementService
|
||||
import io.element.android.features.login.api.LoginEntryPoint
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.libraries.architecture.BackstackView
|
||||
@@ -33,8 +36,11 @@ import io.element.android.libraries.architecture.callback
|
||||
import io.element.android.libraries.architecture.inputs
|
||||
import io.element.android.libraries.designsystem.utils.ForceOrientationInMobileDevices
|
||||
import io.element.android.libraries.designsystem.utils.ScreenOrientation
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlagService
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlags
|
||||
import io.element.android.libraries.matrix.ui.media.ImageLoaderHolder
|
||||
import io.element.android.services.analytics.api.watchers.AnalyticsColdStartWatcher
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.parcelize.Parcelize
|
||||
|
||||
@ContributesNode(AppScope::class)
|
||||
@@ -45,6 +51,8 @@ class NotLoggedInFlowNode(
|
||||
private val loginEntryPoint: LoginEntryPoint,
|
||||
private val imageLoaderHolder: ImageLoaderHolder,
|
||||
private val analyticsColdStartWatcher: AnalyticsColdStartWatcher,
|
||||
private val featureFlagService: FeatureFlagService,
|
||||
private val announcementService: AnnouncementService,
|
||||
) : BaseFlowNode<NotLoggedInFlowNode.NavTarget>(
|
||||
backstack = BackStack(
|
||||
initialElement = NavTarget.Root,
|
||||
@@ -68,6 +76,13 @@ class NotLoggedInFlowNode(
|
||||
override fun onBuilt() {
|
||||
super.onBuilt()
|
||||
analyticsColdStartWatcher.whenLoggingIn()
|
||||
lifecycleScope.launch {
|
||||
// Users who enter the login flow while the feature is available, either for the first time or to add
|
||||
// another account, do not need to be informed about the multi-account feature.
|
||||
if (featureFlagService.isFeatureEnabled(FeatureFlags.MultiAccount)) {
|
||||
announcementService.onAnnouncementDismissed(Announcement.MultiAccount)
|
||||
}
|
||||
}
|
||||
lifecycle.subscribe(
|
||||
onResume = {
|
||||
SingletonImageLoader.setUnsafe(imageLoaderHolder.get())
|
||||
|
||||
@@ -141,6 +141,8 @@ class RootFlowNodeTest : RobolectricTest() {
|
||||
loginEntryPoint = loginEntryPoint,
|
||||
imageLoaderHolder = FakeImageLoaderHolder(),
|
||||
analyticsColdStartWatcher = FakeAnalyticsColdStartWatcher(),
|
||||
featureFlagService = FakeFeatureFlagService(),
|
||||
announcementService = FakeAnnouncementService(),
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
+3
-1
@@ -51,7 +51,9 @@ class DefaultAnnouncementService(
|
||||
if (newNotificationSoundStatus == AnnouncementStatus.Show) {
|
||||
add(Announcement.NewNotificationSound)
|
||||
}
|
||||
if (multiAccountStatus == AnnouncementStatus.Show) {
|
||||
// The multi-account announcement is pending until it is dismissed. Users who were already
|
||||
// logged in when the feature became available still have the default NeverShown status.
|
||||
if (multiAccountStatus != AnnouncementStatus.Shown) {
|
||||
add(Announcement.MultiAccount)
|
||||
}
|
||||
}
|
||||
|
||||
+2
-1
@@ -40,7 +40,8 @@ class DefaultAnnouncementStore(
|
||||
// NOT replay on first launch.
|
||||
// - Fullscreen.Space is disabled
|
||||
// - NewNotificationSound is opted in by AppMigration08
|
||||
// - MultiAccount is NeverShown by default, AppMigration11 sets it to Show or Shown.
|
||||
// - MultiAccount is NeverShown by default, which means pending. It is set to Shown when the user enters
|
||||
// the login flow while the feature is enabled (see NotLoggedInFlowNode), or dismisses the announcement.
|
||||
val defaultStatus = when (announcement) {
|
||||
Announcement.Fullscreen.Space -> AnnouncementStatus.Shown
|
||||
Announcement.NewNotificationSound -> AnnouncementStatus.Shown
|
||||
|
||||
+4
-2
@@ -74,10 +74,12 @@ class DefaultAnnouncementServiceTest {
|
||||
announcementStore = announcementStore,
|
||||
)
|
||||
sut.announcementsToShowFlow().test {
|
||||
assertThat(awaitItem()).isEmpty()
|
||||
// MultiAccount is pending by default
|
||||
assertThat(awaitItem()).containsExactly(Announcement.MultiAccount)
|
||||
announcementStore.setAnnouncementStatus(Announcement.NewNotificationSound, AnnouncementStatus.Show)
|
||||
assertThat(awaitItem()).containsExactly(Announcement.NewNotificationSound)
|
||||
assertThat(awaitItem()).containsExactly(Announcement.NewNotificationSound, Announcement.MultiAccount)
|
||||
announcementStore.setAnnouncementStatus(Announcement.MultiAccount, AnnouncementStatus.Show)
|
||||
// Still pending
|
||||
assertThat(awaitItem()).containsExactly(Announcement.NewNotificationSound, Announcement.MultiAccount)
|
||||
announcementStore.setAnnouncementStatus(Announcement.NewNotificationSound, AnnouncementStatus.Shown)
|
||||
assertThat(awaitItem()).containsExactly(Announcement.MultiAccount)
|
||||
|
||||
-36
@@ -1,36 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.features.migration.impl.migrations
|
||||
|
||||
import dev.zacsweers.metro.AppScope
|
||||
import dev.zacsweers.metro.ContributesIntoSet
|
||||
import io.element.android.features.announcement.api.Announcement
|
||||
import io.element.android.features.announcement.api.AnnouncementService
|
||||
import io.element.android.libraries.sessionstorage.api.SessionStore
|
||||
|
||||
/**
|
||||
* Ensure the multi-account announcement is displayed, but only on application upgrade,
|
||||
* and only if the user does not already have several accounts.
|
||||
* In the other cases, mark the announcement as already shown.
|
||||
*/
|
||||
@ContributesIntoSet(AppScope::class)
|
||||
class AppMigration11(
|
||||
private val sessionStore: SessionStore,
|
||||
private val announcementService: AnnouncementService,
|
||||
) : AppMigration {
|
||||
override val order: Int = 11
|
||||
|
||||
override suspend fun migrate(isFreshInstall: Boolean) {
|
||||
if (isFreshInstall || sessionStore.getAllSessions().size > 1) {
|
||||
// New users, or users who already have several accounts, do not need to be informed about the feature
|
||||
announcementService.onAnnouncementDismissed(Announcement.MultiAccount)
|
||||
} else {
|
||||
announcementService.showAnnouncement(Announcement.MultiAccount)
|
||||
}
|
||||
}
|
||||
}
|
||||
-69
@@ -1,69 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.features.migration.impl.migrations
|
||||
|
||||
import io.element.android.features.announcement.api.Announcement
|
||||
import io.element.android.features.rageshake.test.logs.FakeAnnouncementService
|
||||
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
|
||||
import io.element.android.libraries.sessionstorage.test.aSessionData
|
||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.lambda.value
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Test
|
||||
|
||||
class AppMigration11Test {
|
||||
@Test
|
||||
fun `migration on fresh install should mark the announcement as dismissed`() = runTest {
|
||||
val onAnnouncementDismissedResult = lambdaRecorder<Announcement, Unit> { }
|
||||
val service = FakeAnnouncementService(
|
||||
onAnnouncementDismissedResult = onAnnouncementDismissedResult,
|
||||
)
|
||||
val migration = AppMigration11(
|
||||
sessionStore = InMemorySessionStore(initialList = listOf(aSessionData())),
|
||||
announcementService = service,
|
||||
)
|
||||
migration.migrate(isFreshInstall = true)
|
||||
onAnnouncementDismissedResult.assertions().isCalledOnce()
|
||||
.with(value(Announcement.MultiAccount))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `migration on upgrade with a single account should show the announcement`() = runTest {
|
||||
val showAnnouncementResult = lambdaRecorder<Announcement, Unit> { }
|
||||
val service = FakeAnnouncementService(
|
||||
showAnnouncementResult = showAnnouncementResult,
|
||||
)
|
||||
val migration = AppMigration11(
|
||||
sessionStore = InMemorySessionStore(initialList = listOf(aSessionData())),
|
||||
announcementService = service,
|
||||
)
|
||||
migration.migrate(isFreshInstall = false)
|
||||
showAnnouncementResult.assertions().isCalledOnce()
|
||||
.with(value(Announcement.MultiAccount))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `migration on upgrade with several accounts should mark the announcement as dismissed`() = runTest {
|
||||
val onAnnouncementDismissedResult = lambdaRecorder<Announcement, Unit> { }
|
||||
val service = FakeAnnouncementService(
|
||||
onAnnouncementDismissedResult = onAnnouncementDismissedResult,
|
||||
)
|
||||
val migration = AppMigration11(
|
||||
sessionStore = InMemorySessionStore(
|
||||
initialList = listOf(
|
||||
aSessionData(sessionId = "@alice:server.org"),
|
||||
aSessionData(sessionId = "@bob:server.org"),
|
||||
)
|
||||
),
|
||||
announcementService = service,
|
||||
)
|
||||
migration.migrate(isFreshInstall = false)
|
||||
onAnnouncementDismissedResult.assertions().isCalledOnce()
|
||||
.with(value(Announcement.MultiAccount))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user