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

WebUI: Add support for tracker status (error, warning) filter #22166

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

scratchmex
Copy link

@scratchmex scratchmex commented Jan 14, 2025

implements #19672 in webui

behavior of categories: for context, we consider a tracker in error state whenever it is in either NotWorking, TrackerError or Unreachable. thus

  • error: if all trackers of a torrent are in error state
  • warning: if NOT all trackers ^^

I think the "other error" category from the GUI could be renamed to "Network error" and replace "error" with "Tracker error". The later will be whenever the tracker responds with specific message extracted from libtorrent

open to suggestions :D

TODO:

  • translations
image

will close #21960, close #21796, close #20276, close #20275, close #9849

@glassez
Copy link
Member

glassez commented Jan 15, 2025

I didn't look at any details, but what caught my eye at first glance were the changes to the core classes, which clearly looks unexpected if you only need WebUI to follow regular UI.

@scratchmex
Copy link
Author

I could have done it everything in serialize_torrent.cpp but the pattern there is to call Tracker methods so I followed it. It also has the benefit that we can reuse it in trackersfilterwidget.cpp in the GUI layer, which now has the logic directly in it and cannot be reused from the webui layer. What do you think?

@glassez
Copy link
Member

glassez commented Jan 15, 2025

  • error: if all trackers of a torrent are in error state
  • warning: if NOT all trackers ^^

This is not at all how it behaves in GUI. The "error" filter matches a torrent if at least one tracker entry is in the appropriate "error" state.
"Warning" has nothing to do with errors at all. It matches the torrent if one of its (working) trackers sent the warning message.

@scratchmex
Copy link
Author

ok will change the logic to be that way, good to have it clarified. about modifying core classes, do you have a suggestion about it or is it fine? and in general, any other change suggestion?

@Chocobo1 Chocobo1 added the WebUI WebUI-related issues/changes label Jan 19, 2025
@scratchmex
Copy link
Author

I changed my approach by using the serialize torrent handlers and avoid touching base folder.

@Chocobo1 I tag you because I saw you made some enhancements in the areas I touch. Will appreciate your feedback :D.

@scratchmex
Copy link
Author

scratchmex commented Jan 22, 2025

update on UI look

image

@glassez glassez changed the title add support for tracker status (error, warning) filter in webui WebUI: Add support for tracker status (error, warning) filter Jan 22, 2025
@glassez
Copy link
Member

glassez commented Jan 22, 2025

IIRC, currently WebUI trackers filter uses data provided by "trackers" field of sync/maindata result. So why not enhance it to return state-based items in the same way as regular tracker items?

@scratchmex
Copy link
Author

scratchmex commented Jan 22, 2025

I tried that option but found out it's simpler to calculate the final state on the backend where all the info is already available and expose it as new field tracker_status in sync/maindata than instead, exposing the literal tracker status of each of them, along with their status message and later in the frontend have the logic to calculate the possible filter status

I have the approach you suggested saved there in my stash, that's how I first approached and later found out it was too much info handling

tl;dr: exposing all that info will be nice but for this simple use case I don't think it's worth it

@glassez
Copy link
Member

glassez commented Jan 22, 2025

it's simpler to calculate the final state on the backend where all the info is already available

It is exactly how my suggestion based on "trackers" from sync/maindata supposed to do. It provides data as torrent IDs by tracker URLs. So you need only add extra items with state identifier instead of tracker URL. E.g.:

{
    "https://www.example1.com" : ["torrent1", "torrent2", "torrent3"],
    "https://www.example2.com" : ["torrent1", "torrent2", "torrent3"],
    "@warning" : ["torrent1", "torrent2", "torrent3"],
    "@error" : ["torrent1", "torrent2", "torrent3"]
}

@scratchmex
Copy link
Author

scratchmex commented Jan 22, 2025

Oh I see your point now. So something like this in synccontroller.cpp?

    if (!m_maindataSyncBuf.trackers.isEmpty())
    {
        QJsonObject trackers;
        for (auto it = m_maindataSyncBuf.trackers.cbegin(); it != m_maindataSyncBuf.trackers.cend(); ++it)
            trackers[it.key()] = QJsonArray::fromStringList(it.value());
        trackers["@warning"] = hasWarningIds;
        trackers["@error"] = hasTrackerErrorIds;
        trackers["@other_error"] = hasErrorIds;
        syncData[KEY_TRACKERS] = trackers;
    }

Curious what will be the benefit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment