Skip to content

fix: [SDK-4513] display queued in-app messages on unpause when triggers still match#2682

Merged
sherwinski merged 3 commits into
mainfrom
sherwin/sdk-4513
Jul 8, 2026
Merged

fix: [SDK-4513] display queued in-app messages on unpause when triggers still match#2682
sherwinski merged 3 commits into
mainfrom
sherwin/sdk-4513

Conversation

@sherwinski

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Setting InAppMessages.paused = false now displays messages that were queued while paused (if their triggers still evaluate true) instead of leaving them parked until the next session.

Details

Motivation

Fixes #2647 (SDK-4513). Messages that qualified while paused are added to messageDisplayQueue, but attemptToShowInAppMessage() early-returns while paused, so they stay parked. On unpause, the setter only called evaluateInAppMessages(), which re-attempts display only when a message currently evaluates true — unpausing alone never resumed delivery mid-session, and the queued IAM only appeared after a session boundary (30+ seconds backgrounded). This contradicts the documented behavior: "Changing the value to false resumes normal message delivery for users who qualify based on their triggers."

The unpause branch now:

  1. Prunes queue entries whose triggers no longer evaluate true — matching iOS, which never queues while paused and only re-evaluates current triggers on unpause. Without this, draining the queue could display an IAM on a screen where its trigger conditions no longer hold. Pruned messages are not lost; they remain in messages and display as soon as their triggers qualify again.
  2. Re-evaluates all messages (evaluateInAppMessages()), which re-queues and displays anything that currently qualifies.
  3. Drains any remaining valid queue entries (attemptToShowInAppMessage()).

This supersedes #2671, which unconditionally drained the queue on unpause — including messages whose triggers no longer evaluated true, diverging from iOS behavior.

Scope

Only the paused setter's unpause path in InAppMessagesManager. Trigger evaluation, queueing, redisplay logic, and the pause/dismiss path are unchanged. Not addressed: the reporter's secondary claim that keeping paused = false at all times also fails — that could not be reproduced on Android (mid-session trigger-driven display works correctly in testing) and may be Unity-bridge-specific or environmental.

Testing

Unit testing

Three new tests in InAppMessagesManagerTests.kt ("Message Queue and Display" context), using a stateful InAppStateService mock so the paused/showing state behaves like production:

  • message queued while paused displays on unpause when triggers still evaluate true
  • message queued while paused does not display on unpause when triggers no longer evaluate true (and still displays later once its trigger re-qualifies, proving pruning is not destructive)
  • message displays mid-session when trigger is satisfied after unpausing (the reported scenario's exact ordering: paused = false first, trigger added second)

All 67 module tests pass. spotlessCheck, detekt, and assembleRelease are clean.

Manual testing

Tested on a Pixel 9 emulator (Android 16, ARM64, Play image) with the demo app built against this branch's SDK source (./gradlew :app:assembleGmsDebug), using the shared demo OneSignal app (77e32082-ea27-42e3-a898-c72e141824ef) and its pre-configured iam_type-triggered IAM campaigns. Logs observed via adb logcat -s OneSignalSDK:V.

Scenario 1 — queued while paused, displays on unpause (the reported bug):

  1. Launch the demo app, wait for the push subscription to register and the IAM fetch to complete. Dismiss the "Display on App Open" IAM.
  2. Toggle Pause In-App Messages ON (setPaused(value: true) in logs).
  3. Tap TOP BANNER (adds trigger iam_type=top_banner). Logs show the message added to the queue followed by In app messaging is currently paused, in app messages will not be shown! — the parked state from the ticket.
  4. Toggle Pause In-App Messages OFF.
  5. ✅ The Top Banner IAM displays immediately mid-session (no session boundary), onWillDisplay fires, and the impression POST returns 200. Before this fix, nothing displayed until the app was backgrounded 30+ seconds.

Scenario 2 — stale queued message is pruned, not displayed:

  1. Toggle Pause In-App Messages ON.
  2. Tap CENTER MODAL (queues the iam_type=center_modal IAM; blocked by pause).
  3. In the Triggers section, remove the iam_type trigger (the queued message's triggers no longer evaluate true).
  4. Toggle Pause In-App Messages OFF.
  5. ✅ Nothing displays; logs show There are no IAMs left in the queue! (the stale entry was pruned). fix: [SDK-4513] re-display queued in-app messages on unpause #2671's approach would have wrongly displayed it here.
  6. Tap CENTER MODAL again — the IAM displays immediately, confirming the pruned message was not lost.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

…rs still match

Messages that qualified while paused were parked in messageDisplayQueue
and only re-attempted when a later trigger change happened to re-qualify
a message, so unpausing alone never resumed delivery mid-session. The
unpause branch of the paused setter now prunes queue entries whose
triggers no longer evaluate true (matching iOS, which never queues while
paused), re-evaluates all messages, and drains the remaining queue.

Fixes #2647
@sherwinski

Copy link
Copy Markdown
Contributor Author

@claude review once

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

  • InAppMessagesManager.kt: 8/8 touched executable lines (100.0%) (16 touched lines in diff)

Overall (aggregate gate)

8/8 touched executable lines covered (100.0% — requires ≥ 80%)

📥 View workflow run

@fadi-george

Copy link
Copy Markdown
Contributor

I’d address two queue-edge cases before merging:

  1. The unpause pruning only runs once. If multiple IAMs are queued, message A can display, message B’s triggers can become false while A is showing, and then messageWasDismissed() drains B via attemptToShowInAppMessage() without revalidating eligibility. Consider applying the same eligibility check before dequeueing, not only in the paused = false path.
  2. The paused setter runs the unpause prune/evaluate/drain path on every paused = false assignment, even when already unpaused. A transition guard (wasPaused && !value) would avoid unexpected queue mutation from redundant calls.
    Also worth adding a regression test for the new dismissed-message queue pruning branch.

@sherwinski

Copy link
Copy Markdown
Contributor Author

Addressed your second point in 2f7be84:

The setter now captures wasPaused before assignment and only runs the prune/evaluate/drain path on an actual paused → unpaused transition, so redundant paused = false calls no longer touch the queue. I also added a test covering this. I also added a regression test for the dismissed-message pruning branch so now a dismissed message seeded in the queue is pruned on unpause while a still-valid queued message displays.

Regarding your first point: I generally agree but it's pre-existing behavior. The dismiss-drain path in messageWasDismissed() has always dequeued the next message without revalidating eligibility. Moving the eligibility check into attemptToShowInAppMessage() would change display semantics for every drain path, not just unpause (e.g. a message legitimately queued whose trigger flips false moments later would now be silently dropped mid-session), and it would also diverge further from iOS, which doesn't revalidate at dequeue either. I want to discuss this point with the team to make sure we're ok with that behavior change before moving forward.

@fadi-george

fadi-george commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The latest commit addresses the original unpause transition concern and adds the dismissed-message coverage I asked about.
I agree the “queued item becomes stale later while another IAM is showing” case can be handled separately. A follow-up issue/PR sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: InAppMessages.Paused = false does not re-attempt display of queued IAMs mid-session

2 participants