flood
flood copied to clipboard
server: qBittorrent: Update the status filter behavior
server: qBittorrent: Update the status filter behavior so that UX is in sync with qB GUI/WebUI.
Description
Mainly appends 'pausedDL' torrents to 'Downloading' status.
I understand rtorrent/rutorrent may have the design of classifying torrent status, and I'm not sure if you want to unify the logic for the current three backends, but this PR would be more natural logically for qBittorrent users.
Adjustment based on: https://github.com/CzBiX/qb-web/blob/809d5bab4b5b4566cd3d74eb6e5df243fb4a17c1/src/utils.ts#L4
Types of changes
- [ ] Breaking change (changes that break backward compatibility of public API or CLI - semver MAJOR)
- [ ] New feature (non-breaking change which adds functionality - semver MINOR)
- [x] Bug fix (non-breaking change which fixes an issue - semver PATCH)
Codecov Report
Merging #237 (ecb300f) into master (02aa24e) will decrease coverage by
0.16%. The diff coverage is100.00%.
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
- Coverage 78.79% 78.63% -0.17%
==========================================
Files 57 57
Lines 9211 9212 +1
Branches 917 908 -9
==========================================
- Hits 7258 7244 -14
- Misses 1945 1957 +12
- Partials 8 11 +3
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...services/qBittorrent/util/torrentPropertiesUtil.ts | 60.63% <100.00%> (+0.42%) |
:arrow_up: |
| server/config/passport.ts | 85.36% <0.00%> (-4.88%) |
:arrow_down: |
| server/models/Users.ts | 91.17% <0.00%> (-2.95%) |
:arrow_down: |
| server/services/interfaces/clientGatewayService.ts | 97.70% <0.00%> (-1.15%) |
:arrow_down: |
| server/services/index.ts | 95.62% <0.00%> (-0.55%) |
:arrow_down: |
| ...rver/services/Transmission/clientRequestManager.ts | 74.44% <0.00%> (-0.32%) |
:arrow_down: |
| server/services/rTorrent/clientGatewayService.ts | 70.50% <0.00%> (-0.24%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 02aa24e...ecb300f. Read the comment docs.
None of the changes is effective. Note that switch statements fallthroughs till break and a torrent can’t be downloading and stopped at the same time.
Additionally, rTorrent and ruTorrent are two distinct pieces of software. It is inappropriate to use rTorrent/ruTorrent. Flood connects to rTorrent only, and it doesn’t depend on ruTorrent.
The main reason for this PR is that there is currently no apparent way to filter pausedDL torrents for continuing in the current category. I understand that it would be confusing that a torrent is both in downloading and stopped. Do you think it's an idea to rename stopped to paused like qB?
I only mention ruTorrent because it was de-facto the main rTorrent webui before Flood so maybe there is some experience or design to inherit. Disregard if you don't care
The main reason for this PR is that there is currently no apparent way to filter
pausedDLtorrents for continuing in the current category. I understand that it would be confusing that a torrent is both indownloadingandstopped. Do you think it's an idea to renamestoppedtopausedlike qB?
I don't think that's necessary. IMO there is zero reason to make "stopped" distinct from "paused". It only creates more confusion.
@slikie
Hi, are you still championing this PR?
If yes, you would have to at least make it actually effective. (and respond to my comments)
if no, feel free to close the PR.
and a torrent can’t be
downloadingandstoppedat the same time.
I don't think there's any code logic present against that.
None of the changes is effective. Note that switch statements fallthroughs till
break
I tested it and it is effective. One torrent is only at a state at the time, it won't need to go past break.
I'm still arguing that the current UX doesn't provide a easy way to resume previously paused torrents that aren't finished downloading, which is a necessary function.
There are a lot of circumstances people need to pause their torrents and resume afterwards, and there isn't an apparent queue in flood right now to address that.
Right now I will need to go stopped status, sort by percent complete, and by no way I can tell if one torrent is finished downloading or stuck at 99%.
Hope that address your confusion.
There are a few ways for this:
- Transmission/Deluge: add
pausedand refactorstoppedintocompleted. - rutorrent: add
pausedDLintoDownloading, as I proposed. while there is nostoppedcategory. - qBittorrent: add
pausedDLintoDownloadingand refactorstoppedintoPaused.
You don’t need to change any backend code for this.
What you want is a filter that contains “stopped” but not “completed”.
You don’t need to change any backend code for this.
What you want is a filter that contains “stopped” but not “completed”.
This.
Current change is unnecessary and simply won’t work. A new paused filter will NOT magically appear.
IMO there is zero reason to make "stopped" distinct from "paused". It only creates more confusion.
I was kind of worried about this but I take this as a go-ahead?
The commit changes the public API, and as a result, unfortunately, is a breaking change. Per versioning conventions, I can’t do this without a major release.
And, as i said, backend change is totally unnecessary. You can achieve this purely in frontend by filtering “stopped” and “NOT complete”.
It is an option to add a “Paused” filter that does that in the frontend. However, I would prefer a more general method like “Advanced Filtering” (see Flood-UI/flood#312, though, I can implement this myself in the future).
IMO there is zero reason to make "stopped" distinct from "paused". It only creates more confusion.
I was kind of worried about this but I take this as a go-ahead?
No. I don’t mean that we should replace “stopped” with “paused”. I am just not convinced that a new “paused” status would be useful.