chore: log states via redux #421

Shahroz16 · 2024-08-09T13:51:51Z

closes: https://linear.app/customerio/issue/MBL-454/improve-debug-logs-for-in-app-android

Changes:

Added domain layer for redux to store state changes and have a single source of truth
No change in business logic, but now we can verify which component deviated from the current state
Right now this redux structure is just used for logging states.

More details on the structure can be followed: https://www.notion.so/custio/InAppMessaging-revamp-A-brave-new-event-driven-world-7422938d647944999defc9cca9f113a3?pvs=4

github-actions · 2024-08-09T13:52:07Z

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

java_layout: shahroz/logging-via-redux (1723225628)
kotlin_compose: shahroz/logging-via-redux (1723225632)

Shahroz16 · 2024-08-09T13:54:31Z

core/src/main/kotlin/io/customer/sdk/core/di/DiGraph.kt


 abstract class DiGraph {
    /**
     * Map of dependencies that can be overridden with mocks in test functions.
     */
-    val overrides: MutableMap<String, Any> = mutableMapOf()
+    val overrides = ConcurrentHashMap<String, Any>()


To ensure synchronization.

github-actions · 2024-08-09T13:55:19Z

Build available to test
Version: shahroz-logging-via-redux-SNAPSHOT
Repository: https://s01.oss.sonatype.org/content/repositories/snapshots/

github-actions · 2024-08-09T13:56:43Z

📏 SDK Binary Size Comparison Report

No changes detected in SDK binary size ✅

codecov · 2024-08-09T13:57:27Z

Codecov Report

Attention: Patch coverage is 26.75159% with 115 lines in your changes missing coverage. Please review.

Project coverage is 41.98%. Comparing base (7794cd4) to head (8b30804).
Report is 3 commits behind head on main.

Files	Patch %	Lines
...stomer/messaginginapp/gist/data/listeners/Queue.kt	4.34%	22 Missing ⚠️
...tomer/messaginginapp/gist/presentation/GistView.kt	0.00%	16 Missing ⚠️
...omer/messaginginapp/domain/InAppMessagingAction.kt	17.64%	14 Missing ⚠️
...stomer/messaginginapp/gist/presentation/GistSdk.kt	12.50%	14 Missing ⚠️
...ssaginginapp/gist/presentation/GistModalManager.kt	15.38%	11 Missing ⚠️
...tomer/messaginginapp/domain/InAppMessageReducer.kt	33.33%	0 Missing and 10 partials ⚠️
...saginginapp/gist/presentation/GistModalActivity.kt	0.00%	10 Missing ⚠️
...inginapp/gist/presentation/engine/EngineWebView.kt	0.00%	6 Missing ⚠️
...io/customer/messaginginapp/ModuleMessagingInApp.kt	37.50%	5 Missing ⚠️
...omer/messaginginapp/gist/utilities/ElapsedTimer.kt	0.00%	3 Missing ⚠️
... and 2 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #421      +/-   ##
============================================
- Coverage     41.99%   41.98%   -0.01%     
- Complexity      256      259       +3     
============================================
  Files            94       99       +5     
  Lines          2229     2320      +91     
  Branches        319      344      +25     
============================================
+ Hits            936      974      +38     
- Misses         1204     1247      +43     
- Partials         89       99      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mrehan27

Looks promising. Given that no business logic has changed and testing results were positive, I'm happy shipping this to customers 👍🏻

mrehan27 · 2024-08-09T14:03:39Z

messaginginapp/src/main/java/io/customer/messaginginapp/domain/InAppMessageReducer.kt

+        is InAppMessagingAction.ProcessMessage -> state.copy(currentMessageBeingProcessed = action.message)
+        is InAppMessagingAction.EmbedMessage -> state.copy(currentMessageBeingProcessed = null, currentModalMessage = null)
+        is InAppMessagingAction.ClearUser -> state.copy(currentUser = null, messages = emptyList())
+        else -> state // For actions that don't change state


While this saves some efforts, but it can lead to unexpected results as well for new states. Shouldn't we combine remaining states here instead of else block? 🤔

Yup, this would change, when we start adding more actions, right now scope was just logging,

mrehan27 · 2024-08-09T14:04:28Z

messaginginapp/src/main/java/io/customer/messaginginapp/domain/InAppMessageStore.kt

+import org.reduxkotlin.applyMiddleware
+import org.reduxkotlin.threadsafe.createThreadSafeStore
+
+internal object InAppMessagingStore {


Any particular reason for making this object. Why couldn't we leverage singleton from SDKComponent for this?

We are levering it from singleton, this was added as a precaution and since it doesn't need constructor.

mrehan27 · 2024-08-09T14:06:25Z

messaginginapp/src/main/java/io/customer/messaginginapp/gist/data/listeners/Queue.kt

-                        GIST_TAG,
-                        "Found ${latestMessagesResponse.body()?.count()} messages for user"
-                    )
+                    inAppMessagingManager.dispatch(InAppMessagingAction.LogEvent("Found ${latestMessagesResponse.body()?.count()} messages for user"))


Can't we extract latestMessagesResponse.body() to single variable and reuse in the line below?

mrehan27 · 2024-08-09T14:07:51Z

messaginginapp/src/main/java/io/customer/messaginginapp/gist/presentation/GistModalActivity.kt

        GistSdk.removeListener(this)
        super.onPause()
        overridePendingTransition(0, 0)
    }

    override fun finish() {
+        inAppMessagingManager.dispatch(InAppMessagingAction.LogEvent("GisModelActivity finish"))
        runOnUiThread {


Should we log on animations end too?

mrehan27 · 2024-08-09T14:13:20Z

messaginginapp/src/main/java/io/customer/messaginginapp/gist/presentation/GistSdk.kt

I have some suggestions that apply to code in several areas so adding here.

I think we can now remove GIST_TAG?

For logging the code looks to heavy at the moment

inAppMessagingManager.dispatch(InAppMessagingAction.LogEvent("message"))

Can we create some extension to reduce only for logging as it is used extensively? Something like this:

inAppMessagingManager.dispatchLogEvent("message")

For logging the code looks too heavy at the moment

This would go away, we would not need to log anymore, We are going to log each action and state afterward. This is just a manual action added to let the current logging come through.

Shahroz16 added 3 commits August 9, 2024 04:25

initial implementation

09b1644

improvements

0975c3e

updated version

995f0f1

Shahroz16 commented Aug 9, 2024

View reviewed changes

Shahroz16 requested a review from a team August 9, 2024 13:56

mrehan27 approved these changes Aug 9, 2024

View reviewed changes

Shahroz16 added 3 commits August 9, 2024 22:28

suggestions

3e9a725

remove tag

4c0ed82

remove usage

8b30804

Shahroz16 merged commit 927ba7b into main Aug 9, 2024
33 of 35 checks passed

Shahroz16 deleted the shahroz/logging-via-redux branch August 9, 2024 20:25

Shahroz16 added a commit that referenced this pull request Aug 9, 2024

chore: log states via redux (#421)

3ae7655

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: log states via redux #421

chore: log states via redux #421

Shahroz16 commented Aug 9, 2024

github-actions bot commented Aug 9, 2024 •

edited

Loading

Shahroz16 Aug 9, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024 •

edited

Loading

codecov bot commented Aug 9, 2024 •

edited

Loading

mrehan27 left a comment

mrehan27 Aug 9, 2024

Shahroz16 Aug 9, 2024

mrehan27 Aug 9, 2024

Shahroz16 Aug 9, 2024

mrehan27 Aug 9, 2024

mrehan27 Aug 9, 2024

mrehan27 Aug 9, 2024

Shahroz16 Aug 9, 2024

chore: log states via redux #421

chore: log states via redux #421

Conversation

Shahroz16 commented Aug 9, 2024

github-actions bot commented Aug 9, 2024 • edited Loading

Sample app builds 📱

Choose a reason for hiding this comment

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024 • edited Loading

📏 SDK Binary Size Comparison Report

codecov bot commented Aug 9, 2024 • edited Loading

Codecov Report

mrehan27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 9, 2024 •

edited

Loading

github-actions bot commented Aug 9, 2024 •

edited

Loading

codecov bot commented Aug 9, 2024 •

edited

Loading