fix(sync): append offline liked songs to youtube instead of overwriting#3935
fix(sync): append offline liked songs to youtube instead of overwriting#3935kairosci wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates the liked songs sync logic in ChangesLiked Songs Bidirectional Sync Reconciliation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@suicedia could you verify this solves your problem? Thanks |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt`:
- Line 678: This path directly calls YouTube.likeVideo(song.id, true) without
retries; wrap that call in the same withRetry { ... } used by executeLikeSong()
so transient YouTube API failures are retried, and only update/log local state
on success/final failure as executeLikeSong() does (mirror its error handling
and logging behavior referenced around executeLikeSong() and the existing log at
the current YouTube.likeVideo(...) usage).
- Around line 674-681: The loop in SyncUtils.kt that processes
localSongs.filterNot { it.id in remoteIds || it.song.isLocal } must not call
database.update(song.song.localToggleLike()) when song.song.likedDate == null
because migrated legacy rows can have liked=true with a NULL likedDate; instead
treat null as preserved/backfill-needed: change the branch so that if likedDate
== null you either call YouTube.likeVideo(song.id, true) to ensure remote
like/backfill or simply skip toggling the local like (no database.update), and
leave a TODO/comment to schedule a migration/backfill job; modify the condition
around YouTube.likeVideo(song.id, true) / database.update(...) accordingly
(identify the variables song.song.likedDate, YouTube.likeVideo, database.update,
localToggleLike, localSongs, remoteIds).
🪄 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: 12a957b5-b61a-4b0d-83d7-a6dbcc17a541
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt (1)
671-683:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
LastFullSyncKeyis being compared against the wrong sync boundary.Line 671 assumes this key still represents the previous successful full sync, but
tryAutoSync()updates it before the queued sync even runs. That means an offline like created after the last real sync can already be older thanlastSyncby the time this loop executes, so Line 685 will unlike it locally instead of pushing it to YouTube. Move theLastFullSyncKeywrite to the end of a successfulexecuteFullSync()so this branch compares against the last completed sync, not the current attempt.♻️ Minimal fix direction
fun tryAutoSync() { syncScope.launch { ... syncChannel.send(SyncOperation.FullSync) - - context.dataStore.edit { settings -> - settings[LastFullSyncKey] = LocalDateTime.now().toEpochSecond(ZoneOffset.UTC) - } } } private suspend fun executeFullSync() = withContext(Dispatchers.IO) { ... try { ... executeSyncAutoSyncPlaylists() + context.dataStore.edit { settings -> + settings[LastFullSyncKey] = LocalDateTime.now().toEpochSecond(ZoneOffset.UTC) + } updateState { copy(overallStatus = SyncStatus.Completed, currentOperation = "") } } catch (e: CancellationException) { throw e🤖 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 `@app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt` around lines 671 - 683, The code is comparing likes against LastFullSyncKey which is being set too early; change the flow so LastFullSyncKey represents the last completed full sync by removing/updating any write to LastFullSyncKey in tryAutoSync() and instead writing LastFullSyncKey at the end of executeFullSync() only after a successful completion; ensure the filter/compare that reads LastFullSyncKey (the localSongs loop that references lastSync) continues to read from context.dataStore.get(LastFullSyncKey, 0L) so it now compares against the previous completed sync boundary.
🤖 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.
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt`:
- Around line 671-683: The code is comparing likes against LastFullSyncKey which
is being set too early; change the flow so LastFullSyncKey represents the last
completed full sync by removing/updating any write to LastFullSyncKey in
tryAutoSync() and instead writing LastFullSyncKey at the end of
executeFullSync() only after a successful completion; ensure the filter/compare
that reads LastFullSyncKey (the localSongs loop that references lastSync)
continues to read from context.dataStore.get(LastFullSyncKey, 0L) so it now
compares against the previous completed sync boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3458e504-2788-41f7-908c-fecd281ca5c5
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/utils/SyncUtils.kt
kairosci
force-pushed
the
fix-local-liked-songs-overwrite
branch
from
cbb370d to
218b0b7
Compare
kairosci
force-pushed
the
fix-local-liked-songs-overwrite
branch
from
218b0b7 to
fedbc2c
Compare
1 participant
Problem
Local liked songs are getting overwritten by the YouTube account liked songs when a user logs in, effectively wiping out their offline likes.
Cause
The sync logic in
SyncUtilswas removing local likes for any songs not present in the remote YouTube list, without accounting for songs that were liked while the user was offline or prior to logging in.Solution
LastFullSyncKeytimestamp to determine if a local like was added after the last sync or while offline.isLocal) during the sync so they are never unliked by the YouTube remote state.Testing
Analyzed and tested the sync algorithm to ensure offline likes are preserved and pushed, while still respecting unlikes made remotely.
Related Issues
Summary by CodeRabbit