Fix: update UI when download status changes #113
Merged
Conversation
This file contains 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
mchowning
commented
Jul 14, 2022
| @@ -81,7 +81,7 @@ class DownloadManagerImpl @Inject constructor( | |||
| override val progressUpdates: MutableMap<String, DownloadProgressUpdate> = mutableMapOf() | |||
| override val progressUpdateRelay: Subject<DownloadProgressUpdate> = ReplaySubject.createWithSize(20) | |||
|
|
|||
| private var workManagerListener: LiveData<Pair<List<WorkInfo>, Map<String?, Playable>>>? = null | |||
| private var workManagerListener: LiveData<Pair<List<WorkInfo>, Map<String?, String>>>? = null | |||
There was a problem hiding this comment.
I was very tempted to wrap one or both of these Strings with an inline class in order to make it clear what is what. I can be a little type-happy at times though, so I held off. It might be interesting to use some value classes for things like UUIDs throughout the app though. 🤔
geekygecko
previously approved these changes
Jul 15, 2022
| @@ -159,7 +162,8 @@ class DownloadManagerImpl @Inject constructor( | |||
|
|
|||
| episodeManager.findPlayableByUuid(episodeUUID)?.let { | |||
| episodeManager.updateDownloadTaskId(it, null) | |||
| if (!episode.isDownloaded && it.episodeStatus != EpisodeStatusEnum.NOT_DOWNLOADED) { | |||
| val episode = episodeManager.findPlayableByUuid(episodeUUID) | |||
There was a problem hiding this comment.
Is there a reason for getting the episode out of the database again after the updateDownloadTaskId?
There was a problem hiding this comment.
No, there is not. I just totally overlooked that we already had the episode here as it. 🤦 Thanks for catching that! 🙇 I've addressed this in 6684ff9.
geekygecko
approved these changes
Jul 15, 2022
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.
Description
Fixes #112
When a download is started by adding something to the up next queue (because of the auto download setting), if you switch from an unmetered to a metered connection, the download status still shows that the download is in progress even though the download has actually been paused.
This is occurring because the download is listening for episode changes and putting those episodes into a map keyed on the download task id. That map is not updated however, unless there is a new/removed/changed uuid. As a result, that episode instance's
episodeStatusremains in "waiting for wifi" even once the download starts. Then when the download stops, the app checks if theepisodeStatusneeds updating to "waiting for wifi", but because it checks against the outdated episode instance contained in the map, it doesn't think theepisodeStatusneeds to be updated even though theepisodeStatusof the episode in the db should be updated.This PR addresses that issue by instead retrieving the "current" episode from the database to see if the
episodeStatusneeds to be updated. In addition, I have changed theid -> episodeto instead be anid -> uuidmap to help avoid us from making this mistake again in the future.Test Steps
Checklist