diff --git a/NOTES.md b/NOTES.md index 5cef095..f7223a3 100644 --- a/NOTES.md +++ b/NOTES.md @@ -1,5 +1,10 @@ # Notes +## 2026-05-14 (issue #289) +- `_needsReviewWhere`'s actionless branch now ANDs on `NOT EXISTS person tag`. Delegated tasks (any person-typed `todo_tags` row) are excluded from the daily re-clarification surface — waiting-for cadence belongs to the weekly review, not the daily card. The stale branch is untouched, so a delegated task you nudged today still surfaces. +- `readsFrom` for the three `_needsReviewWhere` callers (`watchNeedsReview`, `getNeedsReview`/`getNeedsReviewCount`/`isNeedsReview`) widened from `{todos}` to `{todos, todoTags, tags}`. Without the widening, Drift's stream invalidation doesn't fire when a person tag is attached/detached, and the live daily card would not drop a newly-tagged task until the next cold reload. +- `FocusSessionPlanningState.reviewPersonTags` is loaded alongside the review snapshot in `advanceStep` (one batched `getPersonTagsForTodos` query) so the new `staleWaitingFor` card variant can name the delegate without a per-frame DAO call. + ## 2026-05-14 (issue #290) - Weekly Review's third step renamed from Projects → Next Actions and broadened to iterate **all** active next actions excluding person-tagged ones (which Waiting For already covers). The disjointness invariant — each task surfaces in at most one wizard step — is now enforced at the SQL layer via `TodoDao.getNextActionsExcludingPersonTagged()`. The old `getNextActionsWithProjectTags()` and `Projects*` symbols are gone. A future dedicated Projects step is deferred until full Projects support lands; resurrecting it will require re-establishing the disjointness matrix (project-tagged ⊂ next-actions). diff --git a/app/lib/database/daos/todo_dao.dart b/app/lib/database/daos/todo_dao.dart index a7e0f52..e54a8cd 100644 --- a/app/lib/database/daos/todo_dao.dart +++ b/app/lib/database/daos/todo_dao.dart @@ -388,19 +388,28 @@ AND ( (last_next_action_completion_at IS NOT NULL AND (last_clarified_at IS NULL OR last_clarified_at < last_next_action_completion_at)) - OR next_action_text IS NULL - OR TRIM(next_action_text) = '' + OR ( + (next_action_text IS NULL OR TRIM(next_action_text) = '') + AND NOT EXISTS ( + SELECT 1 FROM todo_tags tt + JOIN tags tg ON tg.id = tt.tag_id + WHERE tt.todo_id = todos.id AND tg.type = 'person' + ) + ) )'''; /// Stream of tasks needing re-clarification: Stale or Actionless per spec. /// /// Stale: worked on in a session more recently than last clarified. - /// Actionless: no next action defined (regardless of session history). + /// Actionless: no next action defined AND not delegated. Delegated tasks + /// (carrying any person-typed tag) are excluded from the actionless branch — + /// their cadence belongs to the weekly Waiting For review, not the daily + /// re-clarification surface. Stream> watchNeedsReview() { return customSelect( 'SELECT * FROM todos WHERE $_needsReviewWhere ORDER BY created_at', variables: [], - readsFrom: {todos}, + readsFrom: {todos, todoTags, tags}, ).watch().map((rows) => rows.map((r) => todos.map(r.data)).toList()); } @@ -430,7 +439,7 @@ AND ( return customSelect( 'SELECT * FROM todos WHERE $_needsReviewWhere ORDER BY created_at', variables: [], - readsFrom: {todos}, + readsFrom: {todos, todoTags, tags}, ).get().then((rows) => rows.map((r) => todos.map(r.data)).toList()); } @@ -440,7 +449,7 @@ AND ( final rows = await customSelect( 'SELECT COUNT(*) AS cnt FROM todos WHERE $_needsReviewWhere', variables: [], - readsFrom: {todos}, + readsFrom: {todos, todoTags, tags}, ).get(); return rows.first.read('cnt'); } @@ -523,7 +532,7 @@ AND ( final rows = await customSelect( 'SELECT 1 FROM todos WHERE id = ? AND $_needsReviewWhere', variables: [Variable(todoId)], - readsFrom: {todos}, + readsFrom: {todos, todoTags, tags}, ).get(); return rows.isNotEmpty; } diff --git a/app/lib/providers/focus_session_planning_provider.dart b/app/lib/providers/focus_session_planning_provider.dart index 7108106..599b6a7 100644 --- a/app/lib/providers/focus_session_planning_provider.dart +++ b/app/lib/providers/focus_session_planning_provider.dart @@ -28,7 +28,7 @@ import 'database_provider.dart'; import 'synced_preferences_provider.dart'; import 'tag_filter_provider.dart'; -export '../database/gtd_database.dart' show Todo, FocusSession; +export '../database/gtd_database.dart' show Todo, FocusSession, Tag; // --------------------------------------------------------------------------- // Date helpers @@ -272,6 +272,7 @@ class FocusSessionPlanningState { this.reviewedTaskIds = const [], this.reviewNav = const SnapshotNav(), this.reviewActions = const {}, + this.reviewPersonTags = const {}, }); final int currentStep; @@ -316,6 +317,12 @@ class FocusSessionPlanningState { /// when the user navigates back within the review step. final Map reviewActions; + /// Person-typed tags for each task in [reviewNav.items], keyed by task id. + /// Loaded alongside the review snapshot so the card can render the delegate + /// name(s) for the stale waiting-for variant without a per-frame DAO call. + /// Tasks with no person tag are absent from the map. + final Map> reviewPersonTags; + FocusSessionPlanningState copyWith({ int? currentStep, int? availableMinutes, @@ -328,6 +335,7 @@ class FocusSessionPlanningState { List? reviewedTaskIds, SnapshotNav? reviewNav, Map? reviewActions, + Map>? reviewPersonTags, }) => FocusSessionPlanningState( currentStep: currentStep ?? this.currentStep, @@ -341,6 +349,7 @@ class FocusSessionPlanningState { reviewedTaskIds: reviewedTaskIds ?? this.reviewedTaskIds, reviewNav: reviewNav ?? this.reviewNav, reviewActions: reviewActions ?? this.reviewActions, + reviewPersonTags: reviewPersonTags ?? this.reviewPersonTags, ); } @@ -407,9 +416,16 @@ class FocusSessionPlanningNotifier extends Notifier { if (items.isEmpty) { next = 2; } else { + // Batched person-tag lookup so the stale waiting-for card variant + // can render delegate names without a per-frame DAO call. + final personTags = await _db.todoDao.getPersonTagsForTodos( + items.map((t) => t.id).toSet(), + ); + if (!ref.mounted) return; state = state.copyWith( reviewNav: SnapshotNav(items: items), reviewActions: {}, + reviewPersonTags: personTags, ); } } diff --git a/app/lib/screens/planning/steps/task_review_step.dart b/app/lib/screens/planning/steps/task_review_step.dart index 45213e1..4d1f2fa 100644 --- a/app/lib/screens/planning/steps/task_review_step.dart +++ b/app/lib/screens/planning/steps/task_review_step.dart @@ -2,10 +2,12 @@ /// /// Surfaces tasks needing re-clarification one at a time: /// - Stale tasks: worked on in a session more recently than last clarified. -/// - Actionless tasks: no next action defined. +/// - Actionless tasks: no next action defined (excluding delegated tasks). /// /// Context-aware action menu: -/// - Stale tasks show "Still relevant" (stamps last_clarified_at, clears stale). +/// - Stale tasks without a delegate show "Still relevant" (stamps +/// last_clarified_at, clears stale). +/// - Stale delegated tasks show "Still waiting" (same write, waiting-for copy). /// - Actionless tasks omit "Still relevant" — defining an action is required. library; @@ -20,15 +22,39 @@ import '../../../widgets/process_to_handlers.dart'; // Hint enum — derived from task state // --------------------------------------------------------------------------- -enum ReclarifyHint { noNextAction, updatedSinceClarified } +enum ReclarifyHint { + noNextAction, + updatedSinceClarified, + staleWaitingFor, +} + +/// Returns true when [t]'s session timestamps mark it as stale per the +/// `_needsReviewWhere` stale branch: a session completion happened after the +/// last clarification stamp. +bool isStaleReclarification(Todo t) => + t.lastNextActionCompletionAt != null && + (t.lastClarifiedAt == null || + t.lastClarifiedAt!.isBefore(t.lastNextActionCompletionAt!)); /// Mirror of [TodoDao] `_needsReviewWhere`'s actionless predicate: /// `next_action_text IS NULL OR TRIM(next_action_text) = ''`. Whitespace-only /// values land rows in the queue, so the hint must agree (#278). -ReclarifyHint hintFor(Todo t) => - (t.nextActionText == null || t.nextActionText!.trim().isEmpty) - ? ReclarifyHint.noNextAction - : ReclarifyHint.updatedSinceClarified; +/// +/// [hasPersonTag] and [isStale] are computed by the caller (the widget reads +/// person tags from [FocusSessionPlanningState.reviewPersonTags] and stale +/// from [isStaleReclarification]). Keeping the helper pure makes it +/// trivially unit-testable. +ReclarifyHint hintFor( + Todo t, { + required bool hasPersonTag, + required bool isStale, +}) { + if (isStale && hasPersonTag) return ReclarifyHint.staleWaitingFor; + if (t.nextActionText == null || t.nextActionText!.trim().isEmpty) { + return ReclarifyHint.noNextAction; + } + return ReclarifyHint.updatedSinceClarified; +} // --------------------------------------------------------------------------- // Step widget @@ -47,10 +73,16 @@ class TaskReviewStep extends ConsumerWidget { } final task = nav.current!; + final personTags = state.reviewPersonTags[task.id] ?? const []; return _ReviewCard( key: ValueKey(task.id), task: task, - hint: hintFor(task), + hint: hintFor( + task, + hasPersonTag: personTags.isNotEmpty, + isStale: isStaleReclarification(task), + ), + personTags: personTags, previousAction: state.reviewActions[nav.index], ); } @@ -65,12 +97,17 @@ class _ReviewCard extends ConsumerWidget { super.key, required this.task, required this.hint, + this.personTags = const [], this.previousAction, }); final Todo task; final ReclarifyHint hint; + /// Person-typed tags assigned to [task]. Drives the delegate name(s) in + /// the [ReclarifyHint.staleWaitingFor] badge. + final List personTags; + /// Action recorded for this index on a previous pass — drives selection /// affordances and pre-fills dialogs when the user navigates back. final ReviewActionRecord? previousAction; @@ -78,6 +115,32 @@ class _ReviewCard extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { final isActionless = hint == ReclarifyHint.noNextAction; + final isWaitingFor = hint == ReclarifyHint.staleWaitingFor; + final showKeep = !isActionless; + + final badgeBg = isActionless + ? const Color(0xFFFEF3C7) + : const Color(0xFFEFF6FF); + final badgeBorder = isActionless + ? const Color(0xFFFDE68A) + : const Color(0xFFDBEAFE); + final badgeIconColor = isActionless + ? const Color(0xFFD97706) + : const Color(0xFF2563EB); + final badgeTextColor = isActionless + ? const Color(0xFF92400E) + : const Color(0xFF1D4ED8); + final badgeIcon = switch (hint) { + ReclarifyHint.noNextAction => Icons.warning_amber_outlined, + ReclarifyHint.updatedSinceClarified => Icons.update_outlined, + ReclarifyHint.staleWaitingFor => Icons.person_outline, + }; + final badgeText = switch (hint) { + ReclarifyHint.noNextAction => 'No next action defined', + ReclarifyHint.updatedSinceClarified => 'Updated since last clarified', + ReclarifyHint.staleWaitingFor => + 'Waiting for ${personTags.map((t) => t.name).join(', ')} — still waiting?', + }; return ListView( physics: const ClampingScrollPhysics(), @@ -87,39 +150,21 @@ class _ReviewCard extends ConsumerWidget { Container( padding: const EdgeInsets.all(12), decoration: BoxDecoration( - color: isActionless - ? const Color(0xFFFEF3C7) - : const Color(0xFFEFF6FF), + color: badgeBg, borderRadius: BorderRadius.circular(10), - border: Border.all( - color: isActionless - ? const Color(0xFFFDE68A) - : const Color(0xFFDBEAFE), - ), + border: Border.all(color: badgeBorder), ), child: Row( children: [ - Icon( - isActionless - ? Icons.warning_amber_outlined - : Icons.update_outlined, - size: 18, - color: isActionless - ? const Color(0xFFD97706) - : const Color(0xFF2563EB), - ), + Icon(badgeIcon, size: 18, color: badgeIconColor), const SizedBox(width: 8), Expanded( child: Text( - isActionless - ? 'No next action defined' - : 'Updated since last clarified', + badgeText, style: TextStyle( fontSize: 14, fontWeight: FontWeight.w600, - color: isActionless - ? const Color(0xFF92400E) - : const Color(0xFF1D4ED8), + color: badgeTextColor, ), ), ), @@ -147,7 +192,7 @@ class _ReviewCard extends ConsumerWidget { ), ], - // Current next action (if stale) + // Current next action (if present) if (!isActionless && task.nextActionText != null) ...[ const SizedBox(height: 8), Row( @@ -172,8 +217,9 @@ class _ReviewCard extends ConsumerWidget { const SizedBox(height: 28), // Action buttons — defaults plus the dialog modifier on Next. - // Stale tasks add "Still relevant" (keep with a custom label); - // Actionless tasks omit it because defining an action is required. + // Stale tasks add a "keep" variant (label depends on whether the task + // is delegated); Actionless tasks omit it because defining an action + // is required. const _FieldLabel('WHAT DO YOU WANT TO DO?'), const SizedBox(height: 12), @@ -181,10 +227,11 @@ class _ReviewCard extends ConsumerWidget { todo: task, include: { ProcessAction.nextActionDialog, - if (!isActionless) ProcessAction.keep, + if (showKeep) ProcessAction.keep, }, - labels: const { - ProcessAction.keep: 'Still relevant', + labels: { + ProcessAction.keep: + isWaitingFor ? 'Still waiting' : 'Still relevant', ProcessAction.next: 'Update next action…', }, lastAction: _toProcessAction(previousAction?.kind), diff --git a/app/test/database/todo_dao_needs_review_test.dart b/app/test/database/todo_dao_needs_review_test.dart index adf4c54..734a821 100644 --- a/app/test/database/todo_dao_needs_review_test.dart +++ b/app/test/database/todo_dao_needs_review_test.dart @@ -37,6 +37,28 @@ Future _insertClarifiedTask( return id; } +/// Inserts a person-typed tag and links it to [todoId]. Returns the tag id. +Future _attachPersonTag( + GtdDatabase db, + String todoId, { + String name = 'Trixy', +}) async { + final tagId = 'ptag-${DateTime.now().microsecondsSinceEpoch}-$todoId'; + await db.into(db.tags).insert(TagsCompanion( + id: Value(tagId), + name: Value(name), + type: const Value('person'), + userId: Value(_userId), + )); + await db.into(db.todoTags).insert(TodoTagsCompanion( + id: Value('tt-$tagId-$todoId'), + todoId: Value(todoId), + tagId: Value(tagId), + userId: Value(_userId), + )); + return tagId; +} + void main() { setUpAll(configureSqliteForTests); @@ -335,5 +357,118 @@ void main() { expect(await db.todoDao.isNeedsReview(id), isFalse); }); + + // ----- Delegated (person-tagged) actionless branch (#289) ---------------- + + // Delegated + actionless task does NOT surface — waiting-for cadence + // belongs to the weekly review, not the daily re-clarification surface. + test('delegated actionless task — not in result', () async { + final id = await _insertClarifiedTask( + db, + nextActionText: null, + lastNextActionCompletionAt: null, + ); + await _attachPersonTag(db, id); + + final result = await db.todoDao.watchNeedsReview().first; + expect(result, isEmpty); + expect(await db.todoDao.getNeedsReviewCount(), 0); + expect(await db.todoDao.isNeedsReview(id), isFalse); + }); + + // Delegated + whitespace-only next_action_text — also excluded (guards + // the TRIM(...) = '' branch). + test('delegated whitespace-only next_action_text task — not in result', + () async { + final id = await _insertClarifiedTask( + db, + nextActionText: ' ', + lastNextActionCompletionAt: null, + ); + await _attachPersonTag(db, id); + + final result = await db.todoDao.watchNeedsReview().first; + expect(result, isEmpty); + expect(await db.todoDao.isNeedsReview(id), isFalse); + }); + + // Delegated + stale (lastNextActionCompletionAt > lastClarifiedAt) DOES + // surface — the stale branch fires regardless of person-tag presence. + test('delegated stale task — in result (stale branch unaffected)', + () async { + final clarifiedAt = + DateTime.now().subtract(const Duration(hours: 2)).toUtc(); + final completedAt = + DateTime.now().subtract(const Duration(hours: 1)).toUtc(); + final id = await _insertClarifiedTask( + db, + nextActionText: 'Draft email', + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + ); + await _attachPersonTag(db, id); + + final result = await db.todoDao.watchNeedsReview().first; + expect(result, hasLength(1)); + expect(result.first.id, id); + expect(await db.todoDao.isNeedsReview(id), isTrue); + }); + + // Delegated + stale + actionless DOES surface — the stale branch fires + // even when the task is delegated and has no next-action phrase. + test('delegated stale actionless task — in result (stale branch fires)', + () async { + final clarifiedAt = + DateTime.now().subtract(const Duration(hours: 2)).toUtc(); + final completedAt = + DateTime.now().subtract(const Duration(hours: 1)).toUtc(); + final id = await _insertClarifiedTask( + db, + nextActionText: null, + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + ); + await _attachPersonTag(db, id); + + final result = await db.todoDao.watchNeedsReview().first; + expect(result, hasLength(1)); + expect(result.first.id, id); + }); + + // Stream invalidation: attaching a person tag to an actionless task must + // remove it from the live stream. Locks in the readsFrom widening to + // {todos, todoTags, tags}. + test( + 'stream invalidates on person-tag attach — actionless task disappears', + () async { + final id = await _insertClarifiedTask( + db, + nextActionText: null, + lastNextActionCompletionAt: null, + ); + + final emissions = >[]; + final sub = db.todoDao.watchNeedsReview().listen(emissions.add); + + // Let the initial emission settle. + await Future.delayed(const Duration(milliseconds: 50)); + expect(emissions, isNotEmpty, + reason: 'stream should have emitted an initial value'); + expect(emissions.last.any((t) => t.id == id), isTrue, + reason: 'baseline: actionless task should surface'); + + // Attach a person tag — the predicate must now exclude this row. + await _attachPersonTag(db, id); + await Future.delayed(const Duration(milliseconds: 50)); + await sub.cancel(); + + expect(emissions.length, greaterThanOrEqualTo(2), + reason: + 'stream should re-emit after person-tag attach (proves readsFrom ' + 'covers todo_tags/tags)'); + expect(emissions.last.any((t) => t.id == id), isFalse, + reason: + 'after person-tag attach, actionless+delegated task must leave the stream'); + }); }); } diff --git a/app/test/screens/planning/steps/task_review_step_hint_test.dart b/app/test/screens/planning/steps/task_review_step_hint_test.dart index 135c08d..2cf45fd 100644 --- a/app/test/screens/planning/steps/task_review_step_hint_test.dart +++ b/app/test/screens/planning/steps/task_review_step_hint_test.dart @@ -3,7 +3,11 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:jeeves/database/gtd_database.dart'; import 'package:jeeves/screens/planning/steps/task_review_step.dart'; -Todo _todo({String? nextActionText}) { +Todo _todo({ + String? nextActionText, + DateTime? lastNextActionCompletionAt, + DateTime? lastClarifiedAt, +}) { final now = DateTime.now(); return Todo( id: 't', @@ -15,18 +19,24 @@ Todo _todo({String? nextActionText}) { userId: 'u', timeSpentMinutes: 0, nextActionText: nextActionText, + lastNextActionCompletionAt: lastNextActionCompletionAt, + lastClarifiedAt: lastClarifiedAt, ); } void main() { group('hintFor — actionless detection (#278)', () { test('null next_action_text → noNextAction', () { - expect(hintFor(_todo()), ReclarifyHint.noNextAction); + expect( + hintFor(_todo(), hasPersonTag: false, isStale: false), + ReclarifyHint.noNextAction, + ); }); test('whitespace-only next_action_text → noNextAction', () { expect( - hintFor(_todo(nextActionText: ' ')), + hintFor(_todo(nextActionText: ' '), + hasPersonTag: false, isStale: false), ReclarifyHint.noNextAction, reason: 'TodoDao._needsReviewWhere matches both NULL and TRIM("") = "", so ' @@ -35,21 +45,122 @@ void main() { }); test('tabs and newlines → noNextAction', () { - expect(hintFor(_todo(nextActionText: '\t\n ')), ReclarifyHint.noNextAction); + expect( + hintFor(_todo(nextActionText: '\t\n '), + hasPersonTag: false, isStale: false), + ReclarifyHint.noNextAction, + ); }); test('non-empty text → updatedSinceClarified', () { expect( - hintFor(_todo(nextActionText: 'Call Trixy')), + hintFor(_todo(nextActionText: 'Call Trixy'), + hasPersonTag: false, isStale: true), ReclarifyHint.updatedSinceClarified, ); }); test('text with leading/trailing whitespace → updatedSinceClarified', () { expect( - hintFor(_todo(nextActionText: ' Call Trixy ')), + hintFor(_todo(nextActionText: ' Call Trixy '), + hasPersonTag: false, isStale: true), + ReclarifyHint.updatedSinceClarified, + ); + }); + }); + + group('hintFor — staleWaitingFor branch (#289)', () { + test( + 'stale + has person tag + has next-action text → staleWaitingFor (wins ' + 'over updatedSinceClarified)', () { + expect( + hintFor( + _todo(nextActionText: 'Email Trixy'), + hasPersonTag: true, + isStale: true, + ), + ReclarifyHint.staleWaitingFor, + ); + }); + + test( + 'stale + has person tag + no next-action text → staleWaitingFor (wins ' + 'over noNextAction)', () { + expect( + hintFor( + _todo(), + hasPersonTag: true, + isStale: true, + ), + ReclarifyHint.staleWaitingFor, + ); + }); + + test('not stale + has person tag → falls through (helper must be total)', + () { + // Predicate prevents this combo from reaching hintFor in practice; the + // helper still has to return something coherent. + expect( + hintFor(_todo(nextActionText: 'X'), + hasPersonTag: true, isStale: false), ReclarifyHint.updatedSinceClarified, ); + expect( + hintFor(_todo(), hasPersonTag: true, isStale: false), + ReclarifyHint.noNextAction, + ); + }); + + test('stale + no person tag → updatedSinceClarified (unchanged)', () { + expect( + hintFor( + _todo(nextActionText: 'Draft email'), + hasPersonTag: false, + isStale: true, + ), + ReclarifyHint.updatedSinceClarified, + ); + }); + }); + + group('isStaleReclarification', () { + test('null lastNextActionCompletionAt → not stale', () { + expect(isStaleReclarification(_todo()), isFalse); + }); + + test( + 'lastNextActionCompletionAt set, lastClarifiedAt null → stale', + () { + expect( + isStaleReclarification(_todo( + lastNextActionCompletionAt: DateTime.now(), + )), + isTrue, + ); + }); + + test('clarified before completion → stale', () { + final now = DateTime.now(); + expect( + isStaleReclarification(_todo( + lastClarifiedAt: now.subtract(const Duration(hours: 2)), + lastNextActionCompletionAt: + now.subtract(const Duration(hours: 1)), + )), + isTrue, + ); + }); + + test('clarified after completion → not stale', () { + final now = DateTime.now(); + expect( + isStaleReclarification(_todo( + lastClarifiedAt: now, + lastNextActionCompletionAt: + now.subtract(const Duration(hours: 1)), + )), + isFalse, + ); }); }); } diff --git a/app/test/screens/planning/steps/task_review_step_waiting_for_test.dart b/app/test/screens/planning/steps/task_review_step_waiting_for_test.dart new file mode 100644 index 0000000..ff3f37e --- /dev/null +++ b/app/test/screens/planning/steps/task_review_step_waiting_for_test.dart @@ -0,0 +1,222 @@ +/// Widget tests for the [TaskReviewStep] stale-waiting-for branch (#289). +/// +/// Drives the real notifier with an in-memory [GtdDatabase] so the snapshot, +/// person-tag plumbing, and card render exercise the production path. +library; + +import 'package:drift/drift.dart' hide isNotNull, isNull; +import 'package:drift/native.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:jeeves/database/gtd_database.dart'; +import 'package:jeeves/providers/database_provider.dart'; +import 'package:jeeves/providers/focus_session_planning_provider.dart'; +import 'package:jeeves/screens/planning/steps/task_review_step.dart'; + +import '../../../test_helpers.dart'; + +const _userId = 'local'; + +const _staleTaskClarifiedOffset = Duration(hours: 2); +const _staleTaskCompletedOffset = Duration(hours: 1); + +GtdDatabase _openInMemory() => GtdDatabase(NativeDatabase.memory()); + +Future _insertClarifiedTask( + GtdDatabase db, { + String? nextActionText, + DateTime? lastNextActionCompletionAt, + DateTime? lastClarifiedAt, + String title = 'Task', +}) async { + final now = DateTime.now(); + final id = 'task-${now.microsecondsSinceEpoch}-$title'; + await db.into(db.todos).insert(TodosCompanion( + id: Value(id), + title: Value(title), + userId: const Value(_userId), + clarified: const Value(true), + intent: const Value('next'), + createdAt: Value(now), + nextActionText: + nextActionText != null ? Value(nextActionText) : const Value.absent(), + lastNextActionCompletionAt: lastNextActionCompletionAt != null + ? Value(lastNextActionCompletionAt) + : const Value.absent(), + lastClarifiedAt: + lastClarifiedAt != null ? Value(lastClarifiedAt) : const Value.absent(), + )); + return id; +} + +Future _attachPersonTag( + GtdDatabase db, + String todoId, { + required String name, + required String tagId, +}) async { + await db.into(db.tags).insert(TagsCompanion( + id: Value(tagId), + name: Value(name), + type: const Value('person'), + userId: const Value(_userId), + )); + await db.into(db.todoTags).insert(TodoTagsCompanion( + id: Value('tt-$tagId-$todoId'), + todoId: Value(todoId), + tagId: Value(tagId), + userId: const Value(_userId), + )); +} + +Widget _harness(GtdDatabase db) { + return ProviderScope( + overrides: [ + databaseProvider.overrideWithValue(db), + ], + child: const MaterialApp( + home: Scaffold(body: TaskReviewStep()), + ), + ); +} + +/// Loads the review snapshot via the real notifier so the card renders against +/// the same plumbing the planning ritual uses in production. +Future _enterReviewStep(WidgetTester tester, GtdDatabase db) async { + await tester.pumpWidget(_harness(db)); + // Grab the notifier from the running ProviderScope element. + final container = ProviderScope.containerOf( + tester.element(find.byType(TaskReviewStep)), + ); + await container.read(focusSessionPlanningProvider.notifier).advanceStep(); + await tester.pumpAndSettle(); +} + +void main() { + setUpAll(configureSqliteForTests); + + group('TaskReviewStep — staleWaitingFor variant (#289)', () { + late GtdDatabase db; + + setUp(() => db = _openInMemory()); + tearDown(() async => db.close()); + + testWidgets( + 'stale delegated task renders waiting-for badge with delegate name ' + 'and "Still waiting" keep label', (tester) async { + final clarifiedAt = + DateTime.now().subtract(_staleTaskClarifiedOffset).toUtc(); + final completedAt = + DateTime.now().subtract(_staleTaskCompletedOffset).toUtc(); + final id = await _insertClarifiedTask( + db, + nextActionText: 'Email Trixy', + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + title: 'Waiting on Trixy', + ); + await _attachPersonTag(db, id, name: 'Trixy', tagId: 'p-trixy'); + + await _enterReviewStep(tester, db); + + expect(find.textContaining('Waiting for Trixy'), findsOneWidget, + reason: 'badge should name the delegate'); + expect(find.textContaining('still waiting?'), findsOneWidget, + reason: 'badge should pose the keep prompt'); + + // Keep button is relabelled "Still waiting" for the delegated variant. + expect(find.text('Still waiting'), findsOneWidget); + expect(find.text('Still relevant'), findsNothing); + }); + + testWidgets( + 'stale non-delegated task still renders "Still relevant" keep label', + (tester) async { + final clarifiedAt = + DateTime.now().subtract(_staleTaskClarifiedOffset).toUtc(); + final completedAt = + DateTime.now().subtract(_staleTaskCompletedOffset).toUtc(); + await _insertClarifiedTask( + db, + nextActionText: 'Draft email', + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + title: 'No delegate stale', + ); + + await _enterReviewStep(tester, db); + + expect(find.text('Still relevant'), findsOneWidget); + expect(find.text('Still waiting'), findsNothing); + expect(find.text('Updated since last clarified'), findsOneWidget); + }); + + testWidgets( + 'actionless non-delegated task omits both keep variants', + (tester) async { + await _insertClarifiedTask( + db, + nextActionText: null, + title: 'Actionless', + ); + + await _enterReviewStep(tester, db); + + expect(find.text('Still relevant'), findsNothing); + expect(find.text('Still waiting'), findsNothing); + expect(find.text('No next action defined'), findsOneWidget); + }); + + testWidgets( + 'recently-clarified delegated task is filtered out of the review queue', + (tester) async { + // Same shape as the stale-delegated case, but lastClarifiedAt is AFTER + // lastNextActionCompletionAt — so the staleness filter must exclude it. + final completedAt = + DateTime.now().subtract(_staleTaskCompletedOffset).toUtc(); + final clarifiedAt = DateTime.now().toUtc(); + final id = await _insertClarifiedTask( + db, + nextActionText: 'Email Trixy', + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + title: 'Recently re-clarified', + ); + await _attachPersonTag(db, id, name: 'Trixy', tagId: 'p-trixy'); + + await _enterReviewStep(tester, db); + + // Task must not surface in the snapshot — no waiting-for badge, no + // keep variants, and the empty-state card is shown instead. + expect(find.textContaining('Waiting for Trixy'), findsNothing); + expect(find.text('Still waiting'), findsNothing); + expect(find.text('Still relevant'), findsNothing); + expect(find.text('All tasks reviewed!'), findsOneWidget); + }); + + testWidgets( + 'stale delegated task with multiple person tags joins names with comma', + (tester) async { + final clarifiedAt = + DateTime.now().subtract(_staleTaskClarifiedOffset).toUtc(); + final completedAt = + DateTime.now().subtract(_staleTaskCompletedOffset).toUtc(); + final id = await _insertClarifiedTask( + db, + nextActionText: 'Sync with team', + lastClarifiedAt: clarifiedAt, + lastNextActionCompletionAt: completedAt, + title: 'Multi-delegate', + ); + // Tag names are returned ORDER BY name ASC by getPersonTagsForTodos. + await _attachPersonTag(db, id, name: 'Alice', tagId: 'p-alice'); + await _attachPersonTag(db, id, name: 'Bob', tagId: 'p-bob'); + + await _enterReviewStep(tester, db); + + expect(find.textContaining('Waiting for Alice, Bob'), findsOneWidget); + }); + }); +} diff --git a/app/test/screens/tag_cloud_test.dart b/app/test/screens/tag_cloud_test.dart index 6dd8fb7..8eadca6 100644 --- a/app/test/screens/tag_cloud_test.dart +++ b/app/test/screens/tag_cloud_test.dart @@ -14,7 +14,6 @@ import 'package:jeeves/providers/gtd_lists_provider.dart'; import 'package:jeeves/providers/periodic_review_settings_provider.dart'; import 'package:jeeves/providers/tag_filter_provider.dart'; import 'package:jeeves/providers/tags_provider.dart'; -import 'package:jeeves/database/gtd_database.dart' show Tag; import 'package:jeeves/models/focus_session_planning_settings.dart'; import 'package:jeeves/screens/app_shell.dart'; import '../test_helpers.dart';