Skip to content

Commit beede68

Browse files
authored
IAM still show form the queue when IAM is pasued (Reported from Unity) (#942)
* Changes to the aim controller to fix a bug reported from Unity * IAM will show form the queue even when IAM is paused * Must wrap the check around the display action not the action to add to the queue * When IAM is paused adding to the queue works now and displaying will be prevented instead * Modified more triggeredMessages old code * Now we call them dismissedMessages, as triggered is confusing and sounds like it refers to IAM triggers added from the dashboard * `tempTriggeredSet` is now `tempDismissedSet` when get persisted IAMs * Updates to fix unit tests related to IAM functionality * Removed the `ShadowOSInAppMessageController.java`, this was duplicate logic not necessary to test in app messaging * Instead of Shadow for IAM controller, the `OneSignalPackagePrivateHelper.java` was given some helpers to access the current `OSInAppMessageController.java` * `messageDisplayQueue` can accumulate as many IAMs as it would like, but the `inAppMessagingEnabled` flag will control whether or not anything IAMs will show from the queue * `inAppMessageShowing` flag determines if an IAM is actually on the screen or not * Updates from comments on last PR review * `messageDisplayQueue` never gets duplicates of the same IAM * Handle failure case for pulling down all of the HTML for an IAM messageId * Decided to create a list (401 to 404, 410) in OSUtils and a helper to verify whether or not to reattempt a network request based on the status code in the response * Added a constant to represent a max retry count, 3, for now until other cases arise * Make sure when removing, the messageDisplayQueue is not empty' * Dismiss preview IAMs with failed html request * Split logic for dismissing IAM by things we need for all IAMs and non-preview IAMs * Broke a unit test for IAM removing `inAppMessageShowing = false;` * Called in 3 spots, did not account for the 3rd and it broke a unit test, now fixed by adding it back
1 parent 15a14c2 commit beede68

File tree

10 files changed

+197
-143
lines changed

10 files changed

+197
-143
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ boolean dynamicTriggerShouldFire(final OSTrigger trigger) {
4343
currentTimeInterval = new Date().getTime() - sessionLaunchTime.getTime();
4444
break;
4545
case TIME_SINCE_LAST_IN_APP:
46-
if (OSInAppMessageController.getController().isDisplayingInApp())
46+
if (OSInAppMessageController.getController().isInAppMessageShowing())
4747
return false;
4848
Date lastTimeAppDismissed = OSInAppMessageController.getController().lastTimeInAppDismissed;
4949
if (lastTimeAppDismissed == null)

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

Lines changed: 122 additions & 86 deletions
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ void onMessageActionOccurredOnMessage(@NonNull OSInAppMessage message, @NonNull
3434
void onMessageActionOccurredOnPreview(@NonNull OSInAppMessage message, @NonNull JSONObject actionJson) { }
3535

3636
@Override
37-
boolean isDisplayingInApp() { return false; }
37+
boolean isInAppMessageShowing() { return false; }
3838

3939
@Nullable
4040
@Override

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class OSUtils {
6767

6868
static final int UNINITIALIZABLE_STATUS = -999;
6969

70+
public static int MAX_NETWORK_REQUEST_ATTEMPT_COUNT = 3;
71+
static final int[] NO_RETRY_NETWROK_REQUEST_STATUS_CODES = {401, 402, 403, 404, 410};
72+
7073
public enum SchemaType {
7174
DATA("data"),
7275
HTTPS("https"),
@@ -89,6 +92,14 @@ public static SchemaType fromString(String text) {
8992
}
9093
}
9194

95+
public static boolean shouldRetryNetworkRequest(int statusCode) {
96+
for (int code : NO_RETRY_NETWROK_REQUEST_STATUS_CODES)
97+
if (statusCode == code)
98+
return false;
99+
100+
return true;
101+
}
102+
92103
int initializationChecker(Context context, int deviceType, String oneSignalAppId) {
93104
int subscribableStatus = 1;
94105

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class OneSignalPrefs {
7676
public static final String PREFS_OS_ETAG_PREFIX = "PREFS_OS_ETAG_PREFIX_";
7777
public static final String PREFS_OS_HTTP_CACHE_PREFIX = "PREFS_OS_HTTP_CACHE_PREFIX_";
7878
public static final String PREFS_OS_CACHED_IAMS = "PREFS_OS_CACHED_IAMS";
79-
public static final String PREFS_OS_DISPLAYED_IAMS = "PREFS_OS_DISPLAYED_IAMS";
79+
public static final String PREFS_OS_DISMISSED_IAMS = "PREFS_OS_DISPLAYED_IAMS";
8080
public static final String PREFS_OS_IMPRESSIONED_IAMS = "PREFS_OS_IMPRESSIONED_IAMS";
8181
public static final String PREFS_OS_CLICKED_CLICK_IDS_IAMS = "PREFS_OS_CLICKED_CLICK_IDS_IAMS";
8282
public static final String PREFS_OS_RECEIVE_RECEIPTS_ENABLED = "PREFS_OS_RECEIVE_RECEIPTS_ENABLED";

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public static void resetSessionLaunchTime() {
3232

3333
public static void clearTestState() {
3434
OneSignal.pauseInAppMessages(false);
35-
ShadowOSInAppMessageController.displayedMessages.clear();
3635
OSInAppMessageController.getController().messageDisplayQueue.clear();
3736
}
3837

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,20 @@ public OSTestInAppMessageAction(JSONObject json) throws JSONException {
320320

321321
public static void dismissCurrentMessage() {
322322
com.onesignal.OSInAppMessage message = com.onesignal.OSInAppMessageController.getController().getCurrentDisplayedInAppMessage();
323-
com.onesignal.OSInAppMessageController.getController().messageWasDismissed(message);
323+
if (message != null)
324+
com.onesignal.OSInAppMessageController.getController().messageWasDismissed(message);
325+
}
326+
327+
public static boolean isInAppMessageShowing() {
328+
return com.onesignal.OSInAppMessageController.getController().isInAppMessageShowing();
329+
}
330+
331+
public static String getShowingInAppMessageId() {
332+
return com.onesignal.OSInAppMessageController.getController().getCurrentDisplayedInAppMessage().messageId;
333+
}
334+
335+
public static ArrayList<com.onesignal.OSInAppMessage> getInAppMessageDisplayQueue() {
336+
return com.onesignal.OSInAppMessageController.getController().messageDisplayQueue;
324337
}
325338

326339
public static class OSInAppMessageController extends com.onesignal.OSInAppMessageController {
@@ -332,6 +345,7 @@ public static OSInAppMessageController getController() {
332345

333346
return sharedInstance;
334347
}
348+
335349
public void onMessageActionOccurredOnMessage(@NonNull final com.onesignal.OSInAppMessage message, @NonNull final JSONObject actionJson) {
336350
super.onMessageActionOccurredOnMessage(message, actionJson);
337351
}

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

Lines changed: 0 additions & 23 deletions
This file was deleted.

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

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import com.onesignal.ShadowDynamicTimer;
1616
import com.onesignal.ShadowJobService;
1717
import com.onesignal.ShadowNotificationManagerCompat;
18-
import com.onesignal.ShadowOSInAppMessageController;
1918
import com.onesignal.ShadowOSUtils;
2019
import com.onesignal.ShadowOSViewUtils;
2120
import com.onesignal.ShadowOSWebView;
@@ -53,6 +52,7 @@
5352
import static com.test.onesignal.TestHelpers.threadAndTaskWait;
5453
import static junit.framework.Assert.assertEquals;
5554
import static junit.framework.Assert.assertFalse;
55+
import static junit.framework.Assert.assertTrue;
5656

5757
@Config(packageName = "com.onesignal.example",
5858
instrumentedPackages = { "com.onesignal" },
@@ -66,7 +66,6 @@
6666
ShadowNotificationManagerCompat.class,
6767
ShadowJobService.class,
6868
ShadowDynamicTimer.class,
69-
ShadowOSInAppMessageController.class,
7069
ShadowOSWebView.class,
7170
ShadowOSViewUtils.class
7271
},
@@ -134,7 +133,10 @@ public void testDisableInAppMessagingPreventsMessageDisplay() throws Exception {
134133
// We will set the trigger. However, since messaging is disabled, the message should not be shown
135134
OneSignal.addTrigger("test_key", 3);
136135

137-
assertEquals(0, ShadowOSInAppMessageController.displayedMessages.size());
136+
// Make sure 1 IAM is in the display queue
137+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
138+
// Make sure no IAM is showing
139+
assertFalse(OneSignalPackagePrivateHelper.isInAppMessageShowing());
138140
}
139141

140142
/**
@@ -151,16 +153,28 @@ public void testMultipleMessagesDoNotBothDisplay() throws Exception {
151153
}});
152154
threadAndTaskWait();
153155

154-
//both messages should now be valid but only one should display
155-
//which one displays first is undefined and doesn't really matter
156-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
156+
// Make sure 2 items are in the display queue
157+
assertEquals(2, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
158+
// Make sure an IAM is showing
159+
assertTrue(OneSignalPackagePrivateHelper.isInAppMessageShowing());
157160

158-
// dismiss the message
161+
// Dismiss the message
159162
OneSignalPackagePrivateHelper.dismissCurrentMessage();
160163
threadAndTaskWait();
161164

162-
// the second in app message should now be displayed
163-
assertEquals(2, ShadowOSInAppMessageController.displayedMessages.size());
165+
// Make sure 1 item is in the display queue
166+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
167+
// Make sure an IAM is showing
168+
assertTrue(OneSignalPackagePrivateHelper.isInAppMessageShowing());
169+
170+
// Dismiss the message
171+
OneSignalPackagePrivateHelper.dismissCurrentMessage();
172+
threadAndTaskWait();
173+
174+
// Make sure no items are in the display queue
175+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
176+
// Make sure no IAM is showing
177+
assertFalse(OneSignalPackagePrivateHelper.isInAppMessageShowing());
164178
}
165179

166180
// This tests both rotating the device or the app being resumed.
@@ -175,8 +189,9 @@ public void testMessageDismissingWhileDeviceIsRotating() throws Exception {
175189
}});
176190
threadAndTaskWait();
177191

178-
// 2. Assert one IAM was displayed
179-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
192+
// 2. Assert two IAM in the queue and 1 is showing
193+
assertEquals(2, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
194+
assertTrue(OneSignalPackagePrivateHelper.isInAppMessageShowing());
180195

181196
// 3. Rotate device - This will kick off a JS task to get the new height
182197
blankActivityController.pause();
@@ -189,6 +204,10 @@ public void testMessageDismissingWhileDeviceIsRotating() throws Exception {
189204
// 5. Now fire resize event which was scheduled in step 3.
190205
// Test that this does not throw and handles this missing IAM view.
191206
ShadowOSWebView.fireEvalJSCallbacks();
207+
208+
// 6. Make sure only 1 IAM ios left in queue now and it is showing
209+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
210+
assertTrue(OneSignalPackagePrivateHelper.isInAppMessageShowing());
192211
}
193212

194213

@@ -233,7 +252,7 @@ public void testTimedMessageIsDisplayed() throws Exception {
233252
.until(new Callable<Boolean>() {
234253
@Override
235254
public Boolean call() throws Exception {
236-
return ShadowOSInAppMessageController.displayedMessages.size() == 1;
255+
return OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size() == 1;
237256
}
238257
});
239258
}
@@ -276,8 +295,8 @@ public void testAfterLastInAppTimeIsDisplayed() throws Exception {
276295
.untilAsserted(new ThrowingRunnable() {
277296
@Override
278297
public void run() {
279-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
280-
assertEquals(message1.messageId, ShadowOSInAppMessageController.displayedMessages.get(0));
298+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
299+
assertEquals(message1.messageId, OneSignalPackagePrivateHelper.getShowingInAppMessageId());
281300
}
282301
});
283302

@@ -290,8 +309,8 @@ public void run() {
290309
.untilAsserted(new ThrowingRunnable() {
291310
@Override
292311
public void run() {
293-
assertEquals(2, ShadowOSInAppMessageController.displayedMessages.size());
294-
assertEquals(message2.messageId, ShadowOSInAppMessageController.displayedMessages.get(1));
312+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
313+
assertEquals(message2.messageId, OneSignalPackagePrivateHelper.getShowingInAppMessageId());
295314
}
296315
});
297316
}
@@ -324,20 +343,20 @@ public void testTimedMessageDisplayedAfterAllTriggersValid() throws Exception {
324343

325344
// no timer should be scheduled since 'test_key' != 'squirrel'
326345
assertFalse(ShadowDynamicTimer.hasScheduledTimer);
327-
assertEquals(0, ShadowOSInAppMessageController.displayedMessages.size());
346+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
328347

329348
// since we are not actually waiting on any logic to finish, sleeping here is fine
330349
Thread.sleep(20);
331350

332351
// the message still should not be displayed
333-
assertEquals(0, ShadowOSInAppMessageController.displayedMessages.size());
352+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
334353

335354
// after setting this trigger the message should be displayed immediately
336355
OneSignal.addTrigger("test_key", "squirrel");
337356
threadAndTaskWait();
338357

339358
// the message should now have been displayed
340-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
359+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
341360
assertFalse(ShadowDynamicTimer.hasScheduledTimer);
342361
}
343362

@@ -353,7 +372,7 @@ public void useCachedInAppListOnQuickColdRestart() throws Exception {
353372
// Should used cached triggers since we won't be making an on_session call.
354373
// Testing for this by trying to add a trigger that should display an IAM
355374
OneSignal.addTrigger("test_2", 2);
356-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
375+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
357376
}
358377

359378
@Test
@@ -374,7 +393,7 @@ public void useCachedInAppListOnQuickColdRestartWhenInitFromAppClass() throws Ex
374393
// Should used cached triggers since we won't be making an on_session call.
375394
// Testing for this by trying to add a trigger that should display an IAM
376395
OneSignal.addTrigger("test_2", 2);
377-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
396+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
378397
}
379398

380399
@Test
@@ -383,15 +402,15 @@ public void doNotReshowInAppIfDismissed_evenAfterColdRestart() throws Exception
383402
initializeSdkWithMultiplePendingMessages();
384403
// 2. Trigger showing In App and dismiss it
385404
OneSignal.addTrigger("test_2", 2);
386-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
405+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
387406
OneSignalPackagePrivateHelper.dismissCurrentMessage();
388407
// 3. Swipe away app
389408
fastColdRestartApp();
390409
// 4. Cold Start app
391410
initializeSdkWithMultiplePendingMessages();
392411
// 5. Set same trigger, should not display again
393412
OneSignal.addTrigger("test_2", 2);
394-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
413+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
395414
}
396415

397416
@Test
@@ -400,15 +419,15 @@ public void reshowInAppIfDisplayedButNeverDismissedAfterColdRestart() throws Exc
400419
initializeSdkWithMultiplePendingMessages();
401420
// 2. Trigger showing In App
402421
OneSignal.addTrigger("test_2", 2);
403-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
422+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
404423
// 3. Swipe away app
405424
fastColdRestartApp();
406425
// 4. Cold Start app
407426
initializeSdkWithMultiplePendingMessages();
408-
assertEquals(1, ShadowOSInAppMessageController.displayedMessages.size());
427+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
409428
// 5. Set same trigger, should now display again, since it was never dismissed
410429
OneSignal.addTrigger("test_2", 2);
411-
assertEquals(2, ShadowOSInAppMessageController.displayedMessages.size());
430+
assertEquals(1, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
412431
}
413432

414433
@Test
@@ -560,7 +579,7 @@ public void testMessageNotShownForAndroidApi18Lower() throws Exception {
560579
threadAndTaskWait();
561580

562581
// Check no messages exist
563-
assertEquals(0, ShadowOSInAppMessageController.displayedMessages.size());
582+
assertEquals(0, OneSignalPackagePrivateHelper.getInAppMessageDisplayQueue().size());
564583
}
565584

566585
private void setMockRegistrationResponseWithMessages(ArrayList<OSTestInAppMessage> messages) throws JSONException {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import com.onesignal.ShadowDynamicTimer;
1818
import com.onesignal.ShadowJobService;
1919
import com.onesignal.ShadowNotificationManagerCompat;
20-
import com.onesignal.ShadowOSInAppMessageController;
2120
import com.onesignal.ShadowOSUtils;
2221
import com.onesignal.ShadowOneSignalRestClient;
2322
import com.onesignal.ShadowPushRegistratorGCM;
@@ -64,7 +63,6 @@
6463
ShadowNotificationManagerCompat.class,
6564
ShadowJobService.class,
6665
ShadowDynamicTimer.class,
67-
ShadowOSInAppMessageController.class
6866
},
6967
sdk = 26
7068
)

0 commit comments

Comments
 (0)