Storing pageIds to prefs to fix double impression

Author: emawby

Examples/OneSignalDemo/local.properties

@@ -4,6 +4,6 @@
 # Location of the SDK. This is only used by Gradle.
 # For customization when using a Version Control System, please read the
 # header note.
-#Wed Sep 09 11:15:20 PDT 2020
+#Tue Jul 23 19:22:28 PDT 2019
 ndk.dir=/Users/mikechoch/Library/Android/sdk/ndk-bundle
-sdk.dir=/Users/elliotmawby/Library/Android/sdk
+sdk.dir=/Users/mikechoch/Library/Android/sdk

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

@@ -48,12 +48,6 @@ class OSInAppMessage {
     @NonNull
     private Set<String> clickedClickIds;
 
-    /**
-     * IAM carousel pages viewed
-     */
-    @NonNull
-    private Set<String> viewedPageIds = new HashSet<>();
-
     /**
      * Reference to redisplay properties
      */
@@ -216,13 +210,6 @@ void addClickId(String clickId) {
         clickedClickIds.add(clickId);
     }
 
-    @NonNull
-    Set<String> getViewedPageIds() { return viewedPageIds; }
-
-    void clearPageIds() { viewedPageIds.clear(); }
-
-    void addPageId(String pageId) { viewedPageIds.add(pageId); }
-
     OSInAppMessageRedisplayStats getRedisplayStats() {
         return redisplayStats;
     }

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

@@ -51,6 +51,9 @@ class OSInAppMessageController implements OSDynamicTriggerControllerObserver, OS
     //   This means their impression has been successfully posted to our backend and should not be counted again
     @NonNull
     final private Set<String> impressionedMessages;
+    //   This means their impression has been successfully posted to our backend and should not be counted again
+    @NonNull
+    final private Set<String> viewedPageIds;
     // IAM clicks that have been successfully posted to our backend and should not be counted again
     @NonNull
     final private Set<String> clickedClickIds;
@@ -75,6 +78,7 @@ protected OSInAppMessageController(OneSignalDbHelper dbHelper, OSLogger logger)
         dismissedMessages = OSUtils.newConcurrentSet();
         messageDisplayQueue = new ArrayList<>();
         impressionedMessages = OSUtils.newConcurrentSet();
+        viewedPageIds = OSUtils.newConcurrentSet();
         clickedClickIds = OSUtils.newConcurrentSet();
         triggerController = new OSTriggerController(this);
         systemConditionController = new OSSystemConditionController(this);
@@ -96,6 +100,14 @@ protected OSInAppMessageController(OneSignalDbHelper dbHelper, OSLogger logger)
         if (tempImpressionsSet != null)
             impressionedMessages.addAll(tempImpressionsSet);
 
+        Set<String> tempPageImpressionsSet = OneSignalPrefs.getStringSet(
+                OneSignalPrefs.PREFS_ONESIGNAL,
+                OneSignalPrefs.PREFS_OS_PAGE_IMPRESSIONED_IAMS,
+                null
+        );
+        if (tempPageImpressionsSet != null)
+            viewedPageIds.addAll(tempPageImpressionsSet);
+
         Set<String> tempClickedMessageIdsSet = OneSignalPrefs.getStringSet(
                 OneSignalPrefs.PREFS_ONESIGNAL,
                 OneSignalPrefs.PREFS_OS_CLICKED_CLICK_IDS_IAMS,
@@ -417,18 +429,30 @@ else if (action.getUrlTarget() == OSInAppMessageAction.OSInAppMessageActionUrlTy
         }
     }
 
+    private void saveViewedPageIdsToPrefs() {
+        OneSignalPrefs.saveStringSet(
+                OneSignalPrefs.PREFS_ONESIGNAL,
+                OneSignalPrefs.PREFS_OS_PAGE_IMPRESSIONED_IAMS,
+                // Post success, store impressioned pages to disk
+                viewedPageIds);
+    }
+
     private void fireRESTCallForPageChange(@NonNull final OSInAppMessage message, @NonNull final OSInAppMessagePage page) {
         final String variantId = variantIdForMessage(message);
         if (variantId == null)
             return;
 
         final String pageId = page.getPageId();
 
+        final String messagePrefixedPageId = message.messageId + pageId;
+
         // Never send multiple page impressions for the same message UUID unless that page change is from an IAM with redisplay
-        if (message.getViewedPageIds().contains(pageId))
+        if (viewedPageIds.contains(messagePrefixedPageId)) {
+            OneSignal.onesignalLog(OneSignal.LOG_LEVEL.VERBOSE, "Already sent page impression for id: " + pageId);
             return;
+        }
 
-        message.addPageId(page.getPageId());
+        viewedPageIds.add(messagePrefixedPageId);
 
         try {
             JSONObject json = new JSONObject() {{
@@ -443,18 +467,14 @@ private void fireRESTCallForPageChange(@NonNull final OSInAppMessage message, @N
                 @Override
                 void onSuccess(String response) {
                     printHttpSuccessForInAppMessageRequest("page impression", response);
-                    /*OneSignalPrefs.saveStringSet(
-                            OneSignalPrefs.PREFS_ONESIGNAL,
-                            OneSignalPrefs.PREFS_OS_PAGE_IMPRESSIONED_IAMS,
-                            // Post success, store impressioned page to disk
-                            impressionedMessages);*/
+                    saveViewedPageIdsToPrefs();
                 }
 
                 @Override
                 void onFailure(int statusCode, String response, Throwable throwable) {
                     printHttpErrorForInAppMessageRequest("page impression", statusCode, response);
-                    // Post failed, impressionedMessage should be removed and this way another post can be attempted
-                    message.getViewedPageIds().remove(page.getPageId());
+                    // Post failed, viewed page should be removed and this way another post can be attempted
+                    viewedPageIds.remove(messagePrefixedPageId);
                 }
             });
         } catch (JSONException e) {
@@ -548,8 +568,11 @@ private void setDataForRedisplay(OSInAppMessage message) {
 
                 dismissedMessages.remove(message.messageId);
                 impressionedMessages.remove(message.messageId);
+                // Pages from different IAMs should not impact each other so we can clear the entire
+                // list when an IAM is dismissed or we are re-displaying the same one
+                viewedPageIds.clear();
+                saveViewedPageIdsToPrefs();
                 message.clearClickIds();
-                message.clearPageIds();
             }
         }
     }