From cb4843bbefed034170aae35c7eb6a7b8faa59502 Mon Sep 17 00:00:00 2001 From: Usama Date: Thu, 21 May 2026 16:08:48 +0000 Subject: [PATCH 1/3] fix(workspaces): polish free-tier and external-collaborator UX - Hide "Recently approved" badge on free workspaces (auto-created, not approved) - Cap workspace card avatars at 3 + overflow bubble so busy workspaces don't overflow the card - Exclude free-tier workspaces from non-admin discovery and reject access requests against them - Hide "New organisation workspace" button and "Project Defaults" settings for users with no org membership --- .../src/components/settings/MyAccessCard.tsx | 20 +++++++------ .../src/routes/settings/UserSettingsRoute.tsx | 28 +++++++++++++++++-- .../workspaces/WorkspaceSelectorRoute.tsx | 8 ++++-- .../server/dembrane/api/v2/access_requests.py | 8 ++++++ 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/echo/frontend/src/components/settings/MyAccessCard.tsx b/echo/frontend/src/components/settings/MyAccessCard.tsx index 1c545b29..fc291b0d 100644 --- a/echo/frontend/src/components/settings/MyAccessCard.tsx +++ b/echo/frontend/src/components/settings/MyAccessCard.tsx @@ -99,6 +99,8 @@ export const MyAccessCard = () => { const totalOrganisations = data?.organisations.length ?? 0; const totalWorkspaces = data?.workspaces.length ?? 0; + // Externals (no org membership) can't request a workspace. + const canCreateWorkspace = totalOrganisations > 0; return ( @@ -122,14 +124,16 @@ export const MyAccessCard = () => { /> - + {canCreateWorkspace && ( + + )} {byOrganisation.size === 0 ? ( diff --git a/echo/frontend/src/routes/settings/UserSettingsRoute.tsx b/echo/frontend/src/routes/settings/UserSettingsRoute.tsx index 849deb51..aab0f1e3 100644 --- a/echo/frontend/src/routes/settings/UserSettingsRoute.tsx +++ b/echo/frontend/src/routes/settings/UserSettingsRoute.tsx @@ -20,8 +20,10 @@ import { IconScale, IconShieldLock, } from "@tabler/icons-react"; -import { useState } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { useMemo, useState } from "react"; import { useCurrentUser } from "@/components/auth/hooks"; +import { API_BASE_URL } from "@/config"; import { AccountSettingsCard } from "@/components/settings/AccountSettingsCard"; import { AuditLogsCard } from "@/components/settings/AuditLogsCard"; import { ChangePasswordCard } from "@/components/settings/ChangePasswordCard"; @@ -70,6 +72,26 @@ export const UserSettingsRoute = () => { const isTwoFactorEnabled = Boolean(user?.tfa_enabled); + const { data: accessData } = useQuery<{ + organisations: Array<{ id: string }>; + } | null>({ + queryKey: ["v2", "workspaces"], + queryFn: async () => { + const res = await fetch(`${API_BASE_URL}/v2/workspaces`, { + credentials: "include", + }); + if (!res.ok) return null; + return res.json(); + }, + staleTime: 60_000, + }); + const isExternalOnly = (accessData?.organisations.length ?? 0) === 0; + + const visibleSections = useMemo( + () => SECTIONS.filter((s) => !(isExternalOnly && s.id === "project-defaults")), + [isExternalOnly], + ); + return ( @@ -116,7 +138,7 @@ export const UserSettingsRoute = () => { - {SECTIONS.map((section) => ( + {visibleSections.map((section) => ( { )} - {activeSection === "project-defaults" && ( + {activeSection === "project-defaults" && !isExternalOnly && ( <Trans>Project Defaults</Trans> diff --git a/echo/frontend/src/routes/workspaces/WorkspaceSelectorRoute.tsx b/echo/frontend/src/routes/workspaces/WorkspaceSelectorRoute.tsx index ccd5b4dd..311d0839 100644 --- a/echo/frontend/src/routes/workspaces/WorkspaceSelectorRoute.tsx +++ b/echo/frontend/src/routes/workspaces/WorkspaceSelectorRoute.tsx @@ -123,12 +123,14 @@ function AvatarBubbles({ members: MemberPreview[]; count: number; }) { - const overflow = count - members.length; + const MAX_VISIBLE = 3; + const visible = members.slice(0, MAX_VISIBLE); + const overflow = count - visible.length; return ( <Tooltip.Group> <Avatar.Group spacing="sm"> - {members.map((m, i) => ( + {visible.map((m, i) => ( <Tooltip key={`${m.display_name}-${i}`} label={m.display_name} @@ -168,8 +170,10 @@ function WorkspaceCard({ const isAdminOrOwner = workspace.role === "admin" || workspace.role === "owner"; const ONE_DAY_MS = 86_400_000; + // Free workspaces are auto-created on signup, never "approved". const isRecentlyApproved = !!workspace.created_at && + workspace.tier !== "free" && Date.now() - new Date(workspace.created_at).getTime() < ONE_DAY_MS; const [hovered, setHovered] = useState(false); const wsLogo = resolveLogoUrl(workspace.logo_url); diff --git a/echo/server/dembrane/api/v2/access_requests.py b/echo/server/dembrane/api/v2/access_requests.py index 1f2a5a78..011c5c97 100644 --- a/echo/server/dembrane/api/v2/access_requests.py +++ b/echo/server/dembrane/api/v2/access_requests.py @@ -315,6 +315,12 @@ async def request_workspace_access( detail="Workspace not found", # intentional — don't confirm existence ) + if workspace.get("tier") == "free": + raise HTTPException( + status_code=404, + detail="Workspace not found", + ) + org_role = await _org_role(org_id, app_user_id) if org_role is None: raise HTTPException(status_code=403, detail="Not a member of this organisation") @@ -652,6 +658,8 @@ async def list_discoverable_workspaces( } if not is_org_admin: filters["visibility"] = {"_neq": "private"} + # Free tier is the admin's personal allotment — not requestable. + filters["tier"] = {"_neq": "free"} workspaces = await async_directus.get_items( "workspace", From 2890b8210e56484b03241a781982e16ab50732a5 Mon Sep 17 00:00:00 2001 From: Usama <reach.usamazafar@gmail.com> Date: Thu, 21 May 2026 16:54:02 +0000 Subject: [PATCH 2/3] fix(admin): repair Columns filter and table re-renders on usage tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Columns dropdown and period selector on /admin/usage-and-billing appeared inert — checkbox toggles and "Last month" clicks fired and even refetched, but the table never reflected them. Two bugs stacked: 1. Menu.Item's onClick called preventDefault(), which swallowed the bubbled checkbox click and stopped the toggle (same root cause as the org-usage fix in 0ba5830). 2. React Compiler memoized BillingTable's JSX on [footerTotals, table]. TanStack's useReactTable returns a stable reference and mutates the table in place, so the cache always hit and state changes never reached the DOM. SimpleDataTable had the same latent issue affecting Partners / Upgrades sort + search. --- .../src/routes/admin/AdminSettingsRoute.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/echo/frontend/src/routes/admin/AdminSettingsRoute.tsx b/echo/frontend/src/routes/admin/AdminSettingsRoute.tsx index 776c3517..c42d02ca 100644 --- a/echo/frontend/src/routes/admin/AdminSettingsRoute.tsx +++ b/echo/frontend/src/routes/admin/AdminSettingsRoute.tsx @@ -559,6 +559,9 @@ function BillingTable({ total_forecast_eur: number; }; }) { + // useReactTable returns a stable ref; React Compiler caches our JSX on + // it, so state changes never reach the DOM. See frontend/AGENTS.md. + "use no memo"; const [sorting, setSorting] = useState<SortingState>(initialSorting ?? []); // TanStack's ExpandedState is Record<string, boolean> | true, but the // 'true' shorthand isn't useful to us — we always track per-row @@ -1451,19 +1454,25 @@ function UsageAndBillingPanel() { {columnMenuItems.map((c) => ( <Menu.Item key={c.id} - onClick={(e) => e.preventDefault()} closeMenuOnClick={false} + onClick={() => + setColumnVisibility((v) => ({ + ...v, + [c.id]: v[c.id] === false, + })) + } > <Checkbox size="xs" label={c.label} checked={columnVisibility[c.id] !== false} - onChange={(e) => + onChange={() => setColumnVisibility((v) => ({ ...v, - [c.id]: e.currentTarget.checked, + [c.id]: v[c.id] === false, })) } + onClick={(e) => e.stopPropagation()} /> </Menu.Item> ))} @@ -1551,6 +1560,8 @@ function SimpleDataTable<T extends object>({ initialSorting?: SortingState; emptyLabel: string; }) { + // See BillingTable / frontend/AGENTS.md for the rationale. + "use no memo"; const [sorting, setSorting] = useState<SortingState>(initialSorting ?? []); const table = useReactTable<T>({ columns, From 75d67d814f2f25d0f0dc84c7def795e9688217f4 Mon Sep 17 00:00:00 2001 From: Usama <reach.usamazafar@gmail.com> Date: Thu, 21 May 2026 17:18:33 +0000 Subject: [PATCH 3/3] perf(workspace-requests): move staff/requester notifications off the request path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submit and approve/deny were doing N sequential Directus notification writes plus a synchronous SendGrid send inline, adding 5–7s of click latency. Schedule them via FastAPI BackgroundTasks so the response returns immediately. Also drop the redundant workspace re-fetch in _notify_requester_approved by threading the name through from the caller (_upgrade_workspace_for_request now returns (id, name)). Same test surface; affected suite runtime drops from ~71s to ~1.4s, mirroring the production speedup. Five files changed: - server/dembrane/api/v2/workspace_requests.py — extract staff fan-out into _notify_staff_workspace_request_submitted, schedule via BackgroundTasks. - server/dembrane/api/v2/admin.py — BackgroundTasks on decide_workspace_request; background-schedule both notify helpers; drop redundant get_item("workspace", …); _upgrade_workspace_for_request returns (id, name). - server/tests/test_workspace_requests.py, server/tests/test_approve_deny_action.py, server/tests/test_request_notifications.py — pass BackgroundTasks(), drain queue where assertions check side-effects, adjust the tuple-return assertion + drop now-unused workspace mocks. --- echo/server/dembrane/api/v2/admin.py | 35 ++-- .../dembrane/api/v2/workspace_requests.py | 196 +++++++++++------- echo/server/tests/test_approve_deny_action.py | 32 +-- .../tests/test_request_notifications.py | 54 +++-- echo/server/tests/test_workspace_requests.py | 34 +-- 5 files changed, 203 insertions(+), 148 deletions(-) diff --git a/echo/server/dembrane/api/v2/admin.py b/echo/server/dembrane/api/v2/admin.py index aff68237..abe0f20b 100644 --- a/echo/server/dembrane/api/v2/admin.py +++ b/echo/server/dembrane/api/v2/admin.py @@ -21,7 +21,7 @@ from logging import getLogger from datetime import datetime, timezone, timedelta -from fastapi import Query, APIRouter, HTTPException +from fastapi import Query, APIRouter, HTTPException, BackgroundTasks from pydantic import Field, BaseModel from dembrane.email import send_email @@ -858,6 +858,7 @@ async def _notify_requester_approved( granted_tier: str, resulting_ws_id: str, *, + workspace_name: Optional[str] = None, approved_billing_period: Optional[str] = None, proposed_billing_period: Optional[str] = None, ) -> None: @@ -877,13 +878,7 @@ async def _notify_requester_approved( base = (settings.urls.admin_base_url or "").rstrip("/") workspace_url = f"{base}/w/{resulting_ws_id}" if base else f"/w/{resulting_ws_id}" - ws_name = "" - try: - ws = await async_directus.get_item("workspace", resulting_ws_id) - ws_name = (ws or {}).get("name", "") - except Exception: # noqa: BLE001 - pass - ws_name = ws_name or req.get("proposed_name") or "your workspace" + ws_name = workspace_name or req.get("proposed_name") or "your workspace" cadence_label = f"{approved_billing_period} billing" if approved_billing_period else None @@ -1042,10 +1037,11 @@ async def _upgrade_workspace_for_request( granted_tier_expires_at: Optional[str] = None, granted_type_discount: Optional[str] = None, granted_percent_discount: Optional[int] = None, -) -> str: +) -> tuple[str, str]: """Update an existing workspace's tier as part of approving a tier_upgrade request. - Reuses the tier-change logic from set_workspace_tier. + Reuses the tier-change logic from set_workspace_tier. Returns + (workspace_id, workspace_name) so callers can skip a re-fetch. """ from dembrane.policies import TIER_ORDER from dembrane.tier_downgrade import apply_downgrade_effects @@ -1115,7 +1111,7 @@ async def _upgrade_workspace_for_request( req["id"], staff_user_id, ) - return workspace_id + return workspace_id, workspace.get("name") or "" @router.patch( @@ -1126,6 +1122,7 @@ async def decide_workspace_request( request_id: str, body: DecideWorkspaceRequestBody, auth: DependencyDirectusSession, + background_tasks: BackgroundTasks, ) -> DecideWorkspaceRequestResponse: """Staff approve or deny a workspace request. @@ -1208,6 +1205,7 @@ async def decide_workspace_request( ) resulting_ws_id: str + resulting_ws_name: str = "" if req.get("kind") == "new_workspace": resulting_ws_id = await _create_workspace_for_request( req, @@ -1217,13 +1215,15 @@ async def decide_workspace_request( granted_type_discount=body.granted_type_discount, granted_percent_discount=body.granted_percent_discount, ) + # We just created it with this name — no need to re-fetch. + resulting_ws_name = (req.get("proposed_name") or "").strip() elif req.get("kind") == "tier_upgrade": if not req.get("workspace_id"): raise HTTPException( status_code=400, detail="tier_upgrade request missing workspace_id", ) - resulting_ws_id = await _upgrade_workspace_for_request( + resulting_ws_id, resulting_ws_name = await _upgrade_workspace_for_request( req, granted_tier=granted_tier, staff_user_id=staff_user_id, @@ -1250,10 +1250,13 @@ async def decide_workspace_request( await async_directus.update_item("workspace_request", request_id, extra_data) - await _notify_requester_approved( + # Off the request path: SendGrid alone adds ~300–1000ms. + background_tasks.add_task( + _notify_requester_approved, req, granted_tier, resulting_ws_id, + workspace_name=resulting_ws_name, approved_billing_period=approved_billing_period, proposed_billing_period=req.get("proposed_billing_period"), ) @@ -1273,7 +1276,11 @@ async def decide_workspace_request( staff_user_id, ) - await _notify_requester_denied(req, (body.denial_reason or "").strip()) + background_tasks.add_task( + _notify_requester_denied, + req, + (body.denial_reason or "").strip(), + ) return DecideWorkspaceRequestResponse( id=request_id, diff --git a/echo/server/dembrane/api/v2/workspace_requests.py b/echo/server/dembrane/api/v2/workspace_requests.py index e64a8523..de925238 100644 --- a/echo/server/dembrane/api/v2/workspace_requests.py +++ b/echo/server/dembrane/api/v2/workspace_requests.py @@ -3,7 +3,7 @@ from typing import Literal, Optional, Annotated from logging import getLogger -from fastapi import Depends, APIRouter, HTTPException +from fastapi import Depends, APIRouter, HTTPException, BackgroundTasks from pydantic import Field, BaseModel from dembrane.email import send_email @@ -68,10 +68,111 @@ class SubmitWorkspaceRequestResponse(BaseModel): kind: str +async def _notify_staff_workspace_request_submitted( + *, + req_id: str, + kind: str, + org_id: str, + workspace_id: Optional[str], + proposed_tier: str, + proposed_billing_period: Optional[str], + proposed_name: Optional[str], + requester_message: Optional[str], + requester_app_user_id: str, + requester_name: str, + requester_email: str, +) -> None: + """Fan out in-app notifications + emails to all staff. Best-effort — + runs off the request path so SendGrid latency doesn't block the response. + """ + from datetime import datetime, timezone + + from dembrane.email_throttle import queue_digest_item, record_and_check_throttle + + try: + org_name = "" + try: + org = await async_directus.get_item("org", org_id) + org_name = (org or {}).get("name", "") + except Exception: # noqa: BLE001 — best-effort + pass + + kind_label = "new workspace" if kind == "new_workspace" else "tier upgrade" + + tier_segment: str = proposed_tier + if proposed_billing_period: + tier_segment = f"{proposed_tier} · {proposed_billing_period}" + notification_message = f"{org_name} · {tier_segment}" if org_name else tier_segment + + staff_ids = await audience_staff() + await emit_to_audience( + staff_ids, + actor_user_id=requester_app_user_id, + event_code="WORKSPACE_REQUEST_SUBMITTED", + title=f"{requester_name} requested a {kind_label}", + message=notification_message, + action="NAVIGATE_ADMIN_UPGRADES", + ref_org_id=org_id, + ref_workspace_id=workspace_id, + ) + + settings = get_settings() + base = (settings.urls.admin_base_url or "").rstrip("/") + admin_url = f"{base}/admin/upgrades" + + staff_emails = await _resolve_emails(staff_ids) + if not staff_emails: + return + + summary = f"{requester_name} requested a {kind_label}" + if org_name: + summary += f" ({org_name} · {tier_segment})" + else: + summary += f" ({tier_segment})" + + for staff_email_addr in staff_emails: + decision = await record_and_check_throttle( + staff_email_addr, + "WORKSPACE_REQUEST_SUBMITTED", + ) + if decision == "individual": + await send_email( + to=staff_email_addr, + subject=f"Workspace request: {requester_name} · {kind_label}", + template="workspace_request_submitted", + template_data={ + "requester_name": requester_name, + "requester_email": requester_email, + "kind_label": kind_label, + "org_name": org_name, + "proposed_tier": proposed_tier, + "proposed_billing_period": proposed_billing_period, + "proposed_name": proposed_name, + "requester_message": requester_message, + "admin_url": admin_url, + }, + ) + else: + await queue_digest_item( + staff_email_addr, + { + "event_code": "WORKSPACE_REQUEST_SUBMITTED", + "summary": summary, + "timestamp": datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC"), + "admin_url": admin_url, + }, + ) + except Exception: # noqa: BLE001 — background side effect, never fail the caller + logger.exception( + "Background notify-staff failed for workspace_request %s", req_id + ) + + @router.post("", response_model=SubmitWorkspaceRequestResponse) async def submit_workspace_request( body: SubmitWorkspaceRequest, auth: DependencyDirectusSession, + background_tasks: BackgroundTasks, ) -> SubmitWorkspaceRequestResponse: """Submit a workspace request (new workspace or tier upgrade). @@ -205,86 +306,23 @@ async def submit_workspace_request( app_user_id, ) - # Notify all staff (in-app) + email - requester_name = app_user.get("display_name") or "A user" - requester_email = app_user.get("email") or "" - org_name = "" - try: - org = await async_directus.get_item("org", body.org_id) - org_name = (org or {}).get("name", "") - except Exception: # noqa: BLE001 — best-effort - pass - kind_label = "new workspace" if body.kind == "new_workspace" else "tier upgrade" - - # Notification message format: "{org_name} · {tier} · {cadence}" — cadence - # appended only when applicable (pioneer+). Pilot/free omit it so the line - # stays honest about what's actually billable. - tier_segment: str = body.proposed_tier - if body.proposed_billing_period: - tier_segment = f"{body.proposed_tier} · {body.proposed_billing_period}" - notification_message = f"{org_name} · {tier_segment}" if org_name else tier_segment - - staff_ids = await audience_staff() - await emit_to_audience( - staff_ids, - actor_user_id=app_user_id, - event_code="WORKSPACE_REQUEST_SUBMITTED", - title=f"{requester_name} requested a {kind_label}", - message=notification_message, - action="NAVIGATE_ADMIN_UPGRADES", - ref_org_id=body.org_id, - ref_workspace_id=body.workspace_id, + # Off the request path: N sequential Directus writes + N SendGrid calls + # added 2–5s of click latency when done inline. + background_tasks.add_task( + _notify_staff_workspace_request_submitted, + req_id=req_id, + kind=body.kind, + org_id=body.org_id, + workspace_id=body.workspace_id, + proposed_tier=body.proposed_tier, + proposed_billing_period=body.proposed_billing_period, + proposed_name=body.proposed_name, + requester_message=body.requester_message, + requester_app_user_id=app_user_id, + requester_name=app_user.get("display_name") or "A user", + requester_email=app_user.get("email") or "", ) - settings = get_settings() - base = (settings.urls.admin_base_url or "").rstrip("/") - admin_url = f"{base}/admin/upgrades" - - staff_emails = await _resolve_emails(staff_ids) - if staff_emails: - from datetime import datetime, timezone - - from dembrane.email_throttle import queue_digest_item, record_and_check_throttle - - summary = f"{requester_name} requested a {kind_label}" - if org_name: - summary += f" ({org_name} · {tier_segment})" - else: - summary += f" ({tier_segment})" - - for staff_email_addr in staff_emails: - decision = await record_and_check_throttle( - staff_email_addr, - "WORKSPACE_REQUEST_SUBMITTED", - ) - if decision == "individual": - await send_email( - to=staff_email_addr, - subject=f"Workspace request: {requester_name} · {kind_label}", - template="workspace_request_submitted", - template_data={ - "requester_name": requester_name, - "requester_email": requester_email, - "kind_label": kind_label, - "org_name": org_name, - "proposed_tier": body.proposed_tier, - "proposed_billing_period": body.proposed_billing_period, - "proposed_name": body.proposed_name, - "requester_message": body.requester_message, - "admin_url": admin_url, - }, - ) - else: - await queue_digest_item( - staff_email_addr, - { - "event_code": "WORKSPACE_REQUEST_SUBMITTED", - "summary": summary, - "timestamp": datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC"), - "admin_url": admin_url, - }, - ) - return SubmitWorkspaceRequestResponse( id=req_id, status="pending", diff --git a/echo/server/tests/test_approve_deny_action.py b/echo/server/tests/test_approve_deny_action.py index 7251e5ee..1d0af95b 100644 --- a/echo/server/tests/test_approve_deny_action.py +++ b/echo/server/tests/test_approve_deny_action.py @@ -14,6 +14,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from fastapi import BackgroundTasks from pydantic import ValidationError from dembrane.api.v2.admin import ( @@ -236,7 +237,8 @@ async def test_upgrade_tier(self): req, granted_tier="pioneer", staff_user_id="staff-1" ) - assert result == "ws-1" + # Helper now returns (workspace_id, workspace_name). + assert result[0] == "ws-1" update_call = mock_directus.update_item.call_args assert update_call[0][0] == "workspace" assert update_call[0][1] == "ws-1" @@ -388,7 +390,7 @@ async def test_approve_new_workspace_sets_fields(self): ) mock_directus.update_item = AsyncMock() - result = await decide_workspace_request("req-1", body, auth) + result = await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert result.status == "approved" assert result.resulting_workspace_id == "ws-new" @@ -419,7 +421,7 @@ async def test_approve_tier_upgrade(self): with ( patch("dembrane.api.v2.admin.async_directus") as mock_directus, patch("dembrane.app_user.get_app_user_or_raise", new_callable=AsyncMock, return_value={"id": "staff-1"}), - patch("dembrane.api.v2.admin._upgrade_workspace_for_request", new_callable=AsyncMock, return_value="ws-1") as mock_upgrade, + patch("dembrane.api.v2.admin._upgrade_workspace_for_request", new_callable=AsyncMock, return_value=("ws-1", "Existing WS")) as mock_upgrade, ): mock_directus.get_item = AsyncMock( side_effect=[ @@ -429,7 +431,7 @@ async def test_approve_tier_upgrade(self): ) mock_directus.update_item = AsyncMock() - result = await decide_workspace_request("req-1", body, auth) + result = await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert result.status == "approved" assert result.resulting_workspace_id == "ws-1" @@ -461,7 +463,7 @@ async def test_approve_defaults_to_proposed_tier(self): ) mock_directus.update_item = AsyncMock() - _result = await decide_workspace_request("req-1", body, auth) + _result = await decide_workspace_request("req-1", body, auth, BackgroundTasks()) call_kwargs = _mock_create.call_args assert call_kwargs[1]["granted_tier"] == "changemaker" @@ -486,7 +488,7 @@ async def test_already_decided_returns_409(self): mock_directus.get_item = AsyncMock(return_value=decided_req) with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 409 @pytest.mark.asyncio @@ -507,7 +509,7 @@ async def test_not_found_returns_404(self): mock_directus.get_item = AsyncMock(return_value=None) with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-missing", body, auth) + await decide_workspace_request("req-missing", body, auth, BackgroundTasks()) assert exc.value.status_code == 404 @pytest.mark.asyncio @@ -521,7 +523,7 @@ async def test_non_staff_returns_403(self): auth.is_admin = False with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 403 @pytest.mark.asyncio @@ -550,7 +552,7 @@ async def test_unknown_tier_returns_400(self): mock_directus.update_item = AsyncMock() with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 400 @pytest.mark.asyncio @@ -579,7 +581,7 @@ async def test_tier_upgrade_missing_workspace_id_returns_400(self): mock_directus.update_item = AsyncMock() with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 400 @@ -609,7 +611,7 @@ async def test_deny_sets_fields(self): ) mock_directus.update_item = AsyncMock() - result = await decide_workspace_request("req-1", body, auth) + result = await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert result.status == "denied" assert result.resulting_workspace_id is None @@ -642,7 +644,7 @@ async def test_deny_empty_reason_returns_400(self): mock_directus.get_item = AsyncMock(return_value=pending_req) with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 400 @pytest.mark.asyncio @@ -665,7 +667,7 @@ async def test_deny_no_reason_returns_400(self): mock_directus.get_item = AsyncMock(return_value=pending_req) with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 400 @pytest.mark.asyncio @@ -695,7 +697,7 @@ async def test_deny_with_staff_notes(self): ) mock_directus.update_item = AsyncMock() - _result = await decide_workspace_request("req-1", body, auth) + _result = await decide_workspace_request("req-1", body, auth, BackgroundTasks()) claim = mock_directus.update_item.call_args_list[0][0][2] assert claim["staff_notes"] == "Internal: check credit history" @@ -722,7 +724,7 @@ async def test_deny_already_decided_returns_409(self): mock_directus.get_item = AsyncMock(return_value=decided_req) with pytest.raises(HTTPException) as exc: - await decide_workspace_request("req-1", body, auth) + await decide_workspace_request("req-1", body, auth, BackgroundTasks()) assert exc.value.status_code == 409 diff --git a/echo/server/tests/test_request_notifications.py b/echo/server/tests/test_request_notifications.py index d5ab62aa..04831bcb 100644 --- a/echo/server/tests/test_request_notifications.py +++ b/echo/server/tests/test_request_notifications.py @@ -13,6 +13,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from fastapi import BackgroundTasks from dembrane.notifications import ( _SEVERITY_BY_EVENT, @@ -243,7 +244,10 @@ async def test_submit_fires_notification_to_staff(self): patch("dembrane.api.v2.workspace_requests.send_email", mock_send_email), patch("dembrane.api.v2.workspace_requests._resolve_emails", mock_resolve_emails), ): - result = await submit_workspace_request(body, auth) + bg = BackgroundTasks() + result = await submit_workspace_request(body, auth, bg) + # Drain the queue so the mocks see the side-effect calls. + await bg() assert result.status == "pending" @@ -297,7 +301,10 @@ async def test_submit_skips_email_when_no_staff_emails(self): patch("dembrane.api.v2.workspace_requests.send_email", mock_send_email), patch("dembrane.api.v2.workspace_requests._resolve_emails", mock_resolve_emails), ): - result = await submit_workspace_request(body, auth) + bg = BackgroundTasks() + result = await submit_workspace_request(body, auth, bg) + # Drain the queue so the mocks see the side-effect calls. + await bg() assert result.status == "pending" mock_emit.assert_called_once() @@ -342,7 +349,10 @@ async def test_submit_tier_upgrade_fires_notification(self): patch("dembrane.api.v2.workspace_requests.send_email", mock_send_email), patch("dembrane.api.v2.workspace_requests._resolve_emails", mock_resolve_emails), ): - result = await submit_workspace_request(body, auth) + bg = BackgroundTasks() + result = await submit_workspace_request(body, auth, bg) + # Drain the queue so the mocks see the side-effect calls. + await bg() assert result.status == "pending" emit_kwargs = mock_emit.call_args[1] @@ -361,10 +371,10 @@ async def test_approve_fires_notification_and_email(self): from dembrane.api.v2.admin import _notify_requester_approved mock_directus = AsyncMock() - mock_directus.get_item.side_effect = [ - {"name": "My Workspace"}, # workspace lookup - {"display_name": "Alice", "email": "alice@example.com"}, # requester info - ] + # Only the requester lookup now — workspace name comes from the caller. + mock_directus.get_item.return_value = { + "display_name": "Alice", "email": "alice@example.com", + } mock_emit = AsyncMock(return_value="n-1") mock_send_email = AsyncMock(return_value=True) @@ -404,10 +414,7 @@ async def test_approve_skips_email_when_no_requester_email(self): from dembrane.api.v2.admin import _notify_requester_approved mock_directus = AsyncMock() - mock_directus.get_item.side_effect = [ - {"name": "WS"}, - {"display_name": "Alice", "email": ""}, # no email - ] + mock_directus.get_item.return_value = {"display_name": "Alice", "email": ""} mock_emit = AsyncMock(return_value="n-1") mock_send_email = AsyncMock(return_value=True) @@ -447,10 +454,7 @@ async def test_approve_tier_upgrade_message(self): from dembrane.api.v2.admin import _notify_requester_approved mock_directus = AsyncMock() - mock_directus.get_item.side_effect = [ - {"name": "Upgraded WS"}, - {"display_name": "Bob", "email": "bob@x.com"}, - ] + mock_directus.get_item.return_value = {"display_name": "Bob", "email": "bob@x.com"} mock_emit = AsyncMock(return_value="n-1") mock_send_email = AsyncMock(return_value=True) @@ -468,7 +472,7 @@ async def test_approve_tier_upgrade_message(self): patch("dembrane.api.v2.admin.emit", mock_emit), patch("dembrane.api.v2.admin.send_email", mock_send_email), ): - await _notify_requester_approved(req, "changemaker", "ws-2") + await _notify_requester_approved(req, "changemaker", "ws-2", workspace_name="Upgraded WS") assert "tier upgrade" in mock_emit.call_args[1]["title"] assert "Upgraded WS" in mock_emit.call_args[1]["message"] @@ -762,14 +766,16 @@ async def test_approve_calls_notify_approved(self): patch("dembrane.api.v2.admin._create_workspace_for_request", mock_create_ws), patch("dembrane.api.v2.admin._notify_requester_approved", mock_notify_approved), ): - result = await decide_workspace_request("req-1", body, auth) + bg = BackgroundTasks() + result = await decide_workspace_request("req-1", body, auth, bg) + await bg() assert result.status == "approved" - mock_notify_approved.assert_called_once_with( - req_data, - "innovator", - "ws-new", - ) + mock_notify_approved.assert_called_once() + notify_args = mock_notify_approved.call_args + assert notify_args.args == (req_data, "innovator", "ws-new") + # workspace_name is threaded from req["proposed_name"] to skip a re-fetch. + assert notify_args.kwargs.get("workspace_name") == "WS" @pytest.mark.asyncio async def test_deny_calls_notify_denied(self): @@ -815,7 +821,9 @@ async def test_deny_calls_notify_denied(self): patch("dembrane.app_user.get_app_user_or_raise", mock_app_user), patch("dembrane.api.v2.admin._notify_requester_denied", mock_notify_denied), ): - result = await decide_workspace_request("req-1", body, auth) + bg = BackgroundTasks() + result = await decide_workspace_request("req-1", body, auth, bg) + await bg() assert result.status == "denied" mock_notify_denied.assert_called_once_with( diff --git a/echo/server/tests/test_workspace_requests.py b/echo/server/tests/test_workspace_requests.py index c0ad26aa..42f4fbbb 100644 --- a/echo/server/tests/test_workspace_requests.py +++ b/echo/server/tests/test_workspace_requests.py @@ -12,7 +12,7 @@ from unittest.mock import AsyncMock, patch import pytest -from fastapi import HTTPException +from fastapi import HTTPException, BackgroundTasks from pydantic import ValidationError from dembrane.api.v2.workspace_requests import ( @@ -114,7 +114,7 @@ async def test_requires_proposed_name(self): patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 400 assert "proposed_name" in str(exc_info.value.detail) @@ -134,7 +134,7 @@ async def test_org_admin_can_submit(self): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" assert result.kind == "new_workspace" mock_directus.create_item.assert_called_once() @@ -160,7 +160,7 @@ async def test_non_admin_rejected(self): patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 403 @pytest.mark.asyncio @@ -179,7 +179,7 @@ async def test_org_owner_can_submit(self): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" @@ -197,7 +197,7 @@ async def test_requires_workspace_id(self): patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 400 assert "workspace_id" in str(exc_info.value.detail) @@ -226,7 +226,7 @@ async def mock_get_items(collection, _query): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" assert result.kind == "tier_upgrade" @@ -251,7 +251,7 @@ async def mock_get_items(collection, _query): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" @pytest.mark.asyncio @@ -267,7 +267,7 @@ async def test_member_role_rejected(self): patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 403 @pytest.mark.asyncio @@ -290,7 +290,7 @@ async def mock_get_items(collection, _query): patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 409 @@ -327,7 +327,7 @@ async def test_overage_tier_without_cadence_returns_400(self, tier: str): patch("dembrane.api.v2.workspace_requests.async_directus", self._ok_directus()), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 400 assert "proposed_billing_period" in str(exc_info.value.detail) @@ -345,7 +345,7 @@ async def test_pilot_with_cadence_returns_400(self): patch("dembrane.api.v2.workspace_requests.async_directus", self._ok_directus()), ): with pytest.raises(HTTPException) as exc_info: - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert exc_info.value.status_code == 400 assert "pilot" in str(exc_info.value.detail) @@ -363,7 +363,7 @@ async def test_pilot_without_cadence_accepted(self): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value={"id": "au-1"}), patch("dembrane.api.v2.workspace_requests.async_directus", mock_d), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" row = mock_d.create_item.call_args[0][1] assert row["proposed_billing_period"] is None @@ -383,7 +383,7 @@ async def test_pioneer_with_cadence_accepted(self, cadence: str): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value={"id": "au-1"}), patch("dembrane.api.v2.workspace_requests.async_directus", mock_d), ): - result = await submit_workspace_request(body, _mock_auth()) + result = await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) assert result.status == "pending" row = mock_d.create_item.call_args[0][1] assert row["proposed_billing_period"] == cadence @@ -440,7 +440,7 @@ async def test_new_workspace_payload_shape(self): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) row = mock_directus.create_item.call_args[0][1] assert row["kind"] == "new_workspace" assert row["status"] == "pending" @@ -467,7 +467,7 @@ async def test_proposed_name_trimmed(self): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) row = mock_directus.create_item.call_args[0][1] assert row["proposed_name"] == "My Workspace" @@ -494,7 +494,7 @@ async def mock_get_items(collection, _query): patch("dembrane.api.v2.workspace_requests.get_app_user_or_raise", return_value=app_user), patch("dembrane.api.v2.workspace_requests.async_directus", mock_directus), ): - await submit_workspace_request(body, _mock_auth()) + await submit_workspace_request(body, _mock_auth(), BackgroundTasks()) row = mock_directus.create_item.call_args[0][1] assert row["workspace_id"] == "ws-1" assert row["kind"] == "tier_upgrade"