task(SDK-4057) - Revamped Timer Notification Template #682

Anush-Shand · 2024-10-30T11:42:39Z

https://wizrocket.atlassian.net/browse/SDK-4057

The idea is such that the timer notification shown will be the compulsory FGS notification
The existing implementation has been used to prepare the notification builder for the timer template
If the service is not declared in the manifest, then the older approach is used to render the timer notification

- Refactors the code in PushProviders.java to make it more modular - Adds a service which will now show the timer template

- Adds logs

…tions

CTLalit · 2025-01-16T06:50:43Z

clevertap-core/src/main/AndroidManifest.xml

+    <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
+    <uses-permission android:name="android.permission.FOREGROUND_SERVICE_REMOTE_MESSAGING"/>


we need to ask clients to put this permission as it might have some compliances on playstore.

I agree with @CTLalit - this should be mentioned in the docs and needs to be optional. All the code related to FGS should be with a permission check with all cases handled properly

Done
Most SDK's are using these permissions and they are not as critical as like an AlarmManager, it was merged by default in the final manifest

Moving it to the app to be safe

Anush-Shand · 2025-01-17T07:56:53Z

@darshanclevertap @piyush-kukadiya @CTLalit
Another caveat here is that Android enforces that a FGS must call startForeground within 5s https://stackoverflow.com/questions/44425584/context-startforegroundservice-did-not-then-call-service-startforeground/72754189#72754189

Image download can take time for a notification and hence this might lead to a crash

Options

Call startForeground with a placeholder notification and update/dismiss it after notification builder for timer template has been generated
Timebound notification builder for timer template to 4s. If it fails, skip the notification, call startForeground() and stopSelf()

darshanclevertap · 2025-01-17T08:10:34Z

@Anush-Shand - My suggestion is to go with option 1 - Call startForeground with a placeholder notification and update/dismiss it after the notification builder for the timer template has been generated.

Placeholders can have the title & message while the image & timer loads. Once the image loads, we have the notification ID to update the notification. If you could share a demo video of this on Slack, it would help.

Option 2 is risky because if some delay happens more than 4s, the app will crash. See the 2nd comment here -> https://stackoverflow.com/a/45047542/8301966

CTLalit · 2025-01-20T06:17:01Z

clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java

+                                                                Bundle extras) {
+        CleverTapInstanceConfig config = coreState.getConfig();
+        try {
+            synchronized (coreState.getPushProviders().getPushRenderingLock()) {


avoid locking if there is only one push provider available?

coderabbitai · 2025-01-20T06:17:54Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

CTLalit · 2025-01-20T06:18:01Z

clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java

+        try {
+            synchronized (coreState.getPushProviders().getPushRenderingLock()) {
+                config.getLogger().verbose(config.getAccountId(),
+                        "returning push on caller thread with id = " + Thread.currentThread().getId());


maybe print if its main thread.

CTLalit · 2025-01-20T06:21:06Z

clevertap-core/src/main/java/com/clevertap/android/sdk/pushnotification/PushProviders.java

+            if (nb != null) {
+                return nb;
+            }


you are masking the logic behind a nullable NotificationBuilder return method, ideally check from data itself if we should short circuit.

CTLalit · 2025-01-20T06:25:02Z

clevertap-core/src/main/java/com/clevertap/android/sdk/pushnotification/PushProviders.java

+                baseDatabaseManager.loadDBAdapter(context)
+                        .storeUninstallTimestamp();


this might cause lag on main thread, confirm that is not happening.

CTLalit · 2025-01-20T06:25:35Z

clevertap-core/src/main/java/com/clevertap/android/sdk/pushnotification/PushProviders.java

+                        .storeUninstallTimestamp();
+                String pingFreq = extras.getString("pf", "");
+                if (!TextUtils.isEmpty(pingFreq)) {
+                    updatePingFrequencyIfNeeded(context, Integer.parseInt(pingFreq));


Integer.parseInt can throw, make sure it is handled.

CTLalit · 2025-01-20T06:33:23Z

...lates/src/main/java/com/clevertap/android/pushtemplates/PushTemplateNotificationHandler.java

+        return ManifestValidator.isComponentPresentInManifest(applicationContext, "com.clevertap.android.pushtemplates.TimerTemplateService", ManifestValidator.ComponentType.SERVICE)
+                && templateType == TemplateType.TIMER
+                && areRevampedTimerTemplatePermissionsGranted(applicationContext);


templateType == TemplateType.TIMER can be first condition so short circuit happens without much computation

CTLalit · 2025-01-20T06:36:50Z

clevertap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/TemplateRenderer.kt

+                }
+
+                val intent = Intent(context, TimerTemplateService::class.java)
+                context.stopService(intent)
            }, (delay - 100).toLong())


lets see if we can solve for not using such a code in this task.

CTLalit · 2025-01-20T06:40:41Z

...rtap-pushtemplates/src/main/java/com/clevertap/android/pushtemplates/TimerTemplateService.kt

+        val cleverTapAPI = CleverTapAPI.getGlobalInstance(
+            this@TimerTemplateService,
+            PushNotificationUtil.getAccountIdFromNotificationBundle(message)
+        )
+        val config: CleverTapInstanceConfig? = cleverTapAPI?.coreState?.config


can we run this code inside the task itself?

CTLalit · 2025-01-20T06:41:21Z

sample/src/main/AndroidManifest.xml

        <meta-data
            android:name="CLEVERTAP_ACCOUNT_ID"
-            android:value="YOUR ACCT ID" />
+            android:value="TEST-4RW-944-876Z" />
        <meta-data
            android:name="CLEVERTAP_TOKEN"
-            android:value="YOUR ACCT TOKEN" />
+            android:value="TEST-44c-024" />
        <meta-data
            android:name="CLEVERTAP_REGION"
-            android:value="YOUR ACCT REGION"/>
+            android:value="eu1" />


delete these details

Anush-Shand and others added 14 commits October 30, 2024 17:08

task(SDK-4057) - Revamped Timer Notification Template

bd10279

- Refactors the code in PushProviders.java to make it more modular - Adds a service which will now show the timer template

task(SDK-4057) - Fixes name of service

3424ab9

task(SDK-4057) - Fixes a log

de2d3eb

task(SDK-4057) - Fixes TimerTemplateService

c5cae07

task(SDK-4057) - Refactors ManifestValidator

1dc7c41

task(SDK-4057) - Removes redundant imports

91d462e

task(SDK-4057) - More refactoring of TimerTemplateService

5e2b8ac

task(SDK-4057) - More refactoring

7eecabc

- Adds logs

task(SDK-4057) - Adds logs and fixes warnings

84aa6b6

task(SDK-4057) - Improves logs

3a18744

Rename .java to .kt

7a9d8ea

task(SDK-4057) - Convert ManifestValidator to kotlin

65974f5

task(SDK-4057) - Refactors PushProviders.java

06e8970

Merge branch 'develop' into task/SDK-4057/fg_service_timer_template

3741aaf

Anush-Shand marked this pull request as ready for review November 12, 2024 09:02

Anush-Shand and others added 7 commits November 12, 2024 16:06

task(SDK-4057) - Adds tests for ManifestValidator

2e91fb8

Merge branch 'develop' into task/SDK-4057/fg_service_timer_template

721143e

task(SDK-4057) - Refactors storePushNotification into 2 separate func…

a4e6d27

…tions

task(SDK-4057) - Adds a log to help debugging

2e0ff59

task(SDK-4057) - Improves log

0ce9afe

task(SDK-4057) - Stops service even if notif not in tray

7660295

task(SDK-4057) - Stops service later

6d8f170

CTLalit reviewed Jan 16, 2025

View reviewed changes

task(SDK-4057) - Moves permissions to app's manifest instead of the SDK

fd878bf

CTLalit reviewed Jan 20, 2025

View reviewed changes

task(SDK-4057) - Reverts formatting

d8bda66

CTLalit reviewed Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

task(SDK-4057) - Revamped Timer Notification Template #682

task(SDK-4057) - Revamped Timer Notification Template #682

Anush-Shand commented Oct 30, 2024

CTLalit Jan 16, 2025

darshanclevertap Jan 16, 2025

Anush-Shand Jan 17, 2025

Anush-Shand commented Jan 17, 2025

darshanclevertap commented Jan 17, 2025

CTLalit Jan 20, 2025

coderabbitai bot commented Jan 20, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

CTLalit Jan 20, 2025

		<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
		<uses-permission android:name="android.permission.FOREGROUND_SERVICE_REMOTE_MESSAGING"/>

		baseDatabaseManager.loadDBAdapter(context)
		.storeUninstallTimestamp();

task(SDK-4057) - Revamped Timer Notification Template #682

Are you sure you want to change the base?

task(SDK-4057) - Revamped Timer Notification Template #682

Conversation

Anush-Shand commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anush-Shand commented Jan 17, 2025

darshanclevertap commented Jan 17, 2025

Choose a reason for hiding this comment

coderabbitai bot commented Jan 20, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

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

CodeRabbit Configuration File (`.coderabbit.yaml`)