Persist download metadata when a favorite is toggled - #210
Merged
Conversation
A downloaded song keeps its own copy of the metadata on disk, which _collectDownloads() restores into the vault at launch. Two paths write `liked` — InteractionProvider behind the Now Playing star, and FavoriteProvider behind the action sheet — but #199 only taught the former to re-write that copy. Favorite a downloaded song from the action sheet and the on-disk `liked` stayed stale, so the next launch restored the old value and the Now Playing sheet showed the wrong star until a server fetch happened to cover that song. Offline, nothing ever did.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesFavorite metadata synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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.
Downloaded songs sometimes show the wrong favorite state, visible on the Now Playing sheet.
Cause
A downloaded song carries its own copy of the metadata on disk.
DownloadProvider._collectDownloads()deserializes that copy at launch and feeds it throughsyncWithVault, so it becomes the vault's version of the song.Two paths write
liked:InteractionProvider._like/_unlike— the Now Playing star. CallspersistMetadataIfNeeded.FavoriteProvider.toggleOne/unlike— the action sheet, and the Favorites screen. Did not.#199 added the persist to the first path only. So favoriting a downloaded song from the action sheet updated it in memory but left the on-disk copy stale; the next launch restored the old value, and the sheet showed the wrong star until some server fetch happened to cover that song. Offline — the case downloads exist for — nothing ever did.
Song.toJsondoes serializeliked, andDownloadSyncProviderdoes compare it, so the metadata format was never the problem; only one of its two writers was missing.Fix
FavoriteProvidertakes aDownloadProviderand persists after a successful like/unlike, mirroringInteractionProvider. Persisting after the request, not before, keeps a rolled-back optimistic update from being written.Tests
Both paths assert the re-persist, and both failure paths assert it does not happen when the request fails and the toggle is rolled back.
Note
fix/downloaded-song-liked-statusis a stale local branch covering earlier work on this bug; everything in it landed via #199 and it can be deleted.Summary by CodeRabbit