Fix dragging Up Next and syncing at the same time #4862
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition where Up Next list rearrangement could be interrupted by background sync operations that occur every 5 seconds. The solution introduces user interaction tracking to skip sync operations during and shortly after user interactions like dragging to rearrange episodes.
Key Changes:
- Migrated from RxJava to Kotlin Coroutines/Flow for Up Next sync scheduling
- Added user interaction tracking with a 10-second grace period to prevent sync during drag operations
- Converted blocking database operations to suspend functions for better coroutine integration
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| UpNextChangeDao.kt | Converted findAllBlocking() to suspend findAll() function |
| UpNextSyncWorker.kt | Updated to use the new suspend findAll() function |
| UpNextQueue.kt | Added recordUpNextUserInteraction() interface method |
| UpNextQueueImpl.kt | Migrated RxJava sync scheduling to Coroutines Flow; added user interaction tracking and 10-second grace period logic |
| PlaybackManager.kt | Added pass-through method to record user interactions |
| PlayerViewModel.kt | Added method to record user interactions from UI layer |
| UpNextFragment.kt | Calls recordUpNextUserInteraction() on episode drag to prevent sync conflicts |
...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImpl.kt
Outdated
Show resolved
Hide resolved
...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImpl.kt
Outdated
Show resolved
Hide resolved
...ures/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/PlayerViewModel.kt
Outdated
Show resolved
Hide resolved
| fun recentUserInteraction(): Boolean { | ||
| val now = System.currentTimeMillis() | ||
| val timeSinceInteraction = (now - lastUserInteractionTime).milliseconds | ||
| val recentInteraction = timeSinceInteraction < INTERACTION_GRACE_PERIOD | ||
| if (recentInteraction) { | ||
| Timber.i("Skipping sync - user interacted ${timeSinceInteraction.inWholeSeconds} secs ago") | ||
| } | ||
| return recentInteraction | ||
| } |
There was a problem hiding this comment.
The new user interaction tracking behavior lacks test coverage. Consider adding tests to verify that the sync is properly skipped during the grace period after user interaction and that the sync resumes after the grace period expires. This would help ensure the fix works correctly and prevent regressions.
There was a problem hiding this comment.
I have added some tests.
|
Their is also a bug where if you move episodes when stuff is being added to upnext, the new episodes won't be added if you end up moving a episode before they are added to upnext, something I've just noticed over the years |
|
@CookieyedCodes thanks for letting me know. I'm trying to address Up Next issues. Would the episodes be added by features like "Auto add to Up Next"? I have to figure out how to repeat the issue to make sure I can fix it properly. |
|
Yup it would be using auto add to upnext, this is certainly more relatable then my other issue |
Description
There's an issue when rearranging the Up Next list and the Up Next sync that happens every five seconds. At the moment, while you are dragging, the sync can happen in the background, causing the UI to update incorrectly.
Fixes https://linear.app/a8c/issue/PCDROID-375/dragging-up-next-and-syncing-at-the-same-time
Testing Instructions
UpNextSyncto see when the Up Next sync happensScreencast
After
Up.Next.Drag.After.mov
Before
Up.Next.Drag.Before.mov
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...