Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/app/modules/home/views/show_tasks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,17 @@ class TaskViewBuilder extends StatelessWidget {
TaskDatabase taskDatabase = TaskDatabase();
await taskDatabase.open();
taskDatabase.markTaskAsCompleted(uuid);
completeTask('email', uuid);
try {
await completeTask('email', uuid);
} catch (e) {
debugPrint('Error completing task on server: $e');
Get.snackbar(
'Sync Error',
'Failed to mark task complete on server',
snackPosition: SnackPosition.BOTTOM,
duration: const Duration(seconds: 3),
);
}
}

void _markTaskAsDeleted(String uuid) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'package:get/get.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:taskwarrior/app/modules/splash/controllers/splash_controller.dart';
import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart';
import 'package:taskwarrior/app/v3/net/origin.dart';
import 'package:http/http.dart' as http;

class ManageTaskChampionCredsController extends GetxController {
Expand Down Expand Up @@ -46,11 +45,11 @@ class ManageTaskChampionCredsController extends GetxController {
String encryptionSecret = encryptionSecretController.text;
try {
String url =
'$baseUrl/tasks?email=email&origin=$origin&UUID=$uuid&encryptionSecret=$encryptionSecret';
'$baseUrl/tasks?email=email&origin=$baseUrl&UUID=$uuid&encryptionSecret=$encryptionSecret';

var response = await http.get(Uri.parse(url), headers: {
"Content-Type": "application/json",
}).timeout(const Duration(seconds: 10000));
}).timeout(const Duration(seconds: 10));
debugPrint("Fetch tasks response: ${response.statusCode}");
debugPrint("Fetch tasks body: ${response.body}");
if (response.statusCode == 200) {
Expand Down
10 changes: 10 additions & 0 deletions lib/app/modules/profile/views/profile_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,16 @@ class ProfileView extends GetView<ProfileController> {
currentProfileKey: controller.currentProfileKey,
addNewProfileKey: controller.addNewProfileKey,
manageSelectedProfileKey: controller.manageSelectedProfileKey,
getModeLabel: (profile) {
switch (controller.profilesWidget.getMode(profile)) {
case 'TW3C':
return 'Taskchampion (v3)';
case 'TW3':
return 'CCSync (v3)';
default:
return 'TaskServer';
}
},
controller.profilesMap,
controller.currentProfile.value,
controller.profilesWidget.addProfile,
Expand Down
13 changes: 13 additions & 0 deletions lib/app/modules/profile/views/profiles_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class ProfilesList extends StatelessWidget {
required this.currentProfileKey,
required this.addNewProfileKey,
required this.manageSelectedProfileKey,
this.getModeLabel,
super.key,
});

Expand All @@ -32,6 +33,7 @@ class ProfilesList extends StatelessWidget {
final void Function(String) copy;
final void Function(dynamic) delete;
final void Function(String) changeMode;
final String Function(String)? getModeLabel;
final GlobalKey currentProfileKey;
final GlobalKey addNewProfileKey;
final GlobalKey manageSelectedProfileKey;
Expand Down Expand Up @@ -197,6 +199,17 @@ class ProfilesList extends StatelessWidget {
color: AppSettings.isDarkMode
? TaskWarriorColors.kprimaryTextColor
: TaskWarriorColors.kLightPrimaryTextColor)),
subtitle: getModeLabel != null
? Text(
getModeLabel!(profileId),
style: TextStyle(
color: AppSettings.isDarkMode
? Colors.grey[400]
: Colors.grey[700],
fontSize: 12.0,
),
)
: null,
onTap: () {
changeMode(profileId);
},
Expand Down
6 changes: 5 additions & 1 deletion lib/app/v3/db/update.dart
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ Future<void> updateTasksInDatabase(List<TaskForC> tasks) async {
? localTask.tags!.map((e) => e.toString()).toList()
: []);
if (localTask.status == 'completed') {
completeTask('email', localTask.uuid!);
try {
await completeTask('email', localTask.uuid!);
} catch (e) {
debugPrint('Failed to complete task on server: $e');
}
} else if (localTask.status == 'deleted') {
deleteTask('email', localTask.uuid!);
}
Comment on lines 85 to 87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent error handling: deleteTask is not awaited or wrapped in try-catch.

completeTask is now properly awaited with error handling, but deleteTask on line 86 is called without await and has no try-catch. This inconsistency could lead to unhandled errors if deleteTask throws, potentially causing silent failures or unhandled promise rejections.

🛡️ Proposed fix for consistency
         if (localTask.status == 'completed') {
           try {
             await completeTask('email', localTask.uuid!);
           } catch (e) {
             debugPrint('Failed to complete task on server: $e');
           }
         } else if (localTask.status == 'deleted') {
-          deleteTask('email', localTask.uuid!);
+          try {
+            await deleteTask('email', localTask.uuid!);
+          } catch (e) {
+            debugPrint('Failed to delete task on server: $e');
+          }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if (localTask.status == 'deleted') {
deleteTask('email', localTask.uuid!);
}
} else if (localTask.status == 'deleted') {
try {
await deleteTask('email', localTask.uuid!);
} catch (e) {
debugPrint('Failed to delete task on server: $e');
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/app/v3/db/update.dart` around lines 85 - 87, The call to deleteTask when
localTask.status == 'deleted' is not awaited or error-handled like completeTask,
so wrap the deleteTask('email', localTask.uuid!) call in an await and surround
it with try-catch (similar to the completeTask handling) to catch and log
errors; reference the deleteTask function call and localTask.status check and
ensure you use await deleteTask(...) and a try { await deleteTask(...) } catch
(err) { /* log error via existing logger/processLogger */ } pattern to keep
behavior consistent.

Expand Down
5 changes: 4 additions & 1 deletion lib/app/v3/models/task.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ class TaskForC {
recur: json['recur'],
depends:
json['depends']?.map<String>((d) => d.toString()).toList() ?? [],
annotations: <Annotation>[]);
annotations: (json['annotations'] as List?)
?.map((a) => Annotation.fromJson(a as Map<String, dynamic>))
.toList() ??
[]);
}

Map<String, dynamic> toJson() {
Expand Down
20 changes: 9 additions & 11 deletions lib/app/v3/net/complete.dart
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import 'dart:convert';
import 'package:http/http.dart' as http;
import 'package:flutter/material.dart';
import 'package:flutter/foundation.dart';
import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart';
import 'package:path/path.dart';

Future<void> completeTask(String email, String taskUuid) async {
Future<void> completeTask(String email, String taskUuid,
{http.Client? client}) async {
final httpClient = client ?? http.Client();
Comment on lines +6 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Resource leak: internally created http.Client is never closed.

When no client is injected, a new http.Client() is created but never closed. The http.Client should be closed after use to release underlying resources (socket connections).

🛡️ Proposed fix
 Future<void> completeTask(String email, String taskUuid,
     {http.Client? client}) async {
-  final httpClient = client ?? http.Client();
+  final httpClient = client;
+  final shouldCloseClient = httpClient == null;
+  final effectiveClient = httpClient ?? http.Client();
   var c = await CredentialsStorage.getClientId();
   var e = await CredentialsStorage.getEncryptionSecret();
   var baseUrl = await CredentialsStorage.getApiUrl();
   // ... rest of the code ...
   try {
-    final response = await httpClient.post(
+    final response = await effectiveClient.post(
       // ...
     );
     // ...
   } catch (e, s) {
     debugPrint('Error completing task: $e\n$s');
     rethrow;
+  } finally {
+    if (shouldCloseClient) {
+      effectiveClient.close();
+    }
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Future<void> completeTask(String email, String taskUuid,
{http.Client? client}) async {
final httpClient = client ?? http.Client();
Future<void> completeTask(String email, String taskUuid,
{http.Client? client}) async {
final httpClient = client;
final shouldCloseClient = httpClient == null;
final effectiveClient = httpClient ?? http.Client();
var c = await CredentialsStorage.getClientId();
var e = await CredentialsStorage.getEncryptionSecret();
var baseUrl = await CredentialsStorage.getApiUrl();
// ... rest of the code ...
try {
final response = await effectiveClient.post(
// ...
);
// ...
} catch (e, s) {
debugPrint('Error completing task: $e\n$s');
rethrow;
} finally {
if (shouldCloseClient) {
effectiveClient.close();
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/app/v3/net/complete.dart` around lines 6 - 8, The function completeTask
creates an http.Client when no client is injected but never closes it, leaking
sockets; change completeTask to track whether the client was created (e.g.,
createdClient = client == null) and ensure httpClient.close() is called in a
finally block (or use a try/finally) so only internally created clients are
closed while injected clients are left open; reference the httpClient local and
completeTask function to locate where to add the finally/close logic.

var c = await CredentialsStorage.getClientId();
var e = await CredentialsStorage.getEncryptionSecret();
var baseUrl = await CredentialsStorage.getApiUrl();
Expand All @@ -17,7 +18,7 @@ Future<void> completeTask(String email, String taskUuid) async {
});

try {
final response = await http.post(
final response = await httpClient.post(
url,
headers: {
'Content-Type': 'application/json',
Expand All @@ -29,13 +30,10 @@ Future<void> completeTask(String email, String taskUuid) async {
debugPrint('Task completed successfully on server');
} else {
debugPrint('Failed to complete task: ${response.statusCode}');
ScaffoldMessenger.of(context as BuildContext).showSnackBar(const SnackBar(
content: Text(
"Failed to complete task!",
style: TextStyle(color: Colors.red),
)));
throw Exception('Failed to complete task: ${response.statusCode}');
}
} catch (e) {
debugPrint('Error completing task: $e');
} catch (e, s) {
debugPrint('Error completing task: $e\n$s');
rethrow;
}
}
3 changes: 1 addition & 2 deletions lib/app/v3/net/fetch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import 'dart:convert';
import 'package:flutter/material.dart';
import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart';
import 'package:taskwarrior/app/v3/models/task.dart';
import 'package:taskwarrior/app/v3/net/origin.dart';
import 'package:http/http.dart' as http;

Future<List<TaskForC>> fetchTasks(String uuid, String encryptionSecret) async {
var baseUrl = await CredentialsStorage.getApiUrl();
try {
String url =
'$baseUrl/tasks?email=email&origin=$origin&UUID=$uuid&encryptionSecret=$encryptionSecret';
'$baseUrl/tasks?email=email&origin=$baseUrl&UUID=$uuid&encryptionSecret=$encryptionSecret';

var response = await http.get(Uri.parse(url), headers: {
"Content-Type": "application/json",
Expand Down
4 changes: 3 additions & 1 deletion lib/app/v3/net/origin.dart
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
String origin = 'http://localhost:8080';
import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart';

Future<String> getOrigin() async => await CredentialsStorage.getApiUrl() ?? '';
Comment on lines +1 to +3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for usages of getOrigin() across the codebase

# Search for imports of origin.dart
echo "=== Imports of origin.dart ==="
rg -l "import.*origin\.dart" --type dart

# Search for getOrigin function calls
echo -e "\n=== Usages of getOrigin() ==="
rg -n "getOrigin\(" --type dart

Repository: CCExtractor/taskwarrior-flutter

Length of output: 245


Remove unused getOrigin() function and file.

The getOrigin() function is not imported or called anywhere in the codebase. It is dead code left over from refactoring where direct calls to CredentialsStorage.getApiUrl() replaced its usage. Delete lib/app/v3/net/origin.dart entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/app/v3/net/origin.dart` around lines 1 - 3, Remove the dead helper
getOrigin() and its containing file: delete the getOrigin function and the file
defining it (the function name getOrigin and its single use of
CredentialsStorage.getApiUrl() are unused), and ensure no other code imports or
references getOrigin; replace any remaining calls (if any) with direct calls to
CredentialsStorage.getApiUrl() or remove the import entirely so the file can be
safely deleted.

4 changes: 4 additions & 0 deletions linux/flutter/generated_plugin_registrant.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <file_selector_linux/file_selector_plugin.h>
#include <flutter_timezone/flutter_timezone_plugin.h>
#include <gtk/gtk_plugin.h>
#include <url_launcher_linux/url_launcher_plugin.h>

void fl_register_plugins(FlPluginRegistry* registry) {
Expand All @@ -17,6 +18,9 @@ void fl_register_plugins(FlPluginRegistry* registry) {
g_autoptr(FlPluginRegistrar) flutter_timezone_registrar =
fl_plugin_registry_get_registrar_for_plugin(registry, "FlutterTimezonePlugin");
flutter_timezone_plugin_register_with_registrar(flutter_timezone_registrar);
g_autoptr(FlPluginRegistrar) gtk_registrar =
fl_plugin_registry_get_registrar_for_plugin(registry, "GtkPlugin");
gtk_plugin_register_with_registrar(gtk_registrar);
g_autoptr(FlPluginRegistrar) url_launcher_linux_registrar =
fl_plugin_registry_get_registrar_for_plugin(registry, "UrlLauncherPlugin");
url_launcher_plugin_register_with_registrar(url_launcher_linux_registrar);
Expand Down
1 change: 1 addition & 0 deletions linux/flutter/generated_plugins.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
list(APPEND FLUTTER_PLUGIN_LIST
file_selector_linux
flutter_timezone
gtk
url_launcher_linux
)

Expand Down
2 changes: 2 additions & 0 deletions macos/Flutter/GeneratedPluginRegistrant.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import FlutterMacOS
import Foundation

import app_links
import connectivity_plus
import file_picker
import file_picker_writable
Expand All @@ -18,6 +19,7 @@ import sqflite_darwin
import url_launcher_macos

func RegisterGeneratedPlugins(registry: FlutterPluginRegistry) {
AppLinksMacosPlugin.register(with: registry.registrar(forPlugin: "AppLinksMacosPlugin"))
ConnectivityPlusPlugin.register(with: registry.registrar(forPlugin: "ConnectivityPlusPlugin"))
FilePickerPlugin.register(with: registry.registrar(forPlugin: "FilePickerPlugin"))
FilePickerWritablePlugin.register(with: registry.registrar(forPlugin: "FilePickerWritablePlugin"))
Expand Down
131 changes: 128 additions & 3 deletions test/api_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import 'package:http/http.dart' as http;
import 'package:sqflite_common_ffi/sqflite_ffi.dart';
import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart';
import 'package:taskwarrior/app/v3/db/task_database.dart';
import 'package:taskwarrior/app/v3/models/annotation.dart';
import 'package:taskwarrior/app/v3/models/task.dart';
import 'package:taskwarrior/app/v3/net/complete.dart';
import 'package:taskwarrior/app/v3/net/fetch.dart';
import 'package:taskwarrior/app/v3/net/origin.dart';

import 'api_service_test.mocks.dart';

Expand All @@ -27,7 +28,7 @@ void main() {

setUpAll(() {
sqfliteFfiInit();

// Mock SharedPreferences plugin
const MethodChannel('plugins.flutter.io/shared_preferences')
.setMockMethodCallHandler((MethodCall methodCall) async {
Expand Down Expand Up @@ -108,7 +109,7 @@ void main() {
var baseUrl = await CredentialsStorage.getApiUrl();
when(mockClient.get(
Uri.parse(
'$baseUrl/tasks?email=email&origin=$origin&UUID=123&encryptionSecret=secret'),
'$baseUrl/tasks?email=email&origin=$baseUrl&UUID=123&encryptionSecret=secret'),
headers: {
"Content-Type": "application/json",
})).thenAnswer((_) async => http.Response(responseJson, 200));
Expand Down Expand Up @@ -192,4 +193,128 @@ void main() {
expect(() => taskDatabase.fetchTasksFromDatabase(), throwsStateError);
});
});

group('TaskForC annotations', () {
test('fromJson parses annotations from JSON', () {
final json = {
'id': 1,
'description': 'Task with notes',
'project': null,
'status': 'pending',
'uuid': 'abc-123',
'urgency': 2.0,
'priority': null,
'due': null,
'end': null,
'entry': '2024-01-01',
'modified': null,
'annotations': [
{'entry': '2024-05-01', 'description': 'First note'},
{'entry': '2024-05-02', 'description': 'Second note'},
],
};

final task = TaskForC.fromJson(json);

expect(task.annotations, hasLength(2));
expect(task.annotations![0].entry, '2024-05-01');
expect(task.annotations![0].description, 'First note');
expect(task.annotations![1].description, 'Second note');
});

test('fromJson returns empty list when annotations are absent', () {
final json = {
'id': 1,
'description': 'Task no notes',
'project': null,
'status': 'pending',
'uuid': 'abc-456',
'urgency': 1.0,
'priority': null,
'due': null,
'end': null,
'entry': '2024-01-01',
'modified': null,
};

final task = TaskForC.fromJson(json);

expect(task.annotations, isEmpty);
});

test('fromJson returns empty list when annotations are null', () {
final json = {
'id': 1,
'description': 'Task null notes',
'project': null,
'status': 'pending',
'uuid': 'abc-789',
'urgency': 1.0,
'priority': null,
'due': null,
'end': null,
'entry': '2024-01-01',
'modified': null,
'annotations': null,
};

final task = TaskForC.fromJson(json);

expect(task.annotations, isEmpty);
});

test('toJson round-trips annotations', () {
final task = TaskForC(
id: 1,
description: 'Task',
project: null,
status: 'pending',
uuid: '123',
urgency: 1.0,
priority: null,
due: null,
end: null,
entry: '2024-01-01',
modified: null,
tags: [],
start: null,
wait: null,
rtype: null,
recur: null,
depends: [],
annotations: [
Annotation(entry: '2024-01-01', description: 'My note')
]);

final json = task.toJson();

expect(json['annotations'], isA<List>());
expect((json['annotations'] as List)[0]['description'], 'My note');
expect((json['annotations'] as List)[0]['entry'], '2024-01-01');
});
});

group('completeTask', () {
test('throws exception when server returns non-200', () async {
final mockClient = MockClient();
when(mockClient.post(
any,
headers: anyNamed('headers'),
body: anyNamed('body'),
)).thenAnswer((_) async => http.Response('Unauthorized', 401));

await expectLater(
completeTask('email', 'some-uuid', client: mockClient),
throwsException,
);
});
});

group('timeout constant regression', () {
test('credential-check timeout is 10 seconds not 10000', () {
const timeout = Duration(seconds: 10);
expect(timeout.inSeconds, equals(10));
expect(timeout.inMinutes, lessThan(1));
});
});
}
3 changes: 3 additions & 0 deletions windows/flutter/generated_plugin_registrant.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@

#include "generated_plugin_registrant.h"

#include <app_links/app_links_plugin_c_api.h>
#include <connectivity_plus/connectivity_plus_windows_plugin.h>
#include <file_selector_windows/file_selector_windows.h>
#include <flutter_timezone/flutter_timezone_plugin_c_api.h>
#include <permission_handler_windows/permission_handler_windows_plugin.h>
#include <url_launcher_windows/url_launcher_windows.h>

void RegisterPlugins(flutter::PluginRegistry* registry) {
AppLinksPluginCApiRegisterWithRegistrar(
registry->GetRegistrarForPlugin("AppLinksPluginCApi"));
ConnectivityPlusWindowsPluginRegisterWithRegistrar(
registry->GetRegistrarForPlugin("ConnectivityPlusWindowsPlugin"));
FileSelectorWindowsRegisterWithRegistrar(
Expand Down
Loading