diff --git a/CONTEXT.md b/CONTEXT.md index 0b3d3d2b..cd85fd5e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -61,6 +61,18 @@ _Avoid_: Tracking vendor, analytics SDK A Product Usage Event that marks completion or failure of a meaningful user workflow step. _Avoid_: Tap event, raw navigation log, interaction trace +**Schedule**: +A planned commitment with a target time that OnTime helps the user prepare for. +_Avoid_: Event, appointment, alarm + +**Preparation**: +The ordered set of steps a user completes before a Schedule. +_Avoid_: Preparation chain, task list + +**Preparation Step**: +One named action with an expected duration inside a Preparation. +_Avoid_: Task, checklist item, alarm step + **Provider Authentication Completed**: The state where the external Apple or Google account prompt has returned credentials to OnTime. _Avoid_: Login completed, signed in, session ready @@ -174,6 +186,9 @@ _Avoid_: Loaded range, stream range, cached range - An **Analytics Event Parameter** must not contain user-authored text, direct identifiers, tokens, raw exception strings, request bodies, or response bodies. - A **Product Usage Event** uses a stable snake_case name and includes a schema version. - A changed **Product Usage Event** meaning requires a new event name or schema version. +- A **Schedule** may have one **Preparation** for that specific commitment. +- A **Preparation** contains zero or more **Preparation Steps** in user-defined order. +- A user's default **Preparation** may be applied to a **Schedule** and then changed for that Schedule. - A **Schedule** has a **Preparation** whose **Preparation Duration** contributes to preparation-start timing. - **Preparation Duration**, move time, and **Schedule Spare Time** are distinct schedule timing inputs. - User-facing copy should call a scheduled notification a **Schedule Notification**, not an **Alarm**, unless it opens an OnTime screen without the user first tapping a notification. @@ -245,4 +260,5 @@ _Avoid_: Loaded range, stream range, cached range - "Time Sensitive" was too platform-specific for default user-facing status; resolved: fallback iOS delivery should be called notification. - "Status label" was ambiguous across platforms; resolved: Android uses precise notification or notification status, while iOS uses alarm status only for **iOS AlarmKit Alarm**. - "No scheduled alarm" was too capability-specific for an empty state; resolved: use **No Scheduled Notification** across platforms. +- "Preparation chain" describes storage reconstruction, not product language; resolved: the domain concept is an ordered **Preparation** made of **Preparation Steps**. - "Total duration" was ambiguous between **Preparation Duration** and the broader preparation-start timing calculation; resolved: **Preparation Duration** is steps only, while move time and **Schedule Spare Time** are separate inputs. diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index a8b555be..86406344 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -18,42 +18,51 @@ import 'package:uuid/uuid.dart'; part 'database.g.dart'; @Singleton() -@DriftDatabase(tables: [ - Places, - Schedules, - Users, - PreparationSchedules, - PreparationUsers, -], daos: [ - ScheduleDao, - PlaceDao, - UserDao, - PreparationScheduleDao, - PreparationUserDao -]) +@DriftDatabase( + tables: [Places, Schedules, Users, PreparationSchedules, PreparationUsers], + daos: [ + ScheduleDao, + PlaceDao, + UserDao, + PreparationScheduleDao, + PreparationUserDao, + ], +) class AppDatabase extends _$AppDatabase { AppDatabase() : super(_openConnection()); AppDatabase.forTesting(super.e); @override - int get schemaVersion => 3; + int get schemaVersion => 4; @override MigrationStrategy get migration => MigrationStrategy( - onCreate: (Migrator m) async { - await m.createAll(); - }, - onUpgrade: (Migrator m, int from, int to) async { - if (from == 1) { - await m.createTable(preparationSchedules); - await m.createTable(preparationUsers); - } - }, - beforeOpen: (details) async { - await customStatement('PRAGMA foreign_keys = ON'); - }, - ); + onCreate: (Migrator m) async { + await m.createAll(); + }, + onUpgrade: (Migrator m, int from, int to) async { + if (from < 3) { + await m.createTable(preparationSchedules); + await m.createTable(preparationUsers); + } + if (from < 4) { + await _createLookupIndexes(m); + } + }, + beforeOpen: (details) async { + await customStatement('PRAGMA foreign_keys = ON'); + }, + ); + + Future _createLookupIndexes(Migrator m) async { + await m.createIndex(schedulesScheduleTimeIdx); + await m.createIndex(schedulesPlaceIdIdx); + await m.createIndex(preparationSchedulesScheduleIdIdx); + await m.createIndex(preparationSchedulesNextPreparationIdIdx); + await m.createIndex(preparationUsersUserIdIdx); + await m.createIndex(preparationUsersNextPreparationIdIdx); + } static QueryExecutor _openConnection() { return driftDatabase( diff --git a/lib/data/daos/preparation_schedule_dao.dart b/lib/data/daos/preparation_schedule_dao.dart index f992240f..259771db 100644 --- a/lib/data/daos/preparation_schedule_dao.dart +++ b/lib/data/daos/preparation_schedule_dao.dart @@ -1,4 +1,3 @@ -import 'package:collection/collection.dart'; import 'package:drift/drift.dart'; import 'package:on_time_front/data/mappers/domain_persistence_mappers.dart'; import 'package:on_time_front/domain/entities/preparation_step_entity.dart'; @@ -59,29 +58,17 @@ class PreparationScheduleDao extends DatabaseAccessor return PreparationEntity(preparationStepList: []); } - final firstStep = allSteps.firstWhere( - (step) => allSteps.every((other) => other.nextPreparationId != step.id), - orElse: () => allSteps.first, - ); - - final List orderedSteps = []; - PreparationSchedule? currentStep = firstStep; - - while (currentStep != null) { - orderedSteps.add( - PreparationStepEntity( - id: currentStep.id, - preparationName: currentStep.preparationName, - preparationTime: Duration(minutes: currentStep.preparationTime), - nextPreparationId: currentStep.nextPreparationId, - ), - ); - currentStep = allSteps.firstWhereOrNull( - (step) => step.id == currentStep!.nextPreparationId, - ); - } - - return PreparationEntity(preparationStepList: orderedSteps); + return PreparationEntity( + preparationStepList: [ + for (final step in allSteps) + PreparationStepEntity( + id: step.id, + preparationName: step.preparationName, + preparationTime: Duration(minutes: step.preparationTime), + nextPreparationId: step.nextPreparationId, + ), + ], + ).ordered; } Future getPreparationStepById( diff --git a/lib/data/daos/preparation_user_dao.dart b/lib/data/daos/preparation_user_dao.dart index 7b28349f..6df5f702 100644 --- a/lib/data/daos/preparation_user_dao.dart +++ b/lib/data/daos/preparation_user_dao.dart @@ -1,4 +1,3 @@ -import 'package:collection/collection.dart'; import 'package:drift/drift.dart'; import 'package:on_time_front/data/mappers/domain_persistence_mappers.dart'; import 'package:on_time_front/domain/entities/preparation_step_entity.dart'; @@ -49,29 +48,17 @@ class PreparationUserDao extends DatabaseAccessor return PreparationEntity(preparationStepList: []); } - final firstStep = allSteps.firstWhere( - (step) => allSteps.every((other) => other.nextPreparationId != step.id), - orElse: () => allSteps.first, - ); - - final List orderedSteps = []; - PreparationUser? currentStep = firstStep; - - while (currentStep != null) { - orderedSteps.add( - PreparationStepEntity( - id: currentStep.id, - preparationName: currentStep.preparationName, - preparationTime: Duration(minutes: currentStep.preparationTime), - nextPreparationId: currentStep.nextPreparationId, - ), - ); - currentStep = allSteps.firstWhereOrNull( - (step) => step.id == currentStep!.nextPreparationId, - ); - } - - return PreparationEntity(preparationStepList: orderedSteps); + return PreparationEntity( + preparationStepList: [ + for (final step in allSteps) + PreparationStepEntity( + id: step.id, + preparationName: step.preparationName, + preparationTime: Duration(minutes: step.preparationTime), + nextPreparationId: step.nextPreparationId, + ), + ], + ).ordered; } Future getPreparationStepById( diff --git a/lib/data/tables/preparation_schedule_table.dart b/lib/data/tables/preparation_schedule_table.dart index 7b959c05..6eab697b 100644 --- a/lib/data/tables/preparation_schedule_table.dart +++ b/lib/data/tables/preparation_schedule_table.dart @@ -2,6 +2,14 @@ import 'package:drift/drift.dart'; import 'package:on_time_front/data/tables/schedules_table.dart'; import 'package:uuid/uuid.dart'; +@TableIndex( + name: 'preparation_schedules_schedule_id_idx', + columns: {#scheduleId}, +) +@TableIndex( + name: 'preparation_schedules_next_preparation_id_idx', + columns: {#nextPreparationId}, +) class PreparationSchedules extends Table { TextColumn get id => text().clientDefault(() => Uuid().v7())(); TextColumn get scheduleId => text().references(Schedules, #id)(); diff --git a/lib/data/tables/preparation_user_table.dart b/lib/data/tables/preparation_user_table.dart index 3576a843..f31d4fc9 100644 --- a/lib/data/tables/preparation_user_table.dart +++ b/lib/data/tables/preparation_user_table.dart @@ -2,6 +2,11 @@ import 'package:drift/drift.dart'; import 'package:on_time_front/data/tables/user_table.dart'; import 'package:uuid/uuid.dart'; +@TableIndex(name: 'preparation_users_user_id_idx', columns: {#userId}) +@TableIndex( + name: 'preparation_users_next_preparation_id_idx', + columns: {#nextPreparationId}, +) class PreparationUsers extends Table { TextColumn get id => text().clientDefault(() => Uuid().v7())(); TextColumn get userId => text().references(Users, #id)(); diff --git a/lib/data/tables/schedules_table.dart b/lib/data/tables/schedules_table.dart index 03297e4b..17cfcbcc 100644 --- a/lib/data/tables/schedules_table.dart +++ b/lib/data/tables/schedules_table.dart @@ -3,6 +3,8 @@ import 'package:on_time_front/core/utils/json_converters/duration_json_converter import 'package:on_time_front/data/tables/places_table.dart'; import 'package:uuid/uuid.dart'; +@TableIndex(name: 'schedules_schedule_time_idx', columns: {#scheduleTime}) +@TableIndex(name: 'schedules_place_id_idx', columns: {#placeId}) class Schedules extends Table { TextColumn get id => text().clientDefault(() => Uuid().v7())(); TextColumn get placeId => text().references(Places, #id)(); diff --git a/test/core/database/database_index_test.dart b/test/core/database/database_index_test.dart new file mode 100644 index 00000000..0c26600a --- /dev/null +++ b/test/core/database/database_index_test.dart @@ -0,0 +1,91 @@ +import 'package:drift/drift.dart' as drift; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:on_time_front/core/database/database.dart'; + +void main() { + late AppDatabase database; + + setUp(() { + database = AppDatabase.forTesting(NativeDatabase.memory()); + }); + + tearDown(() async { + await database.close(); + }); + + test( + 'creates lookup indexes for schedule and preparation DAO predicates', + () async { + for (final entry in _expectedLookupIndexesByTable.entries) { + final indexNames = await _indexNames(database, entry.key); + + expect( + indexNames, + containsAll(entry.value), + reason: '${entry.key} should declare indexes for DAO lookup paths', + ); + } + }, + ); + + test('adds lookup indexes when upgrading a schema 3 database', () async { + expect(database.schemaVersion, 4); + + await _dropExpectedLookupIndexes(database); + + for (final entry in _expectedLookupIndexesByTable.entries) { + expect( + await _indexNames(database, entry.key), + isNot(containsAll(entry.value)), + ); + } + + await database.migration.onUpgrade(database.createMigrator(), 3, 4); + + for (final entry in _expectedLookupIndexesByTable.entries) { + final indexNames = await _indexNames(database, entry.key); + + expect( + indexNames, + containsAll(entry.value), + reason: '${entry.key} indexes should be added by the 3 -> 4 migration', + ); + } + }); +} + +const _expectedLookupIndexesByTable = { + 'preparation_schedules': { + 'preparation_schedules_schedule_id_idx', + 'preparation_schedules_next_preparation_id_idx', + }, + 'preparation_users': { + 'preparation_users_user_id_idx', + 'preparation_users_next_preparation_id_idx', + }, + 'schedules': {'schedules_schedule_time_idx', 'schedules_place_id_idx'}, +}; + +Future> _indexNames(AppDatabase database, String tableName) async { + final rows = await database + .customSelect( + ''' + SELECT name + FROM sqlite_master + WHERE type = 'index' AND tbl_name = ? + ''', + variables: [drift.Variable.withString(tableName)], + ) + .get(); + + return {for (final row in rows) row.read('name')}; +} + +Future _dropExpectedLookupIndexes(AppDatabase database) async { + for (final indexNames in _expectedLookupIndexesByTable.values) { + for (final indexName in indexNames) { + await database.customStatement('DROP INDEX IF EXISTS $indexName'); + } + } +} diff --git a/test/data/daos/preparation_schedule_dao_test.dart b/test/data/daos/preparation_schedule_dao_test.dart index 95e1eb3d..374c251e 100644 --- a/test/data/daos/preparation_schedule_dao_test.dart +++ b/test/data/daos/preparation_schedule_dao_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:on_time_front/core/database/database.dart'; import 'package:on_time_front/data/daos/preparation_schedule_dao.dart'; import 'package:on_time_front/data/daos/preparation_user_dao.dart'; +import 'package:on_time_front/data/mappers/domain_persistence_mappers.dart'; import 'package:on_time_front/domain/entities/preparation_entity.dart'; import 'package:on_time_front/domain/entities/preparation_step_entity.dart'; import 'package:uuid/uuid.dart'; @@ -207,6 +208,118 @@ void main() { }, ); + test( + 'getPreparationSchedulesByScheduleId orders three linked steps when rows are out of order', + () async { + const lastStep = PreparationStepEntity( + id: 'step-3', + preparationName: 'Step 3: Leave', + preparationTime: Duration(minutes: 15), + nextPreparationId: null, + ); + final middleStep = preparationStep2.copyWith( + nextPreparationId: lastStep.id, + ); + final firstStep = preparationStep1.copyWith( + nextPreparationId: middleStep.id, + ); + + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + lastStep.toPreparationScheduleRow(scheduleId).toCompanion(false), + ); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + middleStep + .toPreparationScheduleRow(scheduleId) + .toCompanion(false), + ); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + firstStep.toPreparationScheduleRow(scheduleId).toCompanion(false), + ); + + final result = await schedulePreparationDao + .getPreparationSchedulesByScheduleId(scheduleId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + lastStep.id, + ]); + }, + ); + + test( + 'getPreparationSchedulesByScheduleId keeps remaining steps when a link is broken', + () async { + final firstStep = preparationStep1.copyWith( + nextPreparationId: 'missing-step', + ); + + await appDatabase.customStatement('PRAGMA foreign_keys = OFF'); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + firstStep.toPreparationScheduleRow(scheduleId).toCompanion(false), + ); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + preparationStep2 + .toPreparationScheduleRow(scheduleId) + .toCompanion(false), + ); + await appDatabase.customStatement('PRAGMA foreign_keys = ON'); + + final result = await schedulePreparationDao + .getPreparationSchedulesByScheduleId(scheduleId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + ]); + }, + ); + + test( + 'getPreparationSchedulesByScheduleId returns each step once when links form a cycle', + () async { + final firstStep = preparationStep1.copyWith( + nextPreparationId: preparationStep2.id, + ); + final secondStep = preparationStep2.copyWith( + nextPreparationId: preparationStep1.id, + ); + + await appDatabase.customStatement('PRAGMA foreign_keys = OFF'); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + firstStep.toPreparationScheduleRow(scheduleId).toCompanion(false), + ); + await appDatabase + .into(appDatabase.preparationSchedules) + .insert( + secondStep + .toPreparationScheduleRow(scheduleId) + .toCompanion(false), + ); + await appDatabase.customStatement('PRAGMA foreign_keys = ON'); + + final result = await schedulePreparationDao + .getPreparationSchedulesByScheduleId(scheduleId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + ]); + }, + ); + test('getPreparationStepById returns the stored step contract', () async { await schedulePreparationDao.createPreparationSchedule( preparationEntity, diff --git a/test/data/daos/preparation_user_dao_test.dart b/test/data/daos/preparation_user_dao_test.dart index 056c06a1..d95b613b 100644 --- a/test/data/daos/preparation_user_dao_test.dart +++ b/test/data/daos/preparation_user_dao_test.dart @@ -3,6 +3,7 @@ import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:on_time_front/core/database/database.dart'; import 'package:on_time_front/data/daos/preparation_user_dao.dart'; +import 'package:on_time_front/data/mappers/domain_persistence_mappers.dart'; import 'package:on_time_front/domain/entities/preparation_entity.dart'; import 'package:on_time_front/domain/entities/preparation_step_entity.dart'; import 'package:uuid/uuid.dart'; @@ -135,6 +136,97 @@ void main() { ); expect(result.preparationStepList[1].nextPreparationId, isNull); }); + + test( + 'should return ordered preparation steps when rows are out of order', + () async { + const lastStep = PreparationStepEntity( + id: 'step-3', + preparationName: 'Step 3: Leave', + preparationTime: Duration(minutes: 15), + nextPreparationId: null, + ); + final middleStep = preparationStep2.copyWith( + nextPreparationId: lastStep.id, + ); + final firstStep = preparationStep1.copyWith( + nextPreparationId: middleStep.id, + ); + + await appDatabase + .into(appDatabase.preparationUsers) + .insert(lastStep.toPreparationUserRow(userId).toCompanion(false)); + await appDatabase + .into(appDatabase.preparationUsers) + .insert(middleStep.toPreparationUserRow(userId).toCompanion(false)); + await appDatabase + .into(appDatabase.preparationUsers) + .insert(firstStep.toPreparationUserRow(userId).toCompanion(false)); + + final result = await userDao.getPreparationUsersByUserId(userId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + lastStep.id, + ]); + }, + ); + + test( + 'should keep remaining preparation steps when a link is broken', + () async { + final firstStep = preparationStep1.copyWith( + nextPreparationId: 'missing-step', + ); + + await appDatabase.customStatement('PRAGMA foreign_keys = OFF'); + await appDatabase + .into(appDatabase.preparationUsers) + .insert(firstStep.toPreparationUserRow(userId).toCompanion(false)); + await appDatabase + .into(appDatabase.preparationUsers) + .insert( + preparationStep2.toPreparationUserRow(userId).toCompanion(false), + ); + await appDatabase.customStatement('PRAGMA foreign_keys = ON'); + + final result = await userDao.getPreparationUsersByUserId(userId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + ]); + }, + ); + + test( + 'should return each preparation step once when links form a cycle', + () async { + final firstStep = preparationStep1.copyWith( + nextPreparationId: preparationStep2.id, + ); + final secondStep = preparationStep2.copyWith( + nextPreparationId: preparationStep1.id, + ); + + await appDatabase.customStatement('PRAGMA foreign_keys = OFF'); + await appDatabase + .into(appDatabase.preparationUsers) + .insert(firstStep.toPreparationUserRow(userId).toCompanion(false)); + await appDatabase + .into(appDatabase.preparationUsers) + .insert(secondStep.toPreparationUserRow(userId).toCompanion(false)); + await appDatabase.customStatement('PRAGMA foreign_keys = ON'); + + final result = await userDao.getPreparationUsersByUserId(userId); + + expect(result.preparationStepList.map((step) => step.id), [ + preparationStep1.id, + preparationStep2.id, + ]); + }, + ); }); group('getPreparationStepById', () {