Podcast Re-subscription fixes #3550
Open
Conversation
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
Ensure sync progress notifications always fire: Wraps importPodcast’s progress counter updates in a defer so every exit path still posts syncProgressPodcastUpto and syncProgressPodcastCount, preserving UI accuracy when we return early.
This allows us to more easily bail out with early returns in cases where we may want to (podcast is deleted or unsubscribed)
Respect server subscription intent for new podcasts: Adds a derived shouldSubscribe flag and passes it through to addFromUuid, preventing re-subscribing podcasts the server reports as unsubscribed. We could also short-circuit the addFromUUID call on shouldSubscribe being false but I figured we may be better off keeping that functionality as is.
Uses the subscribed value instead of updating to isDeleted. This should help in case we only receive subscribed and not isDeleted. We won't want to set subscribed to 0 when isDeleted is missing but subscribed is 0
bjtitus
added
[Type] Other
This behavior was changed a bit last minute to keep the podcast in the DB with the subscribed value set rather than skipping adding it.
|
Version |
|
@bjtitus and I tested this together and we think this PR needs some extra checks to see if all the logic is solid taking in account our server implementation for sync |
|
Version |
|
@bjtitus are you still looking into this? Should I remove myself as reviewer? |
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.
Fixes PCIOS-175
There are two primary fix attempts here:
shouldSubscribewhen we callserverPodcastManager.addFromUuid. Otherwise, we could potentially subscribe to a Podcast which the server already told us is unsubscribed.subscribedbased on theisDeletedvalue. It seems like we should also usesubscribedif only it exists. I'm not 100% sure on why we ever useisDeleted, but I decided to keep that for now. It only happens in specific cases wherecheckIsDeletedistruebut this might be something to look at in the future.So both fixes really come down to using the server's
subscribedvalue where we weren't before.To test
Checklist
CHANGELOG.mdif necessary.