From b38ffd1f17bed0b2be8aa2e1226ea1ee332b4deb Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 13 Nov 2026 05:03:17 -0800 Subject: [PATCH] fix(cloud): EP-3b the audit log names a workspace's new owner from the team roster when the control plane cannot --- .../EnterpriseAuditLog.owner-name.test.tsx | 202 ++++++++++++++++++ .../enterprise/EnterpriseAuditLog.tsx | 24 ++- .../enterprise-audit-account-copy.ts | 11 +- .../enterprise/enterprise-audit-copy.ts | 39 +++- .../enterprise-audit-workspace-copy.ts | 13 +- 5 files changed, 277 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.owner-name.test.tsx diff --git a/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.owner-name.test.tsx b/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.owner-name.test.tsx new file mode 100644 index 00000000000..8cf55a14ae8 --- /dev/null +++ b/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.owner-name.test.tsx @@ -0,0 +1,202 @@ +// @vitest-environment happy-dom +// EP-3b: handing a workspace to a teammate must name them, even when the control plane cannot. +// It only knows people who have used Orca Cloud in the team; the team roster knows everyone. +import { cleanup, render, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import type { CloudAuditEvent } from '../../../../../shared/cloud-api/cloud-api-types' +import type { OrcaProfileAuthStatus } from '../../../../../shared/orca-profiles' +import { EnterpriseAuditLog } from './EnterpriseAuditLog' +import { FIXTURE_NOW, fixtureAuditProps } from './enterprise-audit.fixtures' +import { auditEventSentence, unnamedPersonIn } from './enterprise-audit-copy' + +const ROSTER = { + status: 'ok' as const, + roster: { + viewerRole: 'owner' as const, + canManageMembers: true, + members: [ + { + userId: 'u_ada', + email: 'ada@acme.dev', + displayName: 'Ada Lovelace', + role: 'owner' as const + }, + { + userId: 'u_member', + email: 'member@acme.dev', + displayName: 'Orca Staging Member', + role: 'member' as const + }, + { userId: 'u_noname', email: 'quiet@acme.dev', displayName: '', role: 'member' as const } + ], + pendingInvites: [] + } +} + +const SIGNED_IN: OrcaProfileAuthStatus = { + activeProfileId: 'p1', + configured: true, + state: 'connected', + persistence: 'encrypted', + cloud: { + cloudProfileId: 'cp', + userId: 'u_ada', + email: 'ada@acme.dev', + activeOrgId: 'org_ep3b', + linkedAt: 1 + }, + organizations: [{ orgId: 'org_ep3b', name: 'Acme', role: 'owner' }] +} + +function ownerChange(detail: Record, id = 'own_1'): CloudAuditEvent { + return { + id, + at: FIXTURE_NOW, + category: 'workspaces', + action: 'workspace.owner_changed', + actor: { kind: 'person', userId: 'u_ada', name: 'Ada Lovelace', via: null }, + target: { type: 'workspace', id: 'sbx_1', label: 'endpass-own' }, + detail + } +} + +const listed = (events: CloudAuditEvent[]) => + fixtureAuditProps({ + log: { + status: 'ok', + events, + hasMore: false, + loadingMore: false, + moreError: null, + refreshing: false + } + }) + +const sentences = (container: HTMLElement): string[] => + [...container.querySelectorAll('ul[aria-label="Audit log"] > li')].map( + (row) => row.children[0]?.textContent ?? '' + ) + +let orgMembersList: ReturnType + +beforeEach(() => { + orgMembersList = vi.fn(async () => ROSTER) + vi.stubGlobal('api', { orcaProfiles: { orgMembersList } }) + useAppStore.setState({ orcaProfileAuthStatus: SIGNED_IN }) +}) + +afterEach(() => { + cleanup() + vi.unstubAllGlobals() + useAppStore.setState({ orcaProfileAuthStatus: null }) +}) + +describe('the new owner of a workspace', () => { + it('is named from the team roster when the control plane sent no name', async () => { + // What dev sent on n46: the ids, no toUserName (the new owner had never used Orca Cloud). + const { container } = render( + + ) + // Before the roster answers: the sentence that needs no name, never an id. + expect(sentences(container)).toEqual(['Changed the owner of workspace “endpass-own”']) + await waitFor(() => + expect(sentences(container)).toEqual([ + 'Made Orca Staging Member the owner of workspace “endpass-own”' + ]) + ) + expect(orgMembersList).toHaveBeenCalledWith({ orgId: 'org_ep3b' }) + expect(container.textContent).not.toContain('u_member') + }) + + it('falls back to the email, then to the no-name sentence, and never shows a raw id', async () => { + const { container } = render( + + ) + await waitFor(() => + expect(sentences(container)).toEqual([ + 'Made quiet@acme.dev the owner of workspace “endpass-own”', + 'Changed the owner of workspace “endpass-own”' + ]) + ) + expect(container.textContent).not.toMatch(/u_noname|u_gone/) + }) + + it("keeps the control plane's name when it sent one, and does not read the roster for it", async () => { + const { container } = render( + + ) + expect(sentences(container)).toEqual([ + 'Made Grace Hopper the owner of workspace “endpass-own”', + 'Took ownership of workspace “endpass-own”', + 'Made a former member the owner of workspace “endpass-own”' + ]) + await Promise.resolve() + expect(orgMembersList).not.toHaveBeenCalled() + }) + + it('names the member a role change is about the same way', async () => { + const roleChange: CloudAuditEvent = { + id: 'role_1', + at: FIXTURE_NOW, + category: 'members', + action: 'member.role_changed', + actor: { kind: 'person', userId: 'u_ada', name: 'Ada Lovelace', via: null }, + target: { type: 'member', id: 'u_member', label: null }, + detail: { role: 'admin' } + } + const { container } = render() + await waitFor(() => + expect(sentences(container)).toEqual(['Changed the role of Orca Staging Member']) + ) + // A label from the control plane wins, and needs no roster read. + expect( + unnamedPersonIn({ ...roleChange, target: { type: 'member', id: 'u_member', label: 'Bo' } }) + ).toBeNull() + }) + + it('reads the roster only for rows that point at someone unnamed', () => { + expect(unnamedPersonIn(ownerChange({ fromUserId: 'u_ada', toUserId: 'u_member' }))).toBe( + 'u_member' + ) + expect( + unnamedPersonIn(ownerChange({ toUserId: 'u_member', toUserName: 'Grace Hopper' })) + ).toBeNull() + expect(unnamedPersonIn(ownerChange({ toUserId: 'u_ada' }))).toBeNull() + expect( + unnamedPersonIn({ + ...ownerChange({ toUserId: 'u_member' }), + action: 'workspace.made_private' + }) + ).toBeNull() + // The pure sentence takes any resolver. + expect( + auditEventSentence(ownerChange({ toUserId: 'u_member' }), undefined, undefined, () => 'Bo') + ).toBe('Made Bo the owner of workspace “endpass-own”') + }) +}) diff --git a/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.tsx b/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.tsx index e3eca575956..1d05257c39f 100644 --- a/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.tsx +++ b/src/renderer/src/components/settings/enterprise/EnterpriseAuditLog.tsx @@ -27,7 +27,9 @@ import { } from '../cloud-api/cloud-api-parts' import { SettingsGroup } from '../section-layout/SettingsGroup' import { EnterpriseUpsell, EnterpriseUpsellLine, type EnterpriseContact } from './EnterpriseUpsell' -import { auditCategoryLabel, auditEventSentence } from './enterprise-audit-copy' +import { auditCategoryLabel, auditEventSentence, unnamedPersonIn } from './enterprise-audit-copy' +import { useAppStore } from '@/store' +import { useOrgRoster } from '../../billing/team/use-org-roster' export type AuditLogFilters = { category: CloudAuditCategory | null; actor: string | null } @@ -214,10 +216,28 @@ function Filters({ ) } +/** + * Names teammates from the team roster (the directory the Team page uses), for sentences that + * point at someone the control plane could not name. Read only when a row on screen needs it. + */ +function useRosterNames(events: readonly CloudAuditEvent[]): (userId: string) => string | null { + const orgId = useAppStore((s) => s.orcaProfileAuthStatus?.cloud?.activeOrgId ?? null) + const needed = events.some((event) => unnamedPersonIn(event) !== null) + const roster = useOrgRoster(needed ? orgId : null) + const members = roster.result?.status === 'ok' ? roster.result.roster.members : null + return (userId) => { + const member = members?.find((candidate) => candidate.userId === userId) + return member?.displayName?.trim() || member?.email || null + } +} + +const NO_EVENTS: readonly CloudAuditEvent[] = [] + export function EnterpriseAuditLog(props: EnterpriseAuditLogProps): React.JSX.Element { const t = useTranslate() const locale = useIntlLocale() const { log, filters, exportState } = props + const nameOf = useRosterNames(log.status === 'ok' ? log.events : NO_EVENTS) const filtered = filters.category !== null || filters.actor !== null const canExport = log.status === 'ok' && log.events.length > 0 return ( @@ -347,7 +367,7 @@ export function EnterpriseAuditLog(props: EnterpriseAuditLogProps): React.JSX.El className="flex items-center gap-3 py-2 text-xs" > - {auditEventSentence(event, t, locale)} + {auditEventSentence(event, t, locale, nameOf)} {auditCategoryLabel(event.category, t)} diff --git a/src/renderer/src/components/settings/enterprise/enterprise-audit-account-copy.ts b/src/renderer/src/components/settings/enterprise/enterprise-audit-account-copy.ts index 568503e2864..e2d8b129c9d 100644 --- a/src/renderer/src/components/settings/enterprise/enterprise-audit-account-copy.ts +++ b/src/renderer/src/components/settings/enterprise/enterprise-audit-account-copy.ts @@ -16,8 +16,15 @@ export type ActionCopy = { t: Translate, detail: Detail, locale: string | undefined, - /** The target's name when known, and who did it. */ - about: { name: string | null; actorUserId: string | null } + /** + * The target's name when known, who did it, and a way to name another teammate the event + * points at by id (from the team roster) when the control plane could not. + */ + about: { + name: string | null + actorUserId: string | null + nameOf: (userId: string) => string | null + } ) => string | null } diff --git a/src/renderer/src/components/settings/enterprise/enterprise-audit-copy.ts b/src/renderer/src/components/settings/enterprise/enterprise-audit-copy.ts index 20683274996..03556b4edef 100644 --- a/src/renderer/src/components/settings/enterprise/enterprise-audit-copy.ts +++ b/src/renderer/src/components/settings/enterprise/enterprise-audit-copy.ts @@ -173,9 +173,22 @@ const ACTIONS: Record = { } /** The name to put in the sentence: what the control plane labelled the target, else a recorded name. */ -function targetName(event: CloudAuditEvent): string | null { +/** A teammate the row is about whom the control plane could not name (see unnamedPersonIn). */ +function unnamedMemberTarget(event: CloudAuditEvent): string | null { + const target = event.target + return target?.type === 'member' && target.id && !target.label?.trim() ? target.id : null +} + +function targetName( + event: CloudAuditEvent, + nameOf: (userId: string) => string | null +): string | null { const recorded = event.detail?.name - const name = event.target?.label ?? (typeof recorded === 'string' ? recorded : null) + const member = unnamedMemberTarget(event) + const name = + event.target?.label ?? + (typeof recorded === 'string' ? recorded : null) ?? + (member ? nameOf(member) : null) const clean = name?.trim() return clean ? clean.slice(0, 80) : null } @@ -184,15 +197,17 @@ function targetName(event: CloudAuditEvent): string | null { export function auditEventSentence( event: CloudAuditEvent, t: Translate = translate, - locale?: string + locale?: string, + /** Names a teammate by id from the team roster; the control plane only knows people who have used Orca Cloud. */ + nameOf: (userId: string) => string | null = () => null ): string { const copy = ACTIONS[event.action] if (!copy) { return event.action } - const name = targetName(event) + const name = targetName(event, nameOf) const detailed = event.detail - ? copy.detailed?.(t, event.detail, locale, { name, actorUserId: event.actor.userId }) + ? copy.detailed?.(t, event.detail, locale, { name, actorUserId: event.actor.userId, nameOf }) : null if (detailed) { return detailed @@ -204,3 +219,17 @@ export function auditEventSentence( export function isKnownAuditAction(action: string): boolean { return action in ACTIONS } + +/** + * The teammate a row names in its sentence whom the control plane could not name (it only knows + * people who have used Orca Cloud in this team), so the roster is read only when a row needs it. + */ +export function unnamedPersonIn(event: CloudAuditEvent): string | null { + if (event.action !== 'workspace.owner_changed') { + // A role change or removal whose member the control plane could not name. + return unnamedMemberTarget(event) + } + const owner = event.detail?.toUserId + const named = typeof event.detail?.toUserName === 'string' && event.detail.toUserName.trim() + return typeof owner === 'string' && !named && owner !== event.actor.userId ? owner : null +} diff --git a/src/renderer/src/components/settings/enterprise/enterprise-audit-workspace-copy.ts b/src/renderer/src/components/settings/enterprise/enterprise-audit-workspace-copy.ts index 273c703709a..ce830c19e17 100644 --- a/src/renderer/src/components/settings/enterprise/enterprise-audit-workspace-copy.ts +++ b/src/renderer/src/components/settings/enterprise/enterprise-audit-workspace-copy.ts @@ -82,7 +82,7 @@ export const WORKSPACE_AUDIT_ACTIONS: Record = { ), // Says who got it: themselves, a teammate by name, or someone who has since left. With no // name to give, the plain sentences above are used. The previous owner is left out. - detailed: (t, detail, _locale, { name, actorUserId }) => { + detailed: (t, detail, _locale, { name, actorUserId, nameOf }) => { const owner = typeof detail.toUserId === 'string' ? detail.toUserId : null if (owner && owner === actorUserId) { return name @@ -107,8 +107,15 @@ export const WORKSPACE_AUDIT_ACTIONS: Record = { 'Made a former member the owner of a workspace' ) } - const person = - typeof detail.toUserName === 'string' ? detail.toUserName.trim().slice(0, 80) : '' + // The control plane names people who have used Orca Cloud in this team; a teammate who has + // not is named from the team roster, like the person column. + const person = ( + (typeof detail.toUserName === 'string' ? detail.toUserName : '') || + (owner ? nameOf(owner) : null) || + '' + ) + .trim() + .slice(0, 80) if (!person) { return null }