Repository navigation
Conversation
…alse When autoHandleLandingURL was set to false, tapping a notification would only track the event but not open the app — the transparent NotificationTappedActivity would finish() without starting any activity. On iOS, the OS brings the app to the foreground on notification tap, so autoHandleLandingURL only controls deep link navigation. On Android, the SDK must explicitly launch the app, which was gated behind the same flag. Now, when autoHandleLandingURL is false, the SDK skips deep link navigation but still opens the app via getLaunchIntentForPackage(). Fixes CLIX-176
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughNotification tap handling now launches the app when automatic landing-URL navigation is disabled; added an Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ace6c56196
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ClixLogger.debug( | ||
| "Auto open disabled, skipping landing navigation for ${payload.messageId}" | ||
| ) | ||
| openApp(context) |
There was a problem hiding this comment.
When autoHandleLandingURL is disabled, this new unconditional openApp(context) runs after onNotificationOpened executes, which can override app-defined routing. The README’s custom-handling flow relies on opening a destination from the callback, but starting the launcher immediately afterward can bring the main activity to the top and hide the screen the handler just opened. This regresses the primary autoHandleLandingURL = false use case for apps that implement manual deep-link navigation in onNotificationOpened.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clix/src/main/kotlin/so/clix/core/ClixNotification.kt (1)
304-312:⚠️ Potential issue | 🟠 MajorFallback to app launch when landing navigation fails.
When auto-open is enabled,
openLandingURLIfPresent(...)can returnfalse, but that result is ignored. In that case, notification tap can still end without opening the app.🔧 Proposed fix
- val shouldAutoOpen = autoHandleLandingURL && notificationContext.autoOpenLandingURL - if (shouldAutoOpen) { - openLandingURLIfPresent(context, payload.landingUrl) - } else { - ClixLogger.debug( - "Auto open disabled, skipping landing navigation for ${payload.messageId}" - ) - openApp(context) - } + val shouldAutoOpen = autoHandleLandingURL && notificationContext.autoOpenLandingURL + val openedLanding = shouldAutoOpen && openLandingURLIfPresent(context, payload.landingUrl) + if (!openedLanding) { + if (!shouldAutoOpen) { + ClixLogger.debug( + "Auto open disabled, skipping landing navigation for ${payload.messageId}" + ) + } else { + ClixLogger.warn( + "Landing navigation failed, falling back to app launch for ${payload.messageId}" + ) + } + openApp(context) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clix/src/main/kotlin/so/clix/core/ClixNotification.kt` around lines 304 - 312, The code currently calls openLandingURLIfPresent(context, payload.landingUrl) when shouldAutoOpen is true but ignores its Boolean result; if it returns false the app isn't opened. Change the branch so that when shouldAutoOpen is true you capture the result of openLandingURLIfPresent(...) and, if it returns false, call openApp(context) as a fallback; keep the existing else branch for the disabled case and preserve the ClixLogger.debug message when auto-open is disabled. Ensure you reference openLandingURLIfPresent, openApp, shouldAutoOpen, payload.landingUrl, and notificationContext.autoOpenLandingURL when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@clix/src/main/kotlin/so/clix/core/ClixNotification.kt`:
- Around line 304-312: The code currently calls openLandingURLIfPresent(context,
payload.landingUrl) when shouldAutoOpen is true but ignores its Boolean result;
if it returns false the app isn't opened. Change the branch so that when
shouldAutoOpen is true you capture the result of openLandingURLIfPresent(...)
and, if it returns false, call openApp(context) as a fallback; keep the existing
else branch for the disabled case and preserve the ClixLogger.debug message when
auto-open is disabled. Ensure you reference openLandingURLIfPresent, openApp,
shouldAutoOpen, payload.landingUrl, and notificationContext.autoOpenLandingURL
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 61997e2e-0162-4014-873a-c4c34b9754a0
📒 Files selected for processing (1)
clix/src/main/kotlin/so/clix/core/ClixNotification.kt
Avoid launching the main activity after a custom tap handler has already run, which could override app-defined routing.
1 participant
Summary
autoHandleLandingURL = false일 때 알림을 탭하면 앱이 열리지 않는 문제 수정autoHandleLandingURL플래그에 같이 묶여 있어서false일 때 앱 자체가 열리지 않았음autoHandleLandingURL에 의해 제어되고, 앱 열기는 항상 동작하도록 변경Test plan
autoHandleLandingURL = false로 설정 후 알림 탭 → 앱이 열리는지 확인autoHandleLandingURL = true+ landing URL 있음 → 기존대로 deep link 네비게이션 동작 확인autoHandleLandingURL = true+ landing URL 없음 → 앱 메인 화면 열림 확인autoHandleLandingURL = false+onNotificationOpened핸들러 등록 → 핸들러 호출 + 앱 열림 확인Fixes CLIX-176
Summary by CodeRabbit
Bug Fixes
Chores