diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index 35141f55..0574596f 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -45,6 +45,17 @@ export default ts.config( 'no-undef': 'off' } }, + { + // Auto-generated AsyncAPI DTOs (Modelina output). Empty + // interfaces are legitimate for wire messages whose `data` + // field is intentionally a no-fields object (pure-poke events + // like `notification_received`). See + // `docs/plan/templated-messages.md § Bus event is a pure poke`. + files: ['src/lib/generated/message-bus/**/*.ts'], + rules: { + '@typescript-eslint/no-empty-object-type': 'off' + } + }, { // `static/` holds vendored, verbatim assets (the delta-upload worker and // the wasm-bindgen hash glue) — lint them as the upstream ships them. diff --git a/frontend/src/lib/api/types.ts b/frontend/src/lib/api/types.ts index d8526160..47c9ec64 100644 --- a/frontend/src/lib/api/types.ts +++ b/frontend/src/lib/api/types.ts @@ -971,3 +971,33 @@ export const NOTIFICATION_KIND = { JOB_COMPLETED_FOR_YOU: 'job_completed_for_you', STORAGE_QUOTA_THRESHOLD: 'storage_quota_threshold' } as const; + +/** + * Payload shape for `share_granted` notifications — hand-mirror of + * `crate::domain::entities::notification::SharegrantedPayload`. + * `#[derive(ToSchema)]` on the Rust struct makes it authoritative; + * this interface is a projection for FE type-narrowing until the + * codebase adopts `openapi-typescript` for the REST surface. + * + * See `docs/plan/templated-messages.md § Making payloads a real + * Rust struct` for the rationale — OpenAPI owns the payload; the + * bus event is a pure poke. + */ +export interface SharegrantedPayload { + granter_id: string; + resource_type: string; + resource_id: string; + resource_name?: string; + /** Storage path for `folder` / `file` kinds only. `undefined` + * for drive / calendar / address_book / playlist. */ + resource_path?: string; + /** FE-navigation hint for kinds whose `resource_id` isn't itself + * a folder id. Populated for `drive` (the drive's root folder + * id — the FE routes to `/files/{navigate_folder_id}` because + * drives don't have a browsable URL of their own). Absent for + * folder (uses `resource_id` directly), file (routes to + * `/shared-with-me?file=`), and non-browsable kinds. */ + navigate_folder_id?: string; + role: string; + expires_at?: string; +} diff --git a/frontend/src/lib/components/AppShell.svelte b/frontend/src/lib/components/AppShell.svelte index f40016ac..3bd9a15d 100644 --- a/frontend/src/lib/components/AppShell.svelte +++ b/frontend/src/lib/components/AppShell.svelte @@ -30,6 +30,7 @@ notifications as persistentNotifications, useNotifications } from '$lib/composables/useNotifications.svelte'; + import NotificationRow from '$lib/components/NotificationRow.svelte'; import { errorToast } from '$lib/utils/errors'; import { formatBytes } from '$lib/utils/format'; @@ -346,67 +347,6 @@ const totalUnread = $derived(ui.unread + persistentNotifications.unread); const totalUnreadBadge = $derived(totalUnread > 99 ? '99+' : String(totalUnread)); - /** Format the server-side `created_at` for a persistent row. */ - function formatPersistentTime(iso: string): string { - try { - return formatTime(new Date(iso).getTime()); - } catch { - return ''; - } - } - - /** Human summary for a persistent notification. Kind-specific - * wording lives here so the DTO stays payload-agnostic. */ - function persistentSummary(row: { kind: string; payload: Record }): string { - switch (row.kind) { - case 'share_granted': { - const role = String(row.payload.role ?? 'a role'); - const resType = String(row.payload.resource_type ?? 'resource'); - return t( - 'notifications.persistent.share_granted', - { role, resType }, - `You were granted ${role} on a ${resType}.` - ); - } - case 'new_login_from_new_device': - return t( - 'notifications.persistent.new_device_login', - 'A new device signed into your account.' - ); - case 'job_completed_for_you': { - const name = String(row.payload.name ?? row.payload.job_name ?? 'a job'); - return t('notifications.persistent.job_completed', { name }, `Job "${name}" finished.`); - } - case 'storage_quota_threshold': - return t( - 'notifications.persistent.quota_threshold', - 'You are approaching your storage quota.' - ); - default: - return t( - 'notifications.persistent.generic', - { kind: row.kind }, - `Notification (${row.kind}).` - ); - } - } - - /** Icon for a persistent row's kind. Falls back to a generic bell. */ - function persistentIcon(kind: string): string { - switch (kind) { - case 'share_granted': - return 'user-plus'; - case 'new_login_from_new_device': - return 'shield-alt'; - case 'job_completed_for_you': - return 'check-circle'; - case 'storage_quota_threshold': - return 'database'; - default: - return 'bell'; - } - } - function openMobileSearch() { searchActive = true; requestAnimationFrame(() => searchInputEl?.focus()); @@ -966,32 +906,7 @@ > {/if} {#each persistentNotifications.items as row (row.id)} -
void persistentNotifications.markRead(row.id)} - onkeydown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - void persistentNotifications.markRead(row.id); - } - }} - > - - - -
-
{persistentSummary(row)}
-
- {formatPersistentTime(row.created_at)} -
-
-
+ (notifOpen = false)} /> {/each} {/if} {/if} diff --git a/frontend/src/lib/components/NotificationRow.svelte b/frontend/src/lib/components/NotificationRow.svelte new file mode 100644 index 00000000..fe9da24e --- /dev/null +++ b/frontend/src/lib/components/NotificationRow.svelte @@ -0,0 +1,253 @@ + + + +
+ + + +
+
+ {#if row.kind === NOTIFICATION_KIND.SHARE_GRANTED} + {@const p = row.payload as unknown as SharegrantedPayload} + {@const href = resourceHref(row.payload)} + + + {' '}{t( + 'notifications.share_granted.verb', + 'shared' + )} + {' '} + {#if href} + + + + {p.resource_name ?? p.resource_type} + + {:else} + {p.resource_name ?? p.resource_type} + {/if} + + {' '}({p.role}) + {:else} + + + {t('notifications.persistent.generic', { kind: row.kind }, `Notification (${row.kind}).`)} + + {/if} +
+
{formatPersistentTime(row.created_at)}
+
+ +
+ + diff --git a/frontend/src/lib/composables/useNotifications.svelte.ts b/frontend/src/lib/composables/useNotifications.svelte.ts index 4a388e0a..dd7b2a68 100644 --- a/frontend/src/lib/composables/useNotifications.svelte.ts +++ b/frontend/src/lib/composables/useNotifications.svelte.ts @@ -40,6 +40,8 @@ import { messageBus } from '$lib/message-bus/client.svelte'; import { session } from '$lib/stores/session.svelte'; import { serverConfig } from '$lib/stores/serverConfig.svelte'; +import { ui } from '$lib/stores/ui.svelte'; +import { t } from '$lib/i18n/index.svelte'; import { deleteNotification as apiDelete, getUnreadCount, @@ -47,11 +49,54 @@ import { markAllNotificationsRead, markNotificationRead } from '$lib/api/endpoints/notifications'; -import type { Notification } from '$lib/api/types'; +import type { Notification, SharegrantedPayload } from '$lib/api/types'; +import { NOTIFICATION_KIND } from '$lib/api/types'; import log from 'loglevel'; const bellLog = log.getLogger('oxi:notifications'); +/** + * Plain-string summary of a persistent notification — used for the + * transient toast preview (fade-to-bell UX affordance) and for + * accessible labels where a component can't render rich content. + * + * Rich Svelte rendering lives in `NotificationRow.svelte`. This is + * the string-only equivalent for toasts and aria-labels. + */ +export function summaryFor(row: Notification): string { + switch (row.kind) { + case NOTIFICATION_KIND.SHARE_GRANTED: { + const p = row.payload as unknown as SharegrantedPayload; + const resource = p.resource_name ?? p.resource_type ?? 'a resource'; + return t( + 'notifications.persistent.share_granted', + { role: p.role ?? 'access', resource }, + `Someone shared "${resource}" with you (${p.role ?? 'access'}).` + ); + } + case NOTIFICATION_KIND.NEW_LOGIN_FROM_NEW_DEVICE: + return t( + 'notifications.persistent.new_device_login', + 'A new device signed into your account.' + ); + case NOTIFICATION_KIND.JOB_COMPLETED_FOR_YOU: { + const name = String((row.payload as { name?: unknown }).name ?? 'a job'); + return t('notifications.persistent.job_completed', { name }, `Job "${name}" finished.`); + } + case NOTIFICATION_KIND.STORAGE_QUOTA_THRESHOLD: + return t( + 'notifications.persistent.quota_threshold', + 'You are approaching your storage quota.' + ); + default: + return t( + 'notifications.persistent.generic', + { kind: row.kind }, + `Notification (${row.kind}).` + ); + } +} + /** * Merge `incoming` rows into `existing`, deduplicating on `id`. * Where an id appears in both, the incoming (fresh-from-server) @@ -125,6 +170,13 @@ class NotificationsStore { * Merges via `mergeById` so a concurrent WS push and reconnect * catch-up can't double-count a row that landed twice. * + * Fresh rows — rows the local set didn't have before the merge — + * each fire a transient toast via `ui.notify(..., record: false)` + * so the user gets a peripheral awareness cue that fades to the + * bell (which keeps the row in its persistent history). The + * bell's ring animation plays via `ui.ringBell()` so a single + * bump signals "something new is in there". + * * Silent no-op when the server returns 0 rows — we're already in * sync. Updates `#lastReceivedAt` to the newest of the merged set. */ @@ -144,10 +196,30 @@ class NotificationsStore { limit: 100 }); if (res.items.length > 0) { + // Snapshot the pre-merge id set so we can identify + // which rows are genuinely fresh vs already-known + // (an already-known row can come back on a delta + // fetch if its read_at flipped on another device). + const before = new Set(this.#items.map((n) => n.id)); this.#items = mergeById(this.#items, res.items); // Newest of merged set — take the first item's // created_at since the result is sorted DESC. this.#lastReceivedAt = res.items[0].created_at; + + // Toast preview for every fresh row. Pass + // `record: false` so it doesn't add a phantom + // transient entry to `ui.notifications` that would + // duplicate the persistent row already in + // `notifications.items` (the bell dropdown shows + // them side by side). `ui.ringBell()` bumps the + // bell-ring animation once for the batch. + const fresh = res.items.filter((r) => !before.has(r.id)); + if (fresh.length > 0) { + for (const row of fresh) { + ui.notify(summaryFor(row), 'info', 4000, false); + } + ui.ringBell(); + } } // unread_count is the authoritative live server count — // always update it even when the delta was empty (a row diff --git a/frontend/src/lib/generated/message-bus/NotificationReceivedData.ts b/frontend/src/lib/generated/message-bus/NotificationReceivedData.ts index 2a76e7a7..4ad05083 100644 --- a/frontend/src/lib/generated/message-bus/NotificationReceivedData.ts +++ b/frontend/src/lib/generated/message-bus/NotificationReceivedData.ts @@ -1,8 +1,4 @@ // AUTO-GENERATED — do not edit by hand. // Regenerate with `just asyncapi-ts`. -interface NotificationReceivedData { - created_at: string; - kind: string; - notification_id: string; -} +interface NotificationReceivedData {} export type { NotificationReceivedData as default }; diff --git a/frontend/src/routes/shared-with-me/+page.svelte b/frontend/src/routes/shared-with-me/+page.svelte index 4dbcd55d..78a8df77 100644 --- a/frontend/src/routes/shared-with-me/+page.svelte +++ b/frontend/src/routes/shared-with-me/+page.svelte @@ -4,7 +4,8 @@ import { primeContextPage } from '$lib/utils/listContext'; import { goto } from '$app/navigation'; import { resolve } from '$app/paths'; - import { onMount } from 'svelte'; + import { page } from '$app/state'; + import { onMount, untrack } from 'svelte'; import { addFavorite, dateBucket, @@ -131,6 +132,63 @@ if (viewerOpen) void fileViewer.load(); }); + // ── File-preview deep link (?file=) ────────────────────────────────── + // Notification bell's `share_granted` for a FILE resource links here with + // `?file=` because `/files/{uuid}` requires a folder id and the + // recipient may not have access to the file's parent folder. Every + // recipient of a `share_granted` sees the shared item in this list, so + // this route is the guaranteed-accessible home for the deep link. + // Mirrors the URL↔viewer pattern in `routes/files/[...path]/+page.svelte`. + // See `docs/plan/templated-messages.md § File notification routing`. + // + // URL → viewer. Runs on navigation, on Back/Forward, and once the initial + // listing arrives (the item may not be in `items` yet on cold deep link). + // `untrack` stops re-firing on viewer-state changes so a user-initiated + // close can't be re-opened here. + $effect(() => { + const fileId = page.url.searchParams.get('file'); + const currentItems = items; + untrack(() => { + if (!fileId) { + if (viewerOpen) viewerOpen = false; + return; + } + if (viewerOpen && viewerFile?.id === fileId) return; + const match = currentItems.find((it) => isFile(it) && it.id === fileId); + if (match && isFile(match)) { + viewerFile = match; + viewerOpen = true; + } + // If the file isn't in the current page yet, do nothing — + // the next `load()` completion re-fires this effect (items + // is `$derived`) and picks up the deep link when the item + // lands. A shared file the caller no longer has access to + // (revoked between notification and click) never lands and + // the viewer stays closed — same failure mode as any + // deferred-navigation dangling reference. + }); + }); + + // Viewer → URL: on close (X / Esc / backdrop), drop the `?file=` param. + // `replaceState` so closing doesn't add a history entry. Only act on a + // genuine open→closed transition — on cold deep link the viewer starts + // closed *with* the param while the listing is still loading, and + // stripping it there would race the URL→viewer effect above. + let viewerWasOpen = false; + $effect(() => { + const open = viewerOpen; + const hasParam = page.url.searchParams.get('file') !== null; + untrack(() => { + if (viewerWasOpen && !open && hasParam) { + const url = new URL(page.url); + url.searchParams.delete('file'); + // eslint-disable-next-line svelte/no-navigation-without-resolve + void goto(url, { keepFocus: true, noScroll: true, replaceState: true }); + } + viewerWasOpen = open; + }); + }); + function open(item: FileItem | FolderItem) { if (!isFile(item)) { goto(resolve(`/files/${item.id}`)); @@ -138,6 +196,18 @@ } viewerFile = item; viewerOpen = true; + // Reflect the open file in the URL — makes the view linkable + // (paste-into-slack a `/shared-with-me?file=` and it opens + // straight into the viewer), matches the pattern + // `/files/[...path]` uses, and lets the browser's Back button + // close the viewer. `pushState` (not `replaceState`) so Back + // pops the viewer off history instead of leaving the page. + const url = new URL(page.url); + if (url.searchParams.get('file') !== item.id) { + url.searchParams.set('file', item.id); + // eslint-disable-next-line svelte/no-navigation-without-resolve + void goto(url, { keepFocus: true, noScroll: true }); + } } /** diff --git a/src/application/ports/message_bus_ports.rs b/src/application/ports/message_bus_ports.rs index 84e5ab9e..370f95ea 100644 --- a/src/application/ports/message_bus_ports.rs +++ b/src/application/ports/message_bus_ports.rs @@ -308,23 +308,20 @@ pub enum MessageBusEvent { AuthzChanged { affected_folders: Vec }, /// A new notification was created for the caller — publishes on - /// [`Topic::UserNotifications`]. Payload is deliberately thin: the - /// FE learns "there's something new to look at" and calls - /// `GET /api/notifications` to load the row. Same recovery path a - /// missed push takes on next mount, so the wire event stays a - /// pure poke — no fields the bell needs to render on its own. + /// [`Topic::UserNotifications`]. **Pure cache-invalidation + /// event** — no fields on the wire. The topic itself signals + /// the semantic; the FE responds by refetching `GET + /// /api/notifications` (or a delta via `?after=`), and + /// the REST DTO carries every payload field. /// - /// `kind` is the notification's registered kind slug - /// (`share_granted`, `job_completed_for_you`, - /// `new_login_from_new_device`, `storage_quota_threshold`, …). - /// The FE may use it to route the toast (high-priority kinds pop - /// a toast; low-priority ones just bump the badge) but never - /// treats it as authoritative — the DB row is the truth. - NotificationReceived { - notification_id: Uuid, - kind: String, - created_at: chrono::DateTime, - }, + /// Serde emits `{"event":"notification_received","data":{}}`. + /// + /// Design principle: AsyncAPI defines the envelope + transport; + /// OpenAPI defines the payload. Keeping this event fieldless + /// enforces the split at its strongest — zero schema overlap + /// between the two specs for this kind. See + /// `docs/plan/templated-messages.md § Bus event is a pure poke`. + NotificationReceived, /// A background job's run started. Published on /// [`Topic::Job`]. `started_at` is server wall-clock (RFC 3339 @@ -713,11 +710,7 @@ mod tests { "authz_changed", ), ( - MessageBusEvent::NotificationReceived { - notification_id: Uuid::nil(), - kind: "share_granted".into(), - created_at: chrono::DateTime::::from_timestamp(0, 0).unwrap(), - }, + MessageBusEvent::NotificationReceived, "notification_received", ), ( diff --git a/src/application/services/notification_application_service.rs b/src/application/services/notification_application_service.rs index b7ceaf93..978bc26e 100644 --- a/src/application/services/notification_application_service.rs +++ b/src/application/services/notification_application_service.rs @@ -51,18 +51,15 @@ impl NotificationApplicationService { pub async fn create(&self, new_notif: NewNotification) -> Result { let row = self.repo.create(&new_notif).await?; - // Publish AFTER the row is durable. Silent no-op if the bus - // is disabled at boot (`OXICLOUD_MESSAGEBUS_ENABLE=false`) — - // the WS route is unmounted so the publish just hits a dead - // sender. The FE bell still works: it reads from the DB on - // mount. See plan § "Slice E". + // Publish a pure poke AFTER the row is durable. No fields + // on the wire — the topic itself signals the semantic, the + // FE refetches via REST to render. Silent no-op if the bus + // is disabled at boot (`OXICLOUD_MESSAGEBUS_ENABLE=false`). + // See `docs/plan/templated-messages.md § Bus event is a + // pure poke`. self.bus.publish( &Topic::UserNotifications(row.user_id), - MessageBusEvent::NotificationReceived { - notification_id: row.id, - kind: row.kind.clone(), - created_at: row.created_at, - }, + MessageBusEvent::NotificationReceived, ); Ok(row) diff --git a/src/bin/generate-asyncapi.rs b/src/bin/generate-asyncapi.rs index ef3c5f19..45c111e1 100644 --- a/src/bin/generate-asyncapi.rs +++ b/src/bin/generate-asyncapi.rs @@ -755,15 +755,17 @@ fn folder_deleted_schema() -> Value { // event is just an invalidation. fn notification_received_schema() -> Value { + // Pure cache-invalidation event — no fields on the wire. + // The topic (`user:{u}:notifications`) signals the semantic; + // the FE responds by refetching `GET /api/notifications` + // (or a delta via `?after=`). All payload data lives + // in the REST DTO (OpenAPI), not here. See + // `docs/plan/templated-messages.md § Schema ownership`. json!({ "type": "object", - "description": "A new notification was created for the caller. Payload is intentionally thin — the FE refetches `GET /api/notifications` for the row's full contents. `kind` is the notification's registered kind slug (`share_granted`, `job_completed_for_you`, `new_login_from_new_device`, `storage_quota_threshold`, …); the FE may use it to route a toast for high-priority kinds but never treats it as authoritative.", - "required": ["notification_id", "kind", "created_at"], - "properties": { - "notification_id": { "type": "string", "format": "uuid" }, - "kind": { "type": "string" }, - "created_at": { "type": "string", "format": "date-time" }, - } + "description": "A new notification was created for the caller. Pure cache-invalidation event — no fields on the wire. The FE refetches `GET /api/notifications` on receipt and reads the payload from the REST DTO (see `openapi.json`). Zero schema overlap between the bus wire (this file) and the REST wire — the strict form of the AsyncAPI-defines-envelope / OpenAPI-defines-payload split.", + "additionalProperties": false, + "properties": {} }) } diff --git a/src/domain/entities/notification.rs b/src/domain/entities/notification.rs index a251f971..f5e5ec84 100644 --- a/src/domain/entities/notification.rs +++ b/src/domain/entities/notification.rs @@ -68,3 +68,65 @@ pub struct NewNotification { pub kind: String, pub payload: serde_json::Value, } + +// ════════════════════════════════════════════════════════════════════════════ +// Per-kind payload types (OpenAPI-owned) +// +// Each kind's payload shape lives here as a real Rust struct with +// `#[derive(ToSchema)]`. OpenAPI auto-derives the schema from Rust; +// AsyncAPI never sees these types (the bus event is a pure poke — +// see `docs/plan/templated-messages.md § Schema ownership`). Adding +// a new kind = new struct here + a Rust `kind::` const above + a +// template branch in `NotificationRow.svelte`. +// ════════════════════════════════════════════════════════════════════════════ + +/// Payload written on the DB row when a `share_granted` notification +/// is created. Kind = [`kind::SHARE_GRANTED`]. +/// +/// The `resource_name` and `resource_path` fields are snapshotted at +/// grant time — even if the resource is later renamed or moved, the +/// notification still reflects what it was called when the share +/// happened. `resource_path` is populated for kinds addressable via +/// `/files/[...path]` (folders + files); `None` for calendars, +/// address books, playlists, drives. +/// +/// Wire form matches the `payload` JSONB column exactly — Rust is +/// the source of truth, OpenAPI schema auto-derives via +/// `#[derive(ToSchema)]`. Adding a new field is additive on the +/// JSONB column; no migration needed. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize, utoipa::ToSchema)] +pub struct SharegrantedPayload { + /// The user who created the grant. + pub granter_id: Uuid, + /// Resource kind slug: `folder`, `file`, `drive`, `calendar`, + /// `address_book`, `playlist`. Same string form as + /// [`crate::domain::services::authorization::Resource::type_str`]. + pub resource_type: String, + /// Resource UUID. + pub resource_id: Uuid, + /// Display name at grant time. `None` if the lookup failed at + /// ingest (bell renders a generic fallback in that case). + #[serde(skip_serializing_if = "Option::is_none")] + pub resource_name: Option, + /// Storage path at grant time. Populated for + /// `folder` / `file` kinds; `None` for other kinds. + #[serde(skip_serializing_if = "Option::is_none")] + pub resource_path: Option, + /// FE-navigation hint — the folder id the notification link + /// should route to. Populated when `resource_type` isn't itself + /// a folder-shaped resource but the FE still wants to land in + /// `/files/{id}` (concretely: **drives** — the recipient lands + /// on the drive's root folder). For `resource_type == 'folder'` + /// the FE uses `resource_id` directly and this field stays + /// `None`; same for `file` (routes to `/shared-with-me?file=` + /// via a different path). `None` for calendar / address_book / + /// playlist — those aren't reachable via `/files/*` at all. + #[serde(skip_serializing_if = "Option::is_none")] + pub navigate_folder_id: Option, + /// Role granted (`viewer`, `editor`, `owner`, …). Same string + /// form as [`crate::domain::services::authorization::Role::as_str`]. + pub role: String, + /// Grant expiry, if bounded. `None` = never expires. + #[serde(skip_serializing_if = "Option::is_none")] + pub expires_at: Option>, +} diff --git a/src/infrastructure/services/in_process_message_bus.rs b/src/infrastructure/services/in_process_message_bus.rs index a5646d73..3e8f1ea4 100644 --- a/src/infrastructure/services/in_process_message_bus.rs +++ b/src/infrastructure/services/in_process_message_bus.rs @@ -199,7 +199,7 @@ fn event_kind(event: &MessageBusEvent) -> &'static str { MessageBusEvent::FolderMoved { .. } => "folder_moved", MessageBusEvent::FolderDeleted { .. } => "folder_deleted", MessageBusEvent::AuthzChanged { .. } => "authz_changed", - MessageBusEvent::NotificationReceived { .. } => "notification_received", + MessageBusEvent::NotificationReceived => "notification_received", MessageBusEvent::JobRunStarted { .. } => "job_run_started", MessageBusEvent::JobRunProgress { .. } => "job_run_progress", MessageBusEvent::JobRunEnded { .. } => "job_run_ended", diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index 7c053754..150eeaa8 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -365,6 +365,18 @@ pub async fn create_grant( }, Subject::Token(_) => Vec::new(), }; + + // Enrich the payload with the resource's display name and, + // for browsable kinds (folder / file), its storage path. + // Snapshotted at grant time — the FE bell renders "Alice + // shared 'Q4 Report'", and stays correct even if the folder + // is later renamed. Lookup failure logs a warn + falls back + // to a payload without name/path; the FE renders the generic + // fallback in that case. Only fetched once per grant, then + // reused for every recipient of the fan-out. + let (resource_name, resource_path, navigate_folder_id) = + resolve_resource_display(&state, resource).await; + for rid in recipient_ids { // Self-shares (owner grants themselves via a group they // are also in) would fire a bell on the owner — filter @@ -374,17 +386,27 @@ pub async fn create_grant( if rid == caller_id { continue; } - let payload = serde_json::json!({ - "granter_id": caller_id, - "resource_type": resource.type_str(), - "resource_id": resource.id(), - "role": role.as_str(), - "expires_at": expires_at, - }); + let payload = crate::domain::entities::notification::SharegrantedPayload { + granter_id: caller_id, + resource_type: resource.type_str().to_string(), + resource_id: resource.id(), + resource_name: resource_name.clone(), + resource_path: resource_path.clone(), + navigate_folder_id, + role: role.as_str().to_string(), + expires_at, + }; + let payload_value = match serde_json::to_value(&payload) { + Ok(v) => v, + Err(e) => { + warn!("share_granted payload serialize failed: {e}"); + continue; + } + }; let new_notif = crate::domain::entities::notification::NewNotification { user_id: rid, kind: crate::domain::entities::notification::kind::SHARE_GRANTED.to_string(), - payload, + payload: payload_value, }; if let Err(e) = notif_svc.create(new_notif).await { warn!( @@ -1377,3 +1399,101 @@ pub async fn list_my_shares( // touch it directly. #[allow(dead_code)] fn _ensure_subject_dto_compiles(_: SubjectDto) {} + +/// Look up a resource's display name (`name`) and, for kinds +/// addressable via `/files/[...path]`, its storage path. Fed into +/// [`SharegrantedPayload`] at grant time so the FE bell renders +/// "Alice shared `Q4 Report`" with a clickable link. +/// +/// Best-effort — a lookup failure returns `(None, None)` and the +/// FE bell falls back to a generic template. Slice E's ingester +/// treats bell enrichment as best-effort by design; the grant +/// itself is already durable in `role_grants` when this runs. +/// +/// Only `Resource::Folder` and `Resource::File` carry a +/// `resource_path`. Drives, calendars, address books and playlists +/// have a display name but no `/files/*` route — link renders as +/// bold text via the FE's null-path fallback. +/// Enrichment tuple: `(display_name, storage_path, +/// navigate_folder_id)`. All three fields are optional; each is +/// populated per resource kind (see match arms below). Fed into the +/// `SharegrantedPayload` at grant time. +type ResourceDisplay = (Option, Option, Option); + +async fn resolve_resource_display(state: &AppStateRef, resource: Resource) -> ResourceDisplay { + match resource { + Resource::Folder(id) => { + match state + .applications + .folder_service_concrete + .get_folders_by_ids(&[id.to_string()]) + .await + { + Ok(mut dtos) => match dtos.pop() { + // `navigate_folder_id` stays None for folders — + // the FE uses `resource_id` directly to build + // `/files/{id}`. + Some(dto) => (Some(dto.name), Some(dto.path), None), + None => (None, None, None), + }, + Err(e) => { + warn!("resolve_resource_display: folder {id} lookup failed: {e}"); + (None, None, None) + } + } + } + Resource::File(id) => { + match state + .applications + .file_retrieval_service + .get_files_by_ids(&[id.to_string()]) + .await + { + Ok(mut dtos) => match dtos.pop() { + // File's `path` is the file's own storage path + // — the FE bell renders it in the tooltip but + // the actual link routes to + // `/shared-with-me?file=` (path can't help + // when the recipient has no parent-folder + // access). `navigate_folder_id` stays None. + Some(dto) => (Some(dto.name), Some(dto.path), None), + None => (None, None, None), + }, + Err(e) => { + warn!("resolve_resource_display: file {id} lookup failed: {e}"); + (None, None, None) + } + } + } + Resource::Drive(id) => { + // A drive grant is really "here's the drive, land on + // its root folder." The FE follows `navigate_folder_id` + // into `/files/{root_folder_id}` — the drive itself has + // no browsable URL, but its root folder does. + // + // `resource_name` comes from the root folder's display + // name (drives don't have their own name column; the + // root folder's `storage.folders.name` is the drive's + // canonical label — see `DriveWithRootName`). + match state.drive_repo.get_by_id(id).await { + Ok(dwn) => ( + Some(dwn.root_folder_name), + None, + Some(dwn.drive.root_folder_id), + ), + Err(e) => { + warn!("resolve_resource_display: drive {id} lookup failed: {e:?}"); + (None, None, None) + } + } + } + // Non-browsable resources — no `/files` link at all. + // Calendars / address books / playlists render as bold + // text in the bell via the null-fallback FE path. Fetching + // a name for these is deferred; today the bell shows + // "shared a " for them. + Resource::Calendar(_) | Resource::AddressBook(_) | Resource::Playlist(_) => { + (None, None, None) + } + } +} diff --git a/src/interfaces/api/mod.rs b/src/interfaces/api/mod.rs index 4376d02e..5962a331 100644 --- a/src/interfaces/api/mod.rs +++ b/src/interfaces/api/mod.rs @@ -172,6 +172,12 @@ use crate::interfaces::middleware::server_status::{HeaderPayload, ProgressHeader handlers::recent_handler::record_item_access, handlers::recent_handler::remove_from_recent, handlers::recent_handler::clear_recent_items, + // Notifications (Slice E — bell + retention) + handlers::notifications_handler::list_notifications, + handlers::notifications_handler::unread_count, + handlers::notifications_handler::mark_read, + handlers::notifications_handler::mark_all_read, + handlers::notifications_handler::delete_notification, // Photos handler (free function) handlers::photos_handler::list_photos, handlers::photos_handler::list_photos_geo, @@ -382,6 +388,17 @@ use crate::interfaces::middleware::server_status::{HeaderPayload, ProgressHeader // Public server-config discovery — `GET /api/config`. ServerConfigDto, FeaturesDto, + // Notifications (Slice E) — bell REST DTOs + per-kind + // typed payload structs. The generic `NotificationDto.payload` + // stays `serde_json::Value` on the schema; each per-kind + // struct (e.g. `SharegrantedPayload`) is registered here + // so FE consumers can type-narrow on `kind`. Adding a + // new kind = one more entry here + one Rust struct. + handlers::notifications_handler::NotificationDto, + handlers::notifications_handler::ListResponseDto, + handlers::notifications_handler::UnreadCountDto, + handlers::notifications_handler::MarkAllReadResponseDto, + crate::domain::entities::notification::SharegrantedPayload, HeaderPayload, ProgressHeader, OidcProviderInfoDto, diff --git a/tests/api/rt_bus_check.sh b/tests/api/rt_bus_check.sh index 3ed851f6..32c2b7cb 100755 --- a/tests/api/rt_bus_check.sh +++ b/tests/api/rt_bus_check.sh @@ -779,13 +779,14 @@ fi || { cat "$out_s13"; die "S13: expected 1 event, got $(jq -r '.events | length' "$out_s13")"; } [[ "$(jq -r '.events[0].event' "$out_s13")" == "notification_received" ]] \ || die "S13: wrong event discriminator: $(jq -r '.events[0].event' "$out_s13")" -[[ "$(jq -r '.events[0].data.kind' "$out_s13")" == "share_granted" ]] \ - || die "S13: wrong notification kind: $(jq -r '.events[0].data.kind' "$out_s13")" -# `notification_id` is a fresh UUID stamped by the DB — check it's -# non-empty and non-null. Value asserted by S14 via GET /api/notifications. -[[ -n "$(jq -r '.events[0].data.notification_id' "$out_s13")" ]] \ - && [[ "$(jq -r '.events[0].data.notification_id' "$out_s13")" != "null" ]] \ - || die "S13: notification_id missing on wire payload" +# Pure cache-invalidation event — `data` is intentionally an empty +# object. Everything about the notification (id, kind, payload) is +# on the REST wire (asserted by S14 below), not here. See +# `docs/plan/templated-messages.md § Bus event is a pure poke`. +# If a future change adds fields back to the wire, this assertion +# will fail loudly so the drift is caught. +[[ "$(jq -c '.events[0].data' "$out_s13")" == "{}" ]] \ + || { cat "$out_s13"; die "S13: expected empty data on the wire, got $(jq -c '.events[0].data' "$out_s13")"; } log "S13 OK" # ── Scenario 14 — Notification DB row (Slice E) ───────────────────────────── @@ -814,6 +815,26 @@ first_read_at=$(printf '%s' "$notifs" | jq -r --arg fc "$folder_c" \ 'first(.items[] | select(.kind == "share_granted" and .payload.resource_id == $fc)) | .read_at') [[ "$first_read_at" == "null" ]] \ || die "S14: matched row unexpectedly marked read: read_at=$first_read_at" + +# Payload enrichment (Slice E — templated messages): the ingester +# snapshots the resource's display name + storage path at grant +# time so the FE bell can render "Alice shared 'rt_bus_C_…'" with +# a clickable link. If enrichment silently regressed to ID-only, +# the FE would still work via generic fallback but lose the +# clickable / labelled UX. Assert the enrichment fields landed on +# the folder-C row. +folder_c_name="rt_bus_C_$suffix" +first_name=$(printf '%s' "$notifs" | jq -r --arg fc "$folder_c" \ + 'first(.items[] | select(.kind == "share_granted" and .payload.resource_id == $fc)) | .payload.resource_name') +[[ "$first_name" == "$folder_c_name" ]] \ + || { printf '%s\n' "$notifs" >&2; die "S14: expected resource_name='$folder_c_name', got '$first_name'"; } +first_path=$(printf '%s' "$notifs" | jq -r --arg fc "$folder_c" \ + 'first(.items[] | select(.kind == "share_granted" and .payload.resource_id == $fc)) | .payload.resource_path') +# Path must be non-null AND non-empty AND end with the folder's +# name — the FE builds `/files${path}` for the bell's anchor, so a +# missing/wrong path breaks the click-through. +[[ -n "$first_path" && "$first_path" != "null" && "$first_path" == *"$folder_c_name" ]] \ + || { printf '%s\n' "$notifs" >&2; die "S14: expected resource_path ending with '$folder_c_name', got '$first_path'"; } log "S14 OK" # ── Scenario 15 — Cross-user notifications identity gate ────────────────────