-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix "Merge trackers" always emitting failure #23369
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
base: master
Are you sure you want to change the base?
Fix "Merge trackers" always emitting failure #23369
Conversation
- `handleDuplicateTorrent` should only `emit addTorrentFailed` if merging is disabled, or prevented (due to either torrent being private) - `handleDuplicateTorrent` should return a success code, and callers need to be updated to not always assume failure. - While GUI doesn't do much with the success code - `processTorrent` should also be returning true when a duplicate is handled (either via the dialog, or via `handleDuplicateTorrent` - previously it always returned false) Closes qbittorrent#23367 Closes qbittorrent#21821
No. It is intended to |
If this is true, can we take a step back for a minute and challenge this intention? When an end-user (or API) requests to add a torrent, I think we can agree that what they want to ensure is that the files the torrent describes are being downloaded via the trackers and seeds the torrent specifies, that is all. If merging torrents is allowed, and the result of adding the torrent is a merge operation, then the adding of that torrent has succeeded. I think it would be quite an anti-pattern to force all existing usages monitoring the "addTorrentFailed" socket to now have to start parsing the response to see if the qBittorrent/src/app/application.cpp Lines 932 to 937 in 518fd7c
If you only want to emit But I believe based on all current usages that If you require something to be emitted, then the most correct thing to do here is to add a new event |
|
This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity. |
|
Bump |
handleDuplicateTorrentshould onlyemit addTorrentFailedif merging is disabled, or prevented (due to either torrent being private)handleDuplicateTorrentshould return a success code, and callers need to be updated to not always assume failure.processTorrentshould also be returning true when a duplicate is handled (either via the dialog, or viahandleDuplicateTorrent- previously it always returned false)Closes #23367
Closes #21821
Disclaimer: This is my first PR, I don't have a working local build yet, so I'm counting on CI to catch any errors. Any help testing would be appreciated.