-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/add to up next after sync #3212
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left two minor comments.
It works! 🙂 We'll further monitor it in 7.78 beta.
@@ -291,16 +293,15 @@ class RefreshPodcastsThread( | |||
settings.setRefreshState(settings.getLastSuccessRefreshState() ?: RefreshState.Never) | |||
} | |||
|
|||
private fun updatePodcasts(result: RefreshResponse?): List<String> { | |||
private data class AddedEpisodes(val episodeUuidsAdded: List<String>, val episodesToAddToUpNext: MutableList<Pair<AddToUpNext, PodcastEpisode>>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we convert episodesToAddToUpNext
to immutable List
and make it mutable when needed?
episodesToAddToUpNext.removeAll { runBlocking { true == episodeManager.findByUuid(it.second.uuid)?.isFinished || | ||
true == episodeManager.findByUuid(it.second.uuid)?.isArchived || | ||
playbackManager.upNextQueue.contains(it.second.uuid)} } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we find matching episodes using a single query using findEpisodesByUuids
? Something like below to avoid multiple DB interactions?
val episodeToBeRemovedUuids = episodeManager.findEpisodesByUuids(episodesToAddToUpNext.map { it.second.uuid })
.filter { it.isFinished || it.isArchived || playbackManager.upNextQueue.contains(it.uuid) }
.map { it.uuid }
and then remove those uuids here?
Description
Attempt to defer addition to UpNext these episodes that are new (post update) but might have been already finished, archived or added to UpNext on another device. The update of UpNext got extracted as a separate function, filter was added (potentially causing some slowdown as it would check if episodes aren't finished, archived or already on the UpNext list).
Must admit I'm not 100% sure that UpNext sync will always finish by the time we return from sync(), so if anything logic there might be slightly broken (still don't got into duplicated or missing items on UpNext after some testing)
Fixes #3211
Testing Instructions
Screenshots or Screencast
Need to get my head around how to create these.
Checklist
./gradlew spotlessApply
to automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xml
I have tested any UI changes...
N/A