diff --git a/.changeset/patient-surveys-wait.md b/.changeset/patient-surveys-wait.md new file mode 100644 index 000000000..0c6bae893 --- /dev/null +++ b/.changeset/patient-surveys-wait.md @@ -0,0 +1,6 @@ +--- +"posthog-android": patch +"posthog-android-surveys-compose": patch +--- + +Fix repeat trigger events restarting a survey's `surveyPopupDelaySeconds` delay; a custom `PostHogSurveysDelegate` that can't show a survey must now call `onSurveyClosed`. Flutter apps should use posthog_flutter 5.50.13 or later, since earlier versions can stop showing surveys until the app restarts. diff --git a/posthog-android-surveys-compose/src/main/java/com/posthog/android/surveys/compose/internal/PostHogSurveyHost.kt b/posthog-android-surveys-compose/src/main/java/com/posthog/android/surveys/compose/internal/PostHogSurveyHost.kt index 6b9fedde1..e5e48401f 100644 --- a/posthog-android-surveys-compose/src/main/java/com/posthog/android/surveys/compose/internal/PostHogSurveyHost.kt +++ b/posthog-android-surveys-compose/src/main/java/com/posthog/android/surveys/compose/internal/PostHogSurveyHost.kt @@ -235,7 +235,10 @@ internal class PostHogSurveyHost(private val activityProvider: ActivityProvider) } if (!presented) { + // The SDK holds this survey until it hears back, so report the failed show as a close. + val onClosed = onClosedCallback dismissInternal(notifyClosed = false) + onClosed?.invoke(survey) } } diff --git a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt index c2eb1d931..fa3b491dc 100644 --- a/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt @@ -77,6 +77,10 @@ public class PostHogSurveysIntegration( private val resetGeneration: Long get() = progressStore.resetGeneration private val mainHandler = Handler(Looper.getMainLooper()) private var presentationGeneration = 0L + + // Handed to the delegate but not yet shown. Reserving here, like iOS and the browser, stops a + // repeat trigger event from re-rendering the survey and restarting the delegate's popup delay. + private var pendingPresentation: Long? = null private val lifecycleLock = Any() private var postHog: PostHogInterface? = null @@ -348,7 +352,16 @@ public class PostHogSurveysIntegration( return } - val (generation, presentation) = synchronized(activeSurveyLock) { resetGeneration to ++presentationGeneration } + val (generation, presentation) = + synchronized(activeSurveyLock) { + if (pendingPresentation != null) { + config.logger.log("Cannot show survey - another survey is already pending") + return + } + val presentation = ++presentationGeneration + pendingPresentation = presentation + resetGeneration to presentation + } val displayLanguage = resolveDisplayLanguage() val translations = resolveSurveyTranslations(survey, displayLanguage) val savedProgress = if (supportsSurveyResume()) progressStore.load(survey) else null @@ -373,11 +386,11 @@ public class PostHogSurveysIntegration( ).copy(initialQuestionIndex = progress.questionIndex) // Setup callbacks for delegate call - val onSurveyShown = surveyShownCallback(responseContext, progress) + val onSurveyShown = surveyShownCallback(responseContext, progress, presentation) val onSurveyResponse = surveyResponseCallback(responseContext) - val onSurveyClosed = surveyClosedCallback(responseContext) + val onSurveyClosed = surveyClosedCallback(responseContext, presentation) // Call the delegate to render the survey renderIfCurrent(generation, presentation) { @@ -392,18 +405,32 @@ public class PostHogSurveysIntegration( ) { runOnMain { val current = synchronized(activeSurveyLock) { generation == resetGeneration && presentation == presentationGeneration } - if (!synchronized(lifecycleLock) { isStarted } || !current) return@runOnMain + if (!synchronized(lifecycleLock) { isStarted } || !current) { + releasePendingPresentation(presentation) + return@runOnMain + } render() } } - private fun surveyClosedCallback(responseContext: SurveyResponseContext): OnPostHogSurveyClosed { + private fun releasePendingPresentation(presentation: Long) { + synchronized(activeSurveyLock) { + if (pendingPresentation == presentation) pendingPresentation = null + } + } + + private fun surveyClosedCallback( + responseContext: SurveyResponseContext, + presentation: Long, + ): OnPostHogSurveyClosed { return onSurveyClosed@{ _ -> // Store the original survey for branching logic val originalSurvey = responseContext.survey val distinctId = postHog?.distinctId() val event = synchronized(activeSurveyLock) { + releasePendingPresentation(presentation) + // Validate that this survey matches the currently active survey if (!isActiveAttempt(originalSurvey.id, responseContext.submissionId)) { config.logger.log("[Surveys] Received a close event for a non-active survey") @@ -476,6 +503,7 @@ public class PostHogSurveysIntegration( private fun surveyShownCallback( responseContext: SurveyResponseContext, progress: SurveyProgress, + presentation: Long, ): OnPostHogSurveyShown = { shownSurvey -> val originalSurvey = responseContext.survey @@ -485,6 +513,7 @@ public class PostHogSurveysIntegration( val distinctId = postHog?.distinctId() val event = synchronized(activeSurveyLock) { + releasePendingPresentation(presentation) if (!isCurrentContext(responseContext)) return@synchronized null activateSurvey(originalSurvey, progress, responseContext.wasRestored) @@ -833,7 +862,7 @@ public class PostHogSurveysIntegration( */ internal fun canShowNextSurvey(): Boolean { return synchronized(activeSurveyLock) { - config.cachePreferences?.isAvailable() != false && activeSurvey == null + config.cachePreferences?.isAvailable() != false && activeSurvey == null && pendingPresentation == null } } @@ -882,6 +911,7 @@ public class PostHogSurveysIntegration( private fun clearActiveSurvey() { synchronized(activeSurveyLock) { activeSurvey = null + pendingPresentation = null activeSubmissionId = null activeProgressWasPersisted = false activeSurveyCompleted = false diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDelegateLifecycleTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDelegateLifecycleTest.kt index 32eb4d002..6a34b3546 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDelegateLifecycleTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysDelegateLifecycleTest.kt @@ -123,4 +123,65 @@ internal class PostHogSurveysDelegateLifecycleTest { integration.uninstall() } } + + @Test + fun `survey awaiting shown is not rendered again`() { + val shown = mutableListOf() + val closed = mutableListOf() + config.surveysConfig.surveysDelegate = recordingDelegate(shown, closed) + integration.install(PostHogFake()) + try { + integration.showSurvey(survey) + idleMainLooper() + integration.showSurvey(survey) + idleMainLooper() + assertEquals(1, shown.size) + + shown.single()(displaySurvey()) + closed.single()(displaySurvey()) + integration.showSurvey(survey) + idleMainLooper() + assertEquals(2, shown.size) + } finally { + integration.uninstall() + } + } + + @Test + fun `closing a survey before it shows releases it`() { + val shown = mutableListOf() + val closed = mutableListOf() + config.surveysConfig.surveysDelegate = recordingDelegate(shown, closed) + integration.install(PostHogFake()) + try { + integration.showSurvey(survey) + idleMainLooper() + closed.single()(displaySurvey()) + integration.showSurvey(survey) + idleMainLooper() + assertEquals(2, shown.size) + } finally { + integration.uninstall() + } + } + + private fun recordingDelegate( + shown: MutableList, + closed: MutableList, + ): PostHogSurveysDelegate = + object : PostHogSurveysDelegate by PostHogSurveysDefaultDelegate() { + override fun renderSurvey( + survey: PostHogDisplaySurvey, + onSurveyShown: OnPostHogSurveyShown, + onSurveyResponse: OnPostHogSurveyResponse, + onSurveyClosed: OnPostHogSurveyClosed, + ) { + shown += onSurveyShown + closed += onSurveyClosed + } + } + + private fun displaySurvey() = PostHogDisplaySurvey.toDisplaySurvey(survey) + + private fun idleMainLooper() = org.robolectric.Shadows.shadowOf(android.os.Looper.getMainLooper()).idle() } diff --git a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt index ff3649fff..c3d6a6e04 100644 --- a/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/surveys/PostHogSurveysEventPayloadTest.kt @@ -179,6 +179,7 @@ internal class PostHogSurveysEventPayloadTest { val staleDisplay = assertNotNull(delegate.shownSurvey) val staleShown = assertNotNull(delegate.onSurveyShown) preferences.clear() + assertNotNull(delegate.onSurveyClosed).invoke(staleDisplay) resumed.showSurvey(survey.copy(id = "new-survey")) val currentDisplay = assertNotNull(delegate.shownSurvey) assertNotNull(delegate.onSurveyShown).invoke(currentDisplay) diff --git a/posthog/src/main/java/com/posthog/surveys/PostHogSurveysDelegate.kt b/posthog/src/main/java/com/posthog/surveys/PostHogSurveysDelegate.kt index ad5a53d29..58993b1cc 100644 --- a/posthog/src/main/java/com/posthog/surveys/PostHogSurveysDelegate.kt +++ b/posthog/src/main/java/com/posthog/surveys/PostHogSurveysDelegate.kt @@ -12,6 +12,10 @@ public interface PostHogSurveysDelegate { * Delegates that opt in through [PostHogSurveysResumeAwareDelegate] must start at * [PostHogDisplaySurvey.initialQuestionIndex] to resume an unfinished survey. * + * No other survey is rendered until this one is closed. If the survey can't be + * shown, call [onSurveyClosed] without [onSurveyShown]; the survey isn't marked as seen or + * dismissed and can be rendered again later. + * * @param survey The survey to be displayed to the user * @param onSurveyShown To be called when the survey is successfully displayed to the user * @param onSurveyResponse To be called when the user submits a response to a question