From 117815ef4db6adc3967a7d10567d6556d670a103 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 21 Aug 2026 19:34:11 +0200 Subject: [PATCH] feat(user): show if user is online --- frontend/src/lib/api/endpoints/users.ts | 21 +++++-- .../src/lib/components/UserVignette.svelte | 38 +++++++++++ src/application/dtos/user_dto.rs | 34 +++++++--- .../services/auth_application_service.rs | 63 ++++++++++++------- 4 files changed, 122 insertions(+), 34 deletions(-) diff --git a/frontend/src/lib/api/endpoints/users.ts b/frontend/src/lib/api/endpoints/users.ts index b82727e0..06e71b28 100644 --- a/frontend/src/lib/api/endpoints/users.ts +++ b/frontend/src/lib/api/endpoints/users.ts @@ -16,15 +16,23 @@ export interface ResolvedUser { email: string; image: string | null; isExternal: boolean; + /** Presence — TRUE when the server observed a request on any of this + * user's non-revoked sessions within the last 5 min (backend + * `PublicUserDto.is_online`). Drives the presence dot overlay on + * `` / ``. `false` when the caller's + * source didn't compute presence (a bare `resolveUser(id)` from + * pre-3-layer callers, an older backend build) — dot stays dark. */ + isOnline: boolean; } -/** Subset of the backend `UserDto` we consume here. */ -interface UserDtoShape { +/** Subset of the backend `PublicUserDto` we consume here. */ +interface PublicUserShape { id: string; username?: string | null; email?: string | null; image?: string | null; is_external: boolean; + is_online?: boolean; } // id → in-flight/resolved lookup (the Promise is cached so concurrent callers @@ -41,13 +49,14 @@ export function resolveUser(id: string): Promise { credentials: 'same-origin' }); if (!res.ok) return null; - const u = (await res.json()) as UserDtoShape; + const u = (await res.json()) as PublicUserShape; return { id: u.id, name: u.username?.trim() || u.email || u.id, email: u.email ?? '', image: u.image ?? null, - isExternal: u.is_external + isExternal: u.is_external, + isOnline: u.is_online ?? false }; } catch { return null; @@ -78,6 +87,7 @@ export function seedUser(u: { email: string; image?: string | null; is_external: boolean; + is_online?: boolean; }): void { if (cache.has(u.id)) return; const resolved: ResolvedUser = { @@ -85,7 +95,8 @@ export function seedUser(u: { name: u.username?.trim() || u.email || u.id, email: u.email, image: u.image ?? null, - isExternal: u.is_external + isExternal: u.is_external, + isOnline: u.is_online ?? false }; cache.set(u.id, Promise.resolve(resolved)); } diff --git a/frontend/src/lib/components/UserVignette.svelte b/frontend/src/lib/components/UserVignette.svelte index a1c0fe12..57cd999e 100644 --- a/frontend/src/lib/components/UserVignette.svelte +++ b/frontend/src/lib/components/UserVignette.svelte @@ -33,6 +33,7 @@ const label = $derived(resolved?.name ?? fallbackLabel ?? userId); const email = $derived(resolved?.email || fallbackSublabel || ''); const isExternal = $derived(resolved?.isExternal ?? false); + const isOnline = $derived(resolved?.isOnline ?? false); const image = $derived(resolved?.image ?? null); const colorIndex = $derived(avatarColorIndex(userId)); const initials = $derived(userInitials(label)); @@ -50,6 +51,22 @@ {/if} + {#if isOnline} + + + {/if} {label} @@ -128,6 +145,27 @@ font-size: 9px; } + /* Presence dot — top-right, symmetric with `.uv__badge` at + bottom-right so the two corners don't collide. Slightly smaller + (10x10 vs the badge's 16x16) because it's a pure signal — no + icon, no text. The 2px `--color-bg-surface` border creates a + visual gap between dot and avatar so the green pops out cleanly + regardless of avatar palette (photo, dark initials, light + initials). `box-sizing: border-box` keeps the inner circle's + green footprint at 6x6 — same visual weight the sessions-table + dot has. See `docs/plan/sessions.md` § UI. */ + .uv__presence { + position: absolute; + right: -2px; + top: -2px; + width: 10px; + height: 10px; + border-radius: 50%; + background: var(--color-success-alt); + border: 2px solid var(--color-bg-surface); + box-sizing: border-box; + } + .uv__text { display: flex; flex-direction: column; diff --git a/src/application/dtos/user_dto.rs b/src/application/dtos/user_dto.rs index 8742b1de..6822535d 100644 --- a/src/application/dtos/user_dto.rs +++ b/src/application/dtos/user_dto.rs @@ -161,8 +161,32 @@ pub struct SelfUserDto { pub can_edit_image: bool, } -impl From for PublicUserDto { - fn from(user: User) -> Self { +impl PublicUserDto { + /// Construct a `PublicUserDto` from a `User` entity + an explicit + /// `is_online` signal. + /// + /// **Why not `From`?** The `User` entity models a row in + /// `auth.users`; `is_online` is a cross-table lookup on + /// `auth.sessions` (see the EXISTS subquery in + /// `list_users_with_derived_flags` and `get_user_with_derived_flags` + /// on the user repo). A `From` impl couldn't compute it + /// honestly — it would have to ship a `false` default that lies to + /// the FE presence dot on every emitter that didn't remember to + /// override. Making presence a required constructor argument + /// removes that footgun: every callsite has to declare its intent. + /// + /// Two shapes at the callsite: + /// + /// - Presence matters (single-user `/api/users/{id}`, list + /// projections, self-view): pair with + /// `user_storage.get_user_with_derived_flags(id)` and pass + /// `flags.is_online`. + /// - Presence is out of scope (register / update-profile response, + /// post-mutation echo where the FE ignores the field): pass + /// `false` with a short comment explaining why. The receiver's + /// presence read is a no-op — no dot lights up on the stale + /// value. + pub fn new(user: User, is_online: bool) -> Self { let role = format!("{}", user.role()); let p = user.into_parts(); Self { @@ -174,11 +198,7 @@ impl From for PublicUserDto { is_external: p.is_external, given_name: p.given_name, family_name: p.family_name, - // Single-user paths that don't enrich presence ship `false`. - // List projections (admin users, sharees enriched with - // presence) build via FullUserDto::build below, which - // overrides this from UserDerivedFlags. - is_online: false, + is_online, } } } diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index fc28c146..5b2cd930 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -876,9 +876,7 @@ impl AuthApplicationService { is_external = false, "🛂 user registered", ); - Ok(RegisterResult::Created(Box::new(PublicUserDto::from( - created_user, - )))) + Ok(RegisterResult::Created(Box::new(PublicUserDto::new(created_user, false)))) } /// Create the first admin user during initial system setup. @@ -981,7 +979,7 @@ impl AuthApplicationService { username, created_user.id() ); - Ok(PublicUserDto::from(created_user)) + Ok(PublicUserDto::new(created_user, false)) } pub async fn login( @@ -2182,7 +2180,7 @@ impl AuthApplicationService { lc.dispatch_upgraded_to_internal(&updated).await; } - Ok(PublicUserDto::from(updated)) + Ok(PublicUserDto::new(updated, false)) } /// Admin-driven external → internal promotion. @@ -2293,7 +2291,7 @@ impl AuthApplicationService { "👮🏻‍♂️ external user promoted to internal by admin", ); - Ok(PublicUserDto::from(updated)) + Ok(PublicUserDto::new(updated, false)) } /// `keep_session_id` — when `Some`, revoke every OTHER session for @@ -2550,7 +2548,7 @@ impl AuthApplicationService { pub async fn get_user(&self, user_id: Uuid) -> Result { let user = self.user_storage.get_user_by_id(user_id).await?; - Ok(PublicUserDto::from(user)) + Ok(PublicUserDto::new(user, false)) } /// Cached, image-free lookup of the caller's authorization flags @@ -2875,7 +2873,7 @@ impl AuthApplicationService { if changed.is_empty() && ui_prefs_patch.is_none() { // No-op — return the current user without a DB write. - return Ok(PublicUserDto::from(user)); + return Ok(PublicUserDto::new(user, false)); } // Persist the typed-field changes first (if any). Skip the @@ -2906,7 +2904,7 @@ impl AuthApplicationService { // Refetch so the returned DTO reflects the merged JSONB bag // (the in-memory `user` above holds the pre-merge value). let refreshed = self.user_storage.get_user_by_id(caller_id).await?; - Ok(PublicUserDto::from(refreshed)) + Ok(PublicUserDto::new(refreshed, false)) } // Alias for consistency with handler method @@ -3006,9 +3004,20 @@ impl AuthApplicationService { ) -> Result { // (1) Self — a single fetch suffices (the check compares the input // UUIDs, so the target read is never needed on this path). + // + // Both branches use `get_user_with_derived_flags` (not the narrow + // `get_user_by_id`) so `PublicUserDto.is_online` on the wire + // reflects the same EXISTS subquery the admin list uses. Without + // this the FE presence dot would only light up on list-derived + // paths (admin seed); single fetches from share pickers / group + // members would show every user as offline regardless of real + // state. See `docs/plan/userdto-refactor.md`. if caller_id == target_id { - let caller = self.user_storage.get_user_by_id(caller_id).await?; - return Ok(PublicUserDto::from(caller)); + let (caller, flags) = self + .user_storage + .get_user_with_derived_flags(caller_id) + .await?; + return Ok(PublicUserDto::new(caller, flags.is_online)); } // Caller and target are independent point reads (the self-case already @@ -3016,16 +3025,26 @@ impl AuthApplicationService { // overlap them with `join!` instead of two serial round-trips. // `caller_res?` first preserves the caller-error precedence of the old // sequential form. (benches/ROUND23.md §P1) + // + // `caller` uses the narrow `get_user_by_id` because we only read + // `is_external()` off it for the visibility gate; nothing about + // the caller ships on the wire. Only `target` needs the wider + // projection. let (caller_res, target_res) = tokio::join!( self.user_storage.get_user_by_id(caller_id), - self.user_storage.get_user_by_id(target_id) + self.user_storage.get_user_with_derived_flags(target_id) ); let caller = caller_res?; // Anti-enumeration: NotFound for everything that doesn't pass. // Convert a real NotFound on `target` to the same anonymous 404, // so existence isn't leaked through differential responses. - let target = match target_res { + // + // Destructure the (User, UserDerivedFlags) tuple immediately so + // `target` keeps its historical `User` shape (accessors still + // work below); the flags come along as `target_flags` for the + // `is_online` propagation into the returned `PublicUserDto`. + let (target, target_flags) = match target_res { Ok(u) => u, Err(e) if e.kind == ErrorKind::NotFound => { tracing::info!( @@ -3069,7 +3088,7 @@ impl AuthApplicationService { })?; if related.is_some() { - return Ok(PublicUserDto::from(target)); + return Ok(PublicUserDto::new(target, target_flags.is_online)); } // (3) External callers stop here — no directory enumeration. @@ -3097,12 +3116,12 @@ impl AuthApplicationService { // (4) Internal target + system-address-book exposed: already public. if !target.is_external() && expose_system_users { - return Ok(PublicUserDto::from(target)); + return Ok(PublicUserDto::new(target, target_flags.is_online)); } // (5) Admin caller: always visible. if caller.role() == UserRole::Admin { - return Ok(PublicUserDto::from(target)); + return Ok(PublicUserDto::new(target, target_flags.is_online)); } // (6) No relationship — anti-enumeration NotFound. @@ -3184,7 +3203,7 @@ impl AuthApplicationService { // New method to get user by username - needed for admin user handling pub async fn get_user_by_username(&self, username: &str) -> Result { let user = self.user_storage.get_user_by_username(username).await?; - Ok(PublicUserDto::from(user)) + Ok(PublicUserDto::new(user, false)) } // Method to count how many admin users exist in the system @@ -3208,7 +3227,7 @@ impl AuthApplicationService { offset: i64, ) -> Result, DomainError> { let users = self.user_storage.list_users(limit, offset, false).await?; - Ok(users.into_iter().map(PublicUserDto::from).collect()) + Ok(users.into_iter().map(|u| PublicUserDto::new(u, false)).collect()) } /// Admin-only: lists users including external (grant-only) recipients. @@ -3222,7 +3241,7 @@ impl AuthApplicationService { ) -> Result, DomainError> { self.require_admin_caller(authorization, caller_id).await?; let users = self.user_storage.list_users(limit, offset, true).await?; - Ok(users.into_iter().map(PublicUserDto::from).collect()) + Ok(users.into_iter().map(|u| PublicUserDto::new(u, false)).collect()) } /// Admin-only user listing. Returns `Vec` — same @@ -3277,7 +3296,7 @@ impl AuthApplicationService { limit: i64, ) -> Result, DomainError> { let users = self.user_storage.search_users(query, limit, false).await?; - Ok(users.into_iter().map(PublicUserDto::from).collect()) + Ok(users.into_iter().map(|u| PublicUserDto::new(u, false)).collect()) } /// Username-only search for the NC sharee autocomplete: identical @@ -3541,7 +3560,7 @@ impl AuthApplicationService { created.id(), created.is_external() ); - Ok(PublicUserDto::from(created)) + Ok(PublicUserDto::new(created, false)) } /// Admin-only: reset a user's password. @@ -3640,7 +3659,7 @@ impl AuthApplicationService { /// Get a single user by ID (for admin panel) pub async fn get_user_admin(&self, user_id: Uuid) -> Result { let user = self.user_storage.get_user_by_id(user_id).await?; - Ok(PublicUserDto::from(user)) + Ok(PublicUserDto::new(user, false)) } /// Delete a user by ID (admin only).