From 7582adf914c9858e0ad583c7138683c69277ff52 Mon Sep 17 00:00:00 2001 From: jinliu9508 Date: Thu, 18 Jul 2024 01:25:20 -0400 Subject: [PATCH 1/3] Add a repeat check for trigger combination evaluated to true Summary of changes: 1. When a time-based trigger is evaluated and 'onTriggerCompleted()' is called, we no longer make redisplay messages available to avoid duplicate redisplay of a same message. 2. Modify the logic of 'makeRedisplayMessagesAvailableWithTriggers()' so that a message can be redisplayed only if at least one trigger has changed or a new trigger is added. 3. 'onTriggerConditionChanged()' now accepts a new parameter 'triggerId' that indicates the time-based trigger that fires. --- .../internal/InAppMessagesManager.kt | 19 ++++++++++++------- .../internal/triggers/ITriggerHandler.kt | 2 +- .../triggers/impl/DynamicTriggerController.kt | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index 2e70f1c07a..1b15470ffd 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -451,12 +451,16 @@ internal class InAppMessagesManager( * * Make all messages with redisplay available if: * - Already displayed - * - At least one Trigger has changed + * - At least one existing Trigger has changed OR a new trigger is added */ - private fun makeRedisplayMessagesAvailableWithTriggers(newTriggersKeys: Collection) { + private fun makeRedisplayMessagesAvailableWithTriggers( + newTriggersKeys: Collection, + isNewTriggerAdded: Boolean, + ) { for (message in messages) { - if (!message.isTriggerChanged && redisplayedInAppMessages.contains(message) && - _triggerController.isTriggerOnMessage(message, newTriggersKeys) + if (!message.isTriggerChanged && + redisplayedInAppMessages.contains(message) && + (_triggerController.isTriggerOnMessage(message, newTriggersKeys) || isNewTriggerAdded) ) { Logging.debug("InAppMessagesManager.makeRedisplayMessagesAvailableWithTriggers: Trigger changed for message: $message") message.isTriggerChanged = true @@ -643,7 +647,6 @@ internal class InAppMessagesManager( Logging.debug("InAppMessagesManager.onTriggerCompleted: called with triggerId: $triggerId") val triggerIds: MutableSet = HashSet() triggerIds.add(triggerId) - makeRedisplayMessagesAvailableWithTriggers(triggerIds) } /** @@ -653,9 +656,11 @@ internal class InAppMessagesManager( * * @see OSInAppMessageController.setDataForRedisplay */ - override fun onTriggerConditionChanged() { + override fun onTriggerConditionChanged(triggerId: String) { Logging.debug("InAppMessagesManager.onTriggerConditionChanged()") + makeRedisplayMessagesAvailableWithTriggers(listOf(triggerId), false) + suspendifyOnThread { // This method is called when a time-based trigger timer fires, meaning the message can // probably be shown now. So the current message conditions should be re-evaluated @@ -666,7 +671,7 @@ internal class InAppMessagesManager( override fun onTriggerChanged(newTriggerKey: String) { Logging.debug("InAppMessagesManager.onTriggerChanged(newTriggerKey: $newTriggerKey)") - makeRedisplayMessagesAvailableWithTriggers(listOf(newTriggerKey)) + makeRedisplayMessagesAvailableWithTriggers(listOf(newTriggerKey), true) suspendifyOnThread { // This method is called when a time-based trigger timer fires, meaning the message can diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/ITriggerHandler.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/ITriggerHandler.kt index 861ddcfdf8..c834d32390 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/ITriggerHandler.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/ITriggerHandler.kt @@ -16,7 +16,7 @@ internal interface ITriggerHandler { /** * Called when a time-based trigger (dynamic trigger) will now evaluate to true. */ - fun onTriggerConditionChanged() + fun onTriggerConditionChanged(triggerId: String) /** * Called when a new trigger has been added, or an existing trigger's value has been diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/impl/DynamicTriggerController.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/impl/DynamicTriggerController.kt index 8f12b54575..b7dd35afd7 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/impl/DynamicTriggerController.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/triggers/impl/DynamicTriggerController.kt @@ -89,7 +89,7 @@ internal class DynamicTriggerController( object : TimerTask() { override fun run() { scheduledMessages.remove(triggerId) - events.fire { it.onTriggerConditionChanged() } + events.fire { it.onTriggerConditionChanged(triggerId) } } }, triggerId, From ea912ff34851e04c8fb84b9475723a28019c339c Mon Sep 17 00:00:00 2001 From: jinliu9508 Date: Thu, 18 Jul 2024 01:44:18 -0400 Subject: [PATCH 2/3] Allow redisplay if either an expected trigger has changed or a trigger is added when there is only dynamic trigger --- .../inAppMessages/internal/InAppMessagesManager.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index 1b15470ffd..995ce5f8a4 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -451,7 +451,7 @@ internal class InAppMessagesManager( * * Make all messages with redisplay available if: * - Already displayed - * - At least one existing Trigger has changed OR a new trigger is added + * - At least one existing Trigger has changed OR a new trigger is added when there is only dynamic trigger */ private fun makeRedisplayMessagesAvailableWithTriggers( newTriggersKeys: Collection, @@ -460,7 +460,10 @@ internal class InAppMessagesManager( for (message in messages) { if (!message.isTriggerChanged && redisplayedInAppMessages.contains(message) && - (_triggerController.isTriggerOnMessage(message, newTriggersKeys) || isNewTriggerAdded) + ( + _triggerController.isTriggerOnMessage(message, newTriggersKeys) || + isNewTriggerAdded && _triggerController.messageHasOnlyDynamicTriggers(message) + ) ) { Logging.debug("InAppMessagesManager.makeRedisplayMessagesAvailableWithTriggers: Trigger changed for message: $message") message.isTriggerChanged = true From 26a1dfd6b63c08c5924b88b9f1263b642c2bd90d Mon Sep 17 00:00:00 2001 From: jinliu9508 Date: Wed, 24 Jul 2024 10:36:02 -0400 Subject: [PATCH 3/3] Rephrase if condition for more readability --- .../inAppMessages/internal/InAppMessagesManager.kt | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index 995ce5f8a4..6513e3f548 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -458,13 +458,10 @@ internal class InAppMessagesManager( isNewTriggerAdded: Boolean, ) { for (message in messages) { - if (!message.isTriggerChanged && - redisplayedInAppMessages.contains(message) && - ( - _triggerController.isTriggerOnMessage(message, newTriggersKeys) || - isNewTriggerAdded && _triggerController.messageHasOnlyDynamicTriggers(message) - ) - ) { + val isMessageDisplayed = redisplayedInAppMessages.contains(message) + val isTriggerOnMessage = _triggerController.isTriggerOnMessage(message, newTriggersKeys) + val isOnlyDynamicTriggers = _triggerController.messageHasOnlyDynamicTriggers(message) + if (!message.isTriggerChanged && isMessageDisplayed && (isTriggerOnMessage || isNewTriggerAdded && isOnlyDynamicTriggers)) { Logging.debug("InAppMessagesManager.makeRedisplayMessagesAvailableWithTriggers: Trigger changed for message: $message") message.isTriggerChanged = true }