fixing issue with selecting winners among the old scores (#234)

board_games_companion/lib/extensions/scores_extensions.dart

@@ -14,8 +14,23 @@ extension ScoreExtesions on Score {
 extension ScoresExtesions on Iterable<Score>? {
   List<Score> onlyScoresWithValue() => this?.where((s) => s.hasScore).toList() ?? <Score>[];
 
-  List<Score> winners() =>
-      this?.onlyScoresWithValue().where((s) => s.isWinner).toList() ?? <Score>[];
+  /// Returns winner(s)
+  ///
+  /// In case [Score]s are recorded using the "old" ways, grab the highest/lowest score
+  /// and treat as a winner.
+  List<Score> winners(GameFamily gameFamily) {
+    var winners = this?.onlyScoresWithValue().where((s) => s.isWinner).toList();
+
+    // Get the winner by ordering scores highest/lowest and taking the top one
+    if (winners?.isEmpty ?? true) {
+      final orderedScores = this?.onlyScoresWithValue().sortByScore(gameFamily);
+      if (orderedScores?.isNotEmpty ?? false) {
+        winners = [orderedScores!.first];
+      }
+    }
+
+    return winners ?? [];
+  }
 
   List<Score> onlyCooperativeGames() {
     return this?.where((s) => s.noScoreGameResult?.cooperativeGameResult != null).toList() ??

board_games_companion/lib/pages/playthroughs/playthrough_statistics_view_model.dart

@@ -264,7 +264,7 @@ abstract class _PlaythroughStatisticsViewModel with Store {
     }
 
     final lastPlaythroughBestScores =
-        playthroughScoresByPlaythroughId[lastPlaythrough.id].winners();
+        playthroughScoresByPlaythroughId[lastPlaythrough.id].winners(gameFamily);
     if (lastPlaythroughBestScores.isEmpty) {
       return;
     }

board_games_companion/lib/pages/playthroughs/playthroughs_statistics_page.dart

@@ -1,5 +1,6 @@
 import 'dart:math';
 
+import 'package:board_games_companion/models/player_score.dart';
 import 'package:fl_chart/fl_chart.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_mobx/flutter_mobx.dart';
@@ -181,7 +182,10 @@ class _ScoreBoardGameStatistics extends StatelessWidget {
         ),
       ),
       _SliverSectionWrapper(
-        child: _LastWinnerSection(scoreBoardGameStatistics: scoreBoardGameStatistics),
+        child: _LastWinnerSection(
+          lastGameWinners: scoreBoardGameStatistics.lastGameWinners,
+          lastTimePlayed: scoreBoardGameStatistics.lastTimePlayed,
+        ),
       ),
       SliverPersistentHeader(
         delegate: BgcSliverTitleHeaderDelegate.title(
@@ -435,10 +439,12 @@ class _PlayerStatsDetails extends StatelessWidget {
 class _LastWinnerSection extends StatelessWidget {
   const _LastWinnerSection({
     Key? key,
-    required this.scoreBoardGameStatistics,
+    required this.lastGameWinners,
+    required this.lastTimePlayed,
   }) : super(key: key);
 
-  final ScoreBoardGameStatistics scoreBoardGameStatistics;
+  final List<PlayerScore>? lastGameWinners;
+  final DateTime lastTimePlayed;
 
   @override
   Widget build(BuildContext context) {
@@ -451,17 +457,15 @@ class _LastWinnerSection extends StatelessWidget {
           scrollDirection: Axis.horizontal,
           child: Row(
             children: <Widget>[
-              for (final player
-                  in scoreBoardGameStatistics.lastGameWinners?.map((ps) => ps.player) ??
-                      <Player?>[]) ...[
-                _LastWinnerAvatar(player: player),
+              for (final playerScore in lastGameWinners ?? <PlayerScore>[]) ...[
+                _LastWinnerAvatar(player: playerScore.player),
                 const SizedBox(width: Dimensions.standardSpacing),
               ],
               _LastWinnerPoints(
-                points: scoreBoardGameStatistics.lastGameWinners?.first.score.score,
+                points: lastGameWinners?.first.score.score,
               ),
               const SizedBox(width: Dimensions.standardSpacing),
-              _LastTimePlayed(scoreBoardGameStatistics: scoreBoardGameStatistics),
+              _LastTimePlayed(lastTimePlayed: lastTimePlayed),
             ],
           ),
         ),
@@ -730,20 +734,20 @@ class _LastWinnerAvatar extends StatelessWidget {
 
 class _LastTimePlayed extends StatelessWidget {
   const _LastTimePlayed({
-    required this.scoreBoardGameStatistics,
+    required this.lastTimePlayed,
     Key? key,
   }) : super(key: key);
 
-  final ScoreBoardGameStatistics scoreBoardGameStatistics;
+  final DateTime lastTimePlayed;
 
   @override
   Widget build(BuildContext context) {
     return Row(
       children: <Widget>[
-        CalendarCard(scoreBoardGameStatistics.lastTimePlayed),
+        CalendarCard(lastTimePlayed),
         const SizedBox(width: Dimensions.standardSpacing),
         Text(
-          scoreBoardGameStatistics.lastTimePlayed.toDaysAgo(),
+          lastTimePlayed.toDaysAgo(),
           style: const TextStyle(
             fontSize: Dimensions.smallFontSize,
             fontWeight: FontWeight.normal,

board_games_companion/test/extensions/scores_extensions_test.dart

@@ -0,0 +1,92 @@
+import 'package:board_games_companion/common/enums/game_family.dart';
+import 'package:board_games_companion/models/hive/score.dart';
+import 'package:board_games_companion/models/hive/score_game_results.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  const Score emptyScore = Score(boardGameId: '', id: '', playerId: '', value: '');
+
+  setUp(() {});
+
+  test(
+      'GIVEN a collection of scores '
+      'WHEN a score has the result with the first place '
+      'THEN that single score is a winner ', () {
+    final firstPlaceScore =
+        emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 1, points: 10));
+    final scores = [
+      emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 3, points: 7)),
+      firstPlaceScore,
+      emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 2, points: 9)),
+    ];
+
+    // [GameFamily] doesn't matter in this instance because it's used only for the sake of old scores
+    // without the [ScoreGameResult]
+    final winners = scores.winners(GameFamily.HighestScore);
+
+    expect(winners.length, 1);
+    expect(winners, [firstPlaceScore]);
+  });
+
+  test(
+      'GIVEN a collection of scores '
+      'WHEN a victory is shared between scores '
+      'THEN multiple score are winners ', () {
+    final firstPlaceScore = emptyScore.copyWith(
+      playerId: '1',
+      scoreGameResult: const ScoreGameResult(place: 1, points: 10),
+    );
+    final alsoFirstPlaceScore = emptyScore.copyWith(
+      playerId: '2',
+      scoreGameResult: const ScoreGameResult(place: 1, points: 10),
+    );
+    final scores = [
+      emptyScore.copyWith(scoreGameResult: const ScoreGameResult(place: 3, points: 7)),
+      firstPlaceScore,
+      alsoFirstPlaceScore,
+    ];
+
+    // [GameFamily] doesn't matter in this instance because it's used only for the sake of old scores
+    // without the [ScoreGameResult]
+    final winners = scores.winners(GameFamily.HighestScore);
+
+    expect(winners.length, 2);
+    expect(winners, [firstPlaceScore, alsoFirstPlaceScore]);
+  });
+
+  test(
+      'GIVEN a collection of scores '
+      'WHEN scores are old records, without game score defined '
+      'AND the game family is highest score '
+      'THEN the highest score value should be a winner based ', () {
+    final highestScore = emptyScore.copyWith(playerId: '1', value: '20');
+    final scores = [
+      emptyScore.copyWith(value: '10'),
+      highestScore,
+      emptyScore.copyWith(value: '3'),
+    ];
+
+    final winners = scores.winners(GameFamily.HighestScore);
+
+    expect(winners.length, 1);
+    expect(winners, [highestScore]);
+  });
+
+  test(
+      'GIVEN a collection of scores '
+      'WHEN scores are old records, without game score defined '
+      'AND the game family is lowest score '
+      'THEN the lowest score value should be a winner based ', () {
+    final lowestScore = emptyScore.copyWith(playerId: '1', value: '3');
+    final scores = [
+      emptyScore.copyWith(value: '20'),
+      lowestScore,
+      emptyScore.copyWith(value: '13'),
+    ];
+
+    final winners = scores.winners(GameFamily.LowestScore);
+
+    expect(winners.length, 1);
+    expect(winners, [lowestScore]);
+  });
+}