diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 27e0a922..c842899f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -81,6 +81,13 @@ You can always edit this file by hand instead — the helpers just save effort. answer that latches the card, and reduced motion lands it in a single frame with the colours and the words still changing. +- **The course ending counts what the spec asked for.** The completion moment + now shows the learner's *longest* streak and the number of Module Rewards + they earned, in place of the current streak and the total card count it + stood in with. Both are derived — the longest run comes off the same fold as + the current one, and a Module Reward is a card the content bank points at a + module — so nothing new is stored and a reset still leaves nothing behind. + - **The Today tour is the one the design draws.** A bordered frame travels between the four things it introduces instead of a spotlight snapping between them, the card carries its own counter, dots, Skip and Next, and diff --git a/lib/features/cards/domain/cards_providers.dart b/lib/features/cards/domain/cards_providers.dart index 80ba226c..c1622971 100644 --- a/lib/features/cards/domain/cards_providers.dart +++ b/lib/features/cards/domain/cards_providers.dart @@ -18,12 +18,12 @@ class CardWithCollection { final bool isCollected; } -/// Reads the collected ids **off the snapshot directly** rather than chaining -/// through `collectedCardsProvider.future`. The chained form hits a Riverpod -/// 3.2.1 internal-pause-state assertion (issue #4709) when the -/// `StatefulShellRoute` toggles `TickerMode` after the lesson-completion -/// screen invalidates the inner provider. Callers that mutate collected -/// cards must invalidate this provider alongside `collectedCardsProvider`. +/// Every card the bank holds, paired with whether the learner owns it. +/// +/// Reads the collected ids **off the snapshot directly**: chaining through a +/// provider hit a Riverpod 3.2.1 pause-state assertion (issue #4709) under the +/// `StatefulShellRoute`. So a caller that collects a card invalidates this, +/// and everything showing a collection hangs off it. @riverpod Future> cardsWithCollection(Ref ref) async { final content = ref.watch(contentRepositoryProvider); diff --git a/lib/features/cards/domain/cards_providers.g.dart b/lib/features/cards/domain/cards_providers.g.dart index 16684302..27326a01 100644 --- a/lib/features/cards/domain/cards_providers.g.dart +++ b/lib/features/cards/domain/cards_providers.g.dart @@ -8,22 +8,22 @@ part of 'cards_providers.dart'; // GENERATED CODE - DO NOT MODIFY BY HAND // ignore_for_file: type=lint, type=warning -/// Reads the collected ids **off the snapshot directly** rather than chaining -/// through `collectedCardsProvider.future`. The chained form hits a Riverpod -/// 3.2.1 internal-pause-state assertion (issue #4709) when the -/// `StatefulShellRoute` toggles `TickerMode` after the lesson-completion -/// screen invalidates the inner provider. Callers that mutate collected -/// cards must invalidate this provider alongside `collectedCardsProvider`. +/// Every card the bank holds, paired with whether the learner owns it. +/// +/// Reads the collected ids **off the snapshot directly**: chaining through a +/// provider hit a Riverpod 3.2.1 pause-state assertion (issue #4709) under the +/// `StatefulShellRoute`. So a caller that collects a card invalidates this, +/// and everything showing a collection hangs off it. @ProviderFor(cardsWithCollection) final cardsWithCollectionProvider = CardsWithCollectionProvider._(); -/// Reads the collected ids **off the snapshot directly** rather than chaining -/// through `collectedCardsProvider.future`. The chained form hits a Riverpod -/// 3.2.1 internal-pause-state assertion (issue #4709) when the -/// `StatefulShellRoute` toggles `TickerMode` after the lesson-completion -/// screen invalidates the inner provider. Callers that mutate collected -/// cards must invalidate this provider alongside `collectedCardsProvider`. +/// Every card the bank holds, paired with whether the learner owns it. +/// +/// Reads the collected ids **off the snapshot directly**: chaining through a +/// provider hit a Riverpod 3.2.1 pause-state assertion (issue #4709) under the +/// `StatefulShellRoute`. So a caller that collects a card invalidates this, +/// and everything showing a collection hangs off it. final class CardsWithCollectionProvider extends @@ -35,12 +35,12 @@ final class CardsWithCollectionProvider with $FutureModifier>, $FutureProvider> { - /// Reads the collected ids **off the snapshot directly** rather than chaining - /// through `collectedCardsProvider.future`. The chained form hits a Riverpod - /// 3.2.1 internal-pause-state assertion (issue #4709) when the - /// `StatefulShellRoute` toggles `TickerMode` after the lesson-completion - /// screen invalidates the inner provider. Callers that mutate collected - /// cards must invalidate this provider alongside `collectedCardsProvider`. + /// Every card the bank holds, paired with whether the learner owns it. + /// + /// Reads the collected ids **off the snapshot directly**: chaining through a + /// provider hit a Riverpod 3.2.1 pause-state assertion (issue #4709) under the + /// `StatefulShellRoute`. So a caller that collects a card invalidates this, + /// and everything showing a collection hangs off it. CardsWithCollectionProvider._() : super( from: null, diff --git a/lib/features/cards/domain/module_rewards.dart b/lib/features/cards/domain/module_rewards.dart new file mode 100644 index 00000000..73008ea4 --- /dev/null +++ b/lib/features/cards/domain/module_rewards.dart @@ -0,0 +1,13 @@ +/// The completion moment's Module Reward count. +library; + +import 'package:brew_path/features/cards/domain/cards_providers.dart'; + +/// How many Module Rewards the learner owns, out of [collection]. +/// +/// Derived on every read, never marked when a card is earned: a stored marker +/// would need a merge rule and a reset path of its own (#149). +int collectedModuleRewards(Iterable collection) => + collection + .where((entry) => entry.isCollected && entry.card.isModuleReward) + .length; diff --git a/lib/features/learn/presentation/course_completion_screen.dart b/lib/features/learn/presentation/course_completion_screen.dart index ea447845..d0acd12a 100644 --- a/lib/features/learn/presentation/course_completion_screen.dart +++ b/lib/features/learn/presentation/course_completion_screen.dart @@ -3,6 +3,8 @@ import 'dart:async'; import 'package:brew_path/core/constants/app_routes.dart'; import 'package:brew_path/core/widgets/loading_indicator.dart'; import 'package:brew_path/core/widgets/sticky_action_bar.dart'; +import 'package:brew_path/features/cards/domain/cards_providers.dart'; +import 'package:brew_path/features/cards/domain/module_rewards.dart'; import 'package:brew_path/features/companion/domain/companion_reaction.dart'; import 'package:brew_path/features/companion/presentation/companion_celebration.dart'; import 'package:brew_path/features/learn/domain/course_completion_providers.dart'; @@ -70,8 +72,8 @@ class _CourseCompletionScreenState final mood = context.mood; final lessons = ref.watch(completedLessonsProvider); - final cards = ref.watch(collectedCardsProvider); - final streak = ref.watch(streakProvider); + final cards = ref.watch(cardsWithCollectionProvider); + final streak = ref.watch(streakStatusProvider); // The ending must never paint "0 lessons" for a loading frame; the // moment waits for its own numbers. if (!lessons.hasValue || !cards.hasValue || !streak.hasValue) { @@ -79,8 +81,8 @@ class _CourseCompletionScreenState } final stats = ( lessons: lessons.value?.count ?? 0, - cards: cards.value?.length ?? 0, - streak: streak.value ?? 0, + moduleRewards: collectedModuleRewards(cards.value ?? const []), + longestStreak: streak.value?.longestStreak ?? 0, ); return Scaffold( // The bar takes the bottom inset itself, so this must not consume it @@ -140,17 +142,24 @@ class _CourseCompletionScreenState ); } + /// The three stats the ending reports. + /// + /// #149 rules the last two off the design's own *Cards collected* and + /// *Day streak*, so each label says what its number actually is. Widget _statsSummary(_Stats stats) { return Semantics( label: 'What you did: ${stats.lessons} lessons completed, ' - '${stats.cards} cards collected, ' - 'a ${stats.streak} day streak.', + '${stats.moduleRewards} Module Rewards earned, ' + 'a longest streak of ${stats.longestStreak} days.', child: Column( children: [ _StatRow(label: 'Lessons completed', value: '${stats.lessons}'), - _StatRow(label: 'Cards collected', value: '${stats.cards}'), - _StatRow(label: 'Day streak', value: '${stats.streak}'), + _StatRow( + label: 'Module Rewards', + value: '${stats.moduleRewards}', + ), + _StatRow(label: 'Longest streak', value: '${stats.longestStreak}'), ], ), ); @@ -158,7 +167,7 @@ class _CourseCompletionScreenState } /// The three derived completion stats, travelling together. -typedef _Stats = ({int lessons, int cards, int streak}); +typedef _Stats = ({int lessons, int moduleRewards, int longestStreak}); class _StatRow extends StatelessWidget { const _StatRow({required this.label, required this.value}); diff --git a/lib/features/lessons/presentation/lesson_completion_screen.dart b/lib/features/lessons/presentation/lesson_completion_screen.dart index a4c4d818..b0b0f0fb 100644 --- a/lib/features/lessons/presentation/lesson_completion_screen.dart +++ b/lib/features/lessons/presentation/lesson_completion_screen.dart @@ -80,9 +80,7 @@ class _LessonCompletionScreenState ref.invalidate(modulesWithProgressProvider); ref.invalidate(totalPointsProvider); ref.invalidate(completedLessonsProvider); - ref.invalidate(collectedCardsProvider); - // `cardsWithCollectionProvider` no longer chains through - // `collectedCardsProvider`, so invalidate it explicitly. + // The cards grid and the Module Reward count both hang off this one. ref.invalidate(cardsWithCollectionProvider); // A finished lesson can unlock a Coffee Challenge, and Today and Profile // stay mounted behind this screen — so neither would notice on its own. diff --git a/lib/features/profile/domain/settings_providers.dart b/lib/features/profile/domain/settings_providers.dart index 58fefb56..00705123 100644 --- a/lib/features/profile/domain/settings_providers.dart +++ b/lib/features/profile/domain/settings_providers.dart @@ -108,7 +108,6 @@ Future resetProgress(WidgetRef ref) async { ref.invalidate(totalPointsProvider); ref.invalidate(streakStatusProvider); ref.invalidate(completedLessonsProvider); - ref.invalidate(collectedCardsProvider); ref.invalidate(cardsWithCollectionProvider); ref.invalidate(modulesWithProgressProvider); ref.invalidate(todayLessonProvider); diff --git a/lib/features/progress/domain/progress_providers.dart b/lib/features/progress/domain/progress_providers.dart index ab577615..1ffe64df 100644 --- a/lib/features/progress/domain/progress_providers.dart +++ b/lib/features/progress/domain/progress_providers.dart @@ -17,18 +17,10 @@ part 'progress_providers.g.dart'; /// The learner's points total — **derived, never stored**. /// -/// Two payouts exist and both leave a record: a finished lesson is worth the -/// flat ten it authors, and a challenge's five is implied by its id sitting in -/// the completed set. Summing them here means the total cannot drift from what -/// was actually earned, and Reset Progress needs no rule of its own — clearing -/// the completions clears the total by construction. -/// -/// **The payout is read off the course, not off a copy of it.** The old -/// completions table banked the points on the row; the snapshot stores which -/// lessons are finished and nothing about what they paid, because what a -/// lesson is worth is a fact about the lesson. A finished lesson the content -/// no longer carries therefore pays nothing, which is the same answer a -/// dropped row would have given. +/// Summed from the two records that exist: a finished lesson is worth the flat +/// ten it authors, and a challenge's five is implied by its id in the completed +/// set. The payout is read off the course rather than a banked copy, so a +/// finished lesson the content no longer carries pays nothing. @riverpod Future totalPoints(Ref ref) async { // Every watch resolved before the first await: a rebuild mid-flight must not @@ -49,18 +41,12 @@ Future totalPoints(Ref ref) async { return fromLessons + challengesLogged * PointsValues.challengeCompletion; } -/// The streak, the freeze and the covered days, derived from the snapshot. -/// -/// Read against `DateTime.now()`, so it is only as fresh as the last time it -/// was built — which is why `DayRolloverWatcher` invalidates this on a resume -/// that crossed midnight, rather than letting a value computed before it stand. +/// The qualifying-day set every streak surface folds over — one derivation, so +/// the engine, the save notice and the week strip can never disagree on which +/// days count. /// -/// The day set it folds is assembled by [streakDaySet], which also backfills -/// a learner whose completions predate the day set — see it for why the three -/// sources are unioned rather than ranked. -/// The qualifying-day set every streak surface folds over — one derivation, -/// so the engine, the save notice and the week strip can never disagree on -/// which days count. +/// Assembled by [streakDaySet], which also backfills a learner whose +/// completions predate the day set. @riverpod Future> activeDaySet(Ref ref) async { final completedFuture = ref.watch(completedLessonsProvider.future); @@ -129,25 +115,12 @@ Future completedLessons(Ref ref) async { Future> completedLessonIds(Ref ref) async => (await ref.watch(completedLessonsProvider.future)).ids; -/// The ids of all cards the user has collected, off the progress snapshot. -/// -/// Stored in full rather than derived from the finished lessons: the lesson id -/// space has been rewritten once already on this project, and a derived set -/// would have silently revoked every card the rename touched. -@riverpod -Future> collectedCards(Ref ref) async { - final snapshot = await ref.watch(snapshotRepositoryProvider).read(); - return snapshot.clearedByReset.ownedCollectibles.toList(); -} - /// Highest tree stage ever reached: `max(stored, derived)`, as the field has /// always described itself. /// -/// The stored half is written by first-time lesson completion and never goes -/// down. The derived half is what the *current* course size implies, and it -/// is here to heal a learner whose stored stage predates the writer — taking -/// the max is what stops it doing harm, because a grown course derives lower -/// for the same learner and the stored floor wins. +/// The derived half heals a learner whose stored stage predates the writer; +/// taking the max is what stops it doing harm, because a grown course derives +/// lower for the same learner and the stored floor wins. @riverpod Future treeStage(Ref ref) async { // Every watch resolved before the first await, and **one read** of the @@ -172,10 +145,8 @@ Future treeStage(Ref ref) async { /// /// *Core* means every lesson in every module — the design's `CORE_LESSON_IDS` /// is `MODULES.flatMap(m => m.lessons)`, and the app has no lesson outside a -/// module, so this is not a new content concept and needs no flag on -/// `LessonModel`. It is the very pair [treeStage] already folds over, named -/// once here so the tree screen's counter and the stage it sits under can never -/// disagree about what they are counting. +/// module. The very pair [treeStage] folds over, named once so the tree +/// screen's counter and the stage it sits under cannot disagree. typedef CoreLessonProgress = ({int completed, int total}); /// The learner's progress through the core course. @@ -188,11 +159,9 @@ Future coreLessonProgress(Ref ref) async { /// The month the Profile's closing line names, or null before there is one. /// -/// The rule is [deriveJoinedDate]'s: the install stamp when the database -/// recorded one, and the earliest active day for every device created before -/// it did. The active-day set is read either way rather than only on the -/// fallback, so a stamp arriving later cannot change which providers this one -/// depends on mid-session. +/// The rule is [deriveJoinedDate]'s. The active-day set is read either way +/// rather than only on the fallback, so a stamp arriving later cannot change +/// which providers this one depends on mid-session. @riverpod Future joinedDate(Ref ref) async { final daysFuture = ref.watch(activeDaySetProvider.future); diff --git a/lib/features/progress/domain/progress_providers.g.dart b/lib/features/progress/domain/progress_providers.g.dart index fb072bab..831a0eb9 100644 --- a/lib/features/progress/domain/progress_providers.g.dart +++ b/lib/features/progress/domain/progress_providers.g.dart @@ -10,54 +10,30 @@ part of 'progress_providers.dart'; // ignore_for_file: type=lint, type=warning /// The learner's points total — **derived, never stored**. /// -/// Two payouts exist and both leave a record: a finished lesson is worth the -/// flat ten it authors, and a challenge's five is implied by its id sitting in -/// the completed set. Summing them here means the total cannot drift from what -/// was actually earned, and Reset Progress needs no rule of its own — clearing -/// the completions clears the total by construction. -/// -/// **The payout is read off the course, not off a copy of it.** The old -/// completions table banked the points on the row; the snapshot stores which -/// lessons are finished and nothing about what they paid, because what a -/// lesson is worth is a fact about the lesson. A finished lesson the content -/// no longer carries therefore pays nothing, which is the same answer a -/// dropped row would have given. +/// Summed from the two records that exist: a finished lesson is worth the flat +/// ten it authors, and a challenge's five is implied by its id in the completed +/// set. The payout is read off the course rather than a banked copy, so a +/// finished lesson the content no longer carries pays nothing. @ProviderFor(totalPoints) final totalPointsProvider = TotalPointsProvider._(); /// The learner's points total — **derived, never stored**. /// -/// Two payouts exist and both leave a record: a finished lesson is worth the -/// flat ten it authors, and a challenge's five is implied by its id sitting in -/// the completed set. Summing them here means the total cannot drift from what -/// was actually earned, and Reset Progress needs no rule of its own — clearing -/// the completions clears the total by construction. -/// -/// **The payout is read off the course, not off a copy of it.** The old -/// completions table banked the points on the row; the snapshot stores which -/// lessons are finished and nothing about what they paid, because what a -/// lesson is worth is a fact about the lesson. A finished lesson the content -/// no longer carries therefore pays nothing, which is the same answer a -/// dropped row would have given. +/// Summed from the two records that exist: a finished lesson is worth the flat +/// ten it authors, and a challenge's five is implied by its id in the completed +/// set. The payout is read off the course rather than a banked copy, so a +/// finished lesson the content no longer carries pays nothing. final class TotalPointsProvider extends $FunctionalProvider, int, FutureOr> with $FutureModifier, $FutureProvider { /// The learner's points total — **derived, never stored**. /// - /// Two payouts exist and both leave a record: a finished lesson is worth the - /// flat ten it authors, and a challenge's five is implied by its id sitting in - /// the completed set. Summing them here means the total cannot drift from what - /// was actually earned, and Reset Progress needs no rule of its own — clearing - /// the completions clears the total by construction. - /// - /// **The payout is read off the course, not off a copy of it.** The old - /// completions table banked the points on the row; the snapshot stores which - /// lessons are finished and nothing about what they paid, because what a - /// lesson is worth is a fact about the lesson. A finished lesson the content - /// no longer carries therefore pays nothing, which is the same answer a - /// dropped row would have given. + /// Summed from the two records that exist: a finished lesson is worth the flat + /// ten it authors, and a challenge's five is implied by its id in the completed + /// set. The payout is read off the course rather than a banked copy, so a + /// finished lesson the content no longer carries pays nothing. TotalPointsProvider._() : super( from: null, @@ -85,51 +61,33 @@ final class TotalPointsProvider String _$totalPointsHash() => r'cf138c6951ffe7e42a3e3bcb0631324804feae2d'; -/// The streak, the freeze and the covered days, derived from the snapshot. +/// The qualifying-day set every streak surface folds over — one derivation, so +/// the engine, the save notice and the week strip can never disagree on which +/// days count. /// -/// Read against `DateTime.now()`, so it is only as fresh as the last time it -/// was built — which is why `DayRolloverWatcher` invalidates this on a resume -/// that crossed midnight, rather than letting a value computed before it stand. -/// -/// The day set it folds is assembled by [streakDaySet], which also backfills -/// a learner whose completions predate the day set — see it for why the three -/// sources are unioned rather than ranked. -/// The qualifying-day set every streak surface folds over — one derivation, -/// so the engine, the save notice and the week strip can never disagree on -/// which days count. +/// Assembled by [streakDaySet], which also backfills a learner whose +/// completions predate the day set. @ProviderFor(activeDaySet) final activeDaySetProvider = ActiveDaySetProvider._(); -/// The streak, the freeze and the covered days, derived from the snapshot. -/// -/// Read against `DateTime.now()`, so it is only as fresh as the last time it -/// was built — which is why `DayRolloverWatcher` invalidates this on a resume -/// that crossed midnight, rather than letting a value computed before it stand. +/// The qualifying-day set every streak surface folds over — one derivation, so +/// the engine, the save notice and the week strip can never disagree on which +/// days count. /// -/// The day set it folds is assembled by [streakDaySet], which also backfills -/// a learner whose completions predate the day set — see it for why the three -/// sources are unioned rather than ranked. -/// The qualifying-day set every streak surface folds over — one derivation, -/// so the engine, the save notice and the week strip can never disagree on -/// which days count. +/// Assembled by [streakDaySet], which also backfills a learner whose +/// completions predate the day set. final class ActiveDaySetProvider extends $FunctionalProvider>, Set, FutureOr>> with $FutureModifier>, $FutureProvider> { - /// The streak, the freeze and the covered days, derived from the snapshot. + /// The qualifying-day set every streak surface folds over — one derivation, so + /// the engine, the save notice and the week strip can never disagree on which + /// days count. /// - /// Read against `DateTime.now()`, so it is only as fresh as the last time it - /// was built — which is why `DayRolloverWatcher` invalidates this on a resume - /// that crossed midnight, rather than letting a value computed before it stand. - /// - /// The day set it folds is assembled by [streakDaySet], which also backfills - /// a learner whose completions predate the day set — see it for why the three - /// sources are unioned rather than ranked. - /// The qualifying-day set every streak surface folds over — one derivation, - /// so the engine, the save notice and the week strip can never disagree on - /// which days count. + /// Assembled by [streakDaySet], which also backfills a learner whose + /// completions predate the day set. ActiveDaySetProvider._() : super( from: null, @@ -408,70 +366,12 @@ final class CompletedLessonIdsProvider String _$completedLessonIdsHash() => r'fb927706b8da83b2b28525d2ed2d345baa2acc64'; -/// The ids of all cards the user has collected, off the progress snapshot. -/// -/// Stored in full rather than derived from the finished lessons: the lesson id -/// space has been rewritten once already on this project, and a derived set -/// would have silently revoked every card the rename touched. - -@ProviderFor(collectedCards) -final collectedCardsProvider = CollectedCardsProvider._(); - -/// The ids of all cards the user has collected, off the progress snapshot. -/// -/// Stored in full rather than derived from the finished lessons: the lesson id -/// space has been rewritten once already on this project, and a derived set -/// would have silently revoked every card the rename touched. - -final class CollectedCardsProvider - extends - $FunctionalProvider< - AsyncValue>, - List, - FutureOr> - > - with $FutureModifier>, $FutureProvider> { - /// The ids of all cards the user has collected, off the progress snapshot. - /// - /// Stored in full rather than derived from the finished lessons: the lesson id - /// space has been rewritten once already on this project, and a derived set - /// would have silently revoked every card the rename touched. - CollectedCardsProvider._() - : super( - from: null, - argument: null, - retry: null, - name: r'collectedCardsProvider', - isAutoDispose: true, - dependencies: null, - $allTransitiveDependencies: null, - ); - - @override - String debugGetCreateSourceHash() => _$collectedCardsHash(); - - @$internal - @override - $FutureProviderElement> $createElement( - $ProviderPointer pointer, - ) => $FutureProviderElement(pointer); - - @override - FutureOr> create(Ref ref) { - return collectedCards(ref); - } -} - -String _$collectedCardsHash() => r'442ce2ea146aaad92a581ece9811b9ca444481d6'; - /// Highest tree stage ever reached: `max(stored, derived)`, as the field has /// always described itself. /// -/// The stored half is written by first-time lesson completion and never goes -/// down. The derived half is what the *current* course size implies, and it -/// is here to heal a learner whose stored stage predates the writer — taking -/// the max is what stops it doing harm, because a grown course derives lower -/// for the same learner and the stored floor wins. +/// The derived half heals a learner whose stored stage predates the writer; +/// taking the max is what stops it doing harm, because a grown course derives +/// lower for the same learner and the stored floor wins. @ProviderFor(treeStage) final treeStageProvider = TreeStageProvider._(); @@ -479,11 +379,9 @@ final treeStageProvider = TreeStageProvider._(); /// Highest tree stage ever reached: `max(stored, derived)`, as the field has /// always described itself. /// -/// The stored half is written by first-time lesson completion and never goes -/// down. The derived half is what the *current* course size implies, and it -/// is here to heal a learner whose stored stage predates the writer — taking -/// the max is what stops it doing harm, because a grown course derives lower -/// for the same learner and the stored floor wins. +/// The derived half heals a learner whose stored stage predates the writer; +/// taking the max is what stops it doing harm, because a grown course derives +/// lower for the same learner and the stored floor wins. final class TreeStageProvider extends $FunctionalProvider, int, FutureOr> @@ -491,11 +389,9 @@ final class TreeStageProvider /// Highest tree stage ever reached: `max(stored, derived)`, as the field has /// always described itself. /// - /// The stored half is written by first-time lesson completion and never goes - /// down. The derived half is what the *current* course size implies, and it - /// is here to heal a learner whose stored stage predates the writer — taking - /// the max is what stops it doing harm, because a grown course derives lower - /// for the same learner and the stored floor wins. + /// The derived half heals a learner whose stored stage predates the writer; + /// taking the max is what stops it doing harm, because a grown course derives + /// lower for the same learner and the stored floor wins. TreeStageProvider._() : super( from: null, @@ -572,22 +468,18 @@ String _$coreLessonProgressHash() => /// The month the Profile's closing line names, or null before there is one. /// -/// The rule is [deriveJoinedDate]'s: the install stamp when the database -/// recorded one, and the earliest active day for every device created before -/// it did. The active-day set is read either way rather than only on the -/// fallback, so a stamp arriving later cannot change which providers this one -/// depends on mid-session. +/// The rule is [deriveJoinedDate]'s. The active-day set is read either way +/// rather than only on the fallback, so a stamp arriving later cannot change +/// which providers this one depends on mid-session. @ProviderFor(joinedDate) final joinedDateProvider = JoinedDateProvider._(); /// The month the Profile's closing line names, or null before there is one. /// -/// The rule is [deriveJoinedDate]'s: the install stamp when the database -/// recorded one, and the earliest active day for every device created before -/// it did. The active-day set is read either way rather than only on the -/// fallback, so a stamp arriving later cannot change which providers this one -/// depends on mid-session. +/// The rule is [deriveJoinedDate]'s. The active-day set is read either way +/// rather than only on the fallback, so a stamp arriving later cannot change +/// which providers this one depends on mid-session. final class JoinedDateProvider extends @@ -599,11 +491,9 @@ final class JoinedDateProvider with $FutureModifier, $FutureProvider { /// The month the Profile's closing line names, or null before there is one. /// - /// The rule is [deriveJoinedDate]'s: the install stamp when the database - /// recorded one, and the earliest active day for every device created before - /// it did. The active-day set is read either way rather than only on the - /// fallback, so a stamp arriving later cannot change which providers this one - /// depends on mid-session. + /// The rule is [deriveJoinedDate]'s. The active-day set is read either way + /// rather than only on the fallback, so a stamp arriving later cannot change + /// which providers this one depends on mid-session. JoinedDateProvider._() : super( from: null, diff --git a/lib/features/progress/domain/streak_engine.dart b/lib/features/progress/domain/streak_engine.dart index 872e2112..11efa840 100644 --- a/lib/features/progress/domain/streak_engine.dart +++ b/lib/features/progress/domain/streak_engine.dart @@ -3,29 +3,20 @@ library; import 'package:brew_path/features/progress/domain/streak_status.dart'; -/// Derives the whole streak state from [activeDays], read as of [today]. +/// Derives the whole streak state from [activeDays], read as of [today], both +/// day indices (`epochDay`). They are the **only** inputs: no clock in here, +/// no storage, and no entitlement — §10 makes freezes free for everyone. /// -/// Both are day indices — `epochDay` from `core/utils/date_utils.dart` — and -/// they are the **only** inputs. There is no clock in here, no storage, and -/// deliberately no entitlement: §10 makes freezes free for everyone, so there -/// is no parameter a paid tier could arrive through. -/// -/// **Today is never judged a miss.** A day the learner has not finished yet is -/// not a day they skipped, so an inactive [today] neither spends the freeze nor -/// breaks the streak; both decide when the day is over. Days *after* [today] -/// are ignored rather than folded — a peer whose clock runs ahead can write -/// one, and counting it would open a gap of missed days behind it. -/// -/// The fold walks the gaps between active days rather than every calendar day, -/// so its cost is the size of the set and not the age of the account. +/// **Today is never judged a miss**, and days after it are ignored rather than +/// folded: a peer whose clock runs ahead would otherwise open a gap behind it. StreakStatus deriveStreak({ required Set activeDays, required int today, }) { + final fold = _StreakFold(); final days = activeDays.where((day) => day <= today).toList()..sort(); - if (days.isEmpty) return StreakStatus.idle; + if (days.isEmpty) return fold.status; - final fold = _StreakFold(); // One before the first day, so the opening gap is empty and the first // qualifying day is reached without a miss in front of it. var previous = days.first - 1; @@ -41,12 +32,12 @@ StreakStatus deriveStreak({ /// The fold's cursor. /// /// Mutable and private, which is what keeps [deriveStreak] pure: the state is -/// created, walked and read inside one call and can never be observed -/// mid-fold. Written as a cursor rather than a chain of copies because the -/// rules are read as a sequence of events — one qualifying day, one run of -/// missed days — and each one reads here as the sentence §10 states it in. +/// created, walked and read inside one call. A cursor rather than a chain of +/// copies because the rules read as a sequence of events, and each one reads +/// here as the sentence §10 states it in. class _StreakFold { int _streak = 0; + int _longestStreak = 0; bool _freezeHeld = false; int _towardFreeze = 0; final Set _frozenDays = {}; @@ -54,6 +45,9 @@ class _StreakFold { /// One qualifying day. void qualified() { _streak++; + // The only place the streak rises, so the high-water mark is complete + // here — a break below can then zero the streak without losing it. + if (_streak > _longestStreak) _longestStreak = _streak; // "While a freeze is already held, additional qualifying days do not // accumulate progress toward another one." if (_freezeHeld) return; @@ -90,6 +84,7 @@ class _StreakFold { StreakStatus get status => StreakStatus( streak: _streak, + longestStreak: _longestStreak, freezeHeld: _freezeHeld, daysToNextFreeze: _freezeHeld ? null : freezeEarnDays - _towardFreeze, freezesSpent: _frozenDays.length, @@ -97,17 +92,12 @@ class _StreakFold { ); } -/// Whether growing the day set from [before] to [after] is what earned the -/// freeze, read as of [today]. -/// -/// **A rise, not a state.** `freezeHeld` answers "is one in hand"; every run -/// after the seventh would answer yes, and the design's `FREEZE EARNED` row -/// belongs to the run that actually paid it out — *"the first time most users -/// meet the word 'freeze' — before they ever need one"*. Only the transition -/// says that, so only the transition is asked for. +/// Whether growing the day set from [before] to [after] earned the freeze. /// -/// Both folds run against the same [today], so nothing here can mistake a day -/// rolling over for a freeze being earned. +/// **A rise, not a state.** `freezeHeld` would answer yes for every run after +/// the seventh; the design's `FREEZE EARNED` row belongs to the run that +/// actually paid it out. Both folds run against the same [today], so a day +/// rolling over cannot look like an earn. bool freezeEarnedBetween({ required Set before, required Set after, diff --git a/lib/features/progress/domain/streak_status.dart b/lib/features/progress/domain/streak_status.dart index 9cc89015..54c6b426 100644 --- a/lib/features/progress/domain/streak_status.dart +++ b/lib/features/progress/domain/streak_status.dart @@ -10,25 +10,23 @@ const int freezeEarnDays = 7; /// /// The value is **1**, which is why [StreakStatus.freezeHeld] is a boolean /// rather than a count: the cap is expressed in the type, so no arithmetic can -/// exceed it. The prototype's `FREEZE_CAP = 2` is superseded (#58) and its -/// pip row is dropped with it (#26) — one dash on the covered day and one -/// status line carry what two pips used to. +/// exceed it. The prototype's `FREEZE_CAP = 2` is superseded (#58) and its pip +/// row is dropped with it (#26). const int maxFreezesHeld = 1; -/// The streak, the freeze, and the days a freeze covered — all **derived**, -/// none of them stored. +/// The streak, the freeze, and the days a freeze covered — all **derived**. /// -/// A stored copy of any of these would need a merge rule, and a max-merged -/// counter launders an inflation bug permanently: two devices offline at five -/// days each do not make five, and no later correction can lower the number -/// once it has been written. The active-day set unions instead, which is -/// exactly right, and everything here is recovered by replaying it. +/// A stored copy would need a merge rule, and a max-merged counter launders +/// an inflation bug permanently: two devices offline at five days each do not +/// make five. The active-day set unions instead, and everything here is +/// recovered by replaying it. @immutable class StreakStatus { /// Creates a [StreakStatus]. Prefer `deriveStreak` — this exists for the /// two constants below and for tests that want a fixture. const StreakStatus({ required this.streak, + required this.longestStreak, required this.freezeHeld, required this.daysToNextFreeze, required this.freezesSpent, @@ -40,6 +38,7 @@ class StreakStatus { /// clears the day set this derives from. static const idle = StreakStatus( streak: 0, + longestStreak: 0, freezeHeld: false, daysToNextFreeze: freezeEarnDays, freezesSpent: 0, @@ -53,6 +52,13 @@ class StreakStatus { /// defect that makes deriving the week strip from this number wrong (#26). final int streak; + /// The high-water mark of [streak] over the whole history. + /// + /// Read off the same fold, so the two can never disagree about what a run + /// is: a day a freeze covered joins the run either side of it here too, and + /// a break ends the run for both. + final int longestStreak; + /// Whether an unspent freeze is held. See [maxFreezesHeld] for why this is /// not a count. final bool freezeHeld; @@ -79,6 +85,7 @@ class StreakStatus { identical(this, other) || other is StreakStatus && other.streak == streak && + other.longestStreak == longestStreak && other.freezeHeld == freezeHeld && other.daysToNextFreeze == daysToNextFreeze && other.freezesSpent == freezesSpent && @@ -87,6 +94,7 @@ class StreakStatus { @override int get hashCode => Object.hash( streak, + longestStreak, freezeHeld, daysToNextFreeze, freezesSpent, @@ -95,6 +103,7 @@ class StreakStatus { @override String toString() => - 'StreakStatus(streak: $streak, freezeHeld: $freezeHeld, ' - 'daysToNextFreeze: $daysToNextFreeze, freezesSpent: $freezesSpent)'; + 'StreakStatus(streak: $streak, longestStreak: $longestStreak, ' + 'freezeHeld: $freezeHeld, daysToNextFreeze: $daysToNextFreeze, ' + 'freezesSpent: $freezesSpent)'; } diff --git a/lib/shared/models/coffee_card_model.dart b/lib/shared/models/coffee_card_model.dart index 910bdaa6..497001f4 100644 --- a/lib/shared/models/coffee_card_model.dart +++ b/lib/shared/models/coffee_card_model.dart @@ -5,13 +5,9 @@ part 'coffee_card_model.freezed.dart'; /// A collectible card as the screens show it: the bank's record joined to the /// words of whatever unlocks it. /// -/// **Assembled, never parsed.** There is no `fromJson` here on purpose — no -/// single bank record holds a card's text. The collectible supplies the id and -/// the illustration key, its source lesson or module supplies the title, -/// summary and fact, and the content layer joins the two once. A card built -/// straight from JSON would either be wordless or would need those words -/// duplicated into the collectibles bank, which is the duplication the -/// pipeline exists to prevent. +/// **Assembled, never parsed.** No bank record holds a card's text, so there +/// is no `fromJson` — the collectible supplies the id and the illustration +/// key, its source supplies the words, and the content layer joins them once. @freezed abstract class CoffeeCardModel with _$CoffeeCardModel { /// Creates a [CoffeeCardModel]. @@ -44,4 +40,13 @@ abstract class CoffeeCardModel with _$CoffeeCardModel { /// The module that awards this card, or null when a lesson does. String? moduleId, }) = _CoffeeCardModel; + + const CoffeeCardModel._(); + + /// Whether this is one of the five Module Rewards. + /// + /// [moduleId] carries the collectibles bank's own `unlock.module` pointer + /// and nothing else writes it, so only a module-awarded card has one — a + /// lesson card's owning module lives in [moduleTag]. + bool get isModuleReward => moduleId != null; } diff --git a/lib/shared/models/coffee_card_model.freezed.dart b/lib/shared/models/coffee_card_model.freezed.dart index 2662ea2b..4ece90e5 100644 --- a/lib/shared/models/coffee_card_model.freezed.dart +++ b/lib/shared/models/coffee_card_model.freezed.dart @@ -223,8 +223,8 @@ return $default(_that.id,_that.title,_that.description,_that.fact,_that.moduleTa /// @nodoc -class _CoffeeCardModel implements CoffeeCardModel { - const _CoffeeCardModel({required this.id, required this.title, required this.description, required this.fact, required this.moduleTag, required this.iconName, required this.kind, this.lessonId, this.moduleId}); +class _CoffeeCardModel extends CoffeeCardModel { + const _CoffeeCardModel({required this.id, required this.title, required this.description, required this.fact, required this.moduleTag, required this.iconName, required this.kind, this.lessonId, this.moduleId}): super._(); @override final String id; diff --git a/lib/shared/repositories/content_repository.dart b/lib/shared/repositories/content_repository.dart index 277b27da..a7578ec4 100644 --- a/lib/shared/repositories/content_repository.dart +++ b/lib/shared/repositories/content_repository.dart @@ -18,9 +18,7 @@ part 'content_repository.g.dart'; /// /// Three joins live here and nowhere else: a lesson's owning **module**, the /// **card** a lesson awards, and the **Module Reward card** a module awards. -/// All three are reverse lookups the banks do not store directly, and all three -/// would otherwise be open-coded at every call site with a slightly different -/// answer. +/// Open-coding one at a call site is how two of them come to disagree. class ContentRepository { List? _modules; List? _lessons; @@ -75,19 +73,13 @@ class ContentRepository { return cards.where((card) => card.lessonId == lessonId).firstOrNull; } - /// The Module Reward card [moduleId] awards, or null when no collectible - /// names it. + /// The Module Reward card [moduleId] awards, or null when none names it. /// - /// Matched on the collectible's **own** module pointer, not on the module a - /// card's lesson happens to belong to — every one of the thirty-two lesson - /// cards also carries an owning module, so a match on that would hand back a - /// lesson card for whichever lesson sorted first. A card awarded by a module - /// is the one with no lesson behind it. + /// Matched on the pointer only a Module Reward carries — see + /// [CoffeeCardModel.isModuleReward] for why a lesson card cannot match here. Future getCardForModule(String moduleId) async { final cards = await getCards(); - return cards - .where((card) => card.lessonId == null && card.moduleId == moduleId) - .firstOrNull; + return cards.where((card) => card.moduleId == moduleId).firstOrNull; } /// Loads and caches the twelve Coffee Challenges, in bank order. diff --git a/test/unit/features/cards/collected_module_rewards_test.dart b/test/unit/features/cards/collected_module_rewards_test.dart new file mode 100644 index 00000000..68098e05 --- /dev/null +++ b/test/unit/features/cards/collected_module_rewards_test.dart @@ -0,0 +1,67 @@ +// The completion moment's Module Reward stat, read against the real bank: a +// derivation that miscounted the five would still pass a hand-built fixture. +import 'package:brew_path/features/cards/domain/cards_providers.dart'; +import 'package:brew_path/features/cards/domain/module_rewards.dart'; +import 'package:brew_path/shared/repositories/content_repository.dart'; +import 'package:brew_path/shared/repositories/snapshot_repository.dart'; +import 'package:brew_path/shared/storage/app_database.dart'; +import 'package:drift/native.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../../support/progress_seed.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late AppDatabase db; + late SnapshotRepository snapshots; + + setUp(() { + db = AppDatabase(NativeDatabase.memory()); + AppDatabaseService.instance = db; + snapshots = SnapshotRepository(); + }); + tearDown(() async => db.close()); + + Future ownedRewards() async { + final container = ProviderContainer(); + addTearDown(container.dispose); + return collectedModuleRewards( + await container.read(cardsWithCollectionProvider.future), + ); + } + + test('the bank holds five Module Rewards among its thirty-seven', () async { + final cards = await ContentRepository().getCards(); + + expect(cards, hasLength(37)); + expect(cards.where((card) => card.isModuleReward), hasLength(5)); + }); + + test('a learner who owns nothing owns no Module Reward', () async { + expect(await ownedRewards(), 0); + }); + + test('lesson cards raise the count for none of the five', () async { + await seedCollectible(snapshots, 'c1'); + await seedCollectible(snapshots, 'c2'); + + expect(await ownedRewards(), 0); + }); + + test('a module card counts, and only for itself', () async { + await seedCollectible(snapshots, 'c1'); + await seedCollectible(snapshots, 'cM1'); + + expect(await ownedRewards(), 1); + }); + + test('a finished learner holding every card counts all five', () async { + for (final card in await ContentRepository().getCards()) { + await seedCollectible(snapshots, card.id); + } + + expect(await ownedRewards(), 5); + }); +} diff --git a/test/unit/features/cards/module_rewards_test.dart b/test/unit/features/cards/module_rewards_test.dart new file mode 100644 index 00000000..3265bbd1 --- /dev/null +++ b/test/unit/features/cards/module_rewards_test.dart @@ -0,0 +1,69 @@ +import 'package:brew_path/features/cards/domain/cards_providers.dart'; +import 'package:brew_path/features/cards/domain/module_rewards.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../../../support/content_fixtures.dart'; + +CardWithCollection lessonCard(String id, {required bool collected}) => + CardWithCollection( + card: testCoffeeCard(id: id), + isCollected: collected, + ); + +CardWithCollection moduleCard(String id, {required bool collected}) => + CardWithCollection( + card: testCoffeeCard(id: id, lessonId: null, moduleId: 'm1'), + isCollected: collected, + ); + +void main() { + group('CoffeeCardModel.isModuleReward', () { + test('a card a module awards is one', () { + expect(moduleCard('cM1', collected: true).card.isModuleReward, isTrue); + }); + + test('a card a lesson awards is not, though its module is known', () { + final card = testCoffeeCard(); + + expect(card.moduleTag, isNotEmpty); + expect(card.isModuleReward, isFalse); + }); + }); + + group('collectedModuleRewards', () { + test('counts the module cards owned and none of the lesson cards', () { + expect( + collectedModuleRewards([ + lessonCard('c1', collected: true), + lessonCard('c2', collected: true), + moduleCard('cM1', collected: true), + ]), + 1, + ); + }); + + test('a module card the learner has not earned does not count', () { + expect( + collectedModuleRewards([ + moduleCard('cM1', collected: true), + moduleCard('cM2', collected: false), + ]), + 1, + ); + }); + + test('lesson cards alone count none', () { + expect( + collectedModuleRewards([ + lessonCard('c1', collected: true), + lessonCard('c2', collected: true), + ]), + 0, + ); + }); + + test('an empty collection counts none', () { + expect(collectedModuleRewards(const []), 0); + }); + }); +} diff --git a/test/unit/features/progress/freeze_save_notice_test.dart b/test/unit/features/progress/freeze_save_notice_test.dart index bb14097e..650da341 100644 --- a/test/unit/features/progress/freeze_save_notice_test.dart +++ b/test/unit/features/progress/freeze_save_notice_test.dart @@ -13,6 +13,7 @@ StreakStatus status({ int? daysToNextFreeze = freezeEarnDays, }) => StreakStatus( streak: 7, + longestStreak: 7, freezeHeld: freezeHeld, daysToNextFreeze: daysToNextFreeze, freezesSpent: frozenDays.length, diff --git a/test/unit/features/progress/freeze_status_line_test.dart b/test/unit/features/progress/freeze_status_line_test.dart index 91b9ae11..d7c56d87 100644 --- a/test/unit/features/progress/freeze_status_line_test.dart +++ b/test/unit/features/progress/freeze_status_line_test.dart @@ -13,6 +13,7 @@ StreakStatus status({ Set frozenDays = const {}, }) => StreakStatus( streak: 5, + longestStreak: 5, freezeHeld: freezeHeld, daysToNextFreeze: daysToNextFreeze, freezesSpent: frozenDays.length, diff --git a/test/unit/features/progress/progress_reads_the_snapshot_test.dart b/test/unit/features/progress/progress_reads_the_snapshot_test.dart index ec246c7d..8d87ff2e 100644 --- a/test/unit/features/progress/progress_reads_the_snapshot_test.dart +++ b/test/unit/features/progress/progress_reads_the_snapshot_test.dart @@ -75,12 +75,12 @@ void main() { await seedCollectible(snapshots, 'c1'); final container = harness(); - expect(await container.read(collectedCardsProvider.future), ['c1']); final cards = await container.read(cardsWithCollectionProvider.future); expect( cards.firstWhere((entry) => entry.card.id == 'c1').isCollected, isTrue, ); + expect(cards.where((entry) => entry.isCollected), hasLength(1)); }); test('Today moves past what the snapshot says is finished', () async { diff --git a/test/unit/features/progress/streak_engine_property_test.dart b/test/unit/features/progress/streak_engine_property_test.dart index 21250c91..6dc4c2f5 100644 --- a/test/unit/features/progress/streak_engine_property_test.dart +++ b/test/unit/features/progress/streak_engine_property_test.dart @@ -28,6 +28,7 @@ StreakStatus oracle(Set activeDays, int today) { if (past.isEmpty) return StreakStatus.idle; var streak = 0; + var longest = 0; var freezeHeld = false; var towardFreeze = 0; final frozen = {}; @@ -35,6 +36,7 @@ StreakStatus oracle(Set activeDays, int today) { for (var day = past.first; day < today; day++) { if (activeDays.contains(day)) { streak++; + longest = max(longest, streak); if (!freezeHeld) { towardFreeze++; if (towardFreeze == freezeEarnDays) { @@ -54,6 +56,7 @@ StreakStatus oracle(Set activeDays, int today) { // Today counts when it qualifies and is otherwise left alone. if (activeDays.contains(today)) { streak++; + longest = max(longest, streak); if (!freezeHeld) { towardFreeze++; if (towardFreeze == freezeEarnDays) { @@ -65,6 +68,7 @@ StreakStatus oracle(Set activeDays, int today) { return StreakStatus( streak: streak, + longestStreak: longest, freezeHeld: freezeHeld, daysToNextFreeze: freezeHeld ? null : freezeEarnDays - towardFreeze, freezesSpent: frozen.length, @@ -146,6 +150,14 @@ void main() { }); }); + test('the longest run sits between the current streak and the days', () { + forEachCase((days, today) { + final status = deriveStreak(activeDays: days, today: today); + expect(status.longestStreak, greaterThanOrEqualTo(status.streak)); + expect(status.longestStreak, lessThanOrEqualTo(days.length)); + }); + }); + test('a live streak reaches back no further than a covered day allows', () { forEachCase((days, today) { final status = deriveStreak(activeDays: days, today: today); diff --git a/test/unit/features/progress/streak_engine_test.dart b/test/unit/features/progress/streak_engine_test.dart index c6b683e2..e6dda787 100644 --- a/test/unit/features/progress/streak_engine_test.dart +++ b/test/unit/features/progress/streak_engine_test.dart @@ -215,4 +215,59 @@ void main() { expect(derive({day0 + 5}, today: 0), StreakStatus.idle); }); }); + + group('the longest run the history ever reached', () { + test('an unbroken history reads the same as the current streak', () { + final status = derive(run(0, 5), today: 4); + + expect(status.longestStreak, 5); + expect(status.streak, 5); + }); + + test('five, a gap, then three keeps the five', () { + final status = derive({...run(0, 5), ...run(6, 3)}, today: 8); + + expect(status.streak, 3); + expect(status.longestStreak, 5); + }); + + test('three, a gap, then five reads the later run', () { + final status = derive({...run(0, 3), ...run(4, 5)}, today: 8); + + expect(status.longestStreak, 5); + }); + + test('a broken history the learner has since abandoned keeps its best', () { + final status = derive(run(0, 5), today: 40); + + expect(status.streak, 0); + expect(status.longestStreak, 5); + }); + + test('a covered miss leaves one run, not two', () { + // Seven days, a miss the freeze covers, then one more day: §10 rules + // this a single run of 8, so the longest must not read the 7 either. + final status = derive({...run(0, 7), day0 + 8}, today: 8); + + expect(status.longestStreak, 8); + }); + + test('a run of two misses is a break, so the runs stay separate', () { + final status = derive({...run(0, 7), ...run(9, 2)}, today: 10); + + expect(status.streak, 2); + expect(status.longestStreak, 7); + }); + + test('days ahead of today are ignored here too', () { + final status = derive({...run(0, 3), ...run(20, 9)}, today: 2); + + expect(status.longestStreak, 3); + }); + + test('an empty history has no run at all', () { + expect(derive(const {}, today: 0).longestStreak, 0); + expect(StreakStatus.idle.longestStreak, 0); + }); + }); } diff --git a/test/unit/features/progress/streak_week_test.dart b/test/unit/features/progress/streak_week_test.dart index e1244716..e4f34211 100644 --- a/test/unit/features/progress/streak_week_test.dart +++ b/test/unit/features/progress/streak_week_test.dart @@ -116,6 +116,7 @@ void main() { test('a covered day reads frozen, never done', () { final status = StreakStatus( streak: 3, + longestStreak: 3, freezeHeld: false, daysToNextFreeze: freezeEarnDays, freezesSpent: 1, diff --git a/test/unit/features/tour/micro_tip_session_flags_test.dart b/test/unit/features/tour/micro_tip_session_flags_test.dart index ccd3bf8b..e3fb1009 100644 --- a/test/unit/features/tour/micro_tip_session_flags_test.dart +++ b/test/unit/features/tour/micro_tip_session_flags_test.dart @@ -89,6 +89,7 @@ void main() { group('a freeze earned this session', () { StreakStatus statusWith({required bool freezeHeld}) => StreakStatus( streak: 7, + longestStreak: 7, freezeHeld: freezeHeld, daysToNextFreeze: freezeHeld ? null : 3, freezesSpent: freezeHeld ? 0 : 1, diff --git a/test/widget/course_completion_screen_test.dart b/test/widget/course_completion_screen_test.dart index 34054e9d..c3c26d8b 100644 --- a/test/widget/course_completion_screen_test.dart +++ b/test/widget/course_completion_screen_test.dart @@ -1,10 +1,12 @@ import 'package:brew_path/core/constants/app_routes.dart'; +import 'package:brew_path/features/cards/domain/cards_providers.dart'; import 'package:brew_path/features/companion/application/companion_providers.dart'; import 'package:brew_path/features/companion/domain/companion_lines.dart'; import 'package:brew_path/features/learn/presentation/course_completion_screen.dart'; import 'package:brew_path/features/progress/domain/completed_lessons.dart'; import 'package:brew_path/features/progress/domain/mastery.dart'; import 'package:brew_path/features/progress/domain/progress_providers.dart'; +import 'package:brew_path/features/progress/domain/streak_status.dart'; import 'package:brew_path/shared/repositories/snapshot_repository.dart'; import 'package:brew_path/shared/storage/app_database.dart'; import 'package:drift/native.dart'; @@ -13,9 +15,37 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; +import '../support/content_fixtures.dart'; + const _lessonCount = 32; -const _cardCount = 37; -const _streakDays = 12; +const _moduleRewardCount = 5; +const _longestStreakDays = 12; + +/// A learner whose best run is behind them, so the screen cannot pass by +/// showing the current streak under the new label. +const _streak = StreakStatus( + streak: 3, + longestStreak: _longestStreakDays, + freezeHeld: false, + daysToNextFreeze: freezeEarnDays, + freezesSpent: 0, + frozenDays: {}, +); + +/// The five Module Rewards plus a lesson card, all owned — so a count that +/// took every collected card would read six. +final List _collection = [ + for (var i = 0; i < _moduleRewardCount; i++) + CardWithCollection( + card: testCoffeeCard(id: 'cM$i', lessonId: null, moduleId: 'm$i'), + isCollected: true, + ), + CardWithCollection(card: testCoffeeCard(), isCollected: true), + CardWithCollection( + card: testCoffeeCard(id: 'cM9', lessonId: null, moduleId: 'm9'), + isCollected: false, + ), +]; final CompletedLessons _completed = CompletedLessons( completedOn: { @@ -27,8 +57,6 @@ final CompletedLessons _completed = CompletedLessons( }, ); -final List _cards = [for (var i = 0; i < _cardCount; i++) 'c$i']; - /// One deterministic line so the bubble's copy is assertable. const _lines = CompanionLines({ 'courseComplete': ['You finished the whole course!'], @@ -59,8 +87,8 @@ Future _pump( ProviderScope( overrides: [ completedLessonsProvider.overrideWith((ref) async => _completed), - collectedCardsProvider.overrideWith((ref) async => _cards), - streakProvider.overrideWith((ref) async => _streakDays), + cardsWithCollectionProvider.overrideWith((ref) async => _collection), + streakStatusProvider.overrideWith((ref) async => _streak), companionLinesProvider.overrideWith((ref) async => _lines), ], child: MediaQuery( @@ -93,12 +121,24 @@ void main() { expect(find.text('You finished Beginner Foundations'), findsOneWidget); expect(find.text('Lessons completed'), findsOneWidget); + expect(find.text('Module Rewards'), findsOneWidget); + expect(find.text('Longest streak'), findsOneWidget); expect(find.text('$_lessonCount'), findsOneWidget); - expect(find.text('$_cardCount'), findsOneWidget); - expect(find.text('$_streakDays'), findsOneWidget); + expect(find.text('$_moduleRewardCount'), findsOneWidget); + expect(find.text('$_longestStreakDays'), findsOneWidget); expect(find.text('You finished the whole course!'), findsOneWidget); }); + testWidgets('the streak stat reads the longest run, not the current one', ( + tester, + ) async { + await _pump(tester); + + expect(find.text('${_streak.streak}'), findsNothing); + expect(find.text('Day streak'), findsNothing); + expect(find.text('Cards collected'), findsNothing); + }); + testWidgets('presenting the moment writes the acknowledgement', ( tester, ) async { @@ -136,7 +176,9 @@ void main() { await _pump(tester); expect( - find.bySemanticsLabel(RegExp('What you did.*32 lessons')), + find.bySemanticsLabel( + RegExp('What you did.*32 lessons.*5 Module Rewards.*12 days'), + ), findsOneWidget, ); }); diff --git a/test/widget/features/tour/micro_tip_host_test.dart b/test/widget/features/tour/micro_tip_host_test.dart index 084e7c34..744efa46 100644 --- a/test/widget/features/tour/micro_tip_host_test.dart +++ b/test/widget/features/tour/micro_tip_host_test.dart @@ -184,6 +184,7 @@ void main() { streakStatusProvider.overrideWith( (ref) async => const StreakStatus( streak: 7, + longestStreak: 7, freezeHeld: true, daysToNextFreeze: null, freezesSpent: 0, @@ -256,6 +257,7 @@ void main() { streakStatusProvider.overrideWith( (ref) async => const StreakStatus( streak: 7, + longestStreak: 7, freezeHeld: true, daysToNextFreeze: null, freezesSpent: 0, diff --git a/test/widget/lesson_completion_screen_test.dart b/test/widget/lesson_completion_screen_test.dart index b4f2cc5a..954160ad 100644 --- a/test/widget/lesson_completion_screen_test.dart +++ b/test/widget/lesson_completion_screen_test.dart @@ -1,5 +1,6 @@ import 'package:brew_path/core/constants/app_labels.dart'; import 'package:brew_path/core/widgets/reward_row.dart'; +import 'package:brew_path/features/cards/domain/cards_providers.dart'; import 'package:brew_path/features/cards/presentation/reward_card.dart'; import 'package:brew_path/features/challenges/presentation/challenge_suggestion.dart'; import 'package:brew_path/features/companion/presentation/companion.dart'; @@ -207,7 +208,7 @@ void main() { container.listen(totalPointsProvider, (_, _) {}), container.listen(streakProvider, (_, _) {}), container.listen(completedLessonsProvider, (_, _) {}), - container.listen(collectedCardsProvider, (_, _) {}), + container.listen(cardsWithCollectionProvider, (_, _) {}), ]; addTearDown(() { for (final s in subs) { @@ -225,12 +226,12 @@ void main() { () => container.read(completedLessonsProvider.future), ); final cardsBefore = await tester.runAsync( - () => container.read(collectedCardsProvider.future), + () => container.read(cardsWithCollectionProvider.future), ); expect(pointsBefore, 0); expect(streakBefore, 0); expect(lessonsBefore?.isEmpty, isTrue); - expect(cardsBefore, isEmpty); + expect(cardsBefore?.where((entry) => entry.isCollected), isEmpty); await pumpCompletion(tester, container); // First lesson of m1 — the flat ten it authors. The module moment does @@ -249,12 +250,17 @@ void main() { () => container.read(completedLessonsProvider.future), ); final cardsAfter = await tester.runAsync( - () => container.read(collectedCardsProvider.future), + () => container.read(cardsWithCollectionProvider.future), ); expect(pointsAfter, 10); // m1l1 pays the flat ten it authors expect(streakAfter, 1); expect(lessonsAfter?.ids, {'m1l1'}); - expect(cardsAfter, contains('c1')); + expect( + cardsAfter + ?.where((entry) => entry.isCollected) + .map((entry) => entry.card.id), + contains('c1'), + ); }, ); diff --git a/test/widget/streak_screen_test.dart b/test/widget/streak_screen_test.dart index a7cb5841..36e10cd8 100644 --- a/test/widget/streak_screen_test.dart +++ b/test/widget/streak_screen_test.dart @@ -23,6 +23,7 @@ import '../support/widget_harness.dart'; const _counting = StreakStatus( streak: 12, + longestStreak: 12, freezeHeld: false, daysToNextFreeze: 3, freezesSpent: 0, @@ -31,6 +32,7 @@ const _counting = StreakStatus( const _holding = StreakStatus( streak: 9, + longestStreak: 9, freezeHeld: true, daysToNextFreeze: null, freezesSpent: 0, @@ -41,6 +43,7 @@ const _holding = StreakStatus( /// the fixture the wrap is read off (#498). const _secondWeek = StreakStatus( streak: 10, + longestStreak: 10, freezeHeld: false, daysToNextFreeze: 4, freezesSpent: 0, @@ -50,6 +53,7 @@ const _secondWeek = StreakStatus( /// The day after a closing day: the fill starts over at one seventh. const _weekReopened = StreakStatus( streak: 8, + longestStreak: 8, freezeHeld: true, daysToNextFreeze: null, freezesSpent: 0, @@ -97,6 +101,7 @@ Future _pump( /// A milestone-day fixture: seven in a row, the freeze just earned. const _milestoneDay = StreakStatus( streak: 7, + longestStreak: 7, freezeHeld: true, daysToNextFreeze: null, freezesSpent: 0, @@ -158,6 +163,7 @@ void main() { final now = DateTime.now(); final covered = StreakStatus( streak: 5, + longestStreak: 5, freezeHeld: false, daysToNextFreeze: 7, freezesSpent: 1,