feat(tasks): enhance CSV import with tag support and Todoist format#236
Merged
Conversation
Add support for importing tags from CSV files with new TAGS and LABELS columns. Implement automatic tag creation during import and update existing imports to use async processing. Add Todoist-specific import logic including tag extraction from content (@tag format) and improved error handling. Update UI with new translation keys and improve component structure for better maintainability.
Contributor
Summary of ChangesHello @ahmet-cetinkaya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the task CSV import feature by introducing robust tag support and a specialized import path for Todoist users. It aims to provide a more flexible and user-friendly experience for bringing external task data into the application, ensuring that task metadata like tags are preserved and hierarchies are correctly interpreted. The changes also include performance enhancements through asynchronous processing and a cleaner, more informative user interface for import settings. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Reviewer's GuideRefactors the settings import/export UI to a dedicated external import page while extending the task CSV import pipeline to support tags (generic TAGS column and Todoist LABELS/@tags), tightening error handling, and significantly expanding unit tests and translations for the new behavior. Sequence diagram for the enhanced CSV task import with tags and Todoist formatsequenceDiagram
actor User
participant ExternalImportPage as _ExternalImportPage
participant FileService as IFileService
participant AsyncErrorHandler
participant Mediator
participant ImportHandler as ImportTasksCommandHandler
participant TagQuery as GetListTagsQueryHandler
participant TagSave as SaveTagCommandHandler
participant TaskSave as SaveTaskCommandHandler
participant Overlay as OverlayNotificationHelper
User->>ExternalImportPage: tap import tasks
ExternalImportPage->>FileService: pickFile(csv)
FileService-->>ExternalImportPage: filePath
ExternalImportPage->>ExternalImportPage: setState(selectedTaskFilePath)
User->>ExternalImportPage: press import button
ExternalImportPage->>Overlay: showLoading(importInProgress)
ExternalImportPage->>AsyncErrorHandler: execute(operation: importTasks)
AsyncErrorHandler->>Mediator: send(ImportTasksCommand)
Mediator->>ImportHandler: handle(ImportTasksCommand)
loop rows in CSV
ImportHandler->>ImportHandler: _mapRowToSaveCommand(row, type, colIndices, parentId)
alt type == todoist
ImportHandler->>ImportHandler: _extractTodoistTags(content)
ImportHandler->>ImportHandler: _cleanTodoistTitle(content)
ImportHandler->>ImportHandler: _getOrCreateTagIds(combinedTags)
else type == generic
ImportHandler->>ImportHandler: _getOrCreateTagIds(TAGS)
end
loop tags
ImportHandler->>Mediator: send(GetListTagsQuery)
Mediator->>TagQuery: handle(GetListTagsQuery)
TagQuery-->>Mediator: GetListTagsQueryResponse
alt tag exists (case-insensitive)
Mediator-->>ImportHandler: existing Tag.id
else create new tag
ImportHandler->>Mediator: send(SaveTagCommand)
Mediator->>TagSave: handle(SaveTagCommand)
TagSave-->>Mediator: SaveTagCommandResponse(id)
Mediator-->>ImportHandler: new tag id
end
end
ImportHandler->>Mediator: send(SaveTaskCommand with tagIdsToAdd)
Mediator->>TaskSave: handle(SaveTaskCommand)
TaskSave-->>Mediator: SaveTaskCommandResponse(id)
Mediator-->>ImportHandler: SaveTaskCommandResponse
ImportHandler->>ImportHandler: update parentIdsByIndent
end
ImportHandler-->>Mediator: ImportTasksCommandResponse
Mediator-->>AsyncErrorHandler: ImportTasksCommandResponse
alt import success or partial success
AsyncErrorHandler-->>ExternalImportPage: response
ExternalImportPage->>Overlay: hideNotification
ExternalImportPage->>Overlay: showSuccess(message)
ExternalImportPage->>ExternalImportPage: Navigator.pop()
else error in AsyncErrorHandler
AsyncErrorHandler-->>ExternalImportPage: null
ExternalImportPage->>Overlay: showError(message)
end
ExternalImportPage->>ExternalImportPage: setState(_isProcessing = false)
Updated class diagram for ImportTasksCommandHandler and external import UIclassDiagram
direction LR
class ImportErrorIds {
<<static>> String fileReadError
<<static>> String csvParseError
<<static>> String missingRequiredColumn
<<static>> String dateParseError
<<static>> String invalidFilePath
<<static>> String mediatorError
}
class ImportErrorMessages {
<<static>> String fileNotFound
<<static>> String fileTooLarge
<<static>> String fileReadError
<<static>> String csvParseError
<<static>> String missingRequiredColumn
<<static>> String mediatorError
<<static>> String rowImportError
<<static>> String invalidFilePath
<<static>> String emptyFilePath
<<static>> String negativeCount
}
class TaskImportType {
<<enum>>
generic
todoist
}
class ImportTasksCommand {
+String filePath
+TaskImportType importType
+ImportTasksCommand(String filePath, TaskImportType importType)
}
class ImportTasksCommandResponse {
+int successCount
+int failureCount
+List~String~ errors
+ImportTasksCommandResponse(int successCount, int failureCount, List~String~ errors)
}
class ImportTasksCommandHandler {
-Mediator _mediator
+ImportTasksCommandHandler(Mediator mediator)
+Future~ImportTasksCommandResponse~ handle(ImportTasksCommand request)
-String~null~ _validateFilePath(String filePath)
-Map~String,int~ _getColumnIndices(TaskImportType type, List~dynamic~ header)
-int _getIndent(List~dynamic~ row, int~null~ indentIndex, TaskImportType type)
-Future~SaveTaskCommand~ _mapRowToSaveCommand(List~dynamic~ row, TaskImportType type, Map~String,int~ colIndices, String~null~ parentId)
-Future~SaveTaskCommand~ _mapTodoistRow(List~dynamic~ row, Map~String,int~ idx, String~null~ parentId)
-Future~SaveTaskCommand~ _mapGenericRow(List~dynamic~ row, Map~String,int~ idx)
-List~String~ _extractTodoistTags(String content)
-String _cleanTodoistTitle(String content)
-EisenhowerPriority _mapTodoistPriority(int todoistPriority)
-Future~List~String~~ _getOrCreateTagIds(String~null~ tagsString)
-DateTime~null~ _parseDate(String dateStr)
-void _addError(List~String~ errors, String message)
}
class SaveTaskCommand {
+String~null~ id
+String title
+String~null~ description
+EisenhowerPriority~null~ priority
+DateTime~null~ plannedDate
+DateTime~null~ deadlineDate
+String~null~ parentTaskId
+List~String~~null~ tagIdsToAdd
}
class SaveTaskCommandResponse {
+String id
}
class GetListTagsQuery {
+int pageIndex
+int pageSize
+String~null~ search
}
class GetListTagsQueryResponse {
+List~Tag~ items
}
class SaveTagCommand {
+String~null~ id
+String name
+TagType type
}
class SaveTagCommandResponse {
+String id
}
class Tag {
+String id
+String name
+TagType type
}
class TagType {
<<enum>>
label
other
}
class _ExternalImportPage {
+_ExternalImportPage()
+State createState()
}
class _ExternalImportPageState {
-ITranslationService _translationService
-IFileService _fileService
-String~null~ _selectedTaskFilePath
-TaskImportType _taskImportType
-bool _isProcessing
+Future~void~ _handleTaskFilePick(BuildContext context)
+Future~void~ _handleExternalImportExecute(BuildContext context)
+Widget build(BuildContext context)
}
class _ImportExportActionsDialogState {
-GlobalKey~NavigatorState~ _navigatorKey
-String~null~ _selectedFilePath
-ExportDataFileOptions~null~ _selectedExportOption
-bool _isProcessing
+Widget _buildExternalImportPage(BuildContext context)
}
class ITranslationService {
+String translate(String key)
+String translate(String key, Map~String,String~ namedArgs)
}
class IFileService {
+Future~String~ pickFile(List~String~ allowedExtensions, String dialogTitle)
}
class Mediator {
+Future~TResponse~ send~TRequest,TResponse~(TRequest request)
}
ImportTasksCommand ..> ImportErrorMessages : uses
ImportTasksCommandResponse ..> ImportErrorMessages : validates
ImportTasksCommandHandler ..> ImportTasksCommand : handles
ImportTasksCommandHandler ..> ImportTasksCommandResponse : returns
ImportTasksCommandHandler ..> SaveTaskCommand : creates
ImportTasksCommandHandler ..> GetListTagsQuery : queries
ImportTasksCommandHandler ..> SaveTagCommand : creates
ImportTasksCommandHandler ..> ImportErrorIds : logs
ImportTasksCommandHandler ..> ImportErrorMessages : logs
ImportTasksCommandHandler ..> Mediator : uses
SaveTagCommandResponse ..> Tag : refers
GetListTagsQueryResponse ..> Tag : contains
_ImportExportActionsDialogState --> _ExternalImportPage : builds
_ExternalImportPage --> _ExternalImportPageState : creates state
_ExternalImportPageState ..> ITranslationService : uses
_ExternalImportPageState ..> IFileService : uses
_ExternalImportPageState ..> Mediator : sends ImportTasksCommand
_ExternalImportPageState ..> ImportTasksCommand : constructs
_ExternalImportPageState ..> TaskImportType : selects format
Tag --> TagType : has type
SaveTagCommand --> TagType : sets type
SaveTaskCommand --> Tag : via tagIdsToAdd
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
_ExternalImportPage, the back button now usesNavigator.of(context).pop()instead of the dialog’s_navigatorKey.currentState?.pop(), which may close the entire dialog instead of only popping the inner route; consider using the same navigator key routing as before to preserve the previous navigation behavior. - The new tag resolution logic in
_getOrCreateTagIdsperforms a mediator query and possibly a save for every tag name encountered, which could be expensive for large imports; consider caching tagName → tagId within a single import run to avoid repeated lookups and creations for the same tag. - The
Logger.info('DEBUG: tags=$tags');line in_getOrCreateTagIdslooks like leftover debug logging; consider removing it or replacing it with a more structured, appropriately leveled log message if it’s needed in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_ExternalImportPage`, the back button now uses `Navigator.of(context).pop()` instead of the dialog’s `_navigatorKey.currentState?.pop()`, which may close the entire dialog instead of only popping the inner route; consider using the same navigator key routing as before to preserve the previous navigation behavior.
- The new tag resolution logic in `_getOrCreateTagIds` performs a mediator query and possibly a save for every tag name encountered, which could be expensive for large imports; consider caching tagName → tagId within a single import run to avoid repeated lookups and creations for the same tag.
- The `Logger.info('DEBUG: tags=$tags');` line in `_getOrCreateTagIds` looks like leftover debug logging; consider removing it or replacing it with a more structured, appropriately leveled log message if it’s needed in production.
## Individual Comments
### Comment 1
<location> `src/lib/core/application/features/tasks/commands/import_tasks_command.dart:499-508` </location>
<code_context>
+ Future<List<String>?> _getOrCreateTagIds(String? tagsString) async {
</code_context>
<issue_to_address>
**suggestion (performance):** Tag resolution does repeated per-tag queries without caching, which may become slow on larger imports.
Each tag in each row triggers a `GetListTagsQuery` (with `pageSize: 1`) and possibly `SaveTagCommand`, so imports with many rows/reused tags do O(rows * tags-per-row) mediator calls. Within a single import run, cache tagName → id (e.g., a `Map<String, String>` in the handler) so once a tag is resolved/created you reuse its id for subsequent rows, avoiding redundant queries and speeding up large imports.
Suggested implementation:
```
final List<String> tagIds = [];
for (final tagName in tags) {
// Reuse cached tag id within a single import run if available
if (_tagIdCache.containsKey(tagName)) {
tagIds.add(_tagIdCache[tagName]!);
continue;
}
// Search for existing tag
final query = GetListTagsQuery(
```
1. Add a cache field to the command handler class that owns `_getOrCreateTagIds`, for example:
- `final Map<String, String> _tagIdCache = {};`
Place it as a private field next to the other dependencies/fields (e.g., near the mediator).
2. Wherever in `_getOrCreateTagIds` you currently:
- Resolve an existing tag id from the `GetListTagsQuery` result, and
- Create a new tag via `SaveTagCommand` (or similar),
add lines to populate the cache, e.g.:
- After finding an existing tag: `_tagIdCache[tagName] = resolvedTagId;`
- After creating a new tag: `_tagIdCache[tagName] = createdTagId;`
3. Ensure `_tagIdCache` lives for the duration of a single import run (i.e., per handler instance or per import command execution) so that multiple rows share the same cache, but it is not reused across independent imports.
</issue_to_address>
### Comment 2
<location> `src/lib/core/application/features/tasks/commands/import_tasks_command.dart:503` </location>
<code_context>
+ if (tagsString == null || tagsString.trim().isEmpty) return null;
+
+ final tags = tagsString.split(',').map((t) => t.trim()).where((t) => t.isNotEmpty).toList();
+ Logger.info('DEBUG: tags=$tags');
+ if (tags.isEmpty) return null;
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** The hardcoded 'DEBUG' log message may be too noisy for production and doesn’t add much value.
This logs the tags for every imported row, which will bloat logs and may leak user data. If this isn’t strictly temporary, please remove it or hide it behind a dedicated debug/trace flag.
Suggested implementation:
```
Future<List<String>?> _getOrCreateTagIds(String? tagsString) async {
if (tagsString == null || tagsString.trim().isEmpty) return null;
final tags = tagsString.split(',').map((t) => t.trim()).where((t) => t.isNotEmpty).toList();
```
```
final tags = tagsString.split(',').map((t) => t.trim()).where((t) => t.isNotEmpty).toList();
if (tags.isEmpty) return null;
```
</issue_to_address>
### Comment 3
<location> `src/lib/core/application/features/tasks/commands/import_tasks_command.dart:429-431` </location>
<code_context>
);
}
+ List<String> _extractTodoistTags(String content) {
+ final tagRegex = RegExp(r'@(\S+)');
+ return tagRegex.allMatches(content).map((m) => m.group(1)!).toList();
+ }
+
</code_context>
<issue_to_address>
**issue:** Todoist tag extraction may include trailing punctuation or other separators in the tag names.
Because `@(
\S+)` matches all non-whitespace after `@`, content like `"Buy milk @groceries,"` yields the tag `"groceries,"` (including the comma), which is then used for lookup/creation. If punctuation commonly follows tags, consider either tightening the regex (e.g., stop at punctuation) or normalizing the captured value by stripping trailing punctuation before using it as a tag name.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the CSV import functionality by adding support for tags for both generic and Todoist-specific formats. The implementation includes parsing tags from CSV columns and from the task content for Todoist imports, with logic to create new tags if they don't exist. The UI for import/export has been refactored into a separate, more maintainable widget, which is a great improvement. The test suite has also been expanded to cover the new tag functionality and to improve existing test cases.
My main feedback is on the implementation of the tag creation logic, where a piece of code could be refactored for better readability and maintainability. Overall, this is a solid contribution that adds valuable functionality and improves the codebase.
Comment on lines
+517
to
+527
| final existingTag = response.items.isEmpty | ||
| ? null | ||
| : response.items.firstWhere( | ||
| (t) => t.name.toLowerCase() == tagName.toLowerCase(), | ||
| orElse: () => response.items.first, // Fallback to first if only one result and it's close? | ||
| // Actually firstWhere with orElse is safer. | ||
| ); | ||
|
|
||
| // Re-verify if the name matches exactly (case-insensitive) | ||
| if (existingTag != null && existingTag.name.toLowerCase() == tagName.toLowerCase()) { | ||
| tagIds.add(existingTag.id); |
Contributor
There was a problem hiding this comment.
The logic to find an existing tag is a bit complex and hard to read due to the orElse clause and the subsequent re-verification. The comments also indicate some uncertainty. This can be simplified for better clarity and maintainability by using cast and firstWhere with an orElse that returns null.
final existingTag = response.items.cast<TagListItem?>().firstWhere(
(t) => t!.name.toLowerCase() == tagName.toLowerCase(),
orElse: () => null,
);
if (existingTag != null) {
tagIds.add(existingTag.id);Add in-memory cache for resolved tag IDs during CSV import operations. This prevents redundant database queries for tags that appear multiple times across imported rows, significantly improving performance on large imports. - Add _tagIdCache field to ImportTasksCommandHandler - Cache tag IDs by normalized name (lowercase) - Populate cache after successful lookup or creation - Check cache before querying for existing tags Closes issue raised in PR review about repeated per-tag queries.
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Motivation and Context
This PR enhances the CSV import functionality for tasks by adding support for tags. It implements Todoist-specific import logic with automatic tag creation and improves the overall UX structure of the import/export settings component.
⚙️ Implementation Details
📋 Checklist for Reviewer
🔗 Related
Closes #233