search

nukeop / nuclear

Streaming music player that finds free music for you
https://nuclearplayer.com
GNU Affero General Public License v3.0
12.22k stars 1.07k forks source link

Fix download pausing/resume click behaviour #1713

Closed waldo121 closed 1 month ago

waldo121 commented 1 month ago

Fixes #1712

I simply chose to move a responsability out of the Download Progress Action back to the ipc controller.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 64.49%. Comparing base (280327b) to head (9f9e5d3). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
packages/app/app/actions/downloads.ts 0.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1713 +/- ## ========================================== - Coverage 64.50% 64.49% -0.01% ========================================== Files 392 392 Lines 8404 8405 +1 Branches 765 766 +1 ========================================== Hits 5421 5421 Misses 2323 2323 - Partials 660 661 +1 ``` | [Flag](https://app.codecov.io/gh/nukeop/nuclear/pull/1713/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/nukeop/nuclear/pull/1713/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `64.49% <0.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nukeop commented 1 month ago

I don't get why'd this change anything, but if you say this fixes the problem, I'm grateful.

waldo121 commented 1 month ago

For the record, DOWNLOAD_PROGRESS can be emitted after the download stops due to being paused. So we can imagine the following sequence.

  1. A download is in progress
  2. User clicks on pause -> 1st status change -> item goes from 'started' -> 'paused'
  3. (No more user action) DOWNLOAD_PROGRESS gets sent now -> 2nd status change -> completion is updated -> item goes from 'paused' => 'started' (assuming download is not finished) -> ui is lying

After the fix, DOWNLOAD_PROGRESS handler is not responsible to decide a download is correctly 'started'. So the progress event still happens but has no unexpected impact.