-
Notifications
You must be signed in to change notification settings - Fork 55
Enhance SDK Integration: Auto ActivityLifecycleCallback & CI/CD Environment Support #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a new environment configuration file and updates documentation, integration instructions, and sample applications for CleverTap integration. The changes add new environment variables and methods to initialize the CleverTap SDK dynamically across Android, iOS, and Flutter. Additionally, several hard-coded configuration entries have been removed from native manifest and property files. Modifications in the plugin classes include lifecycle callback registration and error handling improvements when initializing CleverTap. Deprecated initialization methods are marked accordingly to encourage use of the new API. Changes
Sequence Diagram(s)sequenceDiagram
participant FlutterApp
participant PluginAPI
participant MethodChannel
participant NativePlatform
FlutterApp->>PluginAPI: Call initialize(accountId, token, region, targetDomain)
PluginAPI->>MethodChannel: Send "initialize" method call
MethodChannel-->>NativePlatform: Forward initialization call
alt Android Platform
NativePlatform->>DartToNativePlatformCommunicator: initializeCleverTap()
DartToNativePlatformCommunicator->>CleverTapApplication: Register lifecycle callback & set credentials
else iOS Platform
NativePlatform->>CleverTapPlugin: initializeWithConfig:withResult()
Note over CleverTapPlugin: Configure CleverTap instance with provided settings
end
NativePlatform-->>MethodChannel: Return initialization result
MethodChannel-->>PluginAPI: Forward result
PluginAPI-->>FlutterApp: Initialization complete
Possibly related PRs
Suggested reviewers
Poem
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
example/lib/main.dart (1)
133-134
: Consider deprecating this initialization methodThis line uses the older
init
method which appears to have been replaced by the newinitialize
method introduced in this PR. Consider adding a deprecation notice to encourage users to migrate to the new approach.- CleverTapPlugin.init("CLEVERTAP_ACCOUNT_ID", "CLEVERTAP_REGION", "CLEVERTAP_TARGET_DOMAIN"); + // Using deprecated method, consider migrating to CleverTapPlugin.initialize() + CleverTapPlugin.init("CLEVERTAP_ACCOUNT_ID", "CLEVERTAP_REGION", "CLEVERTAP_TARGET_DOMAIN");ios/Classes/CleverTapPlugin.m (1)
46-46
: Consider handling optional targetDomain parameterThe implementation handles the optional
region
parameter but doesn't handle thetargetDomain
parameter that's documented in the iOS integration guide and used in the main.dart example.if (region) { instanceConfig.regionCode = region; } + NSString *targetDomain = config[@"targetDomain"]; + if (targetDomain) { + instanceConfig.domain = targetDomain; + }README.md (2)
2-2
: Minor: Add alt text to image for accessibilityConsider adding alternative text to the image for improved accessibility.
- <img src="https://github.com/CleverTap/clevertap-ios-sdk/blob/master/docs/images/clevertap-logo.png" width="50%"/> + <img src="https://github.com/CleverTap/clevertap-ios-sdk/blob/master/docs/images/clevertap-logo.png" width="50%" alt="CleverTap Logo"/>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2-2: Images should have alternate text (alt text)
null(MD045, no-alt-text)
90-91
: Minor: Improve wording at the start of bullet pointsConsider changing "Checkout" to "Check out" for grammatical correctness.
- - Checkout our [CleverTap Flutter Usage documentation](doc/Usage.md). - - Checkout our [Example Dart project](./example). + - Check out our [CleverTap Flutter Usage documentation](doc/Usage.md). + - Check out our [Example Dart project](./example).🧰 Tools
🪛 LanguageTool
[grammar] ~90-~90: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...er data. ## Documentation & Example - Checkout our [CleverTap Flutter Usage documentat...(SENT_START_NN_DT)
[grammar] ~91-~91: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...r Usage documentation](doc/Usage.md). - Checkout our Example Dart project. ...(SENT_START_NN_DT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.env.example
(1 hunks)README.md
(2 hunks)android/src/main/java/com/clevertap/clevertap_plugin/CleverTapApplication.java
(1 hunks)android/src/main/java/com/clevertap/clevertap_plugin/DartToNativePlatformCommunicator.kt
(2 hunks)doc/Integrate-Android.md
(1 hunks)doc/Integrate-iOS.md
(1 hunks)example/android/app/src/main/AndroidManifest.xml
(0 hunks)example/ios/Runner/Info.plist
(0 hunks)example/lib/main.dart
(1 hunks)example/pubspec.yaml
(0 hunks)ios/Classes/CleverTapPlugin.m
(1 hunks)lib/clevertap_plugin.dart
(1 hunks)
💤 Files with no reviewable changes (3)
- example/ios/Runner/Info.plist
- example/android/app/src/main/AndroidManifest.xml
- example/pubspec.yaml
✅ Files skipped from review due to trivial changes (1)
- .env.example
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
2-2: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
48-48: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 LanguageTool
README.md
[grammar] ~83-~83: A determiner may be missing.
Context: ...ng them in configuration files. - Use least privilege access policies for CI/CD too...
(THE_SUPERLATIVE)
[grammar] ~90-~90: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...er data. ## Documentation & Example - Checkout our [CleverTap Flutter Usage documentat...
(SENT_START_NN_DT)
[grammar] ~91-~91: This sentence should probably be started with a verb instead of the noun ‘Checkout’. If not, consider inserting a comma for better clarity.
Context: ...r Usage documentation](doc/Usage.md). - Checkout our Example Dart project. ...
(SENT_START_NN_DT)
🔇 Additional comments (11)
android/src/main/java/com/clevertap/clevertap_plugin/CleverTapApplication.java (1)
19-19
: Great automation improvement!Automatically registering the ActivityLifecycleCallback eliminates the need for manual registration in the developer workflow, making integration more seamless. This change aligns perfectly with the PR's automation objectives.
doc/Integrate-iOS.md (1)
5-15
: Well-structured credential configuration optionsThe documentation now provides multiple options for credential configuration, enhancing flexibility for developers. The programmatic initialization using
CleverTapPlugin.initialize()
provides a cleaner approach that works well with CI/CD environments.example/lib/main.dart (1)
76-82
: Good example of the new initialization methodThis example clearly demonstrates how to use the new programmatic initialization approach with both required and optional parameters. The comment appropriately indicates this is optional.
ios/Classes/CleverTapPlugin.m (1)
27-48
:❓ Verification inconclusive
Good implementation of iOS SDK initialization
This implementation properly handles the new initialization method with appropriate error handling for required parameters. The code is well-structured and validates inputs before proceeding with initialization.
However, you should add a case for this new method in the
handleMethodCall:result:
method, otherwise it won't be callable from Flutter.
🏁 Script executed:
#!/bin/bash # Check if the method is registered in the handleMethodCall method grep -A5 -B5 "initializeWithConfig:" ios/Classes/CleverTapPlugin.m | grep "handleMethodCall"Length of output: 90
Action Required: Register the New Method in
handleMethodCall:
The implementation for the new iOS SDK initialization is excellent—with proper parameter validation and error handling. However, our verification did not reveal that the new method is registered in the
handleMethodCall:
dispatcher. Please ensure you add a case in thehandleMethodCall:result:
method so that theinitializeWithConfig:
call is properly routed from Flutter.For example, you might add something similar to the following in
ios/Classes/CleverTapPlugin.m
:if ([call.method isEqualToString:@"initializeWithConfig"]) { [self initializeWithConfig:call withResult:result]; return; }After adding this, please verify that Flutter can call this new method as expected.
lib/clevertap_plugin.dart (2)
382-401
: Well-implemented initialization method with comprehensive parameter handlingThe new
initialize
method is a good addition that provides a more flexible way to initialize the CleverTap SDK with account credentials directly from code. This aligns well with the PR objectives to support CI/CD environments through environment variables.The implementation correctly handles both web and non-web platforms, and properly validates required parameters. This will make it easier for developers to integrate CleverTap in CI/CD pipelines where environment variables are commonly used.
404-405
: Appropriate deprecation of the legacy methodGood practice to mark the old method as deprecated while maintaining backward compatibility. The deprecation notice clearly directs users to the new
initialize()
method.doc/Integrate-Android.md (1)
80-98
: Documentation properly updated to reflect new initialization optionsThe documentation now clearly presents two options for initialization, maintaining backward compatibility while introducing the new approach. This aligns perfectly with the PR objective of supporting environment variables for CI/CD pipelines.
The code example is clear and includes all parameters (both required and optional). This will help developers understand how to use the new initialization method.
android/src/main/java/com/clevertap/clevertap_plugin/DartToNativePlatformCommunicator.kt (2)
78-80
: Clean integration of new initialization caseThe addition of the "initialize" case in the method call handler is clean and follows the existing code pattern.
2094-2115
: Well-implemented native initialization method with proper error handlingThis implementation properly:
- Extracts parameters from the method call
- Validates required parameters
- Applies configuration to the CleverTap instance
- Handles exceptions and provides meaningful error messages
The method also correctly differentiates between required and optional parameters, making it robust for various initialization scenarios.
README.md (2)
35-63
: Comprehensive documentation of initialization optionsThe README now clearly documents all three initialization methods with code examples and explanations. This significantly improves the developer experience, especially for those integrating with CI/CD pipelines.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
48-48: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
67-86
: Valuable security measures section addedThe new security measures section provides essential guidance for securely managing credentials, which is particularly important when using environment variables in CI/CD pipelines. The section covers best practices for environment variable management, credential handling, and CI/CD pipeline security.
This is a valuable addition that enhances the security posture of applications using the SDK.
🧰 Tools
🪛 LanguageTool
[grammar] ~83-~83: A determiner may be missing.
Context: ...ng them in configuration files. - Use least privilege access policies for CI/CD too...(THE_SUPERLATIVE)
feat: Deprecate old initialization method in example and provide new usage instructions fix: Add targetDomain configuration in CleverTapPlugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
42-53
: Environment Variables Setup:
The added instructions for setting up environment variables enhance CI/CD support and overall security.
Note: For the code fence starting at line 48, consider specifying a language (e.g.,dotenv
) to improve syntax highlighting and readability in supported editors.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
48-48: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
83-83
: Grammar Suggestion:
Consider revising the sentence at line 83 to include a determiner for clarity. For example: "Use the least privilege access policies for CI/CD tools interacting with CleverTap."🧰 Tools
🪛 LanguageTool
[grammar] ~83-~83: A determiner may be missing.
Context: ...ng them in configuration files. - Use least privilege access policies for CI/CD too...(THE_SUPERLATIVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(2 hunks)example/lib/main.dart
(2 hunks)ios/Classes/CleverTapPlugin.m
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- example/lib/main.dart
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~11-~11: Possible missing preposition found.
Context: ...p-flutter.svg" /> ## Introduction The CleverTap Flutter SDK for Mobile Custom...
(AI_HYDRA_LEO_MISSING_OF)
[grammar] ~83-~83: A determiner may be missing.
Context: ...ng them in configuration files. - Use least privilege access policies for CI/CD too...
(THE_SUPERLATIVE)
🪛 markdownlint-cli2 (0.17.2)
README.md
48-48: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (17)
ios/Classes/CleverTapPlugin.m (1)
27-53
: Good implementation of dynamic configuration for CI/CD environment support.This method provides a clean way to programmatically initialize CleverTap with configuration parameters that can be supplied from environment variables, supporting the PR objective of enabling CI/CD pipeline integration. The error handling for required parameters is correctly implemented.
The implementation properly extracts configuration parameters (accountId, token, region, targetDomain) from the method call and validates that the required parameters are present before initializing CleverTap. The code also supports optional parameters like region and targetDomain, providing flexibility in configuration.
README.md (16)
2-2
: Accessibility Enhancement:
The image tag now includes analt
attribute, which improves accessibility and meets best practices.
11-11
: Header Update:
The "Introduction" header has been simplified by removing the emoji, resulting in a cleaner and more professional look.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing preposition found.
Context: ...p-flutter.svg" /> ## Introduction The CleverTap Flutter SDK for Mobile Custom...(AI_HYDRA_LEO_MISSING_OF)
14-14
: Content Clarity Improvement:
The revised sentence in the introduction now reads more clearly and includes proper punctuation for better readability.
18-18
: Header Simplification:
Changing the header to "Installation and Quick Start" (without emojis) improves clarity and ensures consistency across documentation.
24-24
: Dependency Version Confirmation:
The dependency forclevertap_plugin
is explicitly set to version 3.2.0. Please verify that this version aligns with the intended release and that no compatibility issues exist.
27-27
: Installation Instruction Punctuation:
The instruction now clearly ends with a period, enhancing the professional appearance of the installation steps.
35-37
: SDK Initialization Section:
The new section introducing SDK initialization is clear and well-structured, offering multiple configuration options that cater to different developer needs.
39-40
: Platform-specific Configuration Guidance:
The instructions for using platform-specific configuration files are clear. Ensure that the referenced technical documentation (Android and iOS) is kept up-to-date with these changes.
55-63
: Programmatic Initialization Clarity:
The code snippet for initializing CleverTap programmatically is well-formatted and clearly demonstrates how to pass both required and optional parameters.
65-65
: Cross-Platform Documentation Reference:
Maintaining links to both Android and iOS technical documentation reinforces the integration guidance. Ensure that these links remain valid over time.
67-67
: Introduction of Security Measures Section:
Adding a dedicated "Security Measures" section is a great enhancement, emphasizing the importance of safeguarding credentials and sensitive application data.
69-86
: Comprehensive Security Guidelines:
The expanded security practices—including environment variable management, secure credential handling, and CI/CD pipeline security—provide clear and actionable guidance. This upgrade helps reinforce best practices among developers.🧰 Tools
🪛 LanguageTool
[grammar] ~83-~83: A determiner may be missing.
Context: ...ng them in configuration files. - Use least privilege access policies for CI/CD too...(THE_SUPERLATIVE)
88-91
: Documentation & Example Section Restructure:
The restructured "Documentation & Example" section improves the flow and organization of the README, making it easier for developers to find relevant resources.
93-97
: CleverTap Push Templates SDK Section:
The update to this section clearly delineates information about the CleverTap Push Templates SDK for Android, aiding developers in understanding its purpose and integration steps.
98-100
: Changelog Section Formatting:
Moving and reformatting the Changelog section contributes to a more consistent document structure. This change improves overall readability and maintainability.
102-104
: Questions Section Clarity:
The "Questions" section succinctly directs users on how to get support, ensuring that assistance is readily accessible.
Changelog
1. Automation Enhancements
CleverTapApplication.java
to automatically registerActivityLifecycleCallback
.2. CI/CD Pipeline Support
CLEVERTAP_ACCOUNT_ID
CLEVERTAP_ACCOUNT_TOKEN
CLEVERTAP_ACCOUNT_REGION
(optional)CLEVERTAP_HANDSHAKE_DOMAIN
(optional)3. Security and Documentation Improvements
Summary by CodeRabbit
New Features
Documentation
Chores