diff --git a/application/api/user/teams/routes.py b/application/api/user/teams/routes.py index 5253d89c..5902d39d 100644 --- a/application/api/user/teams/routes.py +++ b/application/api/user/teams/routes.py @@ -30,8 +30,12 @@ from application.api.user.team_authz import ( team_member_required, ) from application.api.user.team_sharing import is_valid_resource_type, owns_resource +from application.events.publisher import publish_user_event from application.storage.db.base_repository import looks_like_uuid +from application.storage.db.repositories.agents import AgentsRepository from application.storage.db.repositories.auth_events import AuthEventsRepository +from application.storage.db.repositories.prompts import PromptsRepository +from application.storage.db.repositories.sources import SourcesRepository from application.storage.db.repositories.team_members import ( ROLE_TEAM_ADMIN, ROLE_TEAM_MEMBER, @@ -41,6 +45,7 @@ from application.storage.db.repositories.team_resource_grants import ( TeamResourceGrantsRepository, ) from application.storage.db.repositories.teams import TeamsRepository +from application.storage.db.repositories.user_tools import UserToolsRepository from application.storage.db.repositories.users import UsersRepository from application.storage.db.session import db_readonly, db_session @@ -95,6 +100,99 @@ def _unique_slug(repo: TeamsRepository, base: str) -> str: return f"{base}-{uuid.uuid4().hex}" +# --- Notifications (best-effort, fire-and-forget; never fail the request) ---- +# Emitted AFTER the mutation commits, over their own read connection, so a +# name-lookup or Redis hiccup can never roll back the share/membership change. + + +def _resource_display_name(conn, resource_type: str, resource_id: str) -> str | None: + """Best-effort human label for a shared resource (ownerless fetch).""" + try: + if resource_type == "agent": + row = AgentsRepository(conn).get_by_id(resource_id) + elif resource_type == "source": + row = SourcesRepository(conn).get_by_id(resource_id) + elif resource_type == "prompt": + row = PromptsRepository(conn).get_for_rendering(resource_id) + elif resource_type == "tool": + row = UserToolsRepository(conn).get_by_id(resource_id) + else: + return None + if not row: + return None + return row.get("custom_name") or row.get("display_name") or row.get("name") + except Exception: + logger.warning("resource name resolve failed (%s)", resource_type, exc_info=True) + return None + + +def _notify_member_added( + team_id: str, new_user: str | None, role: str, actor: str | None +) -> None: + """Toast the new member that they were added to a team.""" + if not new_user or new_user == actor: + return + try: + with db_readonly() as conn: + team = TeamsRepository(conn).get(team_id) + publish_user_event( + new_user, + "team.member_added", + { + "team_id": str(team_id), + "team_name": team.get("name") if team else None, + "role": role, + "added_by": actor, + }, + scope={"kind": "team", "id": str(team_id)}, + ) + except Exception: + logger.warning("member_added notify failed", exc_info=True) + + +def _notify_resource_shared( + actor: str | None, + team_id: str, + resource_type: str, + resource_id: str, + access_level: str, + target_user_id: str | None, +) -> None: + """Toast the recipient(s) of a share. + + Per-member share notifies just that member; whole-team share notifies every + current member except the sharer. + """ + try: + with db_readonly() as conn: + resource_name = _resource_display_name(conn, resource_type, resource_id) + team = TeamsRepository(conn).get(team_id) + if target_user_id: + recipients = [target_user_id] + else: + members = TeamMembersRepository(conn).list_members(team_id) + recipients = list({m["user_id"] for m in members}) + payload = { + "resource_type": resource_type, + "resource_id": str(resource_id), + "resource_name": resource_name, + "access_level": access_level, + "team_id": str(team_id), + "team_name": team.get("name") if team else None, + "shared_by": actor, + } + for recipient in recipients: + if recipient and recipient != actor: + publish_user_event( + recipient, + "resource.shared", + payload, + scope={"kind": "resource", "id": str(resource_id)}, + ) + except Exception: + logger.warning("resource_shared notify failed", exc_info=True) + + @teams_ns.route("/teams") class Teams(Resource): def get(self): @@ -238,6 +336,8 @@ class TeamMembers(Resource): target_user=new_user, role=role, ) + # Post-commit, best-effort: tell the new member they were added. + _notify_member_added(team_id, new_user, role, _current_user()) return make_response(jsonify({"success": True}), 200) except Exception as err: logger.error("Add member failed: %s", err, exc_info=True) @@ -388,6 +488,10 @@ class TeamGrants(Resource): access_level=access_level, target_user_id=target_user_id, ) + # Post-commit, best-effort: tell the recipient(s) it was shared. + _notify_resource_shared( + user, team_id, resource_type, resource_id, access_level, target_user_id + ) return make_response(jsonify({"success": True, "grant": grant}), 201) except Exception as err: logger.error("Share resource failed: %s", err, exc_info=True) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 556b5e37..7e1963c5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -25,6 +25,7 @@ import Setting from './settings'; import Teams from './settings/Teams'; import Notification from './components/Notification'; import ToolApprovalToast from './notifications/ToolApprovalToast'; +import TeamNotificationToast from './notifications/TeamNotificationToast'; function AuthWrapper({ children }: { children: React.ReactNode }) { const { t } = useTranslation(); @@ -90,6 +91,7 @@ function MainLayout() { + ); } diff --git a/frontend/src/events/dispatchEvent.ts b/frontend/src/events/dispatchEvent.ts index c08be023..c8e10cb3 100644 --- a/frontend/src/events/dispatchEvent.ts +++ b/frontend/src/events/dispatchEvent.ts @@ -36,6 +36,10 @@ const KNOWN_TYPES: ReadonlySet = new Set([ 'schedule.cancelled', 'schedule.completed', 'schedule.message.appended', + // Team-sharing notifications (teams/routes.py); consumed by + // TeamNotificationToast via selectRecentEvents. + 'team.member_added', + 'resource.shared', ]); /** diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index 4d07ba2a..321cdc94 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -16,6 +16,14 @@ "editor": "Editor", "viewer": "Viewer" }, + "notifications": { + "memberAddedTitle": "Added to a team", + "memberAddedBody": "You were added to {{team}} as {{role}}", + "sharedTitle": "Shared with you", + "sharedBody": "{{type}} shared with your team: {{name}} ({{access}})", + "sharedBodyNoName": "A {{type}} was shared with your team ({{access}})", + "dismiss": "Dismiss" + }, "demo": [ { "header": "Learn about DocsGPT", diff --git a/frontend/src/notifications/TeamNotificationToast.tsx b/frontend/src/notifications/TeamNotificationToast.tsx new file mode 100644 index 00000000..27fccae9 --- /dev/null +++ b/frontend/src/notifications/TeamNotificationToast.tsx @@ -0,0 +1,169 @@ +import { X } from 'lucide-react'; +import { useCallback, useEffect, useMemo } from 'react'; +import { useTranslation } from 'react-i18next'; +import { useDispatch, useSelector } from 'react-redux'; + +import { Button } from '../components/ui/button'; + +import { + dismissShareNotification, + selectDismissedShareNotifications, + selectRecentEvents, + type SSEEvent, +} from './notificationsSlice'; + +// Backlog replay on reload re-delivers up to ~24h of events; skip anything +// older than the stream-retention window so a stale share doesn't pop. +const MAX_AGE_MS = 24 * 60 * 60 * 1000; +// Informational toasts auto-dismiss after a beat (and persist the dismissal, +// so backlog replay won't re-pop them) — unlike the action-required tool +// approval toast, which stays until acted on. +const AUTO_DISMISS_MS = 8000; +const MAX_VISIBLE = 3; + +const TEAM_NOTIFICATION_TYPES = new Set([ + 'team.member_added', + 'resource.shared', +]); + +/** + * A single team-sharing toast that owns its OWN auto-dismiss timer, keyed on + * its event id. Per-toast (rather than a shared parent timer) so a newly + * arriving sibling toast can't reset an in-flight toast's countdown. + */ +function ShareToast({ + id, + title, + body, + dismissLabel, + onDismiss, +}: { + id: string; + title: string; + body: string; + dismissLabel: string; + onDismiss: (id: string) => void; +}) { + useEffect(() => { + const timer = window.setTimeout(() => onDismiss(id), AUTO_DISMISS_MS); + return () => window.clearTimeout(timer); + }, [id, onDismiss]); + + return ( +
+
+

+ {title} +

+ +
+
+

+ {body} +

+
+
+ ); +} + +/** + * Transient toasts for team-sharing events (``team.member_added`` and + * ``resource.shared``) delivered over the per-user SSE channel. Mirrors the + * ToolApprovalToast surface, but auto-dismisses since these are purely + * informational. Dismissal (manual or timed) is persisted so the SSE backlog + * replay on reload doesn't re-surface an already-seen notification. + */ +export default function TeamNotificationToast() { + const dispatch = useDispatch(); + const { t } = useTranslation(); + const events = useSelector(selectRecentEvents); + const dismissed = useSelector(selectDismissedShareNotifications); + const dismissedSet = useMemo(() => new Set(dismissed), [dismissed]); + + const onDismiss = useCallback( + (id: string) => dispatch(dismissShareNotification(id)), + [dispatch], + ); + + const now = Date.now(); + const visible: SSEEvent[] = []; + const seen = new Set(); + for (const event of events) { + if (!TEAM_NOTIFICATION_TYPES.has(event.type)) continue; + if (!event.id || dismissedSet.has(event.id) || seen.has(event.id)) continue; + if (event.ts) { + const age = now - Date.parse(event.ts); + if (Number.isFinite(age) && age > MAX_AGE_MS) continue; + } + seen.add(event.id); + visible.push(event); + if (visible.length >= MAX_VISIBLE) break; + } + + if (visible.length === 0) return null; + + const describe = (event: SSEEvent): { title: string; body: string } => { + const p = (event.payload ?? {}) as Record; + if (event.type === 'team.member_added') { + const team = (p.team_name as string) || (p.team_id as string) || ''; + const role = + p.role === 'team_admin' + ? t('settings.teams.roleAdmin') + : t('settings.teams.roleMember'); + return { + title: t('notifications.memberAddedTitle'), + body: t('notifications.memberAddedBody', { team, role }), + }; + } + // resource.shared + const type = t(`settings.teams.resourceType.${p.resource_type}`, { + defaultValue: String(p.resource_type ?? 'resource'), + }); + const access = + p.access_level === 'editor' + ? t('teamAccess.editor') + : t('teamAccess.viewer'); + const name = (p.resource_name as string) || ''; + return { + title: t('notifications.sharedTitle'), + body: name + ? t('notifications.sharedBody', { name, type, access }) + : t('notifications.sharedBodyNoName', { type, access }), + }; + }; + + return ( + // Standard bottom-right toast rail. UploadToast / ToolApprovalToast use the + // same corner but rarely co-occur with team-share toasts. +
e.stopPropagation()} + role="status" + aria-live="polite" + aria-atomic="true" + > + {visible.map((event) => { + const { title, body } = describe(event); + return ( + + ); + })} +
+ ); +} diff --git a/frontend/src/notifications/notificationsSlice.ts b/frontend/src/notifications/notificationsSlice.ts index 908ab594..5adbabf7 100644 --- a/frontend/src/notifications/notificationsSlice.ts +++ b/frontend/src/notifications/notificationsSlice.ts @@ -4,6 +4,8 @@ import { RootState } from '../store'; import { loadDismissed, saveDismissed } from './dismissedPersistence'; const DISMISSED_TOOL_APPROVALS_STORAGE_KEY = 'docsgpt:dismissedToolApprovals'; +const DISMISSED_SHARE_NOTIFICATIONS_STORAGE_KEY = + 'docsgpt:dismissedShareNotifications'; /** * Envelope shape published by the backend SSE endpoint @@ -59,11 +61,20 @@ interface NotificationsState { * plausible approval-pending window. */ dismissedToolApprovals: Array<{ id: string; at: number }>; + /** + * Event ids of team-sharing notifications (``team.member_added`` / + * ``resource.shared``) the user dismissed or that auto-dismissed, + * persisted so SSE backlog replay doesn't re-pop them. Same TTL + FIFO + * eviction as the tool-approval set. + */ + dismissedShareNotifications: Array<{ id: string; at: number }>; } const RECENT_EVENTS_CAP = 100; const DISMISSED_TOOL_APPROVALS_CAP = 200; const DISMISSED_TOOL_APPROVALS_TTL_MS = 24 * 60 * 60 * 1000; +const DISMISSED_SHARE_NOTIFICATIONS_CAP = 200; +const DISMISSED_SHARE_NOTIFICATIONS_TTL_MS = 24 * 60 * 60 * 1000; const initialState: NotificationsState = { health: 'connecting', @@ -76,6 +87,10 @@ const initialState: NotificationsState = { DISMISSED_TOOL_APPROVALS_STORAGE_KEY, DISMISSED_TOOL_APPROVALS_TTL_MS, ), + dismissedShareNotifications: loadDismissed( + DISMISSED_SHARE_NOTIFICATIONS_STORAGE_KEY, + DISMISSED_SHARE_NOTIFICATIONS_TTL_MS, + ), }; export const notificationsSlice = createSlice({ @@ -160,6 +175,35 @@ export const notificationsSlice = createSlice({ state.dismissedToolApprovals, ); }, + /** + * Suppress a team-sharing notification (``team.member_added`` / + * ``resource.shared``) by SSE event id — fired on close OR on the + * informational auto-dismiss timer. Persisted so backlog replay on + * reload doesn't re-pop an already-seen notification. + */ + dismissShareNotification: (state, action: PayloadAction) => { + const id = action.payload; + const now = Date.now(); + const cutoff = now - DISMISSED_SHARE_NOTIFICATIONS_TTL_MS; + state.dismissedShareNotifications = + state.dismissedShareNotifications.filter( + (entry) => entry.at >= cutoff && entry.id !== id, + ); + state.dismissedShareNotifications.push({ id, at: now }); + if ( + state.dismissedShareNotifications.length > + DISMISSED_SHARE_NOTIFICATIONS_CAP + ) { + state.dismissedShareNotifications = + state.dismissedShareNotifications.slice( + -DISMISSED_SHARE_NOTIFICATIONS_CAP, + ); + } + saveDismissed( + DISMISSED_SHARE_NOTIFICATIONS_STORAGE_KEY, + state.dismissedShareNotifications, + ); + }, /** * Revoke a pending approval when the backend says it can no longer be * acted on (``tool.approval.cleared`` — the message failed or the @@ -228,6 +272,7 @@ export const { sseLastEventIdAdvanced, clearRecentEvents, dismissToolApproval, + dismissShareNotification, resolveToolApproval, } = notificationsSlice.actions; @@ -255,4 +300,9 @@ export const selectDismissedToolApprovals = createSelector( (entries) => entries.map((entry) => entry.id), ); +export const selectDismissedShareNotifications = createSelector( + (state: RootState) => state.notifications.dismissedShareNotifications, + (entries) => entries.map((entry) => entry.id), +); + export default notificationsSlice.reducer; diff --git a/tests/api/test_teams_endpoints.py b/tests/api/test_teams_endpoints.py index 03067539..fa968348 100644 --- a/tests/api/test_teams_endpoints.py +++ b/tests/api/test_teams_endpoints.py @@ -395,3 +395,176 @@ class TestAdminOversight: _stop(patches) assert resp.status_code == 200 assert json.loads(resp.data)["teams"][0]["member_count"] == 3 + + +@pytest.mark.unit +class TestTeamNotifications: + """Best-effort, post-commit toasts via publish_user_event.""" + + def test_add_member_notifies_new_member(self, client): + members_repo = Mock() + teams_repo = Mock() + teams_repo.get.return_value = {"id": "team-1", "name": "Acme"} + publish = Mock() + patches = _auth(sub="alice", team_role="team_admin") + [ + patch("application.api.user.teams.routes.db_session", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.db_readonly", lambda: _cm(Mock())), + patch( + "application.api.user.teams.routes.TeamMembersRepository", + return_value=members_repo, + ), + patch( + "application.api.user.teams.routes.TeamsRepository", + return_value=teams_repo, + ), + patch("application.api.user.teams.routes.publish_user_event", publish), + ] + _apply(patches) + try: + resp = client.post( + "/api/teams/team-1/members", + json={"user_id": "carol", "role": "team_member"}, + ) + finally: + _stop(patches) + assert resp.status_code == 200 + publish.assert_called_once() + args, _ = publish.call_args + assert args[0] == "carol" + assert args[1] == "team.member_added" + assert args[2]["team_name"] == "Acme" + assert args[2]["role"] == "team_member" + assert args[2]["added_by"] == "alice" + + def test_add_member_skips_self(self, client): + members_repo = Mock() + teams_repo = Mock() + teams_repo.get.return_value = {"id": "team-1", "name": "Acme"} + publish = Mock() + patches = _auth(sub="alice", team_role="team_admin") + [ + patch("application.api.user.teams.routes.db_session", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.db_readonly", lambda: _cm(Mock())), + patch( + "application.api.user.teams.routes.TeamMembersRepository", + return_value=members_repo, + ), + patch( + "application.api.user.teams.routes.TeamsRepository", + return_value=teams_repo, + ), + patch("application.api.user.teams.routes.publish_user_event", publish), + ] + _apply(patches) + try: + resp = client.post( + "/api/teams/team-1/members", + json={"user_id": "alice", "role": "team_admin"}, + ) + finally: + _stop(patches) + assert resp.status_code == 200 + publish.assert_not_called() + + def test_whole_team_share_notifies_all_but_sharer(self, client): + grants_repo = Mock() + grants_repo.grant.return_value = {"id": "g1", "access_level": "editor"} + members_repo = Mock() + members_repo.list_members.return_value = [ + {"user_id": "alice"}, + {"user_id": "bob"}, + {"user_id": "carol"}, + ] + teams_repo = Mock() + teams_repo.get.return_value = {"id": "team-1", "name": "Acme"} + publish = Mock() + patches = _auth(sub="alice", team_role="team_member") + [ + patch("application.api.user.teams.routes.db_session", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.db_readonly", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.owns_resource", return_value=True), + patch( + "application.api.user.teams.routes.TeamResourceGrantsRepository", + return_value=grants_repo, + ), + patch( + "application.api.user.teams.routes.TeamMembersRepository", + return_value=members_repo, + ), + patch( + "application.api.user.teams.routes.TeamsRepository", + return_value=teams_repo, + ), + patch( + "application.api.user.teams.routes._resource_display_name", + return_value="My Agent", + ), + patch("application.api.user.teams.routes.publish_user_event", publish), + ] + _apply(patches) + try: + resp = client.post( + "/api/teams/team-1/grants", + json={ + "resource_type": "agent", + "resource_id": "22222222-2222-2222-2222-222222222222", + "access_level": "editor", + }, + ) + finally: + _stop(patches) + assert resp.status_code == 201 + recipients = {call.args[0] for call in publish.call_args_list} + assert recipients == {"bob", "carol"} # never the sharer (alice) + first = publish.call_args_list[0].args + assert first[1] == "resource.shared" + assert first[2]["resource_name"] == "My Agent" + assert first[2]["access_level"] == "editor" + assert first[2]["shared_by"] == "alice" + + def test_per_member_share_notifies_only_target(self, client): + grants_repo = Mock() + grants_repo.grant.return_value = {"id": "g1", "access_level": "viewer"} + members_repo = Mock() + members_repo.is_member.return_value = True + teams_repo = Mock() + teams_repo.get.return_value = {"id": "team-1", "name": "Acme"} + publish = Mock() + patches = _auth(sub="alice", team_role="team_member") + [ + patch("application.api.user.teams.routes.db_session", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.db_readonly", lambda: _cm(Mock())), + patch("application.api.user.teams.routes.owns_resource", return_value=True), + patch( + "application.api.user.teams.routes.TeamResourceGrantsRepository", + return_value=grants_repo, + ), + patch( + "application.api.user.teams.routes.TeamMembersRepository", + return_value=members_repo, + ), + patch( + "application.api.user.teams.routes.TeamsRepository", + return_value=teams_repo, + ), + patch( + "application.api.user.teams.routes._resource_display_name", + return_value="My Agent", + ), + patch("application.api.user.teams.routes.publish_user_event", publish), + ] + _apply(patches) + try: + resp = client.post( + "/api/teams/team-1/grants", + json={ + "resource_type": "agent", + "resource_id": "22222222-2222-2222-2222-222222222222", + "access_level": "viewer", + "target_user_id": "bob", + }, + ) + finally: + _stop(patches) + assert resp.status_code == 201 + publish.assert_called_once() + args, _ = publish.call_args + assert args[0] == "bob" + assert args[1] == "resource.shared"