diff --git a/apps/sim/app/o/[organizationId]/layout.tsx b/apps/sim/app/o/[organizationId]/layout.tsx
index 7ff0b566493..081b2b437a2 100644
--- a/apps/sim/app/o/[organizationId]/layout.tsx
+++ b/apps/sim/app/o/[organizationId]/layout.tsx
@@ -1,4 +1,5 @@
import { createLogger } from '@sim/logger'
+import { getErrorMessage } from '@sim/utils/errors'
import { dehydrate, HydrationBoundary } from '@tanstack/react-query'
import { cookies } from 'next/headers'
import { redirect } from 'next/navigation'
@@ -65,8 +66,20 @@ export default async function OrganizationLayout({
if (!context) {
return
}
- const peopleEnabled = await canBrowsePeople(organizationId, context.viewer.role)
- if (!context.mothershipAvailable && !context.searchAccess.memberScoped && !peopleEnabled)
+ const peopleEnabledLookup = canBrowsePeople(organizationId, context.viewer.role).catch(
+ (error) => {
+ logger.warn('Could not resolve People navigation access', {
+ organizationId,
+ error: getErrorMessage(error),
+ })
+ return false
+ }
+ )
+ if (
+ !context.mothershipAvailable &&
+ !context.searchAccess.memberScoped &&
+ !(await peopleEnabledLookup)
+ )
redirect(WORKSPACE_SETTINGS_PATH)
const [
@@ -79,6 +92,7 @@ export default async function OrganizationLayout({
changelogEnabled,
projectsEnabled,
simLinksEnabled,
+ peopleEnabled,
] = await Promise.all([
prefetchOrganizationSidebar(
queryClient,
@@ -97,6 +111,7 @@ export default async function OrganizationLayout({
isChangelogEnabled(organizationId),
isProjectsEnabled(),
isSimLinksEnabled(organizationId),
+ peopleEnabledLookup,
])
const initialSidebarCollapsed = cookieStore.get('sidebar_collapsed')?.value === '1'
diff --git a/apps/sim/app/o/[organizationId]/people/[memberId]/person-modal.tsx b/apps/sim/app/o/[organizationId]/people/[memberId]/person-modal.tsx
index 0573dab5afa..ff5fa4f383b 100644
--- a/apps/sim/app/o/[organizationId]/people/[memberId]/person-modal.tsx
+++ b/apps/sim/app/o/[organizationId]/people/[memberId]/person-modal.tsx
@@ -43,11 +43,14 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
department: person.department,
location: person.location,
startDate: person.startDate,
- managerMemberId: person.manager?.memberId ?? null,
+ managerMemberId: person.managerMemberId,
})
const [errors, setErrors] = useState>>({})
const [managerSearch, setManagerSearch] = useState('')
- /** The chosen manager's name, kept with the choice: the search results may no longer include them. */
+ /**
+ * The chosen manager's name, kept with the choice: the search results may no longer include them.
+ * Only a stored manager who is suspended has an id and no name.
+ */
const [managerName, setManagerName] = useState(person.manager?.name ?? null)
const candidates = usePeople(
organizationId,
@@ -64,7 +67,7 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
label: `${candidate.name} (${candidate.email})`,
})),
]
- const managerLabel = (draft.managerMemberId && managerName) || 'No manager'
+ const managerLabel = draft.managerMemberId ? (managerName ?? 'Suspended member') : 'No manager'
const change = (field: K, value: PersonBody[K]) => {
setDraft((current) => ({ ...current, [field]: value }))
diff --git a/apps/sim/lib/api/contracts/people.ts b/apps/sim/lib/api/contracts/people.ts
index 7c904ec4104..4d0d1c88b20 100644
--- a/apps/sim/lib/api/contracts/people.ts
+++ b/apps/sim/lib/api/contracts/people.ts
@@ -24,7 +24,10 @@ const personSchema = personSummarySchema.extend({
location: z.string().nullable(),
/** `YYYY-MM-DD`. */
startDate: z.string().nullable(),
+ /** The manager as shown; `null` when there is none or they are suspended. */
manager: personSummarySchema.nullable(),
+ /** The stored manager link, kept while that manager is suspended. */
+ managerMemberId: z.string().nullable(),
joinedAt: z.iso.datetime(),
})
export type Person = z.output
diff --git a/apps/sim/lib/people/application/use-cases.ts b/apps/sim/lib/people/application/use-cases.ts
index 289aae0ea68..f2a481b46ad 100644
--- a/apps/sim/lib/people/application/use-cases.ts
+++ b/apps/sim/lib/people/application/use-cases.ts
@@ -79,8 +79,9 @@ export const getPerson = defineAuthorizedOrganizationUseCase({
})
/**
- * Replaces one member's profile. The manager must be another active member of the same
- * organization who doesn't already report, directly or not, to this person.
+ * Replaces one member's profile. A new manager must be another active member of the same
+ * organization who doesn't already report, directly or not, to this person. The stored manager,
+ * sent back unchanged, is kept even while suspended, as an import that leaves it out keeps it.
*/
export const updatePerson = defineAuthorizedOrganizationUseCase({
operation: peopleOperations.update,
@@ -93,20 +94,19 @@ export const updatePerson = defineAuthorizedOrganizationUseCase({
}
return db.transaction(async (tx) => {
await repository.lockProfiles(tx, organizationId)
- const active = await repository.findActiveMembers(
- tx,
- organizationId,
- managerMemberId ? [memberId, managerMemberId] : [memberId]
- )
- if (!active.has(memberId)) throw new OrchestrationError('not_found', 'Person not found')
- if (managerMemberId && !active.has(managerMemberId)) {
- throw new OrchestrationError(
- 'validation',
- 'The manager must be a member of this organization'
- )
- }
- if (managerMemberId && (await repository.isInReportingChain(tx, memberId, managerMemberId))) {
- throw new OrchestrationError('validation', 'That manager reports to this person')
+ const current = await repository.getPerson(tx, organizationId, memberId)
+ if (!current) throw new OrchestrationError('not_found', 'Person not found')
+ if (managerMemberId && managerMemberId !== current.managerMemberId) {
+ const active = await repository.findActiveMembers(tx, organizationId, [managerMemberId])
+ if (!active.has(managerMemberId)) {
+ throw new OrchestrationError(
+ 'validation',
+ 'The manager must be a member of this organization'
+ )
+ }
+ if (await repository.isInReportingChain(tx, memberId, managerMemberId)) {
+ throw new OrchestrationError('validation', 'That manager reports to this person')
+ }
}
await repository.upsertProfiles(tx, [{ memberId, ...fields }], context.userId)
const person = await repository.getPerson(tx, organizationId, memberId)
diff --git a/apps/sim/lib/people/repository.ts b/apps/sim/lib/people/repository.ts
index e8584b6c0e5..05f91db2936 100644
--- a/apps/sim/lib/people/repository.ts
+++ b/apps/sim/lib/people/repository.ts
@@ -27,6 +27,7 @@ export interface PersonRecord extends PersonSummary {
location: string | null
startDate: string | null
manager: PersonSummary | null
+ managerMemberId: string | null
joinedAt: Date
}
@@ -96,7 +97,10 @@ async function summariesById(
return new Map(rows.map((row) => [row.memberId, row]))
}
-/** Joins each person to their manager, read once for the whole page. */
+/**
+ * Joins each person to their manager, read once for the whole page. A suspended manager stays
+ * linked by `managerMemberId` but has no summary, since no read lists suspended people.
+ */
async function withManagers(
executor: DbOrTx,
organizationId: string,
@@ -104,9 +108,9 @@ async function withManagers(
): Promise {
const managerIds = [...new Set(rows.flatMap((row) => row.managerMemberId ?? []))]
const managers = await summariesById(executor, organizationId, managerIds)
- return rows.map(({ sortName, managerMemberId, ...row }) => ({
+ return rows.map(({ sortName, ...row }) => ({
...row,
- manager: managerMemberId ? (managers.get(managerMemberId) ?? null) : null,
+ manager: row.managerMemberId ? (managers.get(row.managerMemberId) ?? null) : null,
}))
}
diff --git a/apps/sim/scripts/test-people-e2e.ts b/apps/sim/scripts/test-people-e2e.ts
index abacc77cc56..73edb777113 100644
--- a/apps/sim/scripts/test-people-e2e.ts
+++ b/apps/sim/scripts/test-people-e2e.ts
@@ -86,7 +86,10 @@ async function person(member: Member, memberId: string) {
const result = await api(member, `/${memberId}`, {})
assert(isRecordLike(result.person), 'Expected a person')
return result as {
- person: Record & { manager: { memberId: string } | null }
+ person: Record & {
+ manager: { memberId: string } | null
+ managerMemberId: string | null
+ }
chain: { memberId: string }[]
reports: { memberId: string }[]
}
@@ -297,6 +300,62 @@ async function run() {
assert.equal(row?.manager_member_id, owner.memberId)
})
+ await check(
+ "an edit keeps a suspended manager it doesn't change, and refuses one as a new manager",
+ async () => {
+ const margaret = await seedMember(orgId, 'member', 'Margaret')
+ const edit = (memberId: string, body: Record, expected = 200) =>
+ api(owner, `/${memberId}`, { method: 'PUT', body, expected })
+ await edit(ada.memberId, {
+ title: 'Engineer',
+ department: 'Platform',
+ location: 'Berlin, Germany',
+ startDate: '2021-03-04',
+ managerMemberId: margaret.memberId,
+ })
+ await sql`update "user" set suspended_at = now() where id = ${margaret.userId}`
+
+ const { person: shown } = await person(owner, ada.memberId)
+ assert.equal(shown.manager, null, 'A suspended manager is not shown')
+ assert.equal(shown.managerMemberId, margaret.memberId, 'The stored link is still reported')
+ const saved = await edit(ada.memberId, {
+ title: 'Staff Engineer',
+ department: shown.department,
+ location: shown.location,
+ startDate: shown.startDate,
+ managerMemberId: shown.managerMemberId,
+ })
+ assert(isRecordLike(saved.person), 'Expected a person')
+ assert.equal(saved.person.managerMemberId, margaret.memberId)
+ const row = await profileRow(ada.memberId)
+ assert.equal(row?.title, 'Staff Engineer')
+ assert.equal(row?.manager_member_id, margaret.memberId)
+
+ const refused = await edit(
+ linus.memberId,
+ {
+ title: 'Principal Engineer',
+ department: null,
+ location: null,
+ startDate: '2019-07-01',
+ managerMemberId: margaret.memberId,
+ },
+ 400
+ )
+ assert.equal(refused.error, 'The manager must be a member of this organization')
+ assert.equal((await profileRow(linus.memberId))?.manager_member_id, owner.memberId)
+
+ await edit(ada.memberId, {
+ title: 'Staff Engineer',
+ department: shown.department,
+ location: shown.location,
+ startDate: shown.startDate,
+ managerMemberId: owner.memberId,
+ })
+ assert.equal((await profileRow(ada.memberId))?.manager_member_id, owner.memberId)
+ }
+ )
+
await check(
'leaving the organization removes the profile and clears reporting lines',
async () => {
@@ -308,7 +367,7 @@ async function run() {
)
await check('imports and edits are audited', async () => {
- const expected = { 'org_people.imported': 3, 'org_member.profile_updated': 1 }
+ const expected = { 'org_people.imported': 3, 'org_member.profile_updated': 4 }
let counts: Record = {}
for (let attempt = 0; attempt < 20; attempt++) {
const rows = await sql<