Skip to content

Commit 84a0524

Browse files
committed
Fix IAM redisplay multiple times
* When IAM only has dynamic triggers it can end redisplaying multiple times on the same session * Limit IAM with only dynamic trigger to display once per session * Add test for escenario
1 parent ee96927 commit 84a0524

File tree

4 files changed

+65
-18
lines changed

4 files changed

+65
-18
lines changed

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSInAppMessage.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,13 @@ void setRedisplayStats(int displayQuantity, long lastDisplayTime) {
222222
public String toString() {
223223
return "OSInAppMessage{" +
224224
"messageId='" + messageId + '\'' +
225+
", variants=" + variants +
225226
", triggers=" + triggers +
226227
", clickedClickIds=" + clickedClickIds +
227-
", displayStats=" + redisplayStats +
228+
", redisplayStats=" + redisplayStats +
229+
", displayDuration=" + displayDuration +
230+
", displayedInSession=" + displayedInSession +
231+
", triggerChanged=" + triggerChanged +
228232
", actionTaken=" + actionTaken +
229233
", isPreview=" + isPreview +
230234
'}';

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSInAppMessageController.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -479,10 +479,9 @@ private void setDataForRedisplay(OSInAppMessage message) {
479479
if (messageDismissed && index != -1) {
480480
OSInAppMessage savedIAM = redisplayedInAppMessages.get(index);
481481
message.getRedisplayStats().setDisplayStats(savedIAM.getRedisplayStats());
482+
message.setDisplayedInSession(savedIAM.isDisplayedInSession());
482483

483-
// Message that don't have triggers should display only once per session
484-
boolean triggerHasChanged = message.isTriggerChanged() || (!savedIAM.isDisplayedInSession() && message.triggers.isEmpty());
485-
484+
boolean triggerHasChanged = hasMessageTriggerChanged(message);
486485
OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "setDataForRedisplay: " + message.toString() + " triggerHasChanged: " + triggerHasChanged);
487486

488487
// Check if conditions are correct for redisplay
@@ -498,6 +497,18 @@ private void setDataForRedisplay(OSInAppMessage message) {
498497
}
499498
}
500499

500+
private boolean hasMessageTriggerChanged(OSInAppMessage message) {
501+
// Message that only have dynamic trigger should display only once per session
502+
boolean messageHasOnlyDynamicTrigger = triggerController.messageHasOnlyDynamicTriggers(message);
503+
if (messageHasOnlyDynamicTrigger)
504+
return !message.isDisplayedInSession();
505+
506+
// Message that don't have triggers should display only once per session
507+
boolean shouldMessageDisplayInSession = !message.isDisplayedInSession() && message.triggers.isEmpty();
508+
509+
return message.isTriggerChanged() || shouldMessageDisplayInSession;
510+
}
511+
501512
/**
502513
* Message has passed triggers and de-duplication logic.
503514
* Display message now or add it to the queue to be displayed.

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSTriggerController.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,26 @@ boolean isTriggerOnMessage(OSInAppMessage message, Collection<String> newTrigger
191191
return false;
192192
}
193193

194+
/**
195+
* Part of redisplay logic
196+
*
197+
* If message has only dynamic trigger return true, otherwise false
198+
* */
199+
boolean messageHasOnlyDynamicTriggers(OSInAppMessage message) {
200+
if (message.triggers == null || message.triggers.isEmpty())
201+
return false;
202+
203+
for (ArrayList<OSTrigger> andConditions : message.triggers) {
204+
for (OSTrigger trigger : andConditions) {
205+
if (trigger.kind == OSTrigger.OSTriggerKind.CUSTOM || trigger.kind == OSTrigger.OSTriggerKind.UNKNOWN)
206+
// At least one trigger is not dynamic
207+
return false;
208+
}
209+
}
210+
211+
return true;
212+
}
213+
194214
/**
195215
* Trigger Set/Delete/Persist Logic
196216
*/

OneSignalSDK/unittest/src/test/java/com/test/onesignal/InAppMessageIntegrationTests.java

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,14 @@ private void initializeSdkWithMultiplePendingMessages() throws Exception {
282282
}
283283

284284
@Test
285-
public void testTimedMessageIsDisplayed() throws Exception {
286-
final OSTestInAppMessage message = InAppMessagingHelpers.buildTestMessageWithSingleTrigger(OSTriggerKind.SESSION_TIME, null, OSTestTrigger.OSTriggerOperator.GREATER_THAN.toString(), 0.05);
285+
public void testTimedMessageIsDisplayedOncePerSession() throws Exception {
286+
final OSTestInAppMessage message = InAppMessagingHelpers.buildTestMessageWithSingleTriggerAndRedisplay(
287+
OSTriggerKind.SESSION_TIME,
288+
null,
289+
OSTestTrigger.OSTriggerOperator.GREATER_THAN.toString(),
290+
0.05,
291+
10,
292+
0);
287293

288294
setMockRegistrationResponseWithMessages(new ArrayList<OSTestInAppMessage>() {{
289295
add(message);
@@ -301,31 +307,31 @@ public void testTimedMessageIsDisplayed() throws Exception {
301307
Awaitility.await()
302308
.atMost(new Duration(150, TimeUnit.MILLISECONDS))
303309
.pollInterval(new Duration(10, TimeUnit.MILLISECONDS))
304-
.until(new Callable<Boolean>() {
305-
@Override
306-
public Boolean call() throws Exception {
307-
return OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size() == 1;
308-
}
309-
});
310-
}
310+
.until(() -> OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size() == 1);
311311

312+
OneSignalPackagePrivateHelper.dismissCurrentMessage();
313+
314+
// Check that the IAM is not displayed again
315+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
316+
}
312317

313318
@Test
314-
public void testAfterLastInAppTimeIsDisplayed() throws Exception {
315-
final OSTestInAppMessage message1 = InAppMessagingHelpers.buildTestMessageWithSingleTrigger(
319+
public void testAfterLastInAppTimeIsDisplayedOncePerSession() throws Exception {
320+
final OSTestInAppMessage message1 = InAppMessagingHelpers.buildTestMessageWithSingleTriggerAndRedisplay(
316321
OSTriggerKind.SESSION_TIME,
317322
null,
318323
OSTestTrigger.OSTriggerOperator.GREATER_THAN.toString(),
319-
0.05
320-
);
324+
0.05,
325+
10,
326+
0);
321327

322328
ArrayList<ArrayList<OSTestTrigger>> triggers2 = new ArrayList<ArrayList<OSTestTrigger>>() {{
323329
add(new ArrayList<OSTestTrigger>() {{
324330
add(InAppMessagingHelpers.buildTrigger(OSTriggerKind.SESSION_TIME, null, OSTestTrigger.OSTriggerOperator.GREATER_THAN.toString(), 0.1));
325331
add(InAppMessagingHelpers.buildTrigger(OSTriggerKind.TIME_SINCE_LAST_IN_APP, null, OSTestTrigger.OSTriggerOperator.GREATER_THAN.toString(), 0.05));
326332
}});
327333
}};
328-
final OSTestInAppMessage message2 = InAppMessagingHelpers.buildTestMessageWithMultipleTriggers(triggers2);
334+
final OSTestInAppMessage message2 = InAppMessagingHelpers.buildTestMessageWithMultipleTriggersAndRedisplay(triggers2, 10, 0);
329335

330336
setMockRegistrationResponseWithMessages(new ArrayList<OSTestInAppMessage>() {{
331337
add(message1);
@@ -365,6 +371,12 @@ public void run() {
365371
assertEquals(message2.messageId, OneSignalPackagePrivateHelper.getShowingInAppMessageId());
366372
}
367373
});
374+
375+
376+
OneSignalPackagePrivateHelper.dismissCurrentMessage();
377+
378+
// Check that the IAM is not displayed again
379+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
368380
}
369381

370382
/**

0 commit comments

Comments
 (0)