Skip to content
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

Fix incorrect sorting by "private" column #21041

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

glassez
Copy link
Member

@glassez glassez commented Jul 9, 2024

No description provided.

@glassez glassez added the Hotfix Fix some bug(s) introduced by recently merged commit(s) label Jul 9, 2024
@glassez glassez requested a review from a team July 9, 2024 13:26
@stalkerok
Copy link
Contributor

Yeah, it works.
2024-07-09_180904 2024-07-09_180927

@thalieht
Copy link
Contributor

thalieht commented Jul 9, 2024

Maybe unrelated but i may as well mention it here (not caused by this PR):

Assertion when adding a magnet link from WebUI:

7  (anonymous namespace)::processMap                    synccontroller.cpp     229 0x5555560cc391 
8  SyncController::generateMaindataSyncData             synccontroller.cpp     632 0x5555560cfb0c 
9  SyncController::maindataAction                       synccontroller.cpp     507 0x5555560cdf85 
10 SyncController::qt_static_metacall                   moc_synccontroller.cpp 103 0x55555609d1cd 
13 QMetaObject::invokeMethod<void>                      qobjectdefs.h          376 0x55555612af11 
14 QMetaObject::invokeMethod<>(QObject *, const char *) qobjectdefs.h          413 0x55555612ad2c 
15 APIController::run                                   apicontroller.cpp      59  0x55555612a1d9 
16 WebApplication::doProcessRequest                     webapplication.cpp     358 0x55555611222f 
17 WebApplication::processRequest                       webapplication.cpp     628 0x5555561151fd 
18 Http::Connection::read                               connection.cpp         149 0x555555de7bbf 
19 operator()                                           connection.cpp         58  0x555555de6df6 

@xavier2k6
Copy link
Member

xavier2k6 commented Jul 9, 2024

Should it not display like below?? (like it would if you put those values in excel sheet for example)

ascending order:
N/A
NO
YES

descending order:
YES
NO
N/A

@ManiMatter

This comment was marked as off-topic.

@stalkerok
Copy link
Contributor

On both images, there are several entries that have a "Yes" for privateness and are of status "Stalled", but they are not next to one another. Is that expected?

This can be changed by double sorting.
2024-07-09_195725

@glassez
Copy link
Member Author

glassez commented Jul 9, 2024

Should it not display like below?? (like it would if you put those values in excel sheet for example)

I suppose Excel just sorts it alphabetically while we logically.

@xavier2k6
Copy link
Member

I suppose Excel just sorts it alphabetically while we logically.

I'd prefer alphabetical sorting, but that's just me...will not be a blocker.

src/gui/transferlistsortmodel.cpp Outdated Show resolved Hide resolved
src/gui/transferlistsortmodel.cpp Outdated Show resolved Hide resolved
@glassez
Copy link
Member Author

glassez commented Jul 10, 2024

Maybe unrelated but i may as well mention it here (not caused by this PR):

Assertion when adding a magnet link from WebUI:

Should be fixed now. Could you confirm?

@thalieht
Copy link
Contributor

Maybe unrelated but i may as well mention it here (not caused by this PR):
Assertion when adding a magnet link from WebUI:

Should be fixed now. Could you confirm?

Fixed 👍

@glassez glassez requested a review from Chocobo1 July 11, 2024 13:38
@glassez
Copy link
Member Author

glassez commented Jul 12, 2024

@qbittorrent/bug-handlers, could someone approve it?

Chocobo1
Chocobo1 previously approved these changes Jul 13, 2024
return threeWayCompare(left.toBool(), right.toBool());
if (!leftValid && !rightValid)
return 0;
return rightValid ? -1 : 1;
Copy link
Member

@Chocobo1 Chocobo1 Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking. Usually it is return leftValid ? -1 : 1; and here is special behavior, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here is special behavior, right?

Done by mistake. Fixed.

@glassez glassez merged commit 7f4cb43 into qbittorrent:master Jul 15, 2024
14 checks passed
@glassez glassez deleted the sort-private branch July 15, 2024 05:42
glassez added a commit to sledgehammer999/qBittorrent that referenced this pull request Jul 15, 2024
glassez added a commit to sledgehammer999/qBittorrent that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hotfix Fix some bug(s) introduced by recently merged commit(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants