-
Notifications
You must be signed in to change notification settings - Fork 306
Show account-creation modal recurrently (every 60 days) for logged-out users #5763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
4ee452a
151c63d
77d4395
86c5a0f
c300eeb
d5e9295
0fb8748
99efa04
5999dad
6fcf1ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,6 +188,7 @@ import au.com.shiftyjelly.pocketcasts.ui.helper.FragmentHostListener | |
| import au.com.shiftyjelly.pocketcasts.ui.helper.NavigationBarColor | ||
| import au.com.shiftyjelly.pocketcasts.ui.helper.StatusBarIconColor | ||
| import au.com.shiftyjelly.pocketcasts.ui.theme.Theme | ||
| import au.com.shiftyjelly.pocketcasts.utils.AccountEncouragement | ||
| import au.com.shiftyjelly.pocketcasts.utils.Network | ||
| import au.com.shiftyjelly.pocketcasts.utils.Util | ||
| import au.com.shiftyjelly.pocketcasts.utils.featureflag.Feature | ||
|
|
@@ -229,6 +230,7 @@ import io.reactivex.disposables.CompositeDisposable | |
| import io.reactivex.rxkotlin.addTo | ||
| import io.reactivex.rxkotlin.subscribeBy | ||
| import io.reactivex.schedulers.Schedulers | ||
| import java.time.Instant | ||
| import java.util.Locale | ||
| import java.util.concurrent.TimeUnit | ||
| import javax.inject.Inject | ||
|
|
@@ -354,6 +356,11 @@ class MainActivity : | |
| get() = binding.bottomContainer.height - binding.bottomContainer.paddingBottom | ||
|
|
||
| private var bottomSheetTag: String? = null | ||
|
|
||
| // True once initial onboarding has been launched this session, so the recurring account-creation | ||
| // modal isn't chained onto the same launch (hasCompletedOnboarding() flips to true as soon as | ||
| // onboarding finishes). It shows on the next launch instead. | ||
| private var launchedInitialOnboarding: Boolean = false | ||
|
yaelirub marked this conversation as resolved.
|
||
| private var pendingBottomSheetFragment: Fragment? = null | ||
|
|
||
| override val coroutineContext: CoroutineContext | ||
|
|
@@ -480,6 +487,7 @@ class MainActivity : | |
| val needsLoginPromptAfterRestore = settings.getNeedsLoginPromptAfterRestore() | ||
| // Only show if savedInstanceState is null in order to avoid creating onboarding activity twice. | ||
| if (showOnboarding && savedInstanceState == null) { | ||
| launchedInitialOnboarding = true | ||
| openOnboardingFlow(OnboardingFlow.InitialOnboarding) | ||
| } | ||
|
|
||
|
|
@@ -489,7 +497,9 @@ class MainActivity : | |
| if (savedInstanceState == null && needsLoginPromptAfterRestore) { | ||
| settings.setNeedsLoginPromptAfterRestore(false) | ||
| if (!showOnboarding && !isLoggedIn) { | ||
| settings.showFreeAccountEncouragement.set(false, updateModifiedAt = true) | ||
| // Anchor the recurring clock so encourageAccountCreation() doesn't also show the | ||
| // modal this launch; the next recurring prompt is then one interval out. | ||
| settings.freeAccountEncouragementLastShown.set(Instant.now(), updateModifiedAt = true) | ||
|
yaelirub marked this conversation as resolved.
Outdated
|
||
| openOnboardingFlow(OnboardingFlow.AccountEncouragement) | ||
| } | ||
| } | ||
|
|
@@ -683,21 +693,49 @@ class MainActivity : | |
| private fun encourageAccountCreation() { | ||
| lifecycleScope.launch { | ||
| repeatOnLifecycle(Lifecycle.State.STARTED) { | ||
| val encourageAccountCreation = settings.showFreeAccountEncouragement.value | ||
| if (!encourageAccountCreation) { | ||
| if (!FeatureFlag.isEnabled(Feature.ENCOURAGE_ACCOUNT_CREATION)) { | ||
| return@repeatOnLifecycle | ||
| } | ||
| settings.showFreeAccountEncouragement.set(false, updateModifiedAt = true) | ||
|
|
||
| val isSignedIn = viewModel.signInState.asFlow().first().isSignedIn | ||
| if (isSignedIn) { | ||
| // Don't chain onto the same launch that presented initial onboarding — completing it | ||
| // flips hasCompletedOnboarding() to true, which would otherwise show the modal the | ||
| // moment onboarding is dismissed. It shows on the next launch instead. | ||
| if (launchedInitialOnboarding) { | ||
| return@repeatOnLifecycle | ||
| } | ||
|
|
||
| if (Util.isTablet(this@MainActivity)) { | ||
| AccountBenefitsFragment().show(supportFragmentManager, "account_benefits_fragment") | ||
| } else { | ||
| openOnboardingFlow(OnboardingFlow.AccountEncouragement) | ||
| // Eligible = logged out and past initial onboarding. Shown on the first eligible | ||
| // launch, then every 60 days while the user stays logged out. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to add to the prompt for Opus to reduce the comments to one line? It seems to be overexplaining and producing comments that might not be necessary.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! Updated here: 6fcf1ce. I wish we could use a shared Claude memory |
||
| val isSignedIn = viewModel.signInState.asFlow().first().isSignedIn | ||
| val isEligible = !isSignedIn && settings.hasCompletedOnboarding() | ||
|
|
||
| val decision = AccountEncouragement.decide( | ||
| isEligible = isEligible, | ||
| lastShown = settings.freeAccountEncouragementLastShown.value, | ||
|
yaelirub marked this conversation as resolved.
|
||
| now = Instant.now(), | ||
| ) | ||
|
yaelirub marked this conversation as resolved.
|
||
| when (decision) { | ||
| AccountEncouragement.Decision.Wait -> return@repeatOnLifecycle | ||
|
|
||
| AccountEncouragement.Decision.Show -> { | ||
| // Defer if another bottom sheet (What's New, End of Year, etc.) is already | ||
| // showing or pending: don't stack over it, and don't reset the clock for a | ||
| // modal the user never saw. Retries on the next eligible launch. | ||
| if (bottomSheetTag != null || pendingBottomSheetFragment != null) { | ||
| return@repeatOnLifecycle | ||
| } | ||
|
|
||
| // Reset the clock so the next showing is one interval out. Set before | ||
| // presenting so this STARTED block can't re-show the modal when the activity | ||
| // returns to the foreground after it's dismissed. | ||
| settings.freeAccountEncouragementLastShown.set(Instant.now(), updateModifiedAt = true) | ||
|
yaelirub marked this conversation as resolved.
|
||
|
|
||
| if (Util.isTablet(this@MainActivity)) { | ||
| AccountBenefitsFragment().show(supportFragmentManager, "account_benefits_fragment") | ||
| } else { | ||
| openOnboardingFlow(OnboardingFlow.AccountEncouragement) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import au.com.shiftyjelly.pocketcasts.utils.featureflag.providers.FirebaseRemote | |
| import au.com.shiftyjelly.pocketcasts.utils.featureflag.providers.PreferencesFeatureProvider | ||
| import au.com.shiftyjelly.pocketcasts.utils.getVersionCode | ||
| import dagger.hilt.android.qualifiers.ApplicationContext | ||
| import java.time.Instant | ||
| import javax.inject.Inject | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.cancel | ||
|
|
@@ -129,7 +130,10 @@ class AppLifecycleObserver( | |
| // new installations default to not displaying the tooltip | ||
| settings.showPodcastsRecentlyPlayedSortOrderTooltip.set(false, updateModifiedAt = false) | ||
|
|
||
| settings.showFreeAccountEncouragement.set(false, updateModifiedAt = false) | ||
| // Anchor the account-encouragement cadence on fresh install so the modal waits a full | ||
| // interval before its first show (existing users upgrading leave it null and see it | ||
| // immediately). | ||
| settings.freeAccountEncouragementLastShown.set(Instant.now(), updateModifiedAt = false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (non-blocking) This block is guarded by In practice the worker runs within the first session, so the drift is minutes. Worth knowing, though, that "anchored at install time" is really "anchored at the last process start before migrations ran." |
||
|
|
||
| when (getAppPlatform()) { | ||
| // do nothing because this already defaults to true for all users on automotive | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package au.com.shiftyjelly.pocketcasts.utils | ||
|
|
||
| import java.time.Duration | ||
| import java.time.Instant | ||
|
|
||
| /** | ||
| * Cadence logic for the recurring "Encourage Account Creation" modal shown to logged-out users. | ||
| * | ||
| * The modal is shown to logged-out users who have already completed onboarding, first | ||
| * [interval] after the initial eligible launch and every [interval] thereafter. The initial | ||
| * eligible launch only anchors the clock (returns [Decision.Anchor]) so we don't collide with | ||
| * onboarding. | ||
|
yaelirub marked this conversation as resolved.
Outdated
|
||
| */ | ||
| object AccountEncouragement { | ||
| /** How long to wait between showings of the modal (60 days). */ | ||
| val interval: Duration = Duration.ofDays(60) | ||
|
|
||
| enum class Decision { | ||
| /** Show the modal now. */ | ||
| Show, | ||
|
|
||
| /** Don't show and leave the clock untouched. */ | ||
| Wait, | ||
| } | ||
|
|
||
| /** | ||
| * Pure cadence decision, with no Android/preferences dependencies so it can be unit-tested. | ||
| * | ||
| * Shows on the first eligible launch (no anchor yet), once the interval elapses, or when the | ||
| * stored anchor is in the future (backwards clock / skewed-backup restore). The caller records | ||
| * `now` as the new anchor when it shows. | ||
| * | ||
| * @param isEligible whether the user currently qualifies (flag on, logged out, onboarding done). | ||
| * @param lastShown the persisted anchor, or `null` if the clock has never been started. | ||
| * @param now the current instant. | ||
| * @param interval how long to wait between showings. | ||
| */ | ||
| fun decide( | ||
| isEligible: Boolean, | ||
| lastShown: Instant?, | ||
| now: Instant, | ||
| interval: Duration = this.interval, | ||
| ): Decision { | ||
| if (!isEligible) return Decision.Wait | ||
| if (lastShown == null || lastShown.isAfter(now)) return Decision.Show | ||
| return if (!now.isBefore(lastShown.plus(interval))) Decision.Show else Decision.Wait | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package au.com.shiftyjelly.pocketcasts.utils | ||
|
|
||
| import java.time.Duration | ||
| import java.time.Instant | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Test | ||
|
|
||
| class AccountEncouragementTest { | ||
| private val interval: Duration = Duration.ofDays(60) | ||
| private val now: Instant = Instant.ofEpochSecond(1_700_000_000) | ||
|
|
||
| @Test | ||
| fun `waits when not eligible`() { | ||
| // Even with an elapsed clock, an ineligible user is never shown the modal. | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = false, | ||
| lastShown = now.minus(interval.multipliedBy(2)), | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Wait, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `shows on first eligible launch`() { | ||
| // First eligible launch (no anchor yet) shows immediately, then the clock starts. | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = true, | ||
| lastShown = null, | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Show, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `waits before interval elapses`() { | ||
| // One second short of the interval should not show yet. | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = true, | ||
| lastShown = now.minus(interval).plusSeconds(1), | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Wait, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `shows when interval elapsed`() { | ||
| // Exactly at the interval boundary should show. | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = true, | ||
| lastShown = now.minus(interval), | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Show, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `shows when interval well exceeded`() { | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = true, | ||
| lastShown = now.minus(interval.multipliedBy(3)), | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Show, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `shows when anchor is in the future`() { | ||
| // A future anchor (backwards device clock / restored skewed backup) shows rather than | ||
| // suppressing the modal indefinitely. | ||
| val decision = AccountEncouragement.decide( | ||
| isEligible = true, | ||
| lastShown = now.plus(interval), | ||
| now = now, | ||
| interval = interval, | ||
| ) | ||
|
|
||
| assertEquals(AccountEncouragement.Decision.Show, decision) | ||
| } | ||
|
|
||
| @Test | ||
| fun `default interval is 60 days`() { | ||
| assertEquals(Duration.ofDays(60), AccountEncouragement.interval) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.