Android SDK release 7.2.2 #739
Conversation
* tests(SDK-4215) - Adds UTs for CryptMigrator * tests(SDK-4215) - Fixes deps versions
…d clicked on push notification SDK-4306
fix(inapps) - fix didDismiss() not called when in-app is displayed an…
…cked_not_raised task(SDK-4303) - Fixes notification clicked event not getting raised
WalkthroughThe pull request introduces version 7.2.2 of the CleverTap Android SDK, addressing a critical issue with notification clicked events. The changes span multiple files, including the changelog, documentation, and core SDK components. The update involves modifying version numbers, improving notification event handling, and adding a new test class for encryption migration. The primary focus is on fixing a bug where notification clicked events were not being raised correctly. Changes
Sequence DiagramsequenceDiagram
participant App
participant AnalyticsManager
participant AnalyticsManagerBundler
App->>AnalyticsManager: pushNotificationClickedEvent
AnalyticsManager->>AnalyticsManagerBundler: notificationClickedJson
AnalyticsManagerBundler-->>AnalyticsManager: JSONObject
AnalyticsManager->>App: Process Notification Clicked Event
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
clevertap-core/src/main/java/com/clevertap/android/sdk/InAppNotificationActivity.java (1)
269-285: Consider adding synchronization for thread safety.While the
invokedCallbacksflag prevents duplicate callbacks, the check-then-set operation is not atomic. In a multi-threaded environment, this could lead to race conditions where multiple threads might execute the callback block simultaneously.Consider adding synchronization:
- void didDismiss(Bundle data, boolean killActivity) { + synchronized void didDismiss(Bundle data, boolean killActivity) { if (isAlertVisible) { isAlertVisible = false; }clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManagerBundler.kt (1)
43-54: LGTM! Consider refactoring to reduce code duplication.The implementation looks correct. However, since
notificationClickedJsonandnotificationViewedJsonshare the same structure, consider extracting the common logic into a private method to reduce duplication.@JvmStatic private fun createNotificationJson(root: Bundle, eventName: String): JSONObject { val event = JSONObject() try { val notif = wzrkBundleToJson(root) event.put("evtName", eventName) event.put("evtData", notif) } catch (ignored: Throwable) { //no-op } return event } @JvmStatic fun notificationViewedJson(root: Bundle): JSONObject = createNotificationJson(root, Constants.NOTIFICATION_VIEWED_EVENT_NAME) @JvmStatic fun notificationClickedJson(root: Bundle): JSONObject = createNotificationJson(root, Constants.NOTIFICATION_CLICKED_EVENT_NAME)clevertap-core/src/test/java/com/clevertap/android/sdk/cryption/CryptMigratorTest.kt (1)
52-90: Consider adding edge cases for encryption level validation.The migration tests cover the basic scenarios well. Consider adding test cases for:
- Invalid/unsupported encryption levels
- Null encryption levels
- Migration with empty/corrupted data
clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java (1)
1076-1076: Consider using structured logging.The verbose logging messages could benefit from using structured logging with key-value pairs for better parsing and analysis.
-config.getLogger().verbose(config.getAccountId(), "Constructed custom profile: " + customProfile); +config.getLogger().verbose(config.getAccountId(), "Constructed custom profile - profile={}", customProfile); -config.getLogger().verbose(config.getAccountId(), "Constructed multi-value profile push: " + fields); +config.getLogger().verbose(config.getAccountId(), "Constructed multi-value profile push - fields={}", fields);Also applies to: 1143-1143
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.md(1 hunks)README.md(2 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java(3 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManagerBundler.kt(1 hunks)clevertap-core/src/main/java/com/clevertap/android/sdk/InAppNotificationActivity.java(3 hunks)clevertap-core/src/test/java/com/clevertap/android/sdk/AnalyticsManagerTest.kt(5 hunks)clevertap-core/src/test/java/com/clevertap/android/sdk/cryption/CryptMigratorTest.kt(1 hunks)docs/CTCORECHANGELOG.md(3 hunks)docs/CTGEOFENCE.md(1 hunks)docs/CTPUSHTEMPLATES.md(1 hunks)gradle/libs.versions.toml(1 hunks)sample/build.gradle(2 hunks)templates/CTCORECHANGELOG.md(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/CTPUSHTEMPLATES.md
🧰 Additional context used
🪛 LanguageTool
docs/CTCORECHANGELOG.md
[duplication] ~7-~7: Possible typo: you repeated a word.
Context: ... issue from v7.1.0 onwards: #### Bug Fixes * Fixes an issue where Notification Clicked e...
(ENGLISH_WORD_REPEAT_RULE)
templates/CTCORECHANGELOG.md
[duplication] ~7-~7: Possible typo: you repeated a word.
Context: ... issue from v7.1.0 onwards: #### Bug Fixes * Fixes an issue where Notification Clicked e...
(ENGLISH_WORD_REPEAT_RULE)
CHANGELOG.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ## CHANGE LOG. ### January 21, 2025 * [CleverTap Android SDK v7.2.2](docs/C...
(MISSING_COMMA_AFTER_YEAR)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-static_checks-test-build
🔇 Additional comments (19)
clevertap-core/src/main/java/com/clevertap/android/sdk/InAppNotificationActivity.java (3)
57-58: LGTM! Good addition to prevent duplicate callbacks.The
invokedCallbacksflag is a good practice to ensure dismissal callbacks are only invoked once.
465-471: LGTM! Good cleanup in onDestroy.The implementation properly handles configuration changes and ensures cleanup. The
super.onDestroy()call is correctly placed at the beginning of the method.
269-285: Verify impact on notification dismissal behavior.The changes to
didDismissmodify when the activity is finished. Let's verify that all callers handle this correctly.✅ Verification successful
Changes to didDismiss are safely implemented
The new behavior maintains compatibility with existing callers while adding flexibility for activity lifecycle control. All current usages either rely on the default finishing behavior or explicitly manage the activity lifecycle.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct calls to didDismiss to ensure they're updated rg -l "didDismiss\(" | while read -r file; do echo "=== $file ===" rg -A 2 "didDismiss\(" "$file" done # Search for potential custom implementations that might expect the old behavior ast-grep --pattern 'class $_ extends InAppNotificationActivity { $$$ }'Length of output: 8217
clevertap-core/src/test/java/com/clevertap/android/sdk/cryption/CryptMigratorTest.kt (3)
11-54: LGTM! Well-structured test setup with comprehensive mocking.The test class is well-organized with proper initialization of mocks and test subjects. The setup creates two instances of
CryptMigratorwith different encryption levels, which is good for testing various migration scenarios.
106-232: LGTM! Thorough testing of cached GUID migration.The test cases for cached GUID migration are comprehensive, covering:
- Encryption level changes (none to medium, medium to none)
- Encryption/decryption failures
- Data format validation
287-492: LGTM! Comprehensive testing of database profile migration.The test cases for database profile migration are thorough, covering:
- Encryption level changes
- Field-level encryption
- Error handling
- Data integrity validation
clevertap-core/src/test/java/com/clevertap/android/sdk/AnalyticsManagerTest.kt (1)
229-229: LGTM! Test cases updated correctly for notification clicked events.The test cases have been properly updated to use
notificationClickedJsoninstead ofnotificationViewedJson, maintaining consistency with the changes inAnalyticsManagerBundler.Also applies to: 267-267, 358-358
clevertap-core/src/main/java/com/clevertap/android/sdk/AnalyticsManager.java (2)
497-498: LGTM! Improved logging clarity.The logging message now uses the actual extras object instead of toString(), providing better visibility into the notification data.
504-504: LGTM! Correct event type for notification clicks.The implementation now correctly uses
notificationClickedJsonfor clicked events instead ofnotificationViewedJson.sample/build.gradle (2)
21-22: Version updates look goodVersion code and name are updated consistently following semantic versioning.
162-165: Dependency versions are updated consistentlyAll CleverTap SDK dependencies are updated to their latest compatible versions:
- clevertap-android-sdk: 7.2.2
- clevertap-geofence-sdk: 1.4.0
- push-templates: 1.3.0
- clevertap-hms-sdk: 1.4.0
Also applies to: 167-170
gradle/libs.versions.toml (1)
57-57: Version catalog update looks goodThe CleverTap Android SDK version is updated consistently in the version catalog.
docs/CTGEOFENCE.md (1)
20-20: Documentation version update looks goodThe CleverTap Android SDK version reference is updated consistently in the documentation.
README.md (1)
29-29: README version updates look goodAll references to the CleverTap Android SDK version are updated consistently throughout the README.
Also applies to: 37-37, 49-49
docs/CTCORECHANGELOG.md (2)
3-9: Changelog entry is well documentedThe changelog entry for version 7.2.2 clearly describes the critical bug fix for the notification clicked event issue.
🧰 Tools
🪛 LanguageTool
[duplication] ~7-~7: Possible typo: you repeated a word.
Context: ... issue fromv7.1.0onwards: #### Bug Fixes * Fixes an issue whereNotification Clickede...(ENGLISH_WORD_REPEAT_RULE)
12-13: Advisory notes are helpfulThe advisory notes for previous versions (7.2.1, 7.2.0, 7.1.0) appropriately guide users to upgrade to version 7.2.2 to address the critical issue.
Also applies to: 23-23, 35-36
templates/CTCORECHANGELOG.md (2)
3-9: LGTM! Clear version entry with proper documentation.The version entry follows the changelog format consistently and clearly documents the critical fix.
🧰 Tools
🪛 LanguageTool
[duplication] ~7-~7: Possible typo: you repeated a word.
Context: ... issue fromv7.1.0onwards: #### Bug Fixes * Fixes an issue whereNotification Clickede...(ENGLISH_WORD_REPEAT_RULE)
12-13: LGTM! Appropriate warning notes for previous versions.The warning notes are properly added to alert users about the critical issue and provide clear upgrade guidance.
Also applies to: 23-23
CHANGELOG.md (1)
3-5: LGTM! Consistent changelog entry.The entry maintains the established format and properly links to the detailed changelog.
🧰 Tools
🪛 LanguageTool
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ## CHANGE LOG. ### January 21, 2025 * [CleverTap Android SDK v7.2.2](docs/C...(MISSING_COMMA_AFTER_YEAR)
https://wizrocket.atlassian.net/browse/SDK-4303
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation