fixing score bugs and adding unit tests (#237)

board_games_companion/lib/extensions/scores_extensions.dart

@@ -85,6 +85,15 @@ int compareScores(
   GameFamily gameFamily, {
   bool ignorePlaces = false,
 }) {
+  // Regardless of the game gamily all the scores without a score goes to the bottom of the list
+  if (score.hasScore && !otherScore.hasScore) {
+    return Constants.moveAbove;
+  }
+
+  if (!score.hasScore && otherScore.hasScore) {
+    return Constants.moveBelow;
+  }
+
   switch (gameFamily) {
     case GameFamily.LowestScore:
       // MK Swap scores around

board_games_companion/lib/models/hive/score.dart

@@ -45,10 +45,7 @@ class Score with _$Score {
 
   String? get scoreFormatted => score?.toStringAsFixed(0);
 
-  bool get hasScore =>
-      (value.isNotNullOrBlank && num.tryParse(value!) != null) ||
-      scoreGameResult?.points != null ||
-      noScoreGameResult?.cooperativeGameResult != null;
+  bool get hasScore => score != null || noScoreGameResult?.cooperativeGameResult != null;
 
   bool get isTied => scoreGameResult?.tiebreakerType != null;
 

board_games_companion/lib/pages/edit_playthrough/edit_playthrough_view_model.dart

@@ -174,10 +174,12 @@ abstract class _EditPlaythoughViewModel with Store {
       return;
     }
 
+    final scoreGameResult = playerScore.score.scoreGameResult ?? const ScoreGameResult();
+
     final playerScoreIndex = playerScores.indexOf(playerScore);
     final updatedPlayerScore = playerScore.copyWith(
       score: playerScore.score.copyWith(
-        scoreGameResult: playerScore.score.scoreGameResult!.copyWith(
+        scoreGameResult: scoreGameResult.copyWith(
           points: newScore.toDouble(),
         ),
       ),

board_games_companion/test/extensions/scores_extensions_test.dart

@@ -8,6 +8,105 @@ void main() {
 
   setUp(() {});
 
+  group('SortByScore', () {
+    test(
+        'GIVEN a collection of scores '
+        'WHEN sorting by highest score '
+        'AND comparing only old scores '
+        'THEN scores should be sorted correctly ', () {
+      final firstPlaceScore = emptyScore.copyWith(value: '10');
+      final lastPlaceScore = emptyScore.copyWith(value: '2');
+      final secondPlaceScore = emptyScore.copyWith(value: '7');
+      final scores = [
+        lastPlaceScore,
+        firstPlaceScore,
+        secondPlaceScore,
+      ];
+
+      final sortedScores = scores.sortByScore(GameFamily.HighestScore);
+
+      expect(sortedScores, [firstPlaceScore, secondPlaceScore, lastPlaceScore]);
+    });
+
+    test(
+        'GIVEN a collection of scores '
+        'WHEN sorting by highest score '
+        'AND comparing only old scores with null values '
+        'THEN scores should be sorted correctly ', () {
+      final firstPlaceScore = emptyScore.copyWith(value: '10');
+      final nullValueScore = emptyScore.copyWith(value: null);
+      final secondNullValueScore = emptyScore.copyWith(value: null);
+      final secondPlaceScore = emptyScore.copyWith(value: '7');
+      final scores = [
+        nullValueScore,
+        firstPlaceScore,
+        secondNullValueScore,
+        secondPlaceScore,
+      ];
+
+      final sortedScores = scores.sortByScore(GameFamily.HighestScore);
+
+      expect(sortedScores, [
+        firstPlaceScore,
+        secondPlaceScore,
+        nullValueScore,
+        secondNullValueScore,
+      ]);
+    });
+
+    test(
+        'GIVEN a collection of scores '
+        'WHEN sorting by lowest score '
+        'AND comparing mix of old and new scores '
+        'THEN scores should be sorted correctly ', () {
+      final firstPlaceScore =
+          emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 1, points: 10));
+      final nullValueScore = emptyScore.copyWith(value: null);
+      final secondPlaceScore = emptyScore.copyWith(value: '7');
+      final scores = [
+        nullValueScore,
+        firstPlaceScore,
+        secondPlaceScore,
+      ];
+
+      final sortedScores = scores.sortByScore(GameFamily.LowestScore);
+
+      expect(sortedScores, [
+        secondPlaceScore,
+        firstPlaceScore,
+        nullValueScore,
+      ]);
+    });
+
+    test(
+        'GIVEN a collection of scores '
+        'WHEN sorting by lowest score '
+        'AND comparing mix of old and new scores '
+        'AND have a completely empty scores '
+        'THEN scores should be sorted correctly ', () {
+      final firstPlaceScore =
+          emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 1, points: 10));
+      final nullValueScore = emptyScore.copyWith(value: null, scoreGameResult: null);
+      final secondNullValueScore = emptyScore.copyWith(value: null, scoreGameResult: null);
+      final secondPlaceScore = emptyScore.copyWith(value: '7');
+      final scores = [
+        secondNullValueScore,
+        secondPlaceScore,
+        nullValueScore,
+        firstPlaceScore,
+      ];
+
+      final sortedScores = scores.sortByScore(GameFamily.LowestScore);
+
+      expect(sortedScores, [
+        secondPlaceScore,
+        firstPlaceScore,
+        nullValueScore,
+        secondNullValueScore,
+      ]);
+    });
+  });
+
   test(
       'GIVEN a collection of scores '
       'WHEN a score has the result with the first place '

board_games_companion/test/mocks/game_playthroughs_details_store_mock.dart

@@ -0,0 +1,4 @@
+import 'package:board_games_companion/stores/game_playthroughs_details_store.dart';
+import 'package:mocktail/mocktail.dart';
+
+class MockGamePlaythroughsDetailsStore extends Mock implements GamePlaythroughsDetailsStore {}

board_games_companion/test/models/playthrough_details_test.dart

@@ -1,6 +1,7 @@
 import 'package:board_games_companion/models/hive/player.dart';
 import 'package:board_games_companion/models/hive/playthrough.dart';
 import 'package:board_games_companion/models/hive/score.dart';
+import 'package:board_games_companion/models/hive/score_game_results.dart';
 import 'package:board_games_companion/models/player_score.dart';
 import 'package:board_games_companion/models/playthroughs/playthrough_details.dart';
 import 'package:clock/clock.dart';
@@ -28,17 +29,57 @@ void main() {
 
   test(
       'GIVEN playthrough details  '
-      'WHEN there are scores with the same score '
+      'WHEN there are scores with tiebreakrer type defined '
       'THEN then the hasTies property should be true ', () {
     playthroughDetails = PlaythroughDetails(
       playthrough: emptyPlaythrough,
       playerScores: <PlayerScore>[
-        emptyPlayerScore.copyWith(score: emptyScore.copyWith(value: '100')),
-        emptyPlayerScore.copyWith(score: emptyScore.copyWith(value: '100')),
+        emptyPlayerScore.copyWith(
+          score: emptyScore.copyWith(
+            scoreGameResult: const ScoreGameResult(
+              points: 10,
+              place: 1,
+              tiebreakerType: ScoreTiebreakerType.place,
+            ),
+          ),
+        ),
+        emptyPlayerScore.copyWith(
+          score: emptyScore.copyWith(
+            scoreGameResult: const ScoreGameResult(
+              points: 10,
+              place: 1,
+              tiebreakerType: ScoreTiebreakerType.place,
+            ),
+          ),
+        ),
         emptyPlayerScore.copyWith(score: emptyScore.copyWith(value: '200')),
       ],
     );
 
     expect(playthroughDetails.hasTies, isTrue);
   });
+
+  test(
+      'GIVEN playthrough details  '
+      'WHEN all scores have been enetered '
+      'THEN finished scoring flag should be set to true ', () {
+    playthroughDetails = PlaythroughDetails(
+      playthrough: emptyPlaythrough,
+      playerScores: <PlayerScore>[
+        emptyPlayerScore.copyWith(
+          score: emptyScore.copyWith(
+            scoreGameResult: const ScoreGameResult(points: 10),
+          ),
+        ),
+        emptyPlayerScore.copyWith(
+          score: emptyScore.copyWith(
+            scoreGameResult: const ScoreGameResult(points: 1),
+          ),
+        ),
+        emptyPlayerScore.copyWith(score: emptyScore.copyWith(value: '200')),
+      ],
+    );
+
+    expect(playthroughDetails.finishedScoring, isTrue);
+  });
 }

board_games_companion/test/view_models/edit_playthrough_view_model_test.dart

@@ -0,0 +1,83 @@
+import 'package:board_games_companion/common/enums/game_classification.dart';
+import 'package:board_games_companion/common/enums/game_family.dart';
+import 'package:board_games_companion/models/hive/player.dart';
+import 'package:board_games_companion/models/hive/playthrough.dart';
+import 'package:board_games_companion/models/hive/score.dart';
+import 'package:board_games_companion/models/player_score.dart';
+import 'package:board_games_companion/models/playthroughs/playthrough_details.dart';
+import 'package:board_games_companion/pages/edit_playthrough/edit_playthrough_view_model.dart';
+import 'package:clock/clock.dart';
+import 'package:collection/collection.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:mobx/mobx.dart' show ObservableList;
+import 'package:mocktail/mocktail.dart';
+
+import '../mocks/game_playthroughs_details_store_mock.dart';
+
+void main() {
+  final mockGamePlaythroughsDetailsStore = MockGamePlaythroughsDetailsStore();
+
+  const String mockPlaythroughId = '1';
+  const String mockEmptyPlayerScoreId = '1';
+  const PlayerScore emptyPlayerScore = PlayerScore(
+    player: Player(id: mockEmptyPlayerScoreId),
+    score: Score(
+      boardGameId: '',
+      id: '',
+      playerId: '',
+    ),
+  );
+  final mockPlaythroughDetails = PlaythroughDetails(
+    playerScores: [emptyPlayerScore],
+    playthrough: Playthrough(
+      id: mockPlaythroughId,
+      boardGameId: '',
+      playerIds: [],
+      scoreIds: [],
+      startDate: clock.now(),
+    ),
+  );
+
+  late EditPlaythoughViewModel editPlaythrouhgViewModel;
+
+  setUp(() {
+    when(() => mockGamePlaythroughsDetailsStore.playthroughsDetails).thenReturn(
+      ObservableList.of([mockPlaythroughDetails]),
+    );
+    when(() => mockGamePlaythroughsDetailsStore.gameClassification)
+        .thenReturn(GameClassification.Score);
+    when(() => mockGamePlaythroughsDetailsStore.gameGameFamily).thenReturn(GameFamily.HighestScore);
+
+    editPlaythrouhgViewModel = EditPlaythoughViewModel(
+      mockGamePlaythroughsDetailsStore,
+    );
+    editPlaythrouhgViewModel.setPlaythroughId(mockPlaythroughId);
+  });
+
+  tearDown(() {
+    reset(mockGamePlaythroughsDetailsStore);
+  });
+
+  test(
+      'GIVEN edit playthrough view model '
+      'WHEN setting a playthrough with an id '
+      'THEN then the playthrough details should reflect that ', () {
+    expect(editPlaythrouhgViewModel.playthroughDetails, mockPlaythroughDetails);
+  });
+
+  test(
+      'GIVEN edit playthrough view model '
+      'WHEN updating an empty player score  '
+      'THEN the player score in the view model should reflect the change ', () {
+    final playerScoreToUpdate = mockPlaythroughDetails.playerScores
+        .firstWhereOrNull((element) => element.id == mockEmptyPlayerScoreId);
+    const newScore = 10.0;
+
+    editPlaythrouhgViewModel.updatePlayerScore(playerScoreToUpdate!, newScore);
+
+    final updatedPlayerScore = editPlaythrouhgViewModel.playerScores
+        .firstWhereOrNull((element) => element.id == playerScoreToUpdate.id);
+
+    expect(updatedPlayerScore!.score.scoreGameResult!.points, newScore);
+  });
+}