Fix dragging Up Next and syncing at the same time #4862

geekygecko · 2025-12-17T09:00:47Z

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

Add lots to your Up Next
Go to the Up Next page
Drag and drop an episode
Filter the logs by UpNextSync to see when the Up Next sync happens
When it starts, drag and drop another episode
✅ Check that there are no background sync jobs running until the end

Screencast

After

Up.Next.Drag.After.mov

Before

Up.Next.Drag.Before.mov

Checklist

If this is a user-facing change, I have added an entry in CHANGELOG.md
Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
I have considered whether it makes sense to add tests for my changes
All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
Any jetpack compose components I added or changed are covered by compose previews
I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

I have tested any UI changes...

with different themes
with a landscape orientation
with the device set to have a large display and font size
for accessibility with TalkBack

Copilot

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

...ures/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/PlayerViewModel.kt

Copilot · 2025-12-17T09:05:56Z

...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImpl.kt

+    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
+    }


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.

I have added some tests.

CookieyedCodes · 2025-12-17T10:41:01Z

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

geekygecko · 2025-12-17T10:57:37Z

@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.

CookieyedCodes · 2025-12-17T10:59:58Z

Yup it would be using auto add to upnext, this is certainly more relatable then my other issue

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

...es/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImplTest.kt

sztomek

I may be wrong, but it doesn't seem to work for me:

Screen_recording_20251219_091658.mp4

logs:

2025-12-19 09:17:04.987  6216-6385  BgTask                  au....shiftyjelly.pocketcasts.debug  I  BgTask: UpNextSyncWorker - scheduled
2025-12-19 09:17:05.022  6216-6216  WM-WorkerWrapper        au....shiftyjelly.pocketcasts.debug  D  Starting work for au.com.shiftyjelly.pocketcasts.repositories.sync.UpNextSyncWorker
2025-12-19 09:17:05.022  6216-6374  BgTask                  au....shiftyjelly.pocketcasts.debug  I  BgTask: UpNextSyncWorker - started
2025-12-19 09:17:09.683  6216-6372  BgTask                  au....shiftyjelly.pocketcasts.debug  I  BgTask: UpNextSyncWorker - finished - 4661 ms
2025-12-19 09:17:09.687  6216-6325  WM-WorkerWrapper        au....shiftyjelly.pocketcasts.debug  I  Worker result SUCCESS for Work [ id=87f3fb77-7282-4b1c-b8ec-b07ff70cb122, tags={ au.com.shiftyjelly.pocketcasts.repositories.sync.UpNextSyncWorker,pocket_casts_up_next_sync_worker_tag } ]

sztomek · 2025-12-19T07:56:47Z

...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImpl.kt

 ) : UpNextQueue,
    CoroutineScope {

+    companion object {


💡 alternative, you could define private companion object and let the consts be val XXXX = Y.seconds

I didn't know about private companion object, thanks. I might do it next time.

sztomek · 2025-12-19T07:57:30Z

...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImpl.kt

    private val upNextChangeDao = appDatabase.upNextChangeDao()
    private val podcastDao = appDatabase.podcastDao()
+
+    @Volatile private var lastUserInteractionTime: Long = 0


💡 lastUserInteractionTimeMs would be more precise. Or use a Clock instance

Thanks I have had added Ms

sztomek · 2026-01-05T09:44:59Z

@geekygecko could you please review my comments and update the PR?

dangermattic · 2026-01-06T03:09:17Z

	1 Warning
⚠️	This PR is assigned to the milestone 8.3. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Copilot · 2026-01-06T03:11:00Z

...s/features/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/view/UpNextFragment.kt


    private var userRearrangingFrom: Int? = null
-    private var userDraggingStart: Int? = null
+    private var userDraggingStartMs: Int? = null


The variable name userDraggingStartMs is misleading. The "Ms" suffix suggests it stores milliseconds, but it actually stores an adapter position (integer index). Consider renaming it to userDraggingStartPosition for clarity.

Suggested change

private var userDraggingStartMs: Int? = null

private var userDraggingStartPosition: Int? = null

It doesn't hold the start position.

Copilot · 2026-01-06T03:11:00Z

...es/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImplTest.kt

+    @Test
+    fun `isUserDragging returns true when within grace period`() {
+        upNextQueue.setDragStartTimeForTesting(1_000L)
+        // checking 4 seconds later (within 10 second grace period)


The comment incorrectly states "within 10 second grace period" when the actual DRAG_GRACE_PERIOD is 1 minute (60 seconds). Update the comment to say "within 1 minute grace period" or "within 60 second grace period" for accuracy.

Suggested change

// checking 4 seconds later (within 10 second grace period)

// checking 4 seconds later (within 1 minute grace period)

oops I have changed this.

geekygecko added this to the 8.3 milestone Dec 17, 2025

Copilot AI review requested due to automatic review settings December 17, 2025 09:00

geekygecko added [Type] Bug Not functioning as intended. [Area] Up Next Bug labels Dec 17, 2025

Copilot started reviewing on behalf of geekygecko December 17, 2025 09:01 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

geekygecko marked this pull request as ready for review December 17, 2025 11:33

geekygecko requested a review from a team as a code owner December 17, 2025 11:33

geekygecko requested review from Copilot and sztomek and removed request for a team December 17, 2025 11:33

Copilot started reviewing on behalf of geekygecko December 17, 2025 11:34 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

...es/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/playback/UpNextQueueImplTest.kt Outdated Show resolved Hide resolved

sztomek reviewed Dec 19, 2025

View reviewed changes

bjtitus mentioned this pull request Dec 28, 2025

Up Next reorder delay Automattic/pocket-casts-ios#3836

Merged

3 tasks

geekygecko added 4 commits January 6, 2026 11:35

Fix dragging Up Next and syncing at the same time

51213b6

Add recentUserInteraction tests

b20f253

Fix test name and volatile variable

ffaeeee

Fix drag sync guard

07effe1

Copilot AI review requested due to automatic review settings January 6, 2026 03:08

geekygecko force-pushed the task/up-next-drag branch from 3b847a4 to 07effe1 Compare January 6, 2026 03:08

Copilot started reviewing on behalf of geekygecko January 6, 2026 03:08 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

Fix comment

3ac5a5f

	private var userDraggingStartMs: Int? = null
	private var userDraggingStartPosition: Int? = null

	// checking 4 seconds later (within 10 second grace period)
	// checking 4 seconds later (within 1 minute grace period)

Fix dragging Up Next and syncing at the same time #4862

Are you sure you want to change the base?

Fix dragging Up Next and syncing at the same time #4862

Conversation

geekygecko commented Dec 17, 2025

Description

Testing Instructions

Screencast

Checklist

I have tested any UI changes...

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

geekygecko Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

CookieyedCodes commented Dec 17, 2025

Uh oh!

geekygecko commented Dec 17, 2025

Uh oh!

CookieyedCodes commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

sztomek left a comment

Choose a reason for hiding this comment

Uh oh!

sztomek Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

geekygecko Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

sztomek Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

geekygecko Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

sztomek commented Jan 5, 2026

Uh oh!

dangermattic commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

geekygecko Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

geekygecko Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CookieyedCodes commented Dec 17, 2025 •

edited

Loading