Skip to content

Show last-active time on profile page #1793

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Aug 7, 2025

Fixes #1647.

Draft because I want to write some tests before merging; my manual testing hasn't reached all the different cases in this logic.

gnprice added 4 commits August 6, 2025 17:51
At 4 items high, this is already a bit of a stack of information.
We'll soon add a 5th item, so it'll be good to have it split up.

A logical grouping is to separate the things that are potentially
timely (the name adorned with presence circle and status emoji,
then the status message) from the things that change rarely (the
user's email and role).
Also update the TODO-comment for them to point to the issue.
@gnprice gnprice requested a review from chrisbobbe August 7, 2025 00:54
@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Aug 7, 2025
@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Aug 7, 2025

Would this be a good opportunity to implement refreshing relative times? As for #891; and I guess #293 is also for refreshing a timestamp on the profile page but not a relative one :)

Like maybe we should make a helper widget or a ChangeNotifier or something.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Aug 7, 2025

Are we meant to show this for bot users? I'm not seeing this text on GitHub Bot on CZO on web, but I'm not sure if that's because it's a bot or if it just hasn't ever reported presence.

@gnprice gnprice changed the title Pr last active Show last-active time on profile page Aug 8, 2025
@gnprice
Copy link
Member Author

gnprice commented Aug 8, 2025

This is probably a good prompt to implement refreshing relative times. I think that can be a follow-up, though.

@gnprice
Copy link
Member Author

gnprice commented Aug 8, 2025

Hmm, good question about bot users.

When a user hasn't ever reported presence, that doesn't cause not showing this text. Instead it produces the text "Not active in the last year"; or on web there's also the wrinkle about User.dateJoined described in this comment added in this branch:

  int? userLastActive(int userId) {
    // The corresponding implementation on web is complicated;
    // but the actual behavior seems to be this simple.
    //
    // In web, see buddy_data.user_last_seen_time_status; the last-active time
    // is used only when the status is offline (vs active or idle).
    // The timestamp comes via presence.last_active_date from the data structure
    // fed by presence.status_from_raw.
    //
    // That status_from_raw function sometimes uses idle_timestamp;
    // but only when status idle, where the timestamp will be ignored anyway.
    // It also consults the equivalent of [User.dateJoined] as a fallback,
    // for when processing a user who has "never logged in"... but it's
    // not clear that function ever gets called in such a case.
    // Those wrinkles aside, it always uses active_timestamp.

    return _map[userId]?.activeTimestamp;

So there's probably logic to specifically exclude bots, and we should probably do the same. Seems pretty logical product-wise.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Aug 8, 2025

Thanks, and makes sense! Looks basically fine; I'll plan to take a closer look once you've added screenshots and tests.

Suggestions for things you could also add, or add TODOs for (but I wouldn't block on them):

  • TODO for refreshing relative times, if this will be a followup
  • Tooltip that always offers the full date and time with seconds, in case more precision is desired (also timezone?)
  • When the time appears to be in the future, show the full date/time instead of "now", in case this helps the user
    • detect and debug a wrongly set clock, or
    • (if they've done some of that already) to correctly interpret the data, or at least not be misled when the real time was long ago (not "now")
  • TODOs for advantages of an approach with ffi (see lightbox: Use a friendlier format for the date, like "Yesterday at 4:47 PM" #45):
    • Maybe the ICU library comes with built-in logic to choose "minutes ago" vs. "hours ago" etc., or has a crisp API to say which one we want? (Maybe we want cases for "seconds" or "weeks" or "months" ago too?)
    • Sourcing some localizations from CLDR instead of volunteer translators, for consistency/accuracy:
      • Plurals: "21 minutes ago", "1 hour ago", or n seconds/weeks/months ago if we want those cases too. (Plurals can be hard for translators)
      • Oho, and I wonder (haven't checked) if it localizes the digits, so e.g. in Persian you could get "۲۱ دقیقه پیش"
      • The date/time separator (e.g. "at" in English) for the few date-plus-time renderings I've mentioned here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

profile: Show when the user was last active
2 participants