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

Web: add "Never" and "Mixed" values for "Last Activity" #1390

Closed
wants to merge 1 commit into from

Conversation

Rafostar
Copy link

@Rafostar Rafostar commented Aug 7, 2020

In web interface when data for certain torrent was never received, it last activity shows the number of days since 01.01.1970, which is not really useful. This PR changes this to display text "Never" instead. Also last activity field did not show a "Mixed" value (like other fields) when selecting multiple torrents. This is now fixed too.

Fixes #658

@Rafostar
Copy link
Author

Rafostar commented Aug 7, 2020

Build (Sanity) failure seems unrelated to my PR:

[Step 3/4] 17/23 Test #17: session-test .....................***Failed    0.72 sec
[Step 3/4] [2020-08-07 14:43:03.020] Unable to create session lock file (89): Operation not applicable
[Step 3/4] [2020-08-07 14:43:03.021] Unable to open session lock file (89): Operation not applicable
[Step 3/4] FAIL /export/home/user/buildAgent/work/58721beadbc79d02/libtransmission/session-test.c:67: tr_session_id_is_local(session_id_str_1) (false)

latest = -1;
if (torrents.length < 1) {
str = none;
} else {
baseline = torrents[0].getLastActivity();
for (i = 0; t = torrents[i]; ++i) {
d = t.getLastActivity();
if (latest < d) {
if (baseline != d) {
str = mixed;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure "mixed" should implemented here. If multiple torrents are selected displaying the latest activity looks reasonable. It provide some useful and meaningful information anyway.

Copy link
Author

Choose a reason for hiding this comment

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

I wanted to make it more consistent with other fields that way. For example when you select multiple torrents the "Running Time" you get is a "mixed" instead of the one than run the longest, so I decided to do the same for "Last Activity". Wouldn't selecting a "never" active torrent with another one that was active some time ago and getting reported activity time be more misleading?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about next scenarios. Suppose I have a few douzens of stalled torrents. I can simply select all of them and see what is the latest time they were active.

Ok. It is just a two ways to look on the feature.
Just commented. Descision is yours. :)

@ckerr
Copy link
Member

ckerr commented Oct 18, 2021

Is any of this PR still valid after #1476?

@ckerr ckerr added type:feat A new feature scope:web needs clarification More info is needed before work can proceed labels Oct 18, 2021
@pmarsden
Copy link

Only just noticed this, I wrote my own code to do this 2+ years ago.
It displays 'Never' on torrents that have had no activity, or 'Mixed' if the selected torrents have different values.
Displaying an activity value when you have multiple torrents selected really isnt very useful, since you have no idea which torrent its referring to.

@ckerr
Copy link
Member

ckerr commented Feb 13, 2022

I think this is fixed by #1476.

The current code reads like this:

    // last active at
    if (torrents.length === 0) {
      string = none;
    } else {
      const latest = torrents.reduce(
        (accumulator, t) => Math.max(accumulator, t.getLastActivity()),
        -1
      );
      const now_seconds = Math.floor(now / 1000);
      if (0 < latest && latest <= now_seconds) {
        const idle_secs = now_seconds - latest;
        string =
          idle_secs < 5 ? 'Active now' : `${fmt.timeInterval(idle_secs)} ago`;
      } else {
        string = none;
      }
    }
    setTextContent(e.info.last_activity, string);

@ckerr ckerr closed this Feb 13, 2022
@pmarsden
Copy link

So what does that "fix" do ?
It doesnt look like is displays "Mixed" for multiple selected torrents.

@FluxState
Copy link
Contributor

FluxState commented Feb 15, 2022

@pmarsden It displays the time delta (from now) of the latest active torrent in the selected group. It could be written in this way:

function getTorrentsLastActivity(torrents) {
  const torrentsLastActivities = [];
  for (const torrent of torrents) {
    // Get all last activities from selected torrents.
    const lastActivity = torrent.getLastActivity();
    if (lastActivity > 0) {
      torrentsLastActivities.append(lastActivity);
    }
  }

  // Get latest of them all.
  const lastActivityMax = max(torrentsLastActivities);

  if (isNil(lastActivityMax)) {
    // No last activity data. This should not happen!
    return none;
  }
  
  const now = DateTime.now();
  const latestActivity = DateTime.fromSeconds(lastActivityMax);
  
  if (latestActivity > now) {
    // Latest activity is in the future. This should not happen!
    return none;
  }
  
  const secondsSinceLastActivity = now.diff(latestActivity, 'seconds').toObject().seconds;
  if (secondsSinceLastActivity < 5) {
    // Internationalized 'Active now'.
    return i18n.t('torrent.active_now');
  }
  
  // Humanized string.
  return DateTime.fromSeconds(latestActivity).toRelative();
}

if (isEmpty(torrents)) {
  string = none;
} else {
  string = getTorrentsLastActivity(torrents);
}
setTextContent(e.info.last_activity, string);

@pmarsden
Copy link

So it does not fix the issue then.
It should be displaying "Mixed" if multiple torrents are selected (with different values).
Displaying a time is completely meaningless (useless) since you have no idea which torrent that time refers to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs clarification More info is needed before work can proceed scope:web type:feat A new feature
Development

Successfully merging this pull request may close these issues.

"Last activity" field in web interface
5 participants