refactor: simplify Android widget deep linking by directly launching …#609
refactor: simplify Android widget deep linking by directly launching …#609chan9an wants to merge 3 commits intoCCExtractor:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors the Android widget to a ListView/RemoteViewsFactory model, adds a Task data model and theme-aware layout selection, replaces per-item broadcasts with fill-in intents, and reworks deep-link initialization and controller lifecycle to queue and consume URIs safely across cold starts. Changes
Sequence Diagram(s)sequenceDiagram
participant OS as Android OS
participant Widget as WidgetProvider
participant Main as App main
participant DLS as DeepLinkService
participant Splash as SplashController
participant Home as HomeController
participant UI as App UI
Main->>DLS: Get.putAsync(DeepLinkService).init()
activate DLS
DLS->>OS: getInitialLink() / register uriLinkStream
alt initial link present
DLS->>DLS: store queuedUri (String)
end
DLS-->>Main: initialized service returned
deactivate DLS
Main->>Splash: runApp() -> Splash.onInit()/onReady()
Splash->>DLS: check queuedUri
alt queuedUri exists
Splash->>Home: navigate with deep link
DLS->>DLS: clear queuedUri
else
Splash->>Splash: proceed normal flow
end
OS->>Widget: Widget click -> PendingIntent
Widget->>DLS: deliver intent with deep link payload
alt HomeController registered
DLS->>Home: deliver URI
Home->>UI: perform routing after frame callback
else
DLS->>DLS: queue URI for later consumption
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/main.dart (1)
30-38:⚠️ Potential issue | 🟡 MinorBoot log on line 30 won't be captured to the database.
The
debugPrintoverride that logs to_logDatabaseHelperis defined after the firstdebugPrint("🚀 BOOT: main() started")call on line 30. This initial boot message will only go to the console, not the database. If database logging of boot sequence is important, move the override before line 30.Suggested fix
void main() async { + WidgetsFlutterBinding.ensureInitialized(); + + debugPrint = (String? message, {int? wrapWidth}) { + if (message != null) { + debugPrintSynchronously(message, wrapWidth: wrapWidth); + _logDatabaseHelper.insertLog(message); + } + }; + debugPrint("🚀 BOOT: main() started"); - WidgetsFlutterBinding.ensureInitialized(); - - debugPrint = (String? message, {int? wrapWidth}) { - if (message != null) { - debugPrintSynchronously(message, wrapWidth: wrapWidth); - _logDatabaseHelper.insertLog(message); - } - };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 30 - 38, The boot log isn't captured because the debugPrint override is set after the initial debugPrint("🚀 BOOT: main() started") call; move the override of debugPrint (the lambda using debugPrintSynchronously and _logDatabaseHelper.insertLog) to run before that first debugPrint call in main() so the initial boot message is recorded to the database—ensure the override still calls debugPrintSynchronously and _logDatabaseHelper.insertLog exactly as implemented.
🧹 Nitpick comments (7)
lib/app/services/deep_link_service.dart (2)
25-28: Stream subscription is never cancelled.The
uriLinkStream.listen()subscription is created but not stored or cancelled. WhileGetxServiceinstances are typically permanent, if this service is ever disposed or re-initialized, the subscription will leak. Consider storing and cancelling it inonClose().Store and cancel subscription
class DeepLinkService extends GetxService { late AppLinks _appLinks; String? queuedUri; + StreamSubscription<Uri>? _linkSubscription; Future<void> init() async { _appLinks = AppLinks(); // ... existing code ... - _appLinks.uriLinkStream.listen((uri) { + _linkSubscription = _appLinks.uriLinkStream.listen((uri) { debugPrint('🔗 LINK RECEIVED: $uri'); _handleWidgetUri(uri); }); } + + `@override` + void onClose() { + _linkSubscription?.cancel(); + super.onClose(); + }Add
import 'dart:async';at the top.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/app/services/deep_link_service.dart` around lines 25 - 28, Store the StreamSubscription returned by _appLinks.uriLinkStream.listen(...) (instead of discarding it) by adding an import 'dart:async'; and a field like StreamSubscription? _uriSub in the DeepLinkService, use that subscription to call _handleWidgetUri in the listener, and override onClose() to call _uriSub?.cancel() (and null it) to ensure the subscription is cancelled when the service is disposed.
10-10: Consider keepingqueuedUriprivate or using a getter.Exposing
queuedUrias a public mutable field allows external code to accidentally overwrite or clear the queued URI. A private field with a public getter (and internal setter) would be safer.Suggested encapsulation
- String? queuedUri; + String? _queuedUri; + String? get queuedUri => _queuedUri;Then update internal usages to
_queuedUri.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/app/services/deep_link_service.dart` at line 10, The public mutable field queuedUri should be made private to prevent external mutation: rename queuedUri to _queuedUri and add a public getter (e.g., String? get queuedUri) to expose its value read-only; update all internal references in this file from queuedUri to _queuedUri and keep any internal setters or methods that modify the queued URI using _queuedUri so external callers cannot overwrite it directly.lib/app/modules/home/controllers/home_controller.dart (2)
136-137:Get.isRegistered<DeepLinkService>()check is redundant.Since
DeepLinkServiceis initialized withpermanent: trueinmain.dartbeforerunApp(), it will always be registered whenHomeController.onReady()executes. The check adds defensive safety but is unnecessary given the current initialization order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/app/modules/home/controllers/home_controller.dart` around lines 136 - 137, Remove the redundant registration check in HomeController.onReady: since DeepLinkService is created with permanent: true in main.dart before runApp(), you can directly call Get.find<DeepLinkService>() (or assign final deepLinkService = Get.find<DeepLinkService>();) without wrapping it in Get.isRegistered<DeepLinkService>() — update the code to eliminate the conditional and use the direct lookup (referencing HomeController.onReady, Get.isRegistered<DeepLinkService>, and DeepLinkService).
134-144: The 50ms delay is a fragile timing workaround.Using a fixed delay to wait for Navigator route transitions is brittle—it may be insufficient on slow devices or unnecessary on fast ones. Consider using
WidgetsBinding.instance.addPostFrameCallbackto execute after the current frame completes, or listen to the navigation state.That said, given the complexity of GetX lifecycle and Flutter routing, this pragmatic workaround addresses the immediate issue. The
isClosedguard is a good defensive measure.Alternative using post-frame callback
`@override` void onReady() { super.onReady(); - Future.delayed(const Duration(milliseconds: 50), () { + WidgetsBinding.instance.addPostFrameCallback((_) { if (isClosed) return; if (Get.isRegistered<DeepLinkService>()) { final deepLinkService = Get.find<DeepLinkService>(); if (deepLinkService.queuedUri != null) { debugPrint( "TRACE: HomeController.onReady() consuming deferred queue!"); deepLinkService.consumePendingActions(this); } } }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/app/modules/home/controllers/home_controller.dart` around lines 134 - 144, The fixed 50ms Future.delayed in HomeController.onReady() is a brittle timing hack; replace it by scheduling the deferred deep-link handling to run after the current frame completes (use WidgetsBinding.instance.addPostFrameCallback) or hook into the Navigator/route completion instead, keeping the existing isClosed guard and the DeepLinkService checks (Get.isRegistered<DeepLinkService>(), deepLinkService.queuedUri, deepLinkService.consumePendingActions(this)) so the logic and defensive guard remain identical but executed reliably post-frame or after navigation.lib/main.dart (1)
57-60:unknownRoutefallback is reasonable but could hide routing bugs.The fallback navigates to
INITIALor the first route if an unknown route is requested. While this prevents crashes, it may silently mask routing errors. Consider logging when this fallback is triggered to aid debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 57 - 60, The unknownRoute fallback currently returns AppPages.INITIAL or AppPages.routes.first silently; modify the unknownRoute handler so when the fallback path is used it logs the missing route details (requested route name and settings) before returning the fallback route. Specifically, update the orElse/unknownRoute logic around unknownRoute to capture the RouteSettings (or RouteSettings.name) and call your logger/debugPrint/Get.log with a message like "Unknown route requested: <name>, falling back to AppPages.INITIAL" and then return AppPages.routes.firstWhere(..., orElse: () => AppPages.routes.first) so the same fallback is returned but the event is recorded for debugging.android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt (2)
125-125: Replace empty lifecycle blocks with expression-bodyUnitto silence linter noise.Low-priority cleanup, but it removes detekt
EmptyFunctionBlockwarnings.Proposed refactor
- override fun onCreate() {} + override fun onCreate() = Unit ... - override fun onDestroy() {} + override fun onDestroy() = UnitAlso applies to: 144-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` at line 125, Replace empty lifecycle function bodies with Kotlin expression-body Unit to silence detekt EmptyFunctionBlock warnings: change the empty override of onCreate (override fun onCreate() {}) to use an expression-style Unit return and do the same for other empty lifecycle overrides in this class (e.g., the method at the other empty block referenced, such as onUpdate/onDisabled if present). Locate the overrides by name (onCreate, and the other empty lifecycle method) and transform their bodies to a single-expression Unit to satisfy the linter.
170-175: Remove debugprintlnfrom widget rendering path.This debug artifact creates noisy logs in production code. It's the only instance in the codebase and can be safely removed.
Proposed refactor
fun getDotIdByPriority(p: String): Int { - println("PRIORITY: " + p) if (p.equals("L")) return R.drawable.low_priority_dot if (p.equals("M")) return R.drawable.mid_priority_dot if (p.equals("H")) return R.drawable.high_priority_dot return R.drawable.no_priority_dot }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 170 - 175, The getDotIdByPriority function contains a debug println("PRIORITY: " + p) that should be removed from the widget rendering path; delete that println statement from the TaskWarriorWidgetProvider.getDotIdByPriority method so the function simply returns the appropriate R.drawable.* resource based on p (retain the existing equality checks for "L", "M", "H" and the default return).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Around line 178-195: Guard the index access in
TaskWarriorWidgetProvider.getViewAt by validating that the incoming position is
within 0 until tasks.size and returning an appropriate empty/placeholder
RemoteViews when out-of-bounds to avoid IndexOutOfBoundsException; additionally
make onDataSetChanged atomic by building a new temporary MutableList<Task>
(e.g., newTasks), populate it from SharedPreferences/JSON, then replace tasks
(or clear/addAll) in a single operation so the widget never observes a
partially-cleared list during refresh.
In `@lib/app/modules/splash/controllers/splash_controller.dart`:
- Around line 26-47: The async boot sequence in SplashController.onInit
(initBaseDir, _checkProfiles, setting profilesMap/currentProfile,
checkForUpdate, sendToNextPage) creates a race because GetX does not await
onInit and other controllers call Get.find() immediately; move all async
initialization and state mutations into an async onReady() override (e.g., make
onReady() async and perform await initBaseDir(), await _checkProfiles(),
profilesMap.value = ..., currentProfile.value = ..., await checkForUpdate(),
sendToNextPage()), while keeping only the synchronous deepLinkService queuedUri
check and the early Get.offNamed(Routes.HOME) return in onInit if you need
immediate bypass behavior; alternatively, if you must keep async work in onInit
ensure every dependent controller defers access (use
Future.delayed(Duration.zero, ...) or await SplashController.ready signal) but
preferred fix is relocating the async initialization to onReady to guarantee
dependent controllers see initialized state.
---
Outside diff comments:
In `@lib/main.dart`:
- Around line 30-38: The boot log isn't captured because the debugPrint override
is set after the initial debugPrint("🚀 BOOT: main() started") call; move the
override of debugPrint (the lambda using debugPrintSynchronously and
_logDatabaseHelper.insertLog) to run before that first debugPrint call in main()
so the initial boot message is recorded to the database—ensure the override
still calls debugPrintSynchronously and _logDatabaseHelper.insertLog exactly as
implemented.
---
Nitpick comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Line 125: Replace empty lifecycle function bodies with Kotlin expression-body
Unit to silence detekt EmptyFunctionBlock warnings: change the empty override of
onCreate (override fun onCreate() {}) to use an expression-style Unit return and
do the same for other empty lifecycle overrides in this class (e.g., the method
at the other empty block referenced, such as onUpdate/onDisabled if present).
Locate the overrides by name (onCreate, and the other empty lifecycle method)
and transform their bodies to a single-expression Unit to satisfy the linter.
- Around line 170-175: The getDotIdByPriority function contains a debug
println("PRIORITY: " + p) that should be removed from the widget rendering path;
delete that println statement from the
TaskWarriorWidgetProvider.getDotIdByPriority method so the function simply
returns the appropriate R.drawable.* resource based on p (retain the existing
equality checks for "L", "M", "H" and the default return).
In `@lib/app/modules/home/controllers/home_controller.dart`:
- Around line 136-137: Remove the redundant registration check in
HomeController.onReady: since DeepLinkService is created with permanent: true in
main.dart before runApp(), you can directly call Get.find<DeepLinkService>() (or
assign final deepLinkService = Get.find<DeepLinkService>();) without wrapping it
in Get.isRegistered<DeepLinkService>() — update the code to eliminate the
conditional and use the direct lookup (referencing HomeController.onReady,
Get.isRegistered<DeepLinkService>, and DeepLinkService).
- Around line 134-144: The fixed 50ms Future.delayed in HomeController.onReady()
is a brittle timing hack; replace it by scheduling the deferred deep-link
handling to run after the current frame completes (use
WidgetsBinding.instance.addPostFrameCallback) or hook into the Navigator/route
completion instead, keeping the existing isClosed guard and the DeepLinkService
checks (Get.isRegistered<DeepLinkService>(), deepLinkService.queuedUri,
deepLinkService.consumePendingActions(this)) so the logic and defensive guard
remain identical but executed reliably post-frame or after navigation.
In `@lib/app/services/deep_link_service.dart`:
- Around line 25-28: Store the StreamSubscription returned by
_appLinks.uriLinkStream.listen(...) (instead of discarding it) by adding an
import 'dart:async'; and a field like StreamSubscription? _uriSub in the
DeepLinkService, use that subscription to call _handleWidgetUri in the listener,
and override onClose() to call _uriSub?.cancel() (and null it) to ensure the
subscription is cancelled when the service is disposed.
- Line 10: The public mutable field queuedUri should be made private to prevent
external mutation: rename queuedUri to _queuedUri and add a public getter (e.g.,
String? get queuedUri) to expose its value read-only; update all internal
references in this file from queuedUri to _queuedUri and keep any internal
setters or methods that modify the queued URI using _queuedUri so external
callers cannot overwrite it directly.
In `@lib/main.dart`:
- Around line 57-60: The unknownRoute fallback currently returns
AppPages.INITIAL or AppPages.routes.first silently; modify the unknownRoute
handler so when the fallback path is used it logs the missing route details
(requested route name and settings) before returning the fallback route.
Specifically, update the orElse/unknownRoute logic around unknownRoute to
capture the RouteSettings (or RouteSettings.name) and call your
logger/debugPrint/Get.log with a message like "Unknown route requested: <name>,
falling back to AppPages.INITIAL" and then return
AppPages.routes.firstWhere(..., orElse: () => AppPages.routes.first) so the same
fallback is returned but the event is recorded for debugging.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.ktlib/app/modules/home/controllers/home_controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/app/services/deep_link_service.dartlib/main.dart
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt (4)
222-237: Consider removing unusedurgencyLevelfield.The
urgencyLevelproperty is parsed from JSON but never used ingetViewAt()or elsewhere. If it's for future use, consider adding a TODO comment; otherwise, remove it to keep the data class lean.♻️ Remove unused field
data class Task( val title: String, - val urgencyLevel: String, val uuid: String, val priority: String ) { companion object { fun fromJson(json: JSONObject): Task { val title = json.optString("description", "") - val urgencyLevel = json.optString("urgency", "") val uuid = json.optString("uuid", "") val priority = json.optString("priority", "") - return Task(title, urgencyLevel, uuid, priority) + return Task(title, uuid, priority) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 222 - 237, The Task data class currently includes an unused property urgencyLevel populated in companion::fromJson but never referenced (e.g., in getViewAt()); either remove the urgencyLevel property and stop parsing it in fromJson (update Task(...) constructor usage and fromJson to only extract title, uuid, priority) or if you intend to use it later, add a concise TODO comment above urgencyLevel in the Task class to justify keeping it; update any callers that construct Task instances to match the new constructor shape if you remove the field.
118-121: Unused constructor parametertasksJsonString.The
tasksJsonStringparameter is passed from the service but never used inListViewRemoteViewsFactory. Instead,onDataSetChanged()reads fresh data directly fromSharedPreferences. Either remove the unused parameter or use it inonCreate()for initial population to avoid a redundant SharedPreferences read.♻️ Option 1: Remove unused parameter
class ListViewRemoteViewsFactory( - private val context: Context, - private val tasksJsonString: String? + private val context: Context ) : RemoteViewsService.RemoteViewsFactory {And update the service:
override fun onGetViewFactory(intent: Intent): RemoteViewsFactory { - val tasksJsonString = intent.getStringExtra("tasksJsonString") - return ListViewRemoteViewsFactory(applicationContext, tasksJsonString) + return ListViewRemoteViewsFactory(applicationContext) }♻️ Option 2: Use parameter for initial load in onCreate()
-override fun onCreate() = Unit +override fun onCreate() { + if (!tasksJsonString.isNullOrEmpty()) { + try { + val jsonArray = OrgJSONArray(tasksJsonString) + for (i in 0 until jsonArray.length()) { + tasks.add(Task.fromJson(jsonArray.getJSONObject(i))) + } + } catch (e: JSONException) { + e.printStackTrace() + } + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 118 - 121, The constructor parameter tasksJsonString on ListViewRemoteViewsFactory is unused; either remove it (and update the service that constructs ListViewRemoteViewsFactory) or use it to initialize the internal task state in onCreate() instead of immediately re-reading SharedPreferences in onDataSetChanged(); specifically, if you keep tasksJsonString, read/parse it in ListViewRemoteViewsFactory.onCreate() to populate the factory's data model and only fall back to SharedPreferences in onDataSetChanged() if null, otherwise remove the tasksJsonString parameter from the ListViewRemoteViewsFactory constructor and the caller that supplies it.
22-32: Consider simplifying theme comparison and handling null safely.
sharedPrefs.getString("themeMode", "")returnsString?, and calling.equals("dark")on a nullable receiver is safe but not idiomatic. ThelayoutIdintermediate variable is also unnecessary.♻️ Suggested simplification
fun getLayoutId(context: Context): Int { val sharedPrefs = HomeWidgetPlugin.getData(context) val theme = sharedPrefs.getString("themeMode", "") - val layoutId = - if (theme.equals("dark")) { - R.layout.taskwarrior_layout_dark - } else { - R.layout.taskwarrior_layout - } - return layoutId + return if (theme == "dark") { + R.layout.taskwarrior_layout_dark + } else { + R.layout.taskwarrior_layout + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 22 - 32, The getLayoutId function uses a nullable String from sharedPrefs.getString("themeMode", "") and an unnecessary intermediate layoutId; simplify by reading theme with a non-null default (via the Elvis operator or the second argument) and use the idiomatic == comparison (e.g., theme == "dark") and return the chosen resource directly (no layoutId variable); update the code referencing HomeWidgetPlugin.getData, theme, and getLayoutId to perform a safe null-handling comparison and return R.layout.taskwarrior_layout_dark when the theme equals "dark", otherwise return R.layout.taskwarrior_layout.
151-178: Consider extracting shared theme-reading logic to reduce duplication.Theme preference reading is duplicated in
getLayoutId(),getListItemLayoutId(), andgetListItemLayoutIdForR1(). Also,getDotIdByPrioritywould be more idiomatic using awhenexpression.♻️ Suggested refactor for DRY and idiomatic Kotlin
private fun isDarkTheme(): Boolean { val sharedPrefs = HomeWidgetPlugin.getData(context) return sharedPrefs.getString("themeMode", "") == "dark" } fun getListItemLayoutId(): Int = if (isDarkTheme()) R.layout.listitem_layout_dark else R.layout.listitem_layout fun getListItemLayoutIdForR1(): Int = if (isDarkTheme()) R.layout.no_tasks_found_li_dark else R.layout.no_tasks_found_li fun getDotIdByPriority(p: String): Int = when (p) { "L" -> R.drawable.low_priority_dot "M" -> R.drawable.mid_priority_dot "H" -> R.drawable.high_priority_dot else -> R.drawable.no_priority_dot }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 151 - 178, Extract the duplicated theme-reading into a private helper (e.g., isDarkTheme()) that calls HomeWidgetPlugin.getData(context) and checks sharedPrefs.getString("themeMode","") == "dark", then simplify getListItemLayoutId() and getListItemLayoutIdForR1() to return the appropriate layout based on isDarkTheme(); also refactor getDotIdByPriority(p: String) to use a Kotlin when expression mapping "L"→low_priority_dot, "M"→mid_priority_dot, "H"→high_priority_dot, else→no_priority_dot to make the code more idiomatic and remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/app/services/deep_link_service.dart`:
- Around line 28-31: The stream subscription to _appLinks.uriLinkStream in
_linkSubscription = _appLinks.uriLinkStream.listen((uri) { ... }) lacks an
onError handler; update the listen call to provide an onError callback that
catches and logs/report errors (using debugPrint or your error logger) and
safely ignores malformed/failed events, and optionally provide onDone/cancel
behavior; ensure errors are handled similarly to getInitialLink() and that
_handleWidgetUri(uri) is only called for valid URIs after error checking.
---
Nitpick comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Around line 222-237: The Task data class currently includes an unused property
urgencyLevel populated in companion::fromJson but never referenced (e.g., in
getViewAt()); either remove the urgencyLevel property and stop parsing it in
fromJson (update Task(...) constructor usage and fromJson to only extract title,
uuid, priority) or if you intend to use it later, add a concise TODO comment
above urgencyLevel in the Task class to justify keeping it; update any callers
that construct Task instances to match the new constructor shape if you remove
the field.
- Around line 118-121: The constructor parameter tasksJsonString on
ListViewRemoteViewsFactory is unused; either remove it (and update the service
that constructs ListViewRemoteViewsFactory) or use it to initialize the internal
task state in onCreate() instead of immediately re-reading SharedPreferences in
onDataSetChanged(); specifically, if you keep tasksJsonString, read/parse it in
ListViewRemoteViewsFactory.onCreate() to populate the factory's data model and
only fall back to SharedPreferences in onDataSetChanged() if null, otherwise
remove the tasksJsonString parameter from the ListViewRemoteViewsFactory
constructor and the caller that supplies it.
- Around line 22-32: The getLayoutId function uses a nullable String from
sharedPrefs.getString("themeMode", "") and an unnecessary intermediate layoutId;
simplify by reading theme with a non-null default (via the Elvis operator or the
second argument) and use the idiomatic == comparison (e.g., theme == "dark") and
return the chosen resource directly (no layoutId variable); update the code
referencing HomeWidgetPlugin.getData, theme, and getLayoutId to perform a safe
null-handling comparison and return R.layout.taskwarrior_layout_dark when the
theme equals "dark", otherwise return R.layout.taskwarrior_layout.
- Around line 151-178: Extract the duplicated theme-reading into a private
helper (e.g., isDarkTheme()) that calls HomeWidgetPlugin.getData(context) and
checks sharedPrefs.getString("themeMode","") == "dark", then simplify
getListItemLayoutId() and getListItemLayoutIdForR1() to return the appropriate
layout based on isDarkTheme(); also refactor getDotIdByPriority(p: String) to
use a Kotlin when expression mapping "L"→low_priority_dot, "M"→mid_priority_dot,
"H"→high_priority_dot, else→no_priority_dot to make the code more idiomatic and
remove duplication.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
android/app/src/main/jniLibs/arm64-v8a/libtc_helper.sois excluded by!**/*.soandroid/app/src/main/jniLibs/armeabi-v7a/libtc_helper.sois excluded by!**/*.soandroid/app/src/main/jniLibs/x86_64/libtc_helper.sois excluded by!**/*.sopubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.ktbuild_droidtchelperbuild_log.txtlib/app/modules/home/controllers/home_controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/app/services/deep_link_service.dartlib/main.dartlinux/flutter/generated_plugin_registrant.cclinux/flutter/generated_plugins.cmakemacos/Flutter/GeneratedPluginRegistrant.swiftwindows/flutter/generated_plugin_registrant.ccwindows/flutter/generated_plugins.cmake
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/app/modules/home/controllers/home_controller.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt (1)
118-121: Consider removing unusedtasksJsonStringparameter.The
tasksJsonStringconstructor parameter is passed fromListViewRemoteViewsServicebut never used—onDataSetChanged()correctly reads fresh data from SharedPreferences instead. This is the right behavior for a RemoteViewsFactory, but the unused parameter could be removed for clarity.Proposed cleanup
class ListViewRemoteViewsFactory( - private val context: Context, - private val tasksJsonString: String? + private val context: Context ) : RemoteViewsService.RemoteViewsFactory {override fun onGetViewFactory(intent: Intent): RemoteViewsFactory { - val tasksJsonString = intent.getStringExtra("tasksJsonString") - return ListViewRemoteViewsFactory(applicationContext, tasksJsonString) + return ListViewRemoteViewsFactory(applicationContext) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 118 - 121, The constructor parameter tasksJsonString on class ListViewRemoteViewsFactory is unused; remove that parameter from the constructor and all call sites (e.g. where ListViewRemoteViewsService constructs the factory) and update the class signature to only accept Context, leaving onDataSetChanged() to continue reading SharedPreferences; ensure any references to tasksJsonString are removed so the factory compiles cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/app/services/deep_link_service.dart`:
- Around line 51-57: In consumePendingActions, wrap the Uri.parse(_queuedUri!)
call in a try-catch that catches FormatException (and optionally other
Exceptions) so a malformed _queuedUri doesn’t crash the app; on catch, log the
error (e.g., using debugPrint or your logger) with the bad _queuedUri value and
skip calling _executeAction, then clear _queuedUri as currently done; update the
code around consumePendingActions and its call to _executeAction(Uri.parse(...),
controller) to use the parsed Uri only when parsing succeeds.
---
Nitpick comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Around line 118-121: The constructor parameter tasksJsonString on class
ListViewRemoteViewsFactory is unused; remove that parameter from the constructor
and all call sites (e.g. where ListViewRemoteViewsService constructs the
factory) and update the class signature to only accept Context, leaving
onDataSetChanged() to continue reading SharedPreferences; ensure any references
to tasksJsonString are removed so the factory compiles cleanly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.ktlib/app/modules/home/controllers/home_controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/app/services/deep_link_service.dartlib/main.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/app/modules/home/controllers/home_controller.dart
…checking, and encapsulation
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt (2)
138-140: Consider using Android logging instead ofprintStackTrace().
e.printStackTrace()is less visible on Android. UsingLog.e()integrates better with logcat filtering during debugging.Suggested change
+import android.util.Log + +private const val TAG = "TaskWarriorWidget" + // In onDataSetChanged: } catch (e: JSONException) { - e.printStackTrace() + Log.e(TAG, "Failed to parse tasks JSON", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 138 - 140, In the catch block handling JSONException inside TaskWarriorWidgetProvider (the catch (e: JSONException) clause), replace e.printStackTrace() with Android logging: call android.util.Log.e(TAG, "Failed to parse JSON in <describe operation or method>", e); add a TAG constant (e.g. private const val TAG = "TaskWarriorWidgetProvider") if not present and import android.util.Log so the stack trace is visible in logcat and filterable.
118-121: Unused constructor parametertasksJsonString.The
tasksJsonStringparameter is passed fromListViewRemoteViewsService.onGetViewFactory()but never used in the factory. Instead,onDataSetChanged()reads directly from SharedPreferences. This creates confusion about the intended data flow.Either remove the parameter or use it for initial population in
onCreate().Option 1: Remove the unused parameter
class ListViewRemoteViewsFactory( - private val context: Context, - private val tasksJsonString: String? + private val context: Context ) : RemoteViewsService.RemoteViewsFactory {And in
ListViewRemoteViewsService:override fun onGetViewFactory(intent: Intent): RemoteViewsFactory { - val tasksJsonString = intent.getStringExtra("tasksJsonString") - return ListViewRemoteViewsFactory(applicationContext, tasksJsonString) + return ListViewRemoteViewsFactory(applicationContext) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt` around lines 118 - 121, The constructor parameter tasksJsonString on ListViewRemoteViewsFactory is unused; remove it from the class signature and all callsites (update ListViewRemoteViewsService.onGetViewFactory to instantiate ListViewRemoteViewsFactory(context) instead of passing tasksJsonString), and then remove any internal references to that field; alternatively, if you prefer to keep the parameter, use it to initialize the factory state in ListViewRemoteViewsFactory.onCreate (populate the internal task list from tasksJsonString there) rather than reading directly from SharedPreferences in onDataSetChanged — pick one approach and update constructor, onCreate, and onGetViewFactory consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Around line 209-211: hasStableIds() currently returns true while
getItemId(position: Int) returns position.toLong(), which is unstable; change
getItemId to return a stable identifier derived from each task (e.g., the task's
UUID/hash) so IDs persist across reorders/removals and keep animations correct.
Locate getItemId and the adapter's backing data (where tasks are stored) in
TaskWarriorWidgetProvider.kt, compute a stable Long per task (for example from
task.uuid or by mapping UUIDs to longs) and return that instead of
position.toLong(); leave hasStableIds() as true once stable IDs are implemented.
In `@lib/app/modules/home/controllers/home_controller.dart`:
- Around line 133-137: The post-frame callback unconditionally calls
Get.find<DeepLinkService>() which can crash in tests/alternate entry points
where the service wasn't registered; fix by guarding the lookup using
Get.isRegistered<DeepLinkService>() (or equivalent registration check) before
calling Get.find, and only access deepLinkService.queuedUri if the service is
registered; apply the same guard pattern to the similar usage in
SplashController to prevent startup crashes in non-standard boot paths.
In `@lib/app/modules/splash/controllers/splash_controller.dart`:
- Around line 35-49: Wrap the splash boot sequence (calls to initBaseDir(),
_checkProfiles(), assigning profilesMap/currentProfile, checkForUpdate(), and
sendToNextPage()) in a try/catch (or try/catch/finally) so any thrown error
cannot block navigation; on catch log the error (debugPrint or logger) and
perform a safe fallback navigation (e.g. call Get.offNamed(Routes.HOME) or
sendToNextPage()) and preserve the existing deepLinkService fast-path check for
deepLinkService.queuedUri — ensure the catch/finally always triggers a
navigation to avoid a dead-end.
---
Nitpick comments:
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`:
- Around line 138-140: In the catch block handling JSONException inside
TaskWarriorWidgetProvider (the catch (e: JSONException) clause), replace
e.printStackTrace() with Android logging: call android.util.Log.e(TAG, "Failed
to parse JSON in <describe operation or method>", e); add a TAG constant (e.g.
private const val TAG = "TaskWarriorWidgetProvider") if not present and import
android.util.Log so the stack trace is visible in logcat and filterable.
- Around line 118-121: The constructor parameter tasksJsonString on
ListViewRemoteViewsFactory is unused; remove it from the class signature and all
callsites (update ListViewRemoteViewsService.onGetViewFactory to instantiate
ListViewRemoteViewsFactory(context) instead of passing tasksJsonString), and
then remove any internal references to that field; alternatively, if you prefer
to keep the parameter, use it to initialize the factory state in
ListViewRemoteViewsFactory.onCreate (populate the internal task list from
tasksJsonString there) rather than reading directly from SharedPreferences in
onDataSetChanged — pick one approach and update constructor, onCreate, and
onGetViewFactory consistently.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.ktlib/app/modules/home/controllers/home_controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/app/services/deep_link_service.dartlib/main.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/main.dart
| override fun getItemId(position: Int): Long = position.toLong() | ||
|
|
||
| override fun hasStableIds(): Boolean = true |
There was a problem hiding this comment.
hasStableIds() returns true but getItemId() uses position.
When hasStableIds() returns true, the framework expects getItemId() to return a stable identifier that persists across data changes. Using position.toLong() violates this contract since positions change when items are reordered or removed, potentially causing visual glitches or incorrect item animations.
Proposed fix using task UUID
- override fun getItemId(position: Int): Long = position.toLong()
+ override fun getItemId(position: Int): Long {
+ if (position !in tasks.indices) return position.toLong()
+ return tasks[position].uuid.hashCode().toLong()
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@android/app/src/main/kotlin/com/ccextractor/taskwarriorflutter/TaskWarriorWidgetProvider.kt`
around lines 209 - 211, hasStableIds() currently returns true while
getItemId(position: Int) returns position.toLong(), which is unstable; change
getItemId to return a stable identifier derived from each task (e.g., the task's
UUID/hash) so IDs persist across reorders/removals and keep animations correct.
Locate getItemId and the adapter's backing data (where tasks are stored) in
TaskWarriorWidgetProvider.kt, compute a stable Long per task (for example from
task.uuid or by mapping UUIDs to longs) and return that instead of
position.toLong(); leave hasStableIds() as true once stable IDs are implemented.
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (isClosed) return; | ||
|
|
||
| final deepLinkService = Get.find<DeepLinkService>(); | ||
| if (deepLinkService.queuedUri != null) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify DeepLinkService registration and HomeController usage patterns
rg -nP --type=dart -C2 \
'Get\.(putAsync|put|lazyPut)<DeepLinkService>|Get\.find<DeepLinkService>|Get\.(put|lazyPut)<HomeController>|class\s+HomeController\b'Repository: CCExtractor/taskwarrior-flutter
Length of output: 2202
🏁 Script executed:
#!/bin/bash
# Also search for app initialization and main entry points
fd --type=dart -e dart | head -20 | xargs grep -l "main(|runApp|GetMaterialApp" 2>/dev/null | head -5Repository: CCExtractor/taskwarrior-flutter
Length of output: 246
🏁 Script executed:
#!/bin/bash
# Look for where DeepLinkService is registered
rg -nP --type=dart "DeepLinkService" -A 3 -B 3 | head -100Repository: CCExtractor/taskwarrior-flutter
Length of output: 2532
🏁 Script executed:
#!/bin/bash
# Check for test files that might test HomeController
find . -type f -name "*test*.dart" -o -name "*_test.dart" | head -20Repository: CCExtractor/taskwarrior-flutter
Length of output: 905
🏁 Script executed:
#!/bin/bash
# Check if there are test setup patterns
rg -l "HomeController|setUp\(|DeepLinkService" --type=dart | grep -i testRepository: CCExtractor/taskwarrior-flutter
Length of output: 1693
🏁 Script executed:
#!/bin/bash
# Look at main.dart initialization order more carefully
cat -n lib/main.dart | head -80Repository: CCExtractor/taskwarrior-flutter
Length of output: 2853
🏁 Script executed:
#!/bin/bash
# Check main_test.dart to see test setup
cat -n test/main_test.dart | head -50Repository: CCExtractor/taskwarrior-flutter
Length of output: 1276
🏁 Script executed:
#!/bin/bash
# Search for other possible entry points or test configuration
rg -l "main\(\)|runApp|GetMaterialApp" test/ --type=dart | head -10Repository: CCExtractor/taskwarrior-flutter
Length of output: 495
🏁 Script executed:
#!/bin/bash
# Check if home_page_tour_test properly initializes DI
cat -n test/tour/home_page_tour_test.dart | head -80Repository: CCExtractor/taskwarrior-flutter
Length of output: 3169
Guard Get.find<DeepLinkService>() to avoid startup crashes in non-standard boot paths (tests, alternate entry points).
At line 136, Get.find<DeepLinkService>() is unconditional inside the post-frame callback. While the normal app flow registers DeepLinkService before runApp() via Get.putAsync() in main.dart, this pattern fails in test setups and alternate entry points that skip the standard initialization. Add a registration check to prevent crashes.
🔧 Proposed fix
WidgetsBinding.instance.addPostFrameCallback((_) {
if (isClosed) return;
+ if (!Get.isRegistered<DeepLinkService>()) {
+ debugPrint("🚀 TRACE: DeepLinkService not registered; skipping deferred consume.");
+ return;
+ }
final deepLinkService = Get.find<DeepLinkService>();
if (deepLinkService.queuedUri != null) {
debugPrint(
"🚀 TRACE: HomeController.onReady() consuming deferred queue!");
deepLinkService.consumePendingActions(this);
}
});Note: SplashController line 40 has the same pattern—consider applying the same guard there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/app/modules/home/controllers/home_controller.dart` around lines 133 -
137, The post-frame callback unconditionally calls Get.find<DeepLinkService>()
which can crash in tests/alternate entry points where the service wasn't
registered; fix by guarding the lookup using Get.isRegistered<DeepLinkService>()
(or equivalent registration check) before calling Get.find, and only access
deepLinkService.queuedUri if the service is registered; apply the same guard
pattern to the similar usage in SplashController to prevent startup crashes in
non-standard boot paths.
| await initBaseDir(); | ||
| _checkProfiles(); | ||
| profilesMap.value = _profiles.profilesMap(); | ||
| currentProfile.value = _profiles.getCurrentProfile()!; | ||
|
|
||
| final deepLinkService = Get.find<DeepLinkService>(); | ||
| if (deepLinkService.queuedUri != null) { | ||
| debugPrint("🚀 TRACE: Bypassing Splash routing for queued URI"); | ||
| Get.offNamed(Routes.HOME); | ||
| return; | ||
| } | ||
|
|
||
| await checkForUpdate(); | ||
| initBaseDir().then((_) { | ||
| _checkProfiles(); | ||
| profilesMap.value = _profiles.profilesMap(); | ||
| currentProfile.value = _profiles.getCurrentProfile()!; | ||
| sendToNextPage(); | ||
| }); | ||
| sendToNextPage(); | ||
| } |
There was a problem hiding this comment.
Add a fail-safe around splash boot sequence to prevent route dead-ends.
If any awaited step in Line 35-49 throws, navigation never occurs and users can get stuck on splash.
🛡️ Proposed fix
`@override`
void onReady() async {
super.onReady();
-
- await initBaseDir();
- _checkProfiles();
- profilesMap.value = _profiles.profilesMap();
- currentProfile.value = _profiles.getCurrentProfile()!;
-
- final deepLinkService = Get.find<DeepLinkService>();
- if (deepLinkService.queuedUri != null) {
- debugPrint("🚀 TRACE: Bypassing Splash routing for queued URI");
- Get.offNamed(Routes.HOME);
- return;
- }
-
- await checkForUpdate();
- sendToNextPage();
+ try {
+ await initBaseDir();
+ _checkProfiles();
+ profilesMap.value = _profiles.profilesMap();
+ currentProfile.value = _profiles.getCurrentProfile()!;
+
+ final deepLinkService = Get.find<DeepLinkService>();
+ if (deepLinkService.queuedUri != null) {
+ debugPrint("🚀 TRACE: Bypassing Splash routing for queued URI");
+ Get.offNamed(Routes.HOME);
+ return;
+ }
+
+ await checkForUpdate();
+ sendToNextPage();
+ } catch (e, st) {
+ debugPrint('Splash boot failed: $e');
+ debugPrint('$st');
+ sendToNextPage();
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/app/modules/splash/controllers/splash_controller.dart` around lines 35 -
49, Wrap the splash boot sequence (calls to initBaseDir(), _checkProfiles(),
assigning profilesMap/currentProfile, checkForUpdate(), and sendToNextPage()) in
a try/catch (or try/catch/finally) so any thrown error cannot block navigation;
on catch log the error (debugPrint or logger) and perform a safe fallback
navigation (e.g. call Get.offNamed(Routes.HOME) or sendToNextPage()) and
preserve the existing deepLinkService fast-path check for
deepLinkService.queuedUri — ensure the catch/finally always triggers a
navigation to avoid a dead-end.
Screen.Recording.2026-03-03.at.7.55.34.AM.movWhen the app is running in the background and I try to click on a task, it gets stuck on 'Setting up app' |
Description
The Android widget was basically ghosting users on cold starts. This PR completely revamps the deep-linking pipeline to nuke the Android 12+ Trampoline crash and fix the GetX race conditions that were causing the app to freeze up. We also bumped the target SDK to keep things modern.
Here is the breakdown of the fixes:
getBroadcastforPendingIntent.getActivity()with the correct mutable flags. This safely bypasses the strict Android 12 background launch limits.DeepLinkServiceto initialize synchronously. The Flutter engine now physically waits to catch the OS intent before trying to paint the UI, officially killing the race condition.HomeController.onReady()so the routing fully finishes before the 'Add Task' bottom sheet tries to pop up. No more widget tree deadlocks.targetSdkVersionto 34 so modern Android phones (14/15) stop throwing the "built for an older version" warning.Fixes #594
Screenshots
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor