From 2d2ea18a0c4dd23ac464cc4d32ae7c2edf380f39 Mon Sep 17 00:00:00 2001 From: pec0ra Date: Sun, 10 May 2026 13:09:32 +0200 Subject: [PATCH 1/5] Add option to undo a history entry --- lib/models/settings.dart | 22 ++++ lib/models/setup.dart | 76 ++++++++++++ lib/setup_detail.dart | 254 ++++++++++++++++++++++++++++++--------- lib/setup_edit.dart | 4 +- 4 files changed, 294 insertions(+), 62 deletions(-) diff --git a/lib/models/settings.dart b/lib/models/settings.dart index fca956e..759fc0d 100644 --- a/lib/models/settings.dart +++ b/lib/models/settings.dart @@ -105,6 +105,28 @@ class Settings { SettingType.frontTyrePressure || SettingType.rearTyrePressure => null, }; + void setField(SettingType type, Field? value) { + switch (type) { + case SettingType.airPressure: + airPressure = value; + case SettingType.sag: + sag = value; + case SettingType.volumeSpacer: + volumeSpacer = value; + case SettingType.lsc: + lsc = value; + case SettingType.hsc: + hsc = value; + case SettingType.lsr: + lsr = value; + case SettingType.hsr: + hsr = value; + case SettingType.frontTyrePressure: + case SettingType.rearTyrePressure: + break; + } + } + Settings clone() { return Settings( airPressure: airPressure, diff --git a/lib/models/setup.dart b/lib/models/setup.dart index d3a615d..ec2cdb7 100644 --- a/lib/models/setup.dart +++ b/lib/models/setup.dart @@ -2,6 +2,7 @@ import 'package:suspension_setup/models/settings.dart'; import 'package:suspension_setup/models/tyres.dart'; import 'package:uuid/uuid.dart'; +import 'field.dart'; import 'setting_change.dart'; class Setup { @@ -55,6 +56,81 @@ class Setup { ); } + Setup copyMutable() => Setup.fromJson(toJson()); + + List computeUndo(SettingChanges historyEntry) { + final result = []; + for (final change in historyEntry.changes) { + final Field? currentField; + if (change.suspensionType == SuspensionType.tyre) { + currentField = change.settingType == SettingType.frontTyrePressure + ? tyres.front + : tyres.rear; + } else { + final settings = + change.suspensionType == SuspensionType.fork ? fork : shock; + currentField = settings.fieldFor(change.settingType); + } + + final num? currentValue = currentField?.value; + final bool currentEnabled = currentField != null; + final bool targetEnabled = change.oldEnabled ?? true; + + final bool enabledChanges = currentEnabled != targetEnabled; + final bool valueChanges = + !enabledChanges && currentEnabled && currentValue != change.oldValue; + + if (!enabledChanges && !valueChanges) continue; + + result.add(SettingChange( + suspensionType: change.suspensionType, + settingType: change.settingType, + oldValue: currentValue, + newValue: targetEnabled ? change.oldValue : null, + oldEnabled: enabledChanges ? currentEnabled : null, + newEnabled: enabledChanges ? targetEnabled : null, + )); + } + return result; + } + + void applyChanges(List changes) { + for (final change in changes) { + final bool targetEnabled = change.newEnabled ?? true; + final num? targetValue = change.newValue; + + if (change.suspensionType == SuspensionType.tyre) { + final isFront = change.settingType == SettingType.frontTyrePressure; + final currentField = isFront ? tyres.front : tyres.rear; + final newField = targetEnabled + ? Field( + value: targetValue!, + unit: currentField?.unit ?? + Settings.defaultUnits[change.settingType] ?? + 'PSI') + : null; + if (isFront) { + tyres.front = newField; + } else { + tyres.rear = newField; + } + } else { + final settings = + change.suspensionType == SuspensionType.fork ? fork : shock; + if (targetEnabled) { + final currentField = settings.fieldFor(change.settingType); + final unit = currentField?.unit ?? + Settings.defaultUnits[change.settingType] ?? + ''; + settings.setField( + change.settingType, Field(value: targetValue!, unit: unit)); + } else { + settings.setField(change.settingType, null); + } + } + } + } + Setup clone(bool includeHistory) { return Setup( id: const Uuid().v1(), diff --git a/lib/setup_detail.dart b/lib/setup_detail.dart index 09f0400..04a26aa 100644 --- a/lib/setup_detail.dart +++ b/lib/setup_detail.dart @@ -196,21 +196,25 @@ class History extends StatelessWidget { final Setup setup; - String _changeText(SettingChange change, Setup setup) { - final String unit; + String _unit(SettingChange change, Setup setup) { if (change.suspensionType == SuspensionType.tyre) { - unit = (change.settingType == SettingType.frontTyrePressure + return (change.settingType == SettingType.frontTyrePressure ? setup.tyres.front?.unit : setup.tyres.rear?.unit) ?? + Settings.defaultUnits[change.settingType] ?? ''; - } else { - final settings = change.suspensionType == SuspensionType.fork - ? setup.fork - : setup.shock; - unit = settings.fieldFor(change.settingType)?.unit ?? ''; } - final label = change.settingType.label; + final settings = change.suspensionType == SuspensionType.fork + ? setup.fork + : setup.shock; + return settings.fieldFor(change.settingType)?.unit ?? + Settings.defaultUnits[change.settingType] ?? + ''; + } + String _changeText(SettingChange change, Setup setup) { + final unit = _unit(change, setup); + final label = change.settingType.label; if (change.newEnabled == true) { return '$label: enabled (${change.newValue} $unit)'.trim(); } @@ -220,6 +224,137 @@ class History extends StatelessWidget { return '$label: ${change.oldValue} → ${change.newValue} $unit'.trim(); } + IconData _iconFor(SuspensionType type) => switch (type) { + SuspensionType.fork => SuspensionIcons.fork, + SuspensionType.shock => SuspensionIcons.shock, + SuspensionType.tyre => SuspensionIcons.tyre, + }; + + Future _performUndo( + BuildContext context, + SettingChanges historyEntry, + List actualChanges, + ) async { + final newSetup = setup.copyMutable(); + newSetup.applyChanges(actualChanges); + + final originalComment = historyEntry.comment; + final String autoComment; + if (originalComment != null && + originalComment.isNotEmpty && + originalComment != SettingChanges.defaultComment) { + autoComment = 'Undo: $originalComment'; + } else { + autoComment = + 'Undo: ${DateFormat.yMMMd().add_Hm().format(historyEntry.date)}'; + } + + newSetup.history.add(SettingChanges( + changes: actualChanges, + date: DateTime.now(), + comment: autoComment, + )); + + await Provider.of(context, listen: false) + .upsertSetup(newSetup); + if (!context.mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar( + behavior: SnackBarBehavior.floating, + content: Text('Changes undone'), + ), + ); + } + + void _handleUndo(BuildContext context, SettingChanges historyEntry) { + final actualChanges = setup.computeUndo(historyEntry); + + if (actualChanges.isEmpty) { + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar( + behavior: SnackBarBehavior.floating, + content: Text('Nothing to undo — values are already at those settings'), + ), + ); + return; + } + + showDialog( + context: context, + builder: (dialogContext) => AlertDialog( + title: const Text('Undo change?'), + content: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const Text('The following values will be reverted:'), + const SizedBox(height: 8), + for (final change in actualChanges) + Padding( + padding: const EdgeInsets.symmetric(vertical: 4), + child: TextWithIcon( + text: _changeText(change, setup), + icon: _iconFor(change.suspensionType), + ), + ), + ], + ), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(dialogContext), + child: const Text('Cancel'), + ), + TextButton( + style: TextButton.styleFrom( + foregroundColor: Theme.of(dialogContext).colorScheme.error, + ), + onPressed: () async { + Navigator.pop(dialogContext); + await _performUndo(context, historyEntry, actualChanges); + }, + child: const Text('Undo'), + ), + ], + ), + ); + } + + void _showHistoryItemSheet(BuildContext context, SettingChanges entry) { + final isSetupCreation = entry.comment == SettingChanges.defaultComment; + showModalBottomSheet( + context: context, + useSafeArea: true, + builder: (sheetContext) { + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + ListTile( + title: Text( + DateFormat.yMMMd().add_Hm().format(entry.date), + style: Theme.of(context).textTheme.titleMedium, + ), + subtitle: (entry.comment != null && entry.comment!.isNotEmpty) + ? Text(entry.comment!) + : null, + ), + const Divider(height: 0), + if (!isSetupCreation) + ListTile( + leading: const Icon(Icons.undo), + title: const Text('Undo this change'), + onTap: () { + Navigator.pop(sheetContext); + _handleUndo(context, entry); + }, + ), + ], + ); + }, + ); + } + @override Widget build(BuildContext context) { final theme = Theme.of(context); @@ -238,59 +373,60 @@ class History extends StatelessWidget { theme.colorScheme.surfaceContainerLow, ), clipBehavior: Clip.antiAliasWithSaveLayer, - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Container( - color: theme.colorScheme.secondary.withValues(alpha:0.08), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - ListTile( - title: Text(DateFormat.yMMMd() - .add_Hm() - .format(settingChange.date)), - ), - Divider( - color: theme.colorScheme.secondary.withValues(alpha:0.3), - height: 0, - ), - if (settingChange.comment != null && - settingChange.comment!.isNotEmpty) - Padding( - padding: const EdgeInsets.only( - left: 16, - bottom: 12, - top: 12, - ), - child: TextWithIcon( - text: settingChange.comment!, - icon: Icons.info_outline, + child: InkWell( + onTap: () => _showHistoryItemSheet(context, settingChange), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Container( + color: theme.colorScheme.secondary.withValues(alpha: 0.08), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + ListTile( + title: Text(DateFormat.yMMMd() + .add_Hm() + .format(settingChange.date)), + ), + Divider( + color: theme.colorScheme.secondary + .withValues(alpha: 0.3), + height: 0, + ), + if (settingChange.comment != null && + settingChange.comment!.isNotEmpty) + Padding( + padding: const EdgeInsets.only( + left: 16, + bottom: 12, + top: 12, + ), + child: TextWithIcon( + text: settingChange.comment!, + icon: Icons.info_outline, + ), ), + Divider( + color: theme.colorScheme.secondary + .withValues(alpha: 0.3), + height: 0, ), - Divider( - color: theme.colorScheme.secondary.withValues(alpha:0.3), - height: 0, - ), - ]), - ), - for (SettingChange change in settingChange.changes) - Padding( - padding: const EdgeInsets.only( - left: 16, - bottom: 8, - top: 8, - ), - child: TextWithIcon( - text: _changeText(change, setup), - icon: switch (change.suspensionType) { - SuspensionType.fork => SuspensionIcons.fork, - SuspensionType.shock => SuspensionIcons.shock, - SuspensionType.tyre => SuspensionIcons.tyre, - }, - ), + ]), ), - ], + for (SettingChange change in settingChange.changes) + Padding( + padding: const EdgeInsets.only( + left: 16, + bottom: 8, + top: 8, + ), + child: TextWithIcon( + text: _changeText(change, setup), + icon: _iconFor(change.suspensionType), + ), + ), + ], + ), ), ), ], diff --git a/lib/setup_edit.dart b/lib/setup_edit.dart index 4d11486..8e41f55 100644 --- a/lib/setup_edit.dart +++ b/lib/setup_edit.dart @@ -44,9 +44,7 @@ class _SetupEditState extends State { Future _onSetupChanged(BuildContext context) async { if (_formKey.currentState!.validate()) { - var newSetup = widget.setup != null - ? Setup.fromJson(widget.setup!.toJson()) - : Setup.getDefault(); + var newSetup = widget.setup?.copyMutable() ?? Setup.getDefault(); SettingChanges settingChanges = SettingChanges(changes: [], date: DateTime.now()); From 18f9b6fde2984ef8c9591e129e73c0b199d7cbb4 Mon Sep 17 00:00:00 2001 From: pec0ra Date: Sun, 10 May 2026 13:36:09 +0200 Subject: [PATCH 2/5] Allow editting history entry comments --- integration_test/screenshot_test.dart | 5 +- lib/migrations/migrator.dart | 7 +- lib/migrations/v2_to_v3.dart | 19 ++++ lib/models/setting_change.dart | 7 +- lib/setup_detail.dart | 48 +++++++++- lib/setup_edit.dart | 8 +- test/migrations/migrator_test.dart | 60 +++++++++--- test/migrations/v2_to_v3_test.dart | 131 ++++++++++++++++++++++++++ test/models/setup_test.dart | 6 +- 9 files changed, 263 insertions(+), 28 deletions(-) create mode 100644 lib/migrations/v2_to_v3.dart create mode 100644 test/migrations/v2_to_v3_test.dart diff --git a/integration_test/screenshot_test.dart b/integration_test/screenshot_test.dart index 997470d..7132982 100644 --- a/integration_test/screenshot_test.dart +++ b/integration_test/screenshot_test.dart @@ -81,9 +81,8 @@ Future _screenshot( await binding.takeScreenshot(name); } -// demo.json data migrated to schema v2 const _fixture = { - 'schemaVersion': 2, + 'schemaVersion': 3, 'setups': { 'a5f0c760-7346-11ef-89aa-5137c552c0bc': { 'id': 'a5f0c760-7346-11ef-89aa-5137c552c0bc', @@ -112,6 +111,7 @@ const _fixture = { 'changes': [], 'date': '2024-09-15T11:41:14.071433', 'comment': 'Setup creation', + 'isCreationEntry': true, }, ], }, @@ -144,6 +144,7 @@ const _fixture = { 'changes': [], 'date': '2024-09-15T14:13:48.447488', 'comment': 'Setup creation', + 'isCreationEntry': true, }, { 'changes': [ diff --git a/lib/migrations/migrator.dart b/lib/migrations/migrator.dart index 75df3a6..eedadd2 100644 --- a/lib/migrations/migrator.dart +++ b/lib/migrations/migrator.dart @@ -1,6 +1,7 @@ import 'v1_to_v2.dart'; +import 'v2_to_v3.dart'; -const int currentSchemaVersion = 2; +const int currentSchemaVersion = 3; Map migrateIfNeeded(Map json) { final version = json['schemaVersion'] as int? ?? 1; @@ -9,6 +10,8 @@ Map migrateIfNeeded(Map json) { final setupsOnly = Map.from(data)..remove('schemaVersion'); data = migrateV1ToV2(setupsOnly); } - // Future: if (version < 3) data = migrateV2ToV3(data['setups']); + if (version < 3) { + data = migrateV2ToV3(data); + } return data; } \ No newline at end of file diff --git a/lib/migrations/v2_to_v3.dart b/lib/migrations/v2_to_v3.dart new file mode 100644 index 0000000..43174c8 --- /dev/null +++ b/lib/migrations/v2_to_v3.dart @@ -0,0 +1,19 @@ +/// v2 structure: { schemaVersion: 2, setups: { setupId: { ..., history: [...] } } } +/// v3 structure: { schemaVersion: 3, setups: { setupId: { ..., history: [{ isCreationEntry: true, ... }, ...] } } } +Map migrateV2ToV3(Map v2Data) { + final setups = Map.from(v2Data['setups'] as Map); + final migratedSetups = setups.map( + (id, value) => MapEntry(id, _migrateSetup(value as Map)), + ); + return {'schemaVersion': 3, 'setups': migratedSetups}; +} + +Map _migrateSetup(Map setup) { + final history = setup['history'] as List; + if (history.isEmpty) return Map.from(setup); + final migratedHistory = [ + {...history[0] as Map, 'isCreationEntry': true}, + ...history.skip(1), + ]; + return {...setup, 'history': migratedHistory}; +} \ No newline at end of file diff --git a/lib/models/setting_change.dart b/lib/models/setting_change.dart index 1bb274b..c3f9897 100644 --- a/lib/models/setting_change.dart +++ b/lib/models/setting_change.dart @@ -2,13 +2,13 @@ class SettingChanges { final List changes; final DateTime date; String? comment; - - static const String defaultComment = 'Setup creation'; + final bool isCreationEntry; SettingChanges({ required this.changes, required this.date, this.comment, + this.isCreationEntry = false, }); factory SettingChanges.fromJson(Map json) { @@ -17,6 +17,7 @@ class SettingChanges { json['changes'].map((c) => SettingChange.fromJson(c))), date: DateTime.parse((json['date'])), comment: (json['comment']), + isCreationEntry: json['isCreationEntry'] as bool? ?? false, ); } @@ -25,6 +26,7 @@ class SettingChanges { 'changes': changes.map((c) => c.toJson()).toList(), 'date': date.toIso8601String(), 'comment': comment, + 'isCreationEntry': isCreationEntry, }; } @@ -33,6 +35,7 @@ class SettingChanges { changes: changes.map((e) => e.clone()).toList(), date: date, comment: comment, + isCreationEntry: isCreationEntry, ); } } diff --git a/lib/setup_detail.dart b/lib/setup_detail.dart index 04a26aa..8c0520a 100644 --- a/lib/setup_detail.dart +++ b/lib/setup_detail.dart @@ -242,7 +242,7 @@ class History extends StatelessWidget { final String autoComment; if (originalComment != null && originalComment.isNotEmpty && - originalComment != SettingChanges.defaultComment) { + !historyEntry.isCreationEntry) { autoComment = 'Undo: $originalComment'; } else { autoComment = @@ -321,8 +321,42 @@ class History extends StatelessWidget { ); } + void _showEditCommentDialog(BuildContext context, SettingChanges entry) { + final controller = TextEditingController(text: entry.comment ?? ''); + showDialog( + context: context, + builder: (dialogContext) => AlertDialog( + title: const Text('Edit comment'), + content: TextField( + controller: controller, + autofocus: true, + decoration: const InputDecoration(hintText: 'Add a comment'), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(dialogContext), + child: const Text('Cancel'), + ), + TextButton( + onPressed: () async { + final newComment = controller.text.trim(); + Navigator.pop(dialogContext); + final newSetup = setup.copyMutable(); + final target = newSetup.history.firstWhere( + (e) => e.date == entry.date, + ); + target.comment = newComment.isEmpty ? null : newComment; + await Provider.of(context, listen: false) + .upsertSetup(newSetup); + }, + child: const Text('Save'), + ), + ], + ), + ); + } + void _showHistoryItemSheet(BuildContext context, SettingChanges entry) { - final isSetupCreation = entry.comment == SettingChanges.defaultComment; showModalBottomSheet( context: context, useSafeArea: true, @@ -340,7 +374,15 @@ class History extends StatelessWidget { : null, ), const Divider(height: 0), - if (!isSetupCreation) + ListTile( + leading: const Icon(Icons.edit), + title: const Text('Edit comment'), + onTap: () { + Navigator.pop(sheetContext); + _showEditCommentDialog(context, entry); + }, + ), + if (!entry.isCreationEntry) ListTile( leading: const Icon(Icons.undo), title: const Text('Undo this change'), diff --git a/lib/setup_edit.dart b/lib/setup_edit.dart index 8e41f55..27fe555 100644 --- a/lib/setup_edit.dart +++ b/lib/setup_edit.dart @@ -82,8 +82,12 @@ class _SetupEditState extends State { if (settingChanges.changes.isNotEmpty) { newSetup.history.add(settingChanges); } else if (widget.setup == null || widget.setup!.history.isEmpty) { - settingChanges.comment = SettingChanges.defaultComment; - newSetup.history.add(settingChanges); + newSetup.history.add(SettingChanges( + changes: [], + date: settingChanges.date, + comment: 'Setup creation', + isCreationEntry: true, + )); } newSetup.name = _setupFormController.name.text; diff --git a/test/migrations/migrator_test.dart b/test/migrations/migrator_test.dart index 5354f2d..c5175a7 100644 --- a/test/migrations/migrator_test.dart +++ b/test/migrations/migrator_test.dart @@ -11,7 +11,7 @@ Map _v1Data() => { }, }; -Map _v2Data() => { +Map _v2Data({List history = const []}) => { 'schemaVersion': 2, 'setups': { 'id-1': { @@ -35,43 +35,73 @@ Map _v2Data() => { 'lsr': {'value': 4, 'unit': 'Clicks'}, 'hsr': null, }, - 'history': [], + 'history': history, + }, + }, + }; + +Map _v3Data() => { + 'schemaVersion': 3, + 'setups': { + 'id-1': { + 'id': 'id-1', + 'name': 'Test', + 'fork': { + 'airPressure': {'value': 100, 'unit': 'PSI'}, + }, + 'shock': {}, + 'history': [ + {'changes': [], 'date': '2024-01-01T00:00:00.000Z', 'comment': 'Setup creation', 'isCreationEntry': true}, + ], }, }, }; void main() { group('currentSchemaVersion', () { - test('is 2', () => expect(currentSchemaVersion, 2)); + test('is 3', () => expect(currentSchemaVersion, 3)); }); group('migrateIfNeeded', () { test('migrates data with no schemaVersion (treated as v1)', () { final result = migrateIfNeeded(_v1Data()); - expect(result['schemaVersion'], 2); + expect(result['schemaVersion'], 3); expect(result['setups'], isA()); }); test('migrates data with explicit schemaVersion 1', () { final data = {'schemaVersion': 1, ..._v1Data()}; final result = migrateIfNeeded(data); - expect(result['schemaVersion'], 2); + expect(result['schemaVersion'], 3); }); - test('returns v2 data unchanged', () { - final v2 = _v2Data(); - final result = migrateIfNeeded(v2); - expect(result['schemaVersion'], 2); + test('migrates v2 data to v3', () { + final result = migrateIfNeeded(_v2Data()); + expect(result['schemaVersion'], 3); expect(result['setups']['id-1']['fork']['airPressure']['value'], 100); }); - test('does not re-migrate already-v2 data', () { - final v2 = _v2Data(); + test('does not re-migrate already-v3 data', () { + final result = migrateIfNeeded(_v3Data()); + expect(result['schemaVersion'], 3); + expect(result['setups']['id-1']['history'][0]['isCreationEntry'], true); + }); + + test('v2 to v3 sets isCreationEntry on first history entry', () { + final v2 = _v2Data(history: [ + {'changes': [], 'date': '2024-01-01T00:00:00.000Z', 'comment': 'Setup creation'}, + {'changes': [], 'date': '2024-01-02T00:00:00.000Z', 'comment': 'Rebound tweak'}, + ]); final result = migrateIfNeeded(v2); - // The fork airPressure should still be the v2 field object, not double-wrapped - expect(result['setups']['id-1']['fork']['airPressure'], isA()); - expect(result['setups']['id-1']['fork']['airPressure']['value'], 100); - expect(result['setups']['id-1']['fork']['airPressure']['unit'], 'PSI'); + final history = result['setups']['id-1']['history'] as List; + expect(history[0]['isCreationEntry'], true); + expect(history[1]['isCreationEntry'], isNull); + }); + + test('v2 to v3 leaves empty history unchanged', () { + final result = migrateIfNeeded(_v2Data()); + final history = result['setups']['id-1']['history'] as List; + expect(history, isEmpty); }); test('returns future schema version data unchanged', () { diff --git a/test/migrations/v2_to_v3_test.dart b/test/migrations/v2_to_v3_test.dart new file mode 100644 index 0000000..4b58004 --- /dev/null +++ b/test/migrations/v2_to_v3_test.dart @@ -0,0 +1,131 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:suspension_setup/migrations/v2_to_v3.dart'; + +Map _v2History({ + String date = '2024-01-01T00:00:00.000Z', + String? comment, + List changes = const [], +}) => + { + 'date': date, + 'comment': comment, + 'changes': changes, + }; + +Map _v2Setup({ + String id = 'id-1', + String name = 'Test Setup', + List history = const [], +}) => + { + 'id': id, + 'name': name, + 'fork': {'airPressure': null}, + 'shock': {'airPressure': null}, + 'history': history, + }; + +Map _v2Data([Map? setup]) => { + 'schemaVersion': 2, + 'setups': {'id-1': setup ?? _v2Setup()}, + }; + +void main() { + group('migrateV2ToV3 — output structure', () { + test('bumps schemaVersion to 3', () { + final result = migrateV2ToV3(_v2Data()); + expect(result['schemaVersion'], 3); + }); + + test('preserves setups map key', () { + final result = migrateV2ToV3({'schemaVersion': 2, 'setups': {'my-id': _v2Setup(id: 'my-id')}}); + expect(result['setups'], contains('my-id')); + }); + + test('preserves setup name', () { + final data = _v2Data(_v2Setup(name: 'Enduro Race')); + final setup = migrateV2ToV3(data)['setups']['id-1']; + expect(setup['name'], 'Enduro Race'); + }); + + test('empty v2 setups map produces empty setups map', () { + final result = migrateV2ToV3({'schemaVersion': 2, 'setups': {}}); + expect(result['setups'], isEmpty); + }); + + test('migrates multiple setups independently', () { + final data = { + 'schemaVersion': 2, + 'setups': { + 'id-1': _v2Setup(id: 'id-1', name: 'DH', history: [_v2History()]), + 'id-2': _v2Setup(id: 'id-2', name: 'XC', history: [_v2History()]), + }, + }; + final result = migrateV2ToV3(data)['setups'] as Map; + expect(result['id-1']['name'], 'DH'); + expect(result['id-2']['name'], 'XC'); + expect(result['id-1']['history'][0]['isCreationEntry'], true); + expect(result['id-2']['history'][0]['isCreationEntry'], true); + }); + }); + + group('migrateV2ToV3 — isCreationEntry flag', () { + test('sets isCreationEntry on first history entry', () { + final data = _v2Data(_v2Setup(history: [_v2History()])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history[0]['isCreationEntry'], true); + }); + + test('does not set isCreationEntry on subsequent entries', () { + final data = _v2Data(_v2Setup(history: [ + _v2History(date: '2024-01-01T00:00:00.000Z', comment: 'Setup creation'), + _v2History(date: '2024-01-02T00:00:00.000Z', comment: 'Rebound tweak'), + _v2History(date: '2024-01-03T00:00:00.000Z', comment: 'Air pressure'), + ])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history[0]['isCreationEntry'], true); + expect(history[1]['isCreationEntry'], isNull); + expect(history[2]['isCreationEntry'], isNull); + }); + + test('empty history is left unchanged', () { + final data = _v2Data(_v2Setup(history: [])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history, isEmpty); + }); + }); + + group('migrateV2ToV3 — preserves history entry fields', () { + test('preserves date on first entry', () { + final data = _v2Data(_v2Setup(history: [_v2History(date: '2024-06-15T10:30:00.000Z')])); + final entry = migrateV2ToV3(data)['setups']['id-1']['history'][0]; + expect(entry['date'], '2024-06-15T10:30:00.000Z'); + }); + + test('preserves comment on first entry', () { + final data = _v2Data(_v2Setup(history: [_v2History(comment: 'Setup creation')])); + final entry = migrateV2ToV3(data)['setups']['id-1']['history'][0]; + expect(entry['comment'], 'Setup creation'); + }); + + test('preserves changes list on first entry', () { + final changes = [ + {'settingType': 'airPressure', 'suspensionType': 'fork', 'oldValue': 100, 'newValue': 110}, + ]; + final data = _v2Data(_v2Setup(history: [_v2History(changes: changes)])); + final entry = migrateV2ToV3(data)['setups']['id-1']['history'][0]; + expect(entry['changes'], hasLength(1)); + expect(entry['changes'][0]['newValue'], 110); + }); + + test('preserves all fields on subsequent entries', () { + final data = _v2Data(_v2Setup(history: [ + _v2History(date: '2024-01-01T00:00:00.000Z'), + _v2History(date: '2024-01-02T00:00:00.000Z', comment: 'Rebound tweak'), + ])); + final second = migrateV2ToV3(data)['setups']['id-1']['history'][1]; + expect(second['date'], '2024-01-02T00:00:00.000Z'); + expect(second['comment'], 'Rebound tweak'); + }); + }); +} \ No newline at end of file diff --git a/test/models/setup_test.dart b/test/models/setup_test.dart index 8af8a58..4c9c0ea 100644 --- a/test/models/setup_test.dart +++ b/test/models/setup_test.dart @@ -243,7 +243,8 @@ void main() { SettingChanges( changes: [], date: DateTime.utc(2024, 1, 1), - comment: SettingChanges.defaultComment, + comment: 'Setup creation', + isCreationEntry: true, ), ], ); @@ -255,7 +256,8 @@ void main() { expect(restored.fork.hsr, isNull); expect(restored.shock.sag?.value, 30); expect(restored.history, hasLength(1)); - expect(restored.history.first.comment, SettingChanges.defaultComment); + expect(restored.history.first.comment, 'Setup creation'); + expect(restored.history.first.isCreationEntry, isTrue); }); test('clone with history produces independent copy with new id', () { From 0e85dd2dde2f3a62f42ad46cf82cb364de10ace3 Mon Sep 17 00:00:00 2001 From: pec0ra Date: Sun, 10 May 2026 13:55:12 +0200 Subject: [PATCH 3/5] Add context menu when long pressing a setup --- lib/home_page.dart | 69 ++++++++++++++---------- lib/setup_actions.dart | 84 +++++++++++++++++++++++++++++ lib/setup_detail.dart | 116 ++++++++--------------------------------- 3 files changed, 148 insertions(+), 121 deletions(-) create mode 100644 lib/setup_actions.dart diff --git a/lib/home_page.dart b/lib/home_page.dart index c3be539..57d13e2 100644 --- a/lib/home_page.dart +++ b/lib/home_page.dart @@ -5,6 +5,7 @@ import 'package:provider/provider.dart'; import 'error_screen.dart'; import 'models/setup.dart'; +import 'setup_actions.dart'; import 'setup_detail.dart'; import 'setup_edit.dart'; import 'setup_storage_model.dart'; @@ -137,8 +138,7 @@ class _HomePageState extends State { )), ); }, - // TODO: add context menu in addition to dialog - onLongPress: () => deleteSetup(context, setup, setupModel), + onLongPress: () => _showSetupSheet(context, setup, setupModel), ), ), ], @@ -155,6 +155,46 @@ class _HomePageState extends State { ); } + void _showSetupSheet(BuildContext context, Setup setup, SetupStorageModel setupModel) { + final theme = Theme.of(context); + showModalBottomSheet( + context: context, + useSafeArea: true, + builder: (sheetContext) { + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + ListTile( + title: Text(setup.name, style: theme.textTheme.titleMedium), + subtitle: setup.history.isNotEmpty + ? Text(DateFormat.yMMMd().format(setup.history.last.date)) + : null, + ), + const Divider(height: 0), + ListTile( + leading: const Icon(Icons.copy), + title: const Text('Clone'), + onTap: () { + Navigator.pop(sheetContext); + showCloneSetupDialog(context, setup, + onConfirm: (copyHistory) => + navigateToClone(context, setup, copyHistory)); + }, + ), + ListTile( + leading: Icon(Icons.delete, color: theme.colorScheme.error), + title: Text('Delete', style: TextStyle(color: theme.colorScheme.error)), + onTap: () { + Navigator.pop(sheetContext); + showDeleteSetupDialog(context, setup, setupModel); + }, + ), + ], + ); + }, + ); + } + void createSetup() { Navigator.push( context, @@ -162,31 +202,6 @@ class _HomePageState extends State { ); } - Future deleteSetup( - BuildContext context, Setup setup, SetupStorageModel setupModel) { - return showDialog( - context: context, - builder: (BuildContext context) => AlertDialog( - title: Text('Delete Setup ${setup.name} ?'), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context, 'Cancel'), - child: const Text('Cancel'), - ), - TextButton( - style: TextButton.styleFrom(foregroundColor: Theme.of(context).colorScheme.error), - onPressed: () async { - await setupModel.deleteSetup(setup); - if (!context.mounted) return; - Navigator.pop(context, 'OK'); - }, - child: const Text('Delete'), - ), - ], - ), - ); - } - Future _backup(BuildContext context) async { final success = await Provider.of(context, listen: false) .backup(); diff --git a/lib/setup_actions.dart b/lib/setup_actions.dart new file mode 100644 index 0000000..09d9de1 --- /dev/null +++ b/lib/setup_actions.dart @@ -0,0 +1,84 @@ +import 'package:flutter/material.dart'; + +import 'models/setup.dart'; +import 'setup_edit.dart'; +import 'setup_storage_model.dart'; + +Future showCloneSetupDialog( + BuildContext context, + Setup setup, { + required void Function(bool copyHistory) onConfirm, +}) async { + var copyHistory = false; + await showDialog( + context: context, + builder: (dialogContext) => AlertDialog( + title: Text('Clone Setup \'${setup.name}\'?'), + content: StatefulBuilder( + builder: (context, setState) => CheckboxListTile( + value: copyHistory, + title: const Text('Copy history'), + controlAffinity: ListTileControlAffinity.leading, + onChanged: (value) => setState(() => copyHistory = value ?? false), + ), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(dialogContext), + child: const Text('Cancel'), + ), + TextButton( + onPressed: () { + Navigator.pop(dialogContext); + onConfirm(copyHistory); + }, + child: const Text('OK'), + ), + ], + ), + ); +} + +Future showDeleteSetupDialog( + BuildContext context, + Setup setup, + SetupStorageModel model, { + VoidCallback? onDeleted, +}) async { + await showDialog( + context: context, + builder: (dialogContext) => AlertDialog( + title: Text('Delete Setup \'${setup.name}\'?'), + actions: [ + TextButton( + onPressed: () => Navigator.pop(dialogContext), + child: const Text('Cancel'), + ), + TextButton( + style: TextButton.styleFrom( + foregroundColor: Theme.of(dialogContext).colorScheme.error, + ), + onPressed: () async { + await model.deleteSetup(setup); + if (!dialogContext.mounted) return; + Navigator.pop(dialogContext); + onDeleted?.call(); + }, + child: const Text('Delete'), + ), + ], + ), + ); +} + +void navigateToClone(BuildContext context, Setup setup, bool copyHistory, + {bool replace = false}) { + final route = MaterialPageRoute( + builder: (context) => SetupEdit(setup: setup.clone(copyHistory)), + ); + if (replace) { + Navigator.pushReplacement(context, route); + } else { + Navigator.push(context, route); + } +} \ No newline at end of file diff --git a/lib/setup_detail.dart b/lib/setup_detail.dart index 8c0520a..c054a2d 100644 --- a/lib/setup_detail.dart +++ b/lib/setup_detail.dart @@ -10,6 +10,7 @@ import 'package:url_launcher/url_launcher.dart'; import 'models/setting_change.dart'; import 'models/settings.dart'; import 'setting_tiles.dart'; +import 'setup_actions.dart'; import 'setup_edit.dart'; import 'setup_storage_model.dart'; import 'title_with_icon.dart'; @@ -476,113 +477,40 @@ class History extends StatelessWidget { } } -class OverflowMenu extends StatefulWidget { +class OverflowMenu extends StatelessWidget { const OverflowMenu({super.key, required this.setup}); final Setup setup; - @override - State createState() => _OverflowMenuState(); -} - -class _OverflowMenuState extends State { - bool _copyHistory = false; - - @override - void initState() { - super.initState(); - } - @override Widget build(BuildContext context) { return PopupMenuButton( - itemBuilder: (context) => [ - PopupMenuItem( - onTap: () => duplicate(context), - child: const Row( - spacing: 12, - children: [ - Icon(Icons.copy), - Text('Clone'), - ], - ), - ), - PopupMenuItem( - onTap: () => delete(context), - child: const Row( - spacing: 12, - children: [ - Icon(Icons.delete), - Text('Delete'), - ], - ), - ), - ]); - } - - void duplicate(BuildContext context) { - showDialog( - context: context, - builder: (BuildContext context) => AlertDialog( - title: Text('Duplicate Setup \'${widget.setup.name}\'?'), - content: StatefulBuilder( - builder: (BuildContext context, StateSetter setState) => - CheckboxListTile( - value: _copyHistory, - title: const Text('Copy history'), - controlAffinity: ListTileControlAffinity.leading, - onChanged: (bool? value) { - setState(() { - _copyHistory = value ?? false; - }); - }, + itemBuilder: (context) => [ + PopupMenuItem( + onTap: () => showCloneSetupDialog(context, setup, + onConfirm: (copyHistory) => + navigateToClone(context, setup, copyHistory, replace: true)), + child: const Row( + spacing: 12, + children: [Icon(Icons.copy), Text('Clone')], ), ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context, 'Cancel'), - child: const Text('Cancel'), - ), - TextButton( - onPressed: () { - Navigator.pop(context, 'OK'); - Navigator.pushReplacement( - context, - MaterialPageRoute( - builder: (context) => - SetupEdit(setup: widget.setup.clone(_copyHistory))), - ); - }, - child: const Text('OK'), - ), - ], - ), - ); - } - - void delete(BuildContext context) { - showDialog( - context: context, - builder: (BuildContext context) => AlertDialog( - title: Text('Delete Setup \'${widget.setup.name}\'?'), - actions: [ - TextButton( - onPressed: () => Navigator.pop(context, 'Cancel'), - child: const Text('Cancel'), - ), - TextButton( - style: TextButton.styleFrom(foregroundColor: Theme.of(context).colorScheme.error), - onPressed: () async { - await Provider.of(context, listen: false) - .deleteSetup(widget.setup); + PopupMenuItem( + onTap: () => showDeleteSetupDialog( + context, + setup, + Provider.of(context, listen: false), + onDeleted: () { if (!context.mounted) return; - Navigator.pop(context, 'OK'); Navigator.pop(context); }, - child: const Text('Delete'), ), - ], - ), + child: const Row( + spacing: 12, + children: [Icon(Icons.delete), Text('Delete')], + ), + ), + ], ); } } From e744a3af114bb16d593d0204ef024dafdf0d8e1d Mon Sep 17 00:00:00 2001 From: pec0ra Date: Sun, 10 May 2026 14:28:11 +0200 Subject: [PATCH 4/5] Fixes and improvements --- integration_test/screenshot_test.dart | 4 + lib/migrations/migrator.dart | 2 +- lib/migrations/v2_to_v3.dart | 16 +- lib/models/setting_change.dart | 9 +- lib/models/setup.dart | 4 + lib/setup_actions.dart | 2 +- lib/setup_detail.dart | 4 +- test/migrations/v2_to_v3_test.dart | 31 +- test/models/setup_test.dart | 402 ++++++++++++++++++++++++++ 9 files changed, 464 insertions(+), 10 deletions(-) diff --git a/integration_test/screenshot_test.dart b/integration_test/screenshot_test.dart index 7132982..9de526c 100644 --- a/integration_test/screenshot_test.dart +++ b/integration_test/screenshot_test.dart @@ -108,6 +108,7 @@ const _fixture = { 'tyres': {'front': null, 'rear': null}, 'history': [ { + 'id': 'a5f0c760-7346-11ef-89aa-000000000001', 'changes': [], 'date': '2024-09-15T11:41:14.071433', 'comment': 'Setup creation', @@ -141,12 +142,14 @@ const _fixture = { 'tyres': {'front': null, 'rear': null}, 'history': [ { + 'id': 'f65fdaf0-735b-11ef-8bca-000000000001', 'changes': [], 'date': '2024-09-15T14:13:48.447488', 'comment': 'Setup creation', 'isCreationEntry': true, }, { + 'id': 'f65fdaf0-735b-11ef-8bca-000000000002', 'changes': [ { 'suspensionType': 'shock', @@ -169,6 +172,7 @@ const _fixture = { 'comment': 'Less pressure, more compression', }, { + 'id': 'f65fdaf0-735b-11ef-8bca-000000000003', 'changes': [ { 'suspensionType': 'fork', diff --git a/lib/migrations/migrator.dart b/lib/migrations/migrator.dart index eedadd2..2e864ec 100644 --- a/lib/migrations/migrator.dart +++ b/lib/migrations/migrator.dart @@ -14,4 +14,4 @@ Map migrateIfNeeded(Map json) { data = migrateV2ToV3(data); } return data; -} \ No newline at end of file +} diff --git a/lib/migrations/v2_to_v3.dart b/lib/migrations/v2_to_v3.dart index 43174c8..4f733fa 100644 --- a/lib/migrations/v2_to_v3.dart +++ b/lib/migrations/v2_to_v3.dart @@ -1,5 +1,7 @@ +import 'package:uuid/uuid.dart'; + /// v2 structure: { schemaVersion: 2, setups: { setupId: { ..., history: [...] } } } -/// v3 structure: { schemaVersion: 3, setups: { setupId: { ..., history: [{ isCreationEntry: true, ... }, ...] } } } +/// v3 structure: { schemaVersion: 3, setups: { setupId: { ..., history: [{ id: '...', isCreationEntry: true, ... }, ...] } } } Map migrateV2ToV3(Map v2Data) { final setups = Map.from(v2Data['setups'] as Map); final migratedSetups = setups.map( @@ -12,8 +14,14 @@ Map _migrateSetup(Map setup) { final history = setup['history'] as List; if (history.isEmpty) return Map.from(setup); final migratedHistory = [ - {...history[0] as Map, 'isCreationEntry': true}, - ...history.skip(1), + { + ...history[0] as Map, + 'id': const Uuid().v1(), + 'isCreationEntry': true, + }, + ...history.skip(1).map( + (e) => {...e as Map, 'id': const Uuid().v1()}, + ), ]; return {...setup, 'history': migratedHistory}; -} \ No newline at end of file +} diff --git a/lib/models/setting_change.dart b/lib/models/setting_change.dart index c3f9897..f5464c5 100644 --- a/lib/models/setting_change.dart +++ b/lib/models/setting_change.dart @@ -1,18 +1,23 @@ +import 'package:uuid/uuid.dart'; + class SettingChanges { + final String id; final List changes; final DateTime date; String? comment; final bool isCreationEntry; SettingChanges({ + String? id, required this.changes, required this.date, this.comment, this.isCreationEntry = false, - }); + }) : id = id ?? const Uuid().v1(); factory SettingChanges.fromJson(Map json) { return SettingChanges( + id: json['id'] as String, changes: List.from( json['changes'].map((c) => SettingChange.fromJson(c))), date: DateTime.parse((json['date'])), @@ -23,6 +28,7 @@ class SettingChanges { Map toJson() { return { + 'id': id, 'changes': changes.map((c) => c.toJson()).toList(), 'date': date.toIso8601String(), 'comment': comment, @@ -32,6 +38,7 @@ class SettingChanges { SettingChanges clone() { return SettingChanges( + id: id, changes: changes.map((e) => e.clone()).toList(), date: date, comment: comment, diff --git a/lib/models/setup.dart b/lib/models/setup.dart index ec2cdb7..f708025 100644 --- a/lib/models/setup.dart +++ b/lib/models/setup.dart @@ -59,6 +59,7 @@ class Setup { Setup copyMutable() => Setup.fromJson(toJson()); List computeUndo(SettingChanges historyEntry) { + assert(!historyEntry.isCreationEntry, 'cannot undo a creation entry'); final result = []; for (final change in historyEntry.changes) { final Field? currentField; @@ -98,6 +99,9 @@ class Setup { for (final change in changes) { final bool targetEnabled = change.newEnabled ?? true; final num? targetValue = change.newValue; + assert(!targetEnabled || targetValue != null, + 'targetValue must not be null when targetEnabled is true'); + if (targetEnabled && targetValue == null) continue; if (change.suspensionType == SuspensionType.tyre) { final isFront = change.settingType == SettingType.frontTyrePressure; diff --git a/lib/setup_actions.dart b/lib/setup_actions.dart index 09d9de1..ee3d3cf 100644 --- a/lib/setup_actions.dart +++ b/lib/setup_actions.dart @@ -81,4 +81,4 @@ void navigateToClone(BuildContext context, Setup setup, bool copyHistory, } else { Navigator.push(context, route); } -} \ No newline at end of file +} diff --git a/lib/setup_detail.dart b/lib/setup_detail.dart index c054a2d..3905602 100644 --- a/lib/setup_detail.dart +++ b/lib/setup_detail.dart @@ -344,7 +344,7 @@ class History extends StatelessWidget { Navigator.pop(dialogContext); final newSetup = setup.copyMutable(); final target = newSetup.history.firstWhere( - (e) => e.date == entry.date, + (e) => e.id == entry.id, ); target.comment = newComment.isEmpty ? null : newComment; await Provider.of(context, listen: false) @@ -354,7 +354,7 @@ class History extends StatelessWidget { ), ], ), - ); + ).then((_) => controller.dispose()); } void _showHistoryItemSheet(BuildContext context, SettingChanges entry) { diff --git a/test/migrations/v2_to_v3_test.dart b/test/migrations/v2_to_v3_test.dart index 4b58004..d0d1ec0 100644 --- a/test/migrations/v2_to_v3_test.dart +++ b/test/migrations/v2_to_v3_test.dart @@ -95,6 +95,35 @@ void main() { }); }); + group('migrateV2ToV3 — id field', () { + test('adds id to first history entry', () { + final data = _v2Data(_v2Setup(history: [_v2History()])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history[0]['id'], isA()); + }); + + test('adds id to all history entries', () { + final data = _v2Data(_v2Setup(history: [ + _v2History(date: '2024-01-01T00:00:00.000Z'), + _v2History(date: '2024-01-02T00:00:00.000Z'), + _v2History(date: '2024-01-03T00:00:00.000Z'), + ])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history[0]['id'], isA()); + expect(history[1]['id'], isA()); + expect(history[2]['id'], isA()); + }); + + test('ids are unique across entries', () { + final data = _v2Data(_v2Setup(history: [ + _v2History(date: '2024-01-01T00:00:00.000Z'), + _v2History(date: '2024-01-02T00:00:00.000Z'), + ])); + final history = migrateV2ToV3(data)['setups']['id-1']['history'] as List; + expect(history[0]['id'], isNot(history[1]['id'])); + }); + }); + group('migrateV2ToV3 — preserves history entry fields', () { test('preserves date on first entry', () { final data = _v2Data(_v2Setup(history: [_v2History(date: '2024-06-15T10:30:00.000Z')])); @@ -128,4 +157,4 @@ void main() { expect(second['comment'], 'Rebound tweak'); }); }); -} \ No newline at end of file +} diff --git a/test/models/setup_test.dart b/test/models/setup_test.dart index 4c9c0ea..6f73006 100644 --- a/test/models/setup_test.dart +++ b/test/models/setup_test.dart @@ -200,6 +200,7 @@ void main() { comment: 'Trail ride tuning', ); final restored = SettingChanges.fromJson(changes.toJson()); + expect(restored.id, changes.id); expect(restored.date, date); expect(restored.comment, 'Trail ride tuning'); expect(restored.changes, hasLength(1)); @@ -287,6 +288,30 @@ void main() { expect(clone.history, hasLength(1)); }); + test('copyMutable preserves history entry ids', () { + final entry = SettingChanges(changes: [], date: DateTime.now()); + final original = Setup( + id: 'original-id', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 100, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [entry], + ); + final copy = original.copyMutable(); + expect(copy.history.first.id, entry.id); + }); + test('deserializes legacy JSON without tyres key as empty tyres', () { final json = { 'id': 'legacy-id', @@ -371,4 +396,381 @@ void main() { expect(clone.history, isEmpty); }); }); + + group('computeUndo', () { + Setup _setup({ + num forkAir = 110, + num forkLsc = 10, + num? frontTyre, + num? shock, + }) => + Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: Field(value: forkAir, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: Field(value: forkLsc, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: Field(value: shock ?? 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres( + front: frontTyre != null + ? Field(value: frontTyre, unit: 'PSI') + : null, + ), + history: [], + ); + + SettingChange _change({ + SuspensionType suspension = SuspensionType.fork, + SettingType setting = SettingType.airPressure, + num? oldValue, + num? newValue, + bool? oldEnabled, + bool? newEnabled, + }) => + SettingChange( + suspensionType: suspension, + settingType: setting, + oldValue: oldValue, + newValue: newValue, + oldEnabled: oldEnabled, + newEnabled: newEnabled, + ); + + test('returns undo change when value differs from old value', () { + final setup = _setup(forkAir: 110); + final entry = SettingChanges( + changes: [_change(oldValue: 100, newValue: 110)], + date: DateTime.now(), + ); + final result = setup.computeUndo(entry); + expect(result, hasLength(1)); + expect(result.first.settingType, SettingType.airPressure); + expect(result.first.oldValue, 110); + expect(result.first.newValue, 100); + }); + + test('skips change when current value already equals old value (no-op)', () { + final setup = _setup(forkAir: 100); + final entry = SettingChanges( + changes: [_change(oldValue: 100, newValue: 110)], + date: DateTime.now(), + ); + expect(setup.computeUndo(entry), isEmpty); + }); + + test('returns empty list for entry with no changes', () { + final setup = _setup(); + final entry = SettingChanges(changes: [], date: DateTime.now()); + expect(setup.computeUndo(entry), isEmpty); + }); + + test('asserts when called on a creation entry', () { + final setup = _setup(); + final entry = SettingChanges( + changes: [], + date: DateTime.now(), + isCreationEntry: true, + ); + expect(() => setup.computeUndo(entry), throwsA(isA())); + }); + + test('undo of enable: disables field and sets newValue to null', () { + // change was: disabled→enabled (oldEnabled=false, newEnabled=true, newValue=120) + // current state: field enabled at 120; undo should disable it + final setup = _setup(forkAir: 120); + final entry = SettingChanges( + changes: [ + _change(oldValue: null, newValue: 120, oldEnabled: false, newEnabled: true), + ], + date: DateTime.now(), + ); + final result = setup.computeUndo(entry); + expect(result, hasLength(1)); + expect(result.first.newEnabled, false); + expect(result.first.newValue, isNull); + }); + + test('undo of disable: re-enables field with old value', () { + // change was: enabled→disabled (oldEnabled=true, oldValue=100, newEnabled=false) + // current state: field disabled; undo should enable it at 100 + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 4, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + final entry = SettingChanges( + changes: [ + _change( + setting: SettingType.airPressure, + oldValue: 100, + newValue: null, + oldEnabled: true, + newEnabled: false, + ), + ], + date: DateTime.now(), + ); + final result = setup.computeUndo(entry); + expect(result, hasLength(1)); + expect(result.first.newEnabled, true); + expect(result.first.newValue, 100); + }); + + test('handles tyre pressure undo', () { + final setup = _setup(frontTyre: 24); + final entry = SettingChanges( + changes: [ + _change( + suspension: SuspensionType.tyre, + setting: SettingType.frontTyrePressure, + oldValue: 22, + newValue: 24, + ), + ], + date: DateTime.now(), + ); + final result = setup.computeUndo(entry); + expect(result, hasLength(1)); + expect(result.first.suspensionType, SuspensionType.tyre); + expect(result.first.newValue, 22); + }); + + test('handles multiple fields with partial no-ops', () { + // forkAir already at old value (100), forkLsc changed (8→10) + final setup = _setup(forkAir: 100, forkLsc: 10); + final entry = SettingChanges( + changes: [ + _change(setting: SettingType.airPressure, oldValue: 100, newValue: 110), + _change(setting: SettingType.lsc, oldValue: 8, newValue: 10), + ], + date: DateTime.now(), + ); + final result = setup.computeUndo(entry); + expect(result, hasLength(1)); + expect(result.first.settingType, SettingType.lsc); + expect(result.first.newValue, 8); + }); + }); + + group('applyChanges', () { + test('updates fork field value', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 100, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + setup.applyChanges([ + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.airPressure, + oldValue: 100, + newValue: 90, + ), + ]); + expect(setup.fork.airPressure?.value, 90); + }); + + test('preserves existing field unit when updating value', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 100, unit: 'bar'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + setup.applyChanges([ + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.airPressure, + oldValue: 100, + newValue: 90, + ), + ]); + expect(setup.fork.airPressure?.unit, 'bar'); + }); + + test('disables field when newEnabled is false', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 100, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + volumeSpacer: const Field(value: 2, unit: 'Spacers'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + setup.applyChanges([ + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.volumeSpacer, + oldValue: 2, + newValue: null, + oldEnabled: true, + newEnabled: false, + ), + ]); + expect(setup.fork.volumeSpacer, isNull); + }); + + test('enables field when newEnabled is true', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + setup.applyChanges([ + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.airPressure, + oldValue: null, + newValue: 100, + oldEnabled: false, + newEnabled: true, + ), + ]); + expect(setup.fork.airPressure?.value, 100); + }); + + test('updates tyre pressure', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 100, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 5, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(front: const Field(value: 24, unit: 'PSI')), + history: [], + ); + setup.applyChanges([ + SettingChange( + suspensionType: SuspensionType.tyre, + settingType: SettingType.frontTyrePressure, + oldValue: 24, + newValue: 22, + ), + ]); + expect(setup.tyres.front?.value, 22); + expect(setup.tyres.front?.unit, 'PSI'); + }); + + }); + + group('computeUndo + applyChanges round-trip', () { + test('undo restores setup to state before a value change', () { + final setup = Setup( + id: 'test', + name: 'Test', + fork: Settings( + airPressure: const Field(value: 110, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 10, unit: 'Clicks'), + lsr: const Field(value: 4, unit: 'Clicks'), + ), + shock: Settings( + airPressure: const Field(value: 150, unit: 'PSI'), + sag: const Field(value: 25, unit: '%'), + lsc: const Field(value: 3, unit: 'Clicks'), + lsr: const Field(value: 2, unit: 'Clicks'), + ), + tyres: Tyres(), + history: [], + ); + final entry = SettingChanges( + changes: [ + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.airPressure, + oldValue: 100, + newValue: 110, + ), + SettingChange( + suspensionType: SuspensionType.fork, + settingType: SettingType.lsc, + oldValue: 8, + newValue: 10, + ), + ], + date: DateTime.now(), + ); + final undoChanges = setup.computeUndo(entry); + setup.applyChanges(undoChanges); + expect(setup.fork.airPressure?.value, 100); + expect(setup.fork.lsc?.value, 8); + }); + }); } \ No newline at end of file From 9f2d5bef0591173fa34f5c5204c44aff477c4d3c Mon Sep 17 00:00:00 2001 From: pec0ra Date: Sun, 10 May 2026 14:36:42 +0200 Subject: [PATCH 5/5] Fix variable names --- test/models/setup_test.dart | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/models/setup_test.dart b/test/models/setup_test.dart index 6f73006..4dc4b25 100644 --- a/test/models/setup_test.dart +++ b/test/models/setup_test.dart @@ -398,7 +398,7 @@ void main() { }); group('computeUndo', () { - Setup _setup({ + Setup makeSetup({ num forkAir = 110, num forkLsc = 10, num? frontTyre, @@ -427,7 +427,7 @@ void main() { history: [], ); - SettingChange _change({ + SettingChange makeChange({ SuspensionType suspension = SuspensionType.fork, SettingType setting = SettingType.airPressure, num? oldValue, @@ -445,9 +445,9 @@ void main() { ); test('returns undo change when value differs from old value', () { - final setup = _setup(forkAir: 110); + final setup = makeSetup(forkAir: 110); final entry = SettingChanges( - changes: [_change(oldValue: 100, newValue: 110)], + changes: [makeChange(oldValue: 100, newValue: 110)], date: DateTime.now(), ); final result = setup.computeUndo(entry); @@ -458,22 +458,22 @@ void main() { }); test('skips change when current value already equals old value (no-op)', () { - final setup = _setup(forkAir: 100); + final setup = makeSetup(forkAir: 100); final entry = SettingChanges( - changes: [_change(oldValue: 100, newValue: 110)], + changes: [makeChange(oldValue: 100, newValue: 110)], date: DateTime.now(), ); expect(setup.computeUndo(entry), isEmpty); }); test('returns empty list for entry with no changes', () { - final setup = _setup(); + final setup = makeSetup(); final entry = SettingChanges(changes: [], date: DateTime.now()); expect(setup.computeUndo(entry), isEmpty); }); test('asserts when called on a creation entry', () { - final setup = _setup(); + final setup = makeSetup(); final entry = SettingChanges( changes: [], date: DateTime.now(), @@ -485,10 +485,10 @@ void main() { test('undo of enable: disables field and sets newValue to null', () { // change was: disabled→enabled (oldEnabled=false, newEnabled=true, newValue=120) // current state: field enabled at 120; undo should disable it - final setup = _setup(forkAir: 120); + final setup = makeSetup(forkAir: 120); final entry = SettingChanges( changes: [ - _change(oldValue: null, newValue: 120, oldEnabled: false, newEnabled: true), + makeChange(oldValue: null, newValue: 120, oldEnabled: false, newEnabled: true), ], date: DateTime.now(), ); @@ -520,7 +520,7 @@ void main() { ); final entry = SettingChanges( changes: [ - _change( + makeChange( setting: SettingType.airPressure, oldValue: 100, newValue: null, @@ -537,10 +537,10 @@ void main() { }); test('handles tyre pressure undo', () { - final setup = _setup(frontTyre: 24); + final setup = makeSetup(frontTyre: 24); final entry = SettingChanges( changes: [ - _change( + makeChange( suspension: SuspensionType.tyre, setting: SettingType.frontTyrePressure, oldValue: 22, @@ -557,11 +557,11 @@ void main() { test('handles multiple fields with partial no-ops', () { // forkAir already at old value (100), forkLsc changed (8→10) - final setup = _setup(forkAir: 100, forkLsc: 10); + final setup = makeSetup(forkAir: 100, forkLsc: 10); final entry = SettingChanges( changes: [ - _change(setting: SettingType.airPressure, oldValue: 100, newValue: 110), - _change(setting: SettingType.lsc, oldValue: 8, newValue: 10), + makeChange(setting: SettingType.airPressure, oldValue: 100, newValue: 110), + makeChange(setting: SettingType.lsc, oldValue: 8, newValue: 10), ], date: DateTime.now(), );