Fix/learn quiz course flaws#3787
Merged
Merged
Conversation
- Reset quiz state between activities so answers are not preselected - Shuffle ranking quiz options and require a reorder before submitting - Compute max points from actual gradeable quiz counts so stages are reachable - Persist in-lesson quiz attempts and free-text responses across resume - Keep best lesson result on replay instead of overwriting - Reject quizzes with missing or invalid answer keys in the parser - Fix article audio progress jumping to 100% on pause and body text swapping during highlight - Add pause/scrub controls to direct videos and support YouTube shorts/embed/live URLs - Add explicit close button to lesson sheet and disable accidental swipe dismissal - Show earned stars and a Done option on the lesson finish pane - Show friendly empty state for lessons without content - Remove hardcoded certificate rarity claim - Send selected answer index in quiz attempt sync payload - Remove unreferenced legacy lesson/swiper widgets
- Send real catalog activity _id in quiz attempts so server-side answer
verification runs (was sending the positional index)
- Include selected_indices (multi-choice) and selected_order (ranking)
in quiz attempt payloads
- Report free-text attempts as quiz_attempts entries; drop undocumented
free_text_response and total_activities fields
- Send furthest_activity_index per spec
- Fix offline sync body to {device_id, updates} (was {progress})
- Send platform as ios/android (was invalid "mobile") plus app_version
- Include required device_id in progress link request
- Log server-computed stars/points/stage from progress responses
Contributor
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR updates Learn lesson contracts, catalog metadata, API-backed progress synchronization, session persistence, completion handling, and activity controls. It also removes legacy lesson page and widget implementations and changes completion and modal UI behavior. ChangesLearn lesson and progress overhaul
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Learner
participant LearnLessonExperience
participant LearnLessonCompletionService
participant LearnProgressService
participant LearnSyncService
Learner->>LearnLessonExperience: complete activities
LearnLessonExperience->>LearnProgressService: save attempts and drafts
Learner->>LearnLessonExperience: finish lesson
LearnLessonExperience->>LearnLessonCompletionService: completeLesson
LearnLessonCompletionService->>LearnProgressService: save result and clear session
LearnLessonCompletionService->>LearnSyncService: report completion
LearnSyncService-->>LearnLessonCompletionService: buffer or submit progress
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mobile/lib/src/app/learn/services/learn_lesson_experience_service.dart`:
- Around line 116-122: The ranking validation in
learn_lesson_experience_service.dart is missing a check that correctOrder has
the same length as options. Update the ranking branch in the validation logic to
require correctOrder.length == options.length in addition to the existing null,
set-uniqueness, and bounds checks. Use the LearnQuizFormat.ranking case in the
service’s validation method and keep the validation aligned with gradeRanking so
a malformed expected order cannot make the quiz permanently ungradable.
In `@src/mobile/lib/src/app/learn/widgets/experience/learn_video_activity.dart`:
- Line 54: The direct video listener is being registered after an async
initialize path without checking widget lifecycle, which can cause a disposed
ChangeNotifier assertion if dispose() runs first. Update the LearnVideoActivity
flow around _directController!.initialize() and _onDirectVideoTick so that you
return early when !mounted before calling addListener(), keeping the listener
registration safely after initialization only while the widget is still mounted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ef630dd-8ebb-4c9c-8108-95f42e9c7ee4
📒 Files selected for processing (18)
src/mobile/lib/src/app/learn/formatting/learn_display_text.dartsrc/mobile/lib/src/app/learn/models/learn_lesson_activity.dartsrc/mobile/lib/src/app/learn/pages/lesson_finished.dartsrc/mobile/lib/src/app/learn/pages/lesson_page.dartsrc/mobile/lib/src/app/learn/services/learn_lesson_experience_service.dartsrc/mobile/lib/src/app/learn/services/learn_progress_service.dartsrc/mobile/lib/src/app/learn/services/learn_quiz_scoring_service.dartsrc/mobile/lib/src/app/learn/services/learn_sync_service.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_article_activity.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_course_certificate.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_experience_shell.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_lesson_experience.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_lesson_finish_pane.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_quiz_ranking.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_video_activity.dartsrc/mobile/lib/src/app/learn/widgets/kya_swiper.dartsrc/mobile/lib/src/app/learn/widgets/learn_bottom_sheets.dartsrc/mobile/lib/src/app/learn/widgets/learn_lesson_activities.dart
💤 Files with no reviewable changes (5)
- src/mobile/lib/src/app/learn/widgets/kya_swiper.dart
- src/mobile/lib/src/app/learn/pages/lesson_finished.dart
- src/mobile/lib/src/app/learn/pages/lesson_page.dart
- src/mobile/lib/src/app/learn/widgets/learn_lesson_activities.dart
- src/mobile/lib/src/app/learn/widgets/experience/learn_course_certificate.dart
- Fetch GET /learn/progress (JWT or guest identity) and parse the object-keyed lessons map with optional furthest_activity_index - Merge server progress into local storage keeping the best of each lesson (completion, furthest step, stars/points never downgraded) - Hydrate on app start after guest session/pending sync, and after guest progress is linked to an account on sign-in
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the mobile Learn lesson experience to address quiz/course flow issues by improving lesson playback controls, strengthening progress/session persistence, and tightening the modal UX to avoid accidental dismissal.
Changes:
- Added in-lesson session persistence (quiz attempts + free-text drafts) and improved completion reporting payloads/logging.
- Improved lesson media/quiz UX (direct video playback controls + progress indicator, ranking drag handles + better CTA gating).
- Refined completion/empty states and modal behavior (non-dismissible lesson sheet + explicit close button, clearer finish actions).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mobile/lib/src/app/learn/widgets/learn_lesson_activities.dart | Removed legacy activity card/button widgets (now superseded by experience widgets). |
| src/mobile/lib/src/app/learn/widgets/learn_bottom_sheets.dart | Prevents accidental lesson dismissal by disabling scrim-tap and drag-to-dismiss. |
| src/mobile/lib/src/app/learn/widgets/kya_swiper.dart | Removed unused placeholder widget. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_video_activity.dart | Adds tap-to-play/pause and a progress scrubber for direct videos. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_quiz_ranking.dart | Improves reorder UX (handle drag) and avoids pre-filled correct order. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_lesson_finish_pane.dart | Shows stars and standardizes completion actions (Next + Done). |
| src/mobile/lib/src/app/learn/widgets/experience/learn_lesson_experience.dart | Restores in-progress session state, logs progress, fixes completion flow and empty-state UI. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_experience_shell.dart | Adds explicit close (X) control and supports hiding the drag handle. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_course_certificate.dart | Removes course “rarity label” display. |
| src/mobile/lib/src/app/learn/widgets/experience/learn_article_activity.dart | Makes TTS completion/cancel behavior more reliable and guards highlighting rules. |
| src/mobile/lib/src/app/learn/services/learn_sync_service.dart | Expands completion payload (furthest activity index, quiz attempt details), adds app/platform metadata, updates offline sync format. |
| src/mobile/lib/src/app/learn/services/learn_quiz_scoring_service.dart | Records selected answers/order in grading results for reporting/resume. |
| src/mobile/lib/src/app/learn/services/learn_progress_service.dart | Adds per-lesson session storage and changes maxPoints calculation to match graded quiz count. |
| src/mobile/lib/src/app/learn/services/learn_lesson_experience_service.dart | Adds activityId support and validates quiz answer keys; exposes gradedQuizCount(). |
| src/mobile/lib/src/app/learn/pages/lesson_page.dart | Removes legacy lesson page host widget. |
| src/mobile/lib/src/app/learn/pages/lesson_finished.dart | Removes legacy lesson finished widget. |
| src/mobile/lib/src/app/learn/models/learn_lesson_activity.dart | Adds activityId and richer LearnQuizGrade metadata. |
| src/mobile/lib/src/app/learn/formatting/learn_display_text.dart | Makes activity ordinal formatting safer for out-of-range indices; removes rarity-label helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…o-platform/AirQo-frontend into fix/learn-quiz-course-flaws
- Send free_text_response in lesson completion PUT and restore it (plus quiz_score_ratio) when hydrating progress from the server - Attach JWT to progress writes so logged-in completions are attributed by account identity, not device/guest resolution - Adopt server-driven stages and max_points from the catalog response - Flush buffered offline progress on reconnect instead of next launch - Report furthest_activity_index as the real catalog index when invalid activities are skipped - Make catalog token optional (public endpoint) and use a valid UUIDv4 in the device-id fallback path
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/mobile/lib/src/app/learn/pages/kya_page.dart (1)
166-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
applyCatalogMetaout of the build method.Calling
applyCatalogMetainsideBlocBuilderruns it on every rebuild — includingValueListenableBuilderrevision ticks from progress changes. It mutates static state and performs list sorting on each invocation. Consider applying catalog metadata once when the catalog is first received (e.g., in aBlocListeneror whenstatetransitions toLessonsLoaded), rather than on every frame.♻️ Suggested refactor
Widget _buildCoursesContent() { return BlocBuilder<KyaBloc, KyaState>( + listenWhen: (prev, next) => prev is! LessonsLoaded && next is LessonsLoaded, + listener: (context, state) { + final catalog = switch (state) { + LessonsLoaded s => s.model, + _ => null, + }; + if (catalog != null) LearnCatalog.applyCatalogMeta(catalog); + }, builder: (context, state) { // ... existing code ... - if (catalog != null) LearnCatalog.applyCatalogMeta(catalog); final courses = catalog != null ? LearnCatalog.buildFromV2Catalog(catalog.courses) : const <LearnCourseViewModel>[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/src/app/learn/pages/kya_page.dart` at line 166, Move the LearnCatalog.applyCatalogMeta call out of the KyaPage build path. Right now the BlocBuilder in KyaPage invokes applyCatalogMeta on every rebuild, so update the page to apply catalog metadata once when LessonsLoaded/catalog is first received instead of inside the widget tree; a BlocListener or state-transition handling around the existing catalog/state logic is the right place. Keep the catalog-loading UI in build, but ensure the static mutation and sorting in LearnCatalog.applyCatalogMeta only happen once per catalog change.src/mobile/lib/main.dart (1)
263-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent error swallowing degrades observability.
Both
.catchError((_) {})blocks discard all errors with no logging. IfsyncPendingProgress,hydrateLocalProgress, orlinkProgressToAccountfail, there's no diagnostic trail. At minimum, log the error so failures are traceable.♻️ Add error logging to both catch handlers
void _initLearnGuestSession() { LearnSyncService.instance.ensureGuestSession().then((_) async { await LearnSyncService.instance.syncPendingProgress(); await LearnSyncService.instance.hydrateLocalProgress(); - }).catchError((_) {}); + }).catchError((e) { + logError('Learn guest session init failed: $e'); + }); }.linkProgressToAccount(token) .then((_) => LearnSyncService.instance .hydrateLocalProgress(authToken: token)) - .catchError((_) {}); + .catchError((e) { + logError('Learn account linking/hydration failed: $e'); + });Also applies to: 331-333
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/main.dart` around lines 263 - 266, Both catchError handlers are swallowing failures without any visibility, so update the error handling around LearnSyncService.instance.ensureGuestSession(), syncPendingProgress(), hydrateLocalProgress(), and linkProgressToAccount() to log the caught exception instead of ignoring it. Keep the existing async flow, but in each catchError callback record the error through the app’s logging mechanism with enough context to identify which sync step failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mobile/lib/src/app/learn/models/learn_v2_catalog.dart`:
- Around line 28-35: The JSON parsing for LearnV2Catalog.maxPoints is too strict
and will fail when the server sends numeric values as doubles; update
LearnV2Catalog.fromJson to defensively handle both int and double inputs instead
of using a direct cast. Apply the same fix in LearnV2Stage.fromJson for the
index field, using a safe numeric conversion pattern similar to the existing
stages parsing so floating-point JSON values do not throw.
In `@src/mobile/lib/src/app/learn/services/learn_sync_service.dart`:
- Around line 82-83: `LearnSyncService` is parsing `stars` and `pointsEarned`
with direct `as int?` casts, which can fail when the JSON value is a numeric
double like 0.0. Update the deserialization logic in `LearnSyncService` to use
the same safe numeric handling pattern already used for `furthestActivityIndex`
and `quizScoreRatio`, converting any `num` to an int and defaulting to 0
otherwise. Keep the fix localized to the mapping that builds the learn sync
model so both fields accept int or double input safely.
---
Nitpick comments:
In `@src/mobile/lib/main.dart`:
- Around line 263-266: Both catchError handlers are swallowing failures without
any visibility, so update the error handling around
LearnSyncService.instance.ensureGuestSession(), syncPendingProgress(),
hydrateLocalProgress(), and linkProgressToAccount() to log the caught exception
instead of ignoring it. Keep the existing async flow, but in each catchError
callback record the error through the app’s logging mechanism with enough
context to identify which sync step failed.
In `@src/mobile/lib/src/app/learn/pages/kya_page.dart`:
- Line 166: Move the LearnCatalog.applyCatalogMeta call out of the KyaPage build
path. Right now the BlocBuilder in KyaPage invokes applyCatalogMeta on every
rebuild, so update the page to apply catalog metadata once when
LessonsLoaded/catalog is first received instead of inside the widget tree; a
BlocListener or state-transition handling around the existing catalog/state
logic is the right place. Keep the catalog-loading UI in build, but ensure the
static mutation and sorting in LearnCatalog.applyCatalogMeta only happen once
per catalog change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63a727ee-5730-4dd4-9cd8-ea5b4c78afd4
📒 Files selected for processing (13)
src/mobile/lib/main.dartsrc/mobile/lib/src/app/learn/models/learn_course_structure.dartsrc/mobile/lib/src/app/learn/models/learn_v2_catalog.dartsrc/mobile/lib/src/app/learn/pages/kya_page.dartsrc/mobile/lib/src/app/learn/repository/learn_repository.dartsrc/mobile/lib/src/app/learn/services/learn_lesson_experience_service.dartsrc/mobile/lib/src/app/learn/services/learn_progress_service.dartsrc/mobile/lib/src/app/learn/services/learn_sync_service.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_lesson_experience.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_quiz_ranking.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_video_activity.dartsrc/mobile/lib/src/app/shared/utils/device_id_manager.dartsrc/mobile/lib/src/meta/utils/api_utils.dart
✅ Files skipped from review due to trivial changes (1)
- src/mobile/lib/src/meta/utils/api_utils.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- src/mobile/lib/src/app/learn/widgets/experience/learn_quiz_ranking.dart
- src/mobile/lib/src/app/learn/services/learn_lesson_experience_service.dart
- src/mobile/lib/src/app/learn/widgets/experience/learn_lesson_experience.dart
- src/mobile/lib/src/app/learn/services/learn_progress_service.dart
- Extract LearnApiClient: pure transport for the five Learn endpoints; LearnSyncServiceImpl becomes an orchestrator (session lifecycle, buffering, hydration) with constructor-injected collaborators (api client, progress store, cache manager, auth/device/prefs/version providers), defaulting to production wiring - Make LearnSyncService.instance settable so tests can substitute a fake - Move QuizAttemptData/LearnServerProgress models to models/learn_progress_models.dart (re-exported for compatibility) - Extract LearnLessonCompletionService: scoring, persistence, session cleanup and server report move out of the lesson experience widget; widget accepts injectable progress/completion services - Replace LearnCatalog's mutable stage/max-points statics with an immutable LearnCatalogMeta snapshot swapped on catalog load - Add injectable deps to LearnRepositoryImpl (cache manager, token provider) and a visibleForTesting LearnProgressService constructor - Stub isOffline in kya_bloc_test setUp; with regenerated mocks all 10 Learn bloc tests pass (previously 4 failed on MissingStubError)
- LearnApiClient: endpoint paths, identity headers (JWT/guest/device), body shapes, response parsing, non-2xx handling via MockClient - LearnSyncServiceImpl: guest session lifecycle, JWT-attributed completion reports, offline buffering and flush, server-to-local hydration, guest link/unlink — all through injected fakes - LearnLessonCompletionService: scoring excludes free text, local persistence, session cleanup, sync report contents - LearnProgressService: best-result replay rule, server merge (ratio adoption, no free-text clobber, no downgrades) - LearnCatalogMeta: stage sorting/filtering, fallback ladder, server max_points preference 36 Learn tests passing
JSON numbers can arrive as doubles; direct 'as int?' casts on max_points, stage index, course_number, and activity order would throw and fail the whole catalog parse. Coerce via a shared helper and cover with parsing tests.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/mobile/test/app/learn/services/learn_api_client_test.dart (1)
50-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider splitting the combined test into two focused tests.
The test "falls back to session_id and returns null on failure" asserts two distinct behaviors: session_id fallback parsing and null return on non-2xx. Splitting them would improve failure diagnostics when one behavior breaks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/test/app/learn/services/learn_api_client_test.dart` around lines 50 - 72, Split the combined createAnonymousSession test into two focused tests: one validating session_id fallback parsing and another validating null return for a non-2xx response. Keep the existing setup and assertions, using descriptive test names so failures clearly identify the affected behavior.src/mobile/lib/src/app/learn/services/learn_sync_service.dart (1)
305-323: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
syncPendingProgressagainst concurrent invocation.
drain()reads the buffer without clearing it;clear()only runs after a successful sync. Between those two points (which span anawaiton the API call), a secondsyncPendingProgress()call — triggered by_ensureReconnectFlush,linkProgressToAccount, or app startup — can read the same pending entries and submit duplicate sync requests. If the server's sync endpoint isn't idempotent, this could double-count progress.🔒 Proposed fix: add a re-entrancy guard
bool _isSyncing = false; + `@override` Future<void> syncPendingProgress() async { await _ensurePrefs(); + if (_isSyncing) return; + _isSyncing = true; final pending = _buffer!.drain(); if (pending == null || pending.isEmpty) return; try { final synced = await _api.postSyncUpdates( identity: await _identity(), updates: pending, ); if (synced) { await _buffer!.clear(); loggy.info('Synced ${pending.length} pending Learn progress entries'); } } catch (e) { loggy.warning('Pending Learn progress sync failed: $e'); + } finally { + _isSyncing = false; } }Verify whether the server's
/progress/syncendpoint is idempotent — if it is, duplicate submissions are harmless and this becomes lower priority.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mobile/lib/src/app/learn/services/learn_sync_service.dart` around lines 305 - 323, Add a re-entrancy guard to syncPendingProgress so concurrent calls cannot drain and submit the same buffer entries. Track whether a sync is already in progress, return early for overlapping invocations, and reliably reset the guard with try/finally around the existing API submission and buffer-clearing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mobile/lib/src/app/learn/models/learn_progress_models.dart`:
- Around line 68-69: Replace the unsafe casts for stars, pointsEarned, and
totalPoints in the relevant Learn progress model constructors with safe numeric
checks, following the existing quizScoreRatio is-num pattern and converting
accepted num values to int as appropriate; preserve 0 defaults for missing or
invalid values.
---
Nitpick comments:
In `@src/mobile/lib/src/app/learn/services/learn_sync_service.dart`:
- Around line 305-323: Add a re-entrancy guard to syncPendingProgress so
concurrent calls cannot drain and submit the same buffer entries. Track whether
a sync is already in progress, return early for overlapping invocations, and
reliably reset the guard with try/finally around the existing API submission and
buffer-clearing logic.
In `@src/mobile/test/app/learn/services/learn_api_client_test.dart`:
- Around line 50-72: Split the combined createAnonymousSession test into two
focused tests: one validating session_id fallback parsing and another validating
null return for a non-2xx response. Keep the existing setup and assertions,
using descriptive test names so failures clearly identify the affected behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0b4bba8-3f2f-46f9-a6f8-1ec58d4446cc
📒 Files selected for processing (14)
src/mobile/lib/src/app/learn/models/learn_course_structure.dartsrc/mobile/lib/src/app/learn/models/learn_progress_models.dartsrc/mobile/lib/src/app/learn/repository/learn_repository.dartsrc/mobile/lib/src/app/learn/services/learn_api_client.dartsrc/mobile/lib/src/app/learn/services/learn_lesson_completion_service.dartsrc/mobile/lib/src/app/learn/services/learn_progress_service.dartsrc/mobile/lib/src/app/learn/services/learn_sync_service.dartsrc/mobile/lib/src/app/learn/widgets/experience/learn_lesson_experience.dartsrc/mobile/test/app/learn/bloc/kya_bloc_test.dartsrc/mobile/test/app/learn/models/learn_course_structure_test.dartsrc/mobile/test/app/learn/services/learn_api_client_test.dartsrc/mobile/test/app/learn/services/learn_lesson_completion_service_test.dartsrc/mobile/test/app/learn/services/learn_progress_service_test.dartsrc/mobile/test/app/learn/services/learn_sync_service_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mobile/lib/src/app/learn/services/learn_progress_service.dart
stars, points_earned, and total_points used direct 'as int?' casts that throw when the server sends doubles, failing the whole progress parse; furthest_activity_index silently became null on a double, losing resume position. Coerce all four via a shared helper and cover with parsing tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements