feat: riverpod migration

[Nirajn2311]

TODO: Update description

[Nirajn2311]

Every scenario across the 3 sections have been tested in both Android and iOS simulators so those sections should be good for a review

[huyenltnguyen]

We should use this image instead (no underscore below the emblem): https://cdn.freecodecamp.org/platform/universal/fcc_banner_new.png

[huyenltnguyen]

I think we could remove these Flutter icons?

• mobile-app-new/android/app/src/main/res/mipmap-hdpi/ic_launcher.png
• mobile-app-new/android/app/src/main/res/mipmap-mdpi/ic_launcher.png
• mobile-app-new/android/app/src/main/res/mipmap-xhdpi/ic_launcher.png
• mobile-app-new/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.png
• mobile-app-new/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.png

[huyenltnguyen]

For this one, we can grab the SVG from https://github.com/freeCodeCamp/freeCodeCamp/blob/main/client/src/assets/icons/default-avatar.tsx.

The new default image looks like this:

Screenshot

(Sorry, I know this is an existing issue, but I didn't catch it until now 😅 )

[huyenltnguyen]

Could we remove these Flutter icons?

• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.png
• mobile-app-new/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.png

[Sembauke]

freeCodeCamp mobile PR #1843 review: Stacked to Riverpod

PR: #1843

Reviewed commit: 90b44a48b92a54fb0f2b791c651fd0a812000964.

Scope: News, Podcasts, Code Radio, shared audio, persistence, and provider lifecycles in mobile-app-new, compared with the existing mobile-app. Learn is intentionally excluded because its migration is unfinished. This report consolidates the nine findings from both review passes.

Severity: P1 means a high-priority issue to resolve before shipping the replacement app because existing user data is at risk. P2 means a functional correctness issue requiring a fix.

Validation: Eight findings were exercised with focused tests using the application code and mocked platform or network boundaries. The Code Radio notification Stop/resume finding was verified by tracing the application and installed just_audio implementation. No Android/iOS device or simulator end-to-end run was performed. The upgrade tests use legacy-storage fixtures, not an actual installed-app upgrade.

Finding 1 (P1): Import SQLite data before treating missing JSON as an empty library

Locations: podcasts/repositories/storage_repository.dart:148–150, news/repositories/bookmark_repository.dart:37–39, and podcasts/services/library_service.dart:101–110.

Problem: The new repositories read only the JSON stores. If an existing user still has SQLite-only bookmarks or downloads, the new app presents an empty library. The podcast cleanup path can subsequently delete that user's downloaded audio.

Reproduction: Start with a legacy podcasts.db containing episode A and A's downloaded audio, but no storage/podcast-downloads.json. Read the new downloads library: A is missing. Download episode B, creating JSON containing B only. On a subsequent startup, the orphan sweep reads that JSON and deletes A's audio because A has no JSON row. For bookmarks, start with bookmarked-article.db and no bookmark JSON; the new service returns no bookmarks, and saving a new bookmark writes JSON containing only the new entry.

Cause: Both repositories treat absent JSON as no usable records without importing the old database. The podcast sweep correctly skips cleanup while the store is missing or unreadable, but after the first new download the incomplete JSON becomes readable. Cleanup then mistakes legacy audio for orphaned files.

Impact: Existing bookmarks become inaccessible, and existing podcast audio can be permanently deleted. The bookmark SQLite file itself is not deleted by the reproduced path. Installing the intermediate JSON-storage release is not sufficient for every user: bookmark migration in the old app is lazy, so someone who never opened the relevant UI can still have SQLite-only data.

Suggested fix: Preserve the legacy import path, make it idempotent, and merge legacy entries safely if JSON already exists. Gate orphan cleanup on completed migration, not merely on readable JSON. Test a direct SQLite upgrade, an intermediate release whose lazy migration never ran, and a mixed SQLite/JSON state.

Validation: Real legacy database fixtures and the actual new repositories/services were used with isolated storage directories. The podcast fixture lost A's audio after creating B's JSON entry and running the actual sweep. The bookmark fixture returned an empty list and then wrote only the new bookmark. Controls confirmed existing v7 JSON bookmarks remain readable and legacy podcast JSON can migrate while retaining records.

Applicability: This is an upgrade blocker when the new implementation replaces the installed app and inherits its storage. A clean installation of the separate scaffold does not exercise that path.

Finding 2 (P2): Keep download dependencies alive until transfers and persistence finish

Locations: podcasts/controllers/downloads_controller.dart:40–56, podcasts/repositories/storage_repository.dart:51–53, and podcasts/controllers/library_controller.dart:13–17.

Problem: The downloads notifier keeps itself alive during a transfer, but it reads the library service without retaining its provider dependency. Leaving Podcasts can dispose the service and storage providers while the original operation still holds their objects. Returning creates another storage repository operating on the same file.

Reproduction: Start download A, leave Podcasts so its library subscriptions are removed, return, and start download B before A finishes. Allow both transfers to finish together. A and B can now persist through separate repository instances. A second variant is to start one download, leave the section, and let it finish in the background.

Cause: ref.read(podcastLibraryServiceProvider) does not make that provider a watched dependency of the pinned notifier. Storage serialization is per repository instance, so it no longer protects the shared file once two instances coexist. After completion, reading an otherwise unobserved podcastLibraryProvider.notifier and awaiting refresh() also allows that notifier to be disposed before its state update.

Impact: Concurrent completion can lose a download entry. A successful background transfer can also be reported as DownloadFailed when its subsequent library refresh throws a disposed-reference exception. That misleading state offers a retry for an episode already saved.

Suggested fix: Retain the library service and its storage dependency throughout the operation, and ensure writes to the same store share one serialization mechanism. Refresh or invalidate the UI library in a way that does not await a state update on an unobserved disposable notifier. Do not classify a UI refresh failure as a failed transfer.

Validation: Tests used the actual providers, controllers, library service, and file-backed storage with controlled transfer completion. Leaving and returning created two storage repositories; concurrent completion retained only one episode. The single-background-download case persisted the episode but reported failure. A control using one repository preserved both concurrent updates.

Finding 3 (P2): Ignore superseded audio loads instead of stopping or relabeling the current source

Locations: services/audio_service.dart:229–231 and services/audio_service.dart:179–180.

Problem: Multiple source loads share one player, but their completion and error handlers do not check whether they still own it. An older interrupted request can stop or overwrite the state of a newer selection.

Reproduction: Start loading podcast episode A, then select B before A finishes buffering. Replacing A's source interrupts A's pending load. A's catch block calls the shared stop(), aborting B too. The episode play buttons remain enabled while their spinner is displayed, so this sequence is reachable from the UI.

Related reproduction: After loading podcast A, start a slow Code Radio load, then select podcast B before the radio load completes. When the radio request is interrupted, its catch restores the previously captured configuration for A, even though B has become the current media item.

Cause: loadEpisode() handles every load exception by stopping the current player. loadCodeRadio() handles every failure by restoring its captured previous configuration. Neither distinguishes an active request's failure from a superseded request's cancellation.

Impact: The latest selected podcast may fail to start. In the radio variant, B's displayed media and the handler's episode identity disagree, so controls and saved progress can refer to A.

Suggested fix: Give source changes an operation identity or serialize them with explicit cancellation semantics. Before publishing media, restoring configuration, stopping, or continuing playback after an await, verify that the operation is still current. An interrupted old load must not mutate a newer load's state.

Validation: Focused tests exercised the actual audio handler and installed just_audio with a mocked native player. The podcast race returned false for B and left both episode identity and media item null. The radio race successfully loaded media B but left the handler's episode identity as A.

Finding 4 (P2): Do not save the previous episode's position against the next episode

Locations: services/audio_service.dart:217–221, podcasts/services/playback_service.dart:114–130, and podcasts/services/playback_service.dart:146–159.

Problem: loadEpisode() changes _audioConfig to B before it replaces A's source. A position event during that interval is therefore attributed to B.

Reproduction: Play A at ten minutes, switch to an unplayed B, and stop B shortly after it starts. Later play B again. B can resume at ten minutes despite having played for less than a second.

Cause: _audioConfig changes before the awaited setSpeed() and setAudioSource() calls. The speed change can emit a playback/position event from A. PodcastPlaybackService._onPosition() labels that event with the handler's current episode ID, which is already B. Positions under 30 seconds are subsequently ignored rather than clearing the incorrect saved position.

Impact: Resume progress is corrupted across episodes. The problem does not require overlapping user-initiated loads; an ordinary sequential switch can trigger it.

Suggested fix: Keep each progress event associated with the source that produced it. Suppress persistence during a source transition or publish the new episode identity only after the corresponding source is active. Ensure an old-source event cannot write or clear progress for the new episode.

Validation: The actual handler and playback service were exercised with just_audio's native boundary mocked and preferences isolated. A was at 600 seconds, B ran for about 50 milliseconds, and B's saved resume position was nevertheless 600 seconds.

Finding 5 (P2): Reinitialize Code Radio after notification Stop

Locations: code_radio/ui/player/player_viewmodel.dart:61, services/audio_service.dart:110–131, services/audio_service.dart:274, and code_radio/services/code_radio_service.dart:66–78.

Problem: The in-app toggle treats a stopped radio session as though it were merely paused.

Reproduction: Keep the Code Radio screen open, press Stop in Android's media notification, then press Play in the app.

Cause: stop() clears the audio configuration, stream URL, and media item. The Code Radio service sees the unloaded state and cancels its station subscription. The toggle then calls only handler.play(). just_audio retains its source after stop and can resume it, but the app's playback-event listener returns immediately while _audioConfig is null. The still-mounted radio provider does not automatically restart the station subscription.

Impact: Audio can restart while the screen continues showing Play. Further taps do not pause it, and normal notification and metadata updates are not restored.

Suggested fix: Distinguish paused playback from a stopped or unloaded session. For the latter, restart the station subscription and reload the radio configuration/source before playing. Keep ordinary pause/resume behavior separate from that reinitialization.

Validation: Verified by tracing the app's stop, subscription, toggle, and playback-event paths, plus the installed just_audio 0.10.6 behavior that retains a stopped source. This particular Android notification interaction was not run on a device or in a focused runtime test.

Finding 6 (P2): Bind episode controls to the episode displayed on the page

Locations: podcasts/ui/episode/episode_view.dart:157–177 and podcasts/ui/episode/episode_view.dart:253.

Problem: Opening an episode page does not load that episode, but its seek, rewind, forward, and speed controls operate on the shared player's current source.

Reproduction: Load and pause A, open B's detail page without pressing Play, then drag B's scrubber. B's duration and a zero position are displayed, but the seek command changes A's position.

Cause: isSeekable checks only for a positive duration, not whether the displayed episode is current. The seek method accepts only a position, so it cannot enforce the target episode. The other transport controls similarly dispatch directly to the shared handler.

Impact: A user manipulates an episode other than the one shown. Seeking B before starting it also does not establish B's requested starting position.

Suggested fix: Disable source-specific controls until the displayed episode is loaded, or explicitly load that episode before applying the operation. Passing an episode identity through the controller provides a second guard against controlling the wrong source.

Validation: A widget test used the actual episode view and player controller with a fake handler holding paused A. B's slider was enabled. Seeking halfway through B's ten-minute duration sent a five-minute seek to the handler while its episode identity remained A.

Finding 7 (P2): Honor cancellation during the remainder of the download operation

Locations: podcasts/services/library_service.dart:43–60 and podcasts/controllers/downloads_controller.dart:80–83.

Problem: The UI still offers cancellation while artwork is downloading, but cancellation only controls the already-finished audio request.

Reproduction: Download an episode whose audio completes before a slow artwork request. While artwork is pending and the operation is still displayed as running, press Cancel. Allow artwork to finish.

Cause: The cancel token is passed to downloadEpisode() but not to saveArtwork(). After awaiting artwork, the service neither checks cancellation nor verifies that the operation is still active before committing the episode and showing a completion notification.

Impact: The episode is saved and completion is reported even though the user canceled the operation and its running state was cleared.

Suggested fix: Define the cancellation boundary explicitly. While the UI advertises cancellation, propagate or check the token through subsequent stages and before persistence/notification. Clean up uncommitted files safely, or mark the operation as committed and remove the cancellation affordance once cancellation can no longer be honored.

Validation: A test delayed artwork completion while using the actual library service and downloads controller. It canceled at that stage, released the artwork operation, and found the canceled episode committed to storage.

Finding 8 (P2): Avoid leaving repeated searches in a loading state

Location: news/ui/search/search_viewmodel.dart:25–32.

Problem: Clearing the field changes the notifier's results but does not change the query retained by Algolia's searcher. Repeating the same term can therefore set loading without producing a new response.

Reproduction: Search for python and wait for results. Clear the field, wait, then paste python again so no intermediate query is submitted. The spinner remains.

Cause: The empty-term branch returns before calling the search service. Algolia still holds python. On the next identical query, the notifier sets AsyncLoading, but the searcher's distinct-state filtering suppresses another request and response. Nothing transitions the notifier out of loading.

Impact: Search becomes stuck for this ordinary clear-and-repeat interaction until another query or provider reset changes the searcher's state.

Suggested fix: Keep the searcher's query synchronized with the cleared field, or deliberately reuse/reissue a repeated query without entering a loading state that has no corresponding response. Preserve sensible behavior for empty queries and pending responses.

Validation: The actual notifier/service and real Algolia 1.6.0 HitsSearcher were tested with a fake remote. The first query settled; after clear and repeat, the remote request count remained one and state remained loading. The indefinite stall follows from the verified distinct-query behavior, not from waiting indefinitely in the test.

Finding 9 (P2): Reject pagination results after disposal or refresh

Locations: news/controllers/feed_controller.dart:64–81 and the analogous path in podcasts/ui/episode_list/episode_list_viewmodel.dart:91–106.

Problem: fetchNextPage() captures the current feed, awaits a request, then writes state without confirming that its provider and request generation are still current.

Reproduction A: Trigger pagination, navigate away so the provider is disposed, then let the request complete. The success-path state assignment throws; the catch attempts another state assignment and throws a disposed-reference exception too.

Reproduction B: Trigger pagination, refresh the feed before the old append request finishes, and let the refreshed first page complete. Then complete the old append. Its captured pre-refresh state overwrites the refreshed state and cursor.

Cause: The method has neither cancellation tied to provider disposal/rebuild nor an operation-generation check. A mounted check alone is insufficient for a refresh where the notifier survives and now represents a newer request generation.

Impact: Navigating away can produce an unhandled async exception. Refresh can be undone by a late pagination response, bringing back stale results and pagination metadata. This review did not demonstrate a whole-app crash.

Suggested fix: Cancel pending work on disposal or invalidation where possible, and capture a generation/token for every append. Reject results and errors from an obsolete generation before assigning state. The error path needs the same protection as the success path.

Validation: Tests exercised the actual News notifier with a controlled fake API. One reproduced the disposed-reference exception. Another confirmed that a refreshed cursor was replaced by the old pagination cursor. Those test pages used empty post lists; replacement of visible articles follows from the same state-copy code. The matching podcast pattern was identified by inspection rather than a separate pagination runtime test.

Additional validation observations

flutter analyze --no-pub lib passed using Flutter 3.44.2 and Dart 3.12.2. The initial full analysis reported only the checkout's missing local .env asset. A non-secret local stub was added for review testing; no tracked application source was changed.

The shipped test/widget_test.dart fails with No ProviderScope found because it pumps MyApp directly. It also still contains the generated counter-app assertions. Replace it with an application smoke test using the required provider overrides and assertions for the actual UI.

Focused review tests are separate from the PR's tests. Some deliberately assert the existing defective behavior and pass when it is reproduced; others assert the desired behavior and fail on this revision. These outcomes should not be described as a passing application test suite. There was no live Algolia request, native audio playback, or production-data mutation during validation.

[huyenltnguyen]

I've pulled the changes down to test. Everything seems to be working as expected, though I'm wondering if this might just be an issue on my end. I'm using 16e in the simulator, and the text in both News and Podcast seems a bit large: there are only about 5-6 words per line.

Screenshots