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.

Sign up for GitHub

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

Jump to bottom

WebUI: Add log viewer #18290

Merged
merged 3 commits into from
Jan 16, 2023
Merged

WebUI: Add log viewer #18290

merged 3 commits into from
Jan 16, 2023

Conversation

brvphoenix
Copy link
Contributor

@brvphoenix brvphoenix commented Dec 25, 2022 •
edited
Loading

Supersede #18189.

@ttys3

qbt1

@brvphoenix brvphoenix force-pushed the new_log_view branch 2 times, most recently from f049a89 to ee95b0d Compare December 25, 2022 16:02
@thalieht thalieht added the WebUI WebUI-related issues/changes label Dec 25, 2022
@brvphoenix brvphoenix changed the title feat: add logs viewer for webui WebUI: add log viewer Dec 25, 2022
@glassez glassez requested review from a team December 25, 2022 18:49
@ttys3 ttys3 mentioned this pull request Dec 26, 2022
Closed
@thalieht
Copy link
Contributor

The only problem i found (or maybe personal preference) was the annoying stretching of columns when trying to resize them (i'm just a tester). Also i'm wondering about the purpose of the Blocked column in Blocked IP's. Aren't all entries there always blocked?

@brvphoenix
Copy link
Contributor Author

The only problem i found (or maybe personal preference) was the annoying stretching of columns when trying to resize them (i'm just a tester).

Do you want something like this?
qbt1

Also i'm wondering about the purpose of the Blocked column in Blocked IP's. Aren't all entries there always blocked?

Not always.

qBittorrent/src/base/bittorrent/sessionimpl.cpp

Line 5586 in b33dc7d

Logger::instance()->addPeer(ip, true, reason);

qBittorrent/src/base/bittorrent/sessionimpl.cpp

Line 5593 in b33dc7d

Logger::instance()->addPeer(ip, false);

@thalieht
Copy link
Contributor

Do you want something like this?

Absolutely.

Also i'm wondering about the purpose of the Blocked column in Blocked IP's. Aren't all entries there always blocked?

Not always.

Thanks but looking at the code the peer is still not "not blocked" it is banned instead so writing in that column "blocked false" is misleading.

@brvphoenix brvphoenix force-pushed the new_log_view branch 2 times, most recently from f335422 to 39f7068 Compare December 28, 2022 08:31
@brvphoenix
Copy link
Contributor Author

Thanks but looking at the code the peer is still not "not blocked" it is banned instead so writing in that column "blocked false" is misleading.

Change Blocked (true/false) to Status (blocked/banned).

@Lagicrus
Copy link

Hey, loving this concept, but I wonder if two tweaks could be made (sorry if these are out of scope)

  • Text for "clearAll" feels like it should be "Clear All" (unless the gif is out of date)
  • Having a link to/tooltip/etc for how to use the filter? At a glance, I have 0 idea where your "-peer" option came from, for example, having an easy to access way to see what you can use it for beyond just "text" would be nice. But I guess that is going into it a bit

@brvphoenix
Copy link
Contributor Author

  • Having a link to/tooltip/etc for how to use the filter? At a glance, I have 0 idea where your "-peer" option came from, for example, having an easy to access way to see what you can use it for beyond just "text" would be nice. But I guess that is going into it a bit

Perhaps, you should create a separate issue for help because this feature is not introduced by this pr.

@brvphoenix brvphoenix force-pushed the new_log_view branch 2 times, most recently from 08b460b to ab7512a Compare December 28, 2022 15:04
@thalieht
Copy link
Contributor

Only one problem remains: When logging in the WebUI and changing to Execution Log's Blocked IP's, the list isn't fetched immediately like the log.

@brvphoenix brvphoenix force-pushed the new_log_view branch 2 times, most recently from 58087ba to 4d28a04 Compare December 31, 2022 14:03
@brvphoenix
Copy link
Contributor Author

Only one problem remains: When logging in the WebUI and changing to Execution Log's Blocked IP's, the list isn't fetched immediately like the log.

Should be fixed now.

thalieht
thalieht previously approved these changes Dec 31, 2022
View reviewed changes
Copy link
Contributor

@thalieht thalieht left a comment

Choose a reason for hiding this comment

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

LGTM (as a tester)

@brvphoenix brvphoenix requested a review from thalieht December 31, 2022 15:41
@brvphoenix
Copy link
Contributor Author

LGTM (as a tester)

Thanks. Remove some redundant codes in new push.

thalieht
thalieht previously approved these changes Dec 31, 2022
View reviewed changes
@glassez glassez added this to the 4.6.0 milestone Jan 3, 2023
@sledgehammer999
Copy link
Member

Please edit the commit message. It should indicate that it includes code copied from elsewhere and its license. A short mention should suffice.

@sledgehammer999
Copy link
Member

Please edit the commit message. It should indicate that it includes code copied from elsewhere and its license. A short mention should suffice.

Also an appropriate entry in the AUTHORS file should be made too.

@brvphoenix
Copy link
Contributor Author

Please edit the commit message. It should indicate that it includes code copied from elsewhere and its license. A short mention should suffice.

Also an appropriate entry in the AUTHORS file should be made too.

Added.

@ttys3 @brvphoenix
WebUI: add log viewer
f041107 
The javascript implementation of multi-select menu is from the source
https://github.com/PhilippeMarcMeyer/vanillaSelectBox. It is licensed
under the MIT License. Some minor fixes is made to pass the lint.

Co-authored-by: brvphoenix <30111323+brvphoenix@users.noreply.github.com>
Chocobo1
Chocobo1 reviewed Jan 4, 2023
View reviewed changes
AUTHORS Outdated Show resolved Hide resolved
@brvphoenix brvphoenix force-pushed the new_log_view branch 2 times, most recently from 5725a95 to 2270d87 Compare January 4, 2023 12:13
@fengqi
Copy link

fengqi commented Jan 5, 2023

Hi, are you consider increasing the tracker announce logs

@glassez glassez requested review from a team January 14, 2023 09:34
@glassez glassez changed the title WebUI: add log viewer WebUI: Add log viewer Jan 16, 2023
@glassez glassez merged commit 0d376e7 into qbittorrent:master Jan 16, 2023
@glassez
Copy link
Member

glassez commented Jan 16, 2023

@brvphoenix, @ttys3
Thank you.

@brvphoenix brvphoenix deleted the new_log_view branch January 31, 2023 14:26
@thalieht thalieht mentioned this pull request Sep 24, 2023
Open
61 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Chocobo1 Chocobo1 Chocobo1 left review comments

@glassez glassez glassez approved these changes

@thalieht thalieht thalieht approved these changes

@xavier2k6 xavier2k6 xavier2k6 approved these changes

Assignees
No one assigned
Labels
WebUI WebUI-related issues/changes
Projects
None yet
Milestone
4.6.0
Development

Successfully merging this pull request may close these issues.
9 participants
@brvphoenix @thalieht @Lagicrus @sledgehammer999 @fengqi @glassez @Chocobo1 @xavier2k6 @ttys3