From 654679eced6e8f7ac038434da758e0ab771c4940 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Mon, 7 Sep 2026 22:09:31 -0400 Subject: [PATCH 01/13] #666 redesign edit position page as one page with autosave Replaces the three-tab edit position page with a single max-w-5xl scroll of SectionCards: status moves out of the form and into header actions (mirroring the application detail page), and every field autosaves on blur through four narrow server actions instead of one whole-form submit. Also fixes managers staying editable on an archived position. Co-Authored-By: Claude Sonnet 4.6 --- .../manage/positions/[id]/edit/loading.tsx | 62 ++-- .../manage/positions/[id]/edit/page.tsx | 267 +++++++++------ components/features/markdown-field.tsx | 25 +- components/features/pipeline-summary.tsx | 19 +- .../position-applications-summary.tsx | 56 ++++ .../position-availability-section.tsx | 178 ++++++++++ components/features/position-danger-zone.tsx | 10 +- components/features/position-details-form.tsx | 311 ------------------ .../features/position-details-readonly.tsx | 65 ---- .../features/position-details-section.tsx | 153 +++++++++ components/features/position-edit-tabs.tsx | 49 --- .../features/position-managers-readonly.tsx | 49 +++ .../features/position-questions-section.tsx | 4 +- .../position-status-header-actions.tsx | 181 ++++++++++ components/ui/section-card.tsx | 10 +- docs/DESIGN.md | 4 +- docs/PERMISSIONS.md | 15 +- docs/WORKFLOWS.md | 44 +-- lib/constants.ts | 136 ++++++-- lib/use-autosave.ts | 89 +++++ prisma/actions/position-actions.ts | 223 +++++++++---- tests/db/position-archive.test.ts | 59 +++- tests/db/position-field-saves.test.ts | 174 ++++++++++ tests/db/position-publish-permission.test.ts | 64 ++-- tests/db/position-transitions.test.ts | 63 ++-- tests/unit/constants.test.ts | 53 +++ tests/unit/position-transitions.test.ts | 79 +++-- 27 files changed, 1644 insertions(+), 798 deletions(-) create mode 100644 components/features/position-applications-summary.tsx create mode 100644 components/features/position-availability-section.tsx delete mode 100644 components/features/position-details-form.tsx delete mode 100644 components/features/position-details-readonly.tsx create mode 100644 components/features/position-details-section.tsx delete mode 100644 components/features/position-edit-tabs.tsx create mode 100644 components/features/position-managers-readonly.tsx create mode 100644 components/features/position-status-header-actions.tsx create mode 100644 lib/use-autosave.ts create mode 100644 tests/db/position-field-saves.test.ts diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx index 9ed4519b..fb336b86 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx @@ -1,52 +1,36 @@ +import { SectionCardSkeleton } from '@/components/ui/section-card'; import { Skeleton } from '@/components/ui/skeleton'; export default function EditPositionLoading() { return ( -
- {/* PageHeader skeleton: back link + title + description */} +
+ {/* PageHeader skeleton: back link + title + badge, one action bar */}
- - -
- -
- {/* Tab bar skeleton */} -
- - - -
- - {/* Tab content skeleton — loose shape fits both the editable form and the archived read-only view */} -
-
- - -
-
- - +
+
+ +
-
- - -
-
- - -
-
- - -
- +
+
- {/* Danger zone skeleton — admin-only, shown speculatively to avoid layout shift */} -
- + {/* Details */} + + {/* Availability */} + + {/* Managers */} + + {/* Applications summary */} + + {/* Questions */} + + + {/* Danger zone skeleton — compact, admin-only, shown speculatively to avoid layout shift */} +
+
diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx index 66682150..9bdaf63f 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -18,15 +18,20 @@ import { isPositionActive, } from '@/lib/utils'; +import { PositionApplicationsSummary } from '@/components/features/position-applications-summary'; +import { PositionAvailabilitySection } from '@/components/features/position-availability-section'; import { PositionDangerZone } from '@/components/features/position-danger-zone'; -import { PositionDetailsForm } from '@/components/features/position-details-form'; -import { PositionDetailsReadonly } from '@/components/features/position-details-readonly'; -import { PositionEditTabs } from '@/components/features/position-edit-tabs'; +import { PositionDetailsSection } from '@/components/features/position-details-section'; +import { PositionManagersReadonly } from '@/components/features/position-managers-readonly'; import { PositionManagersSection } from '@/components/features/position-managers-section'; import { PositionQuestionsReadonly } from '@/components/features/position-questions-readonly'; import { PositionQuestionsSection } from '@/components/features/position-questions-section'; +import { PositionStatusHeaderActions } from '@/components/features/position-status-header-actions'; +import { PositionStatusBadge } from '@/components/features/status-badge'; import { PageHeader } from '@/components/layouts/page-header'; import { LocalTime } from '@/components/ui/local-time'; +import { Markdown } from '@/components/ui/markdown'; +import { SectionCard } from '@/components/ui/section-card'; import { WarningCallout } from '@/components/ui/warning-callout'; interface EditPositionPageProps { @@ -55,6 +60,8 @@ export default async function EditPositionPage({ const user = await requireListedManagerOr404(position.managers); const canEdit = user.isAdmin || isPositionActive(position); + const closesAtPast = + position.closesAt !== null && position.closesAt < new Date(); const staleCloseDate = isOpenPastCloseDate(position) ? position.closesAt : null; @@ -65,25 +72,79 @@ export default async function EditPositionPage({ : null; const draftPastOpenDate = draftPastDate?.label === 'Was scheduled to open'; - const deletionSummary = user.isAdmin - ? await getPositionDeletionSummary(position.id) - : null; + const [deletionSummary, stats] = await Promise.all([ + user.isAdmin ? getPositionDeletionSummary(position.id) : null, + getPositionApplicationStats([position.id]), + ]); - // One groupBy, reused by the archived callout and the close confirmation. - const stats = await getPositionApplicationStats([position.id]); + // One groupBy, reused by the archived callout and the applications summary. const counts = stats.get(position.id)?.counts ?? {}; + const total = stats.get(position.id)?.total ?? 0; const unresolvedTotal = UNRESOLVED_APPLICATION_STATUSES.reduce( (sum, status) => sum + (counts[status] ?? 0), 0, ); + const availabilityWarnings = (staleCloseDate || draftPastDate) && ( +
+ {staleCloseDate && ( + +
+

+ Applicants see this position as Closed. +

+

+ Its close date passed on{' '} + , so it + stopped accepting applications even though its status is still + Open. Give it a future close date to reopen it, or choose Close to + make that explicit. +

+
+ + )} + {draftPastDate && ( + +
+

+ This position was scheduled to{' '} + {draftPastOpenDate ? 'open' : 'close'}. +

+

+ Its {draftPastOpenDate ? 'open' : 'close'} date passed on{' '} + , but + it's still a draft, so applicants can't see it. Give it + a future {draftPastOpenDate ? 'open' : 'close'} date, or{' '} + {user.isAdmin + ? 'choose Publish to open it now.' + : 'ask an admin to publish it.'} +

+
+ + )} +
+ ); + return ( -
+
} + actions={ + canEdit ? ( + + ) : undefined + } /> {!canEdit && ( @@ -112,103 +173,117 @@ export default async function EditPositionPage({ )} - - {staleCloseDate && ( - -
-

- Applicants see this position as Closed. -

-

- Its close date passed on{' '} - - , so it stopped accepting applications even though - its status is still Open. Change Closes At below to - a future date to reopen it, or set Status to Closed - to make that explicit. -

-
- - )} - {draftPastDate && ( - -
-

- This position was scheduled to{' '} - {draftPastOpenDate ? 'open' : 'close'}. -

-

- Its {draftPastOpenDate ? 'open' : 'close'} date - passed on{' '} - - , but its status is still Draft, so applicants - can't see it. Change{' '} - {draftPastOpenDate ? 'Opens At' : 'Closes At'} below - to a future date, or set Status to Open to publish - it now. -

-
- - )} -
- ) + +
+ {canEdit ? ( + + ) : ( +
+
+

Title

+

{position.title}

+
+
+

Description

+ {position.description ? ( + + ) : ( +

+ No description +

+ )} +
+
+ )} +
+ + + +
+ {canEdit ? ( + ) : ( - +
+
Opens
+
+ {position.opensAt ? ( + + ) : ( + Not set + )} +
+
+
+
Closes
+
+ {position.closesAt ? ( + + ) : ( + Not set + )} +
+
+ + )} +
+ + + +
+ {canEdit ? ( + - ) - } - questionsContent={ - canEdit ? ( + ) : ( + + )} +
+ + + +
+ +
+ + + +
+ {canEdit ? ( ) : ( - ) - } - managersContent={ - - } - /> + )} +
+ {deletionSummary && ( (); - const isSubmitting = formState.isSubmitting; +interface MarkdownFieldProps { + disabled?: boolean; + footer?: ReactNode; + onCommit?: (value: string) => void; +} + +// Context is typed as the minimal shape both consumers actually share — the +// create dialog's full PositionFormValues structurally satisfies it too. +export function MarkdownField({ + disabled, + footer, + onCommit, +}: MarkdownFieldProps) { + const { control, formState } = useFormContext<{ description: string }>(); + const isSubmitting = disabled ?? formState.isSubmitting; const [mode, setMode] = useState<'write' | 'preview'>('write'); const textareaRef = useRef(null); @@ -368,6 +380,10 @@ export function MarkdownField() { }} onPaste={handlePaste} onKeyDown={handleKeyDown} + onBlur={(e) => { + field.onBlur(); + onCommit?.(e.target.value); + }} rows={10} disabled={isSubmitting} placeholder="Describe the role, responsibilities, and what you're looking for. Markdown is supported." @@ -415,6 +431,7 @@ export function MarkdownField() { )} + {footer} ); }} diff --git a/components/features/pipeline-summary.tsx b/components/features/pipeline-summary.tsx index 126c3784..cd248bf2 100644 --- a/components/features/pipeline-summary.tsx +++ b/components/features/pipeline-summary.tsx @@ -1,7 +1,7 @@ -import { $Enums } from '@/prisma/client'; import { getApplicationStatusCounts } from '@/prisma/data/applications'; import { + APPLICATION_PIPELINE_STATUSES, APPLICATION_STATUS_BADGE_VARIANT, APPLICATION_STATUS_LABELS, STATUS_BADGE_VARIANT_TO_DOT, @@ -13,16 +13,6 @@ import { StatCard } from '@/components/features/stat-card'; import { Card, CardContent } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -// Statuses surfaced in the pipeline summary (excludes draft). -const PIPELINE_STATUSES = [ - $Enums.ApplicationStatus.applied, - $Enums.ApplicationStatus.reached_out, - $Enums.ApplicationStatus.interview_scheduled, - $Enums.ApplicationStatus.reviewing, - $Enums.ApplicationStatus.accepted, - $Enums.ApplicationStatus.rejected, -] as const; - interface PipelineSummaryProps { reviewer: Reviewer; } @@ -30,7 +20,10 @@ interface PipelineSummaryProps { export async function PipelineSummary({ reviewer }: PipelineSummaryProps) { const counts = await getApplicationStatusCounts(reviewer); - const total = PIPELINE_STATUSES.reduce((sum, s) => sum + (counts[s] ?? 0), 0); + const total = APPLICATION_PIPELINE_STATUSES.reduce( + (sum, s) => sum + (counts[s] ?? 0), + 0, + ); return (
@@ -43,7 +36,7 @@ export async function PipelineSummary({ reviewer }: PipelineSummaryProps) { className="col-span-2 md:col-span-1" /> - {PIPELINE_STATUSES.map((status) => { + {APPLICATION_PIPELINE_STATUSES.map((status) => { const count = counts[status] ?? 0; const variant = APPLICATION_STATUS_BADGE_VARIANT[status]; const dotClass = STATUS_BADGE_VARIANT_TO_DOT[variant]; diff --git a/components/features/position-applications-summary.tsx b/components/features/position-applications-summary.tsx new file mode 100644 index 00000000..60ce8eda --- /dev/null +++ b/components/features/position-applications-summary.tsx @@ -0,0 +1,56 @@ +import type { $Enums } from '@/prisma/client'; + +import { + APPLICATION_PIPELINE_STATUSES, + APPLICATION_STATUS_BADGE_VARIANT, + APPLICATION_STATUS_LABELS, + STATUS_BADGE_VARIANT_TO_DOT, +} from '@/lib/constants'; +import { CONCEPT_ICONS } from '@/lib/icons'; + +import { StatCard } from '@/components/features/stat-card'; +import { SectionCardEmpty } from '@/components/ui/section-card'; + +interface PositionApplicationsSummaryProps { + counts: Partial>; + total: number; +} + +// getPositionApplicationStats is already fetched for this page to compute +// one number for a callout — this surfaces the full breakdown, nearly free. +export function PositionApplicationsSummary({ + counts, + total, +}: PositionApplicationsSummaryProps) { + if (total === 0) + return ( + + ); + + return ( +
+ + {APPLICATION_PIPELINE_STATUSES.map((status) => { + const count = counts[status] ?? 0; + const variant = APPLICATION_STATUS_BADGE_VARIANT[status]; + return ( + + ); + })} +
+ ); +} diff --git a/components/features/position-availability-section.tsx b/components/features/position-availability-section.tsx new file mode 100644 index 00000000..2816be55 --- /dev/null +++ b/components/features/position-availability-section.tsx @@ -0,0 +1,178 @@ +'use client'; + +import type { ReactNode } from 'react'; +import { useRef } from 'react'; +import { useForm } from 'react-hook-form'; + +import { updatePositionSchedule } from '@/prisma/actions/position-actions'; + +import { + POSITION_CLOSES_AT_ORDER_ERROR, + POSITION_OPENS_AT_ORDER_ERROR, + positionPastDateIssues, +} from '@/lib/constants'; +import { toOrgDayString } from '@/lib/dates'; +import { autosaveStatusText, useAutosave } from '@/lib/use-autosave'; +import { ActionError, isError } from '@/lib/utils'; + +import { + Form, + FormControl, + FormDescription, + FormField, + FormItem, + FormLabel, + FormMessage, +} from '@/components/ui/form'; +import { Input } from '@/components/ui/input'; + +interface ScheduleValues { + opensAt: string; + closesAt: string; +} + +interface PositionAvailabilitySectionProps { + positionId: string; + opensAt: string | null; + closesAt: string | null; + // Server-rendered warning callout(s) about a status/date divergence. + warnings?: ReactNode; +} + +// No zodResolver — the pair saves together, so validation runs inside the +// commit handler against the loaded row's previous values, mirroring +// validatePositionDates/positionPastDateIssues (lib/constants.ts) rather than +// a schema built for a single combined submit. +export function PositionAvailabilitySection({ + positionId, + opensAt, + closesAt, + warnings, +}: PositionAvailabilitySectionProps) { + const initial: ScheduleValues = { + opensAt: opensAt ?? '', + closesAt: closesAt ?? '', + }; + const form = useForm({ defaultValues: initial }); + // The last-saved pair — positionPastDateIssues only flags a date actually + // changed since this, so an untouched past date stays saveable. + const lastSavedRef = useRef(initial); + + const scheduleAutosave = useAutosave({ + initialValue: initial, + save: async (value: ScheduleValues) => { + const result = await updatePositionSchedule({ + id: positionId, + opensAt: value.opensAt || undefined, + closesAt: value.closesAt || undefined, + }); + if (isError(result)) throw new ActionError(result.error); + lastSavedRef.current = value; + }, + }); + + function validate(values: ScheduleValues): boolean { + form.clearErrors(); + let valid = true; + + if (values.opensAt && values.closesAt && values.opensAt > values.closesAt) { + form.setError('opensAt', { message: POSITION_OPENS_AT_ORDER_ERROR }); + form.setError('closesAt', { message: POSITION_CLOSES_AT_ORDER_ERROR }); + valid = false; + } + + for (const issue of positionPastDateIssues( + values, + toOrgDayString(new Date()), + lastSavedRef.current, + )) { + form.setError(issue.path, { message: issue.message }); + valid = false; + } + + return valid; + } + + function handleFieldBlur() { + const values = form.getValues(); + if (validate(values)) scheduleAutosave.commit(values); + } + + const statusText = autosaveStatusText( + scheduleAutosave.status, + scheduleAutosave.error, + ); + + return ( +
+ + {warnings} + +
+ ( + + Opens At + + { + field.onBlur(); + handleFieldBlur(); + }} + /> + + + Applications open at 12:00 AM Eastern on this day. + + + + )} + /> + + ( + + Closes At + + { + field.onBlur(); + handleFieldBlur(); + }} + /> + + + Applications close at 11:59 PM Eastern on this day. + + + + )} + /> +
+ + {statusText && ( +

+ {statusText} +

+ )} +
+ + ); +} diff --git a/components/features/position-danger-zone.tsx b/components/features/position-danger-zone.tsx index e95d6855..47d41657 100644 --- a/components/features/position-danger-zone.tsx +++ b/components/features/position-danger-zone.tsx @@ -44,11 +44,13 @@ export function PositionDangerZone({ const blockedReasonId = `delete-position-blocked-${positionId}`; return ( - - - Delete Position + + + +

Delete Position

+ - +

Deleting hides this position everywhere — the positions list, search results and any direct link. This can't be undone from the app. diff --git a/components/features/position-details-form.tsx b/components/features/position-details-form.tsx deleted file mode 100644 index a11a194d..00000000 --- a/components/features/position-details-form.tsx +++ /dev/null @@ -1,311 +0,0 @@ -'use client'; - -import type { ReactNode } from 'react'; -import { useMemo, useState } from 'react'; -import { useForm, useWatch } from 'react-hook-form'; - -import { zodResolver } from '@hookform/resolvers/zod'; -import { toast } from 'sonner'; - -import { updatePosition } from '@/prisma/actions/position-actions'; -import type { PositionStatus } from '@/prisma/client'; - -import { - POSITION_DRAFT_CLOSE_HINT, - POSITION_OPEN_REQUIRES_ADMIN_HINT, - POSITION_REOPEN_PAST_CLOSE_HINT, - POSITION_UNPUBLISH_BLOCKED_HINT, - type PositionFormValues, - getPositionStatusOptions, - makePositionFormSchema, -} from '@/lib/constants'; -import { toOrgDayString } from '@/lib/dates'; -import { ACTION_ICONS } from '@/lib/icons'; - -import { MarkdownField } from '@/components/features/markdown-field'; -import { Button } from '@/components/ui/button'; -import { ConfirmDialog } from '@/components/ui/confirm-dialog'; -import { - Form, - FormControl, - FormDescription, - FormField, - FormItem, - FormLabel, - FormMessage, -} from '@/components/ui/form'; -import { Input } from '@/components/ui/input'; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from '@/components/ui/select'; - -interface PositionDetailsFormProps { - position: { - id: string; - title: string; - description: string; - status: PositionStatus; - opensAt: string | null; - closesAt: string | null; - }; - isAdmin: boolean; - hasApplications: boolean; - unresolvedApplicationCount: number; - // Server-rendered warning callout(s) about a status/date divergence, shown - // right under the Status field so they read as one status-related section. - statusNotice?: ReactNode; -} - -function closeConfirmDescription(count: number): string { - const application = count === 1 ? 'application is' : 'applications are'; - return `${count} ${application} still in progress. Closing stops new applications; the ones you have stay reviewable.`; -} - -const REOPEN_CONFIRM_DESCRIPTION = - 'This position becomes listed and applyable again. Existing applications and decisions are unchanged.'; - -// Always visible, not dialog-triggered: shadcn Form primitives directly, no FormDialog. -export function PositionDetailsForm({ - position, - isAdmin, - hasApplications, - unresolvedApplicationCount, - statusNotice, -}: PositionDetailsFormProps) { - const schema = useMemo( - () => - makePositionFormSchema(toOrgDayString(new Date()), { - opensAt: position.opensAt ?? undefined, - closesAt: position.closesAt ?? undefined, - }), - [position.opensAt, position.closesAt], - ); - - const form = useForm({ - resolver: zodResolver(schema), - defaultValues: { - title: position.title, - description: position.description, - status: position.status, - opensAt: position.opensAt ?? '', - closesAt: position.closesAt ?? '', - }, - }); - const isSubmitting = form.formState.isSubmitting; - - const [pendingValues, setPendingValues] = useState( - null, - ); - const [isSaving, setIsSaving] = useState(false); - const disabled = isSubmitting || isSaving; - - const watchedClosesAt = useWatch({ control: form.control, name: 'closesAt' }); - const closesAtPast = - !!watchedClosesAt && watchedClosesAt < toOrgDayString(new Date()); - - const statusOptions = getPositionStatusOptions(isAdmin, position.status, { - hasApplications, - closesAtPast, - }); - - // Precedence: reopen-past-close -> unpublish-blocked -> draft-close. - const transitionHint = - position.status === 'closed' && closesAtPast - ? POSITION_REOPEN_PAST_CLOSE_HINT - : hasApplications && position.status !== 'draft' - ? POSITION_UNPUBLISH_BLOCKED_HINT - : position.status === 'draft' - ? POSITION_DRAFT_CLOSE_HINT - : null; - const showAdminHint = !isAdmin && position.status !== 'open'; - - async function save(data: PositionFormValues) { - setIsSaving(true); - try { - const result = await updatePosition({ - id: position.id, - ...data, - opensAt: data.opensAt || undefined, - closesAt: data.closesAt || undefined, - }); - - if (result && 'error' in result) { - toast.error(result.error); - } else { - toast.success('Position updated'); - } - } catch (error) { - console.error(error); - toast.error('Something went wrong. Please try again.'); - } finally { - setIsSaving(false); - setPendingValues(null); - } - } - - function onSubmit(data: PositionFormValues) { - const needsCloseConfirm = - position.status === 'open' && - data.status === 'closed' && - unresolvedApplicationCount > 0; - const needsReopenConfirm = - position.status === 'closed' && data.status === 'open'; - - if (needsCloseConfirm || needsReopenConfirm) { - setPendingValues(data); - return; - } - void save(data); - } - - const confirmMove: 'close' | 'reopen' | null = - pendingValues === null - ? null - : pendingValues.status === 'closed' - ? 'close' - : 'reopen'; - - return ( -

- void form.handleSubmit(onSubmit)(e)} - className="flex flex-col gap-4" - > - ( - - Title - - - - - - )} - /> - - - - ( - - Status - - {(showAdminHint || transitionHint) && ( - - {showAdminHint && ( - - {POSITION_OPEN_REQUIRES_ADMIN_HINT} - - )} - {transitionHint && ( - {transitionHint} - )} - - )} - - - )} - /> - - {statusNotice} - -
- ( - - Opens At - - - - - Applications open at 12:00 AM Eastern on this day. - - - - )} - /> - - ( - - Closes At - - - - - Applications close at 11:59 PM Eastern on this day. - - - - )} - /> -
- -
- -
- - - { - if (!open && !isSaving) setPendingValues(null); - }} - title={ - confirmMove === 'close' - ? 'Close this position?' - : 'Reopen this position?' - } - description={ - confirmMove === 'close' - ? closeConfirmDescription(unresolvedApplicationCount) - : REOPEN_CONFIRM_DESCRIPTION - } - confirmLabel={ - confirmMove === 'close' ? 'Close position' : 'Reopen position' - } - pendingLabel={confirmMove === 'close' ? 'Closing…' : 'Reopening…'} - isPending={isSaving} - onConfirm={() => { - if (pendingValues) void save(pendingValues); - }} - /> - - ); -} diff --git a/components/features/position-details-readonly.tsx b/components/features/position-details-readonly.tsx deleted file mode 100644 index bc4134b7..00000000 --- a/components/features/position-details-readonly.tsx +++ /dev/null @@ -1,65 +0,0 @@ -import type { PositionStatus } from '@/prisma/client'; - -import { PositionStatusBadge } from '@/components/features/status-badge'; -import { LocalTime } from '@/components/ui/local-time'; - -interface PositionDetailsReadonlyProps { - position: { - title: string; - description: string; - status: PositionStatus; - opensAt: Date | null; - closesAt: Date | null; - }; -} - -export function PositionDetailsReadonly({ - position, -}: PositionDetailsReadonlyProps) { - return ( -
-
-
Title
-
{position.title}
-
- -
-
Description
-
- {position.description || ( - No description - )} -
-
- -
-
Status
-
- -
-
- -
-
Opens
-
- {position.opensAt ? ( - - ) : ( - Not set - )} -
-
- -
-
Closes
-
- {position.closesAt ? ( - - ) : ( - Not set - )} -
-
-
- ); -} diff --git a/components/features/position-details-section.tsx b/components/features/position-details-section.tsx new file mode 100644 index 00000000..342b35c0 --- /dev/null +++ b/components/features/position-details-section.tsx @@ -0,0 +1,153 @@ +'use client'; + +import { useForm } from 'react-hook-form'; + +import { zodResolver } from '@hookform/resolvers/zod'; +import { z } from 'zod/v4'; + +import { + updatePositionDescription, + updatePositionTitle, +} from '@/prisma/actions/position-actions'; + +import { + positionDescriptionSchema, + positionTitleSchema, +} from '@/lib/constants'; +import { autosaveStatusText, useAutosave } from '@/lib/use-autosave'; +import { ActionError, isError } from '@/lib/utils'; + +import { MarkdownField } from '@/components/features/markdown-field'; +import { + Form, + FormControl, + FormDescription, + FormField, + FormItem, + FormLabel, + FormMessage, +} from '@/components/ui/form'; +import { Input } from '@/components/ui/input'; + +const detailsSchema = z.object({ + title: positionTitleSchema, + description: positionDescriptionSchema, +}); +type DetailsValues = z.infer; + +interface PositionDetailsSectionProps { + positionId: string; + title: string; + description: string; +} + +export function PositionDetailsSection({ + positionId, + title, + description, +}: PositionDetailsSectionProps) { + const form = useForm({ + resolver: zodResolver(detailsSchema), + defaultValues: { title, description }, + mode: 'onBlur', + }); + + const titleAutosave = useAutosave({ + initialValue: title, + save: async (value: string) => { + const result = await updatePositionTitle({ + id: positionId, + title: value, + }); + if (isError(result)) throw new ActionError(result.error); + }, + }); + + const descriptionAutosave = useAutosave({ + initialValue: description, + save: async (value: string) => { + const result = await updatePositionDescription({ + id: positionId, + description: value, + }); + if (isError(result)) throw new ActionError(result.error); + }, + }); + + return ( +
+ + { + const statusText = autosaveStatusText( + titleAutosave.status, + titleAutosave.error, + ); + return ( + + Title + + { + field.onBlur(); + const value = e.target.value; + void (async () => { + const valid = await form.trigger('title'); + if (valid) titleAutosave.commit(value); + })(); + }} + /> + + + {statusText && ( + + {statusText} + + )} + + ); + }} + /> + + { + void (async () => { + const valid = await form.trigger('description'); + if (valid) descriptionAutosave.commit(value); + })(); + }} + footer={(() => { + const statusText = autosaveStatusText( + descriptionAutosave.status, + descriptionAutosave.error, + ); + return statusText ? ( +

+ {statusText} +

+ ) : undefined; + })()} + /> + + + ); +} diff --git a/components/features/position-edit-tabs.tsx b/components/features/position-edit-tabs.tsx deleted file mode 100644 index 8beaf263..00000000 --- a/components/features/position-edit-tabs.tsx +++ /dev/null @@ -1,49 +0,0 @@ -'use client'; - -import type { ReactNode } from 'react'; - -import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; - -interface PositionEditTabsProps { - detailsContent: ReactNode; - questionsContent: ReactNode; - managersContent: ReactNode | null; -} - -export function PositionEditTabs({ - detailsContent, - questionsContent, - managersContent, -}: PositionEditTabsProps) { - return ( - - - - Details - - - Questions - - {managersContent && ( - - Managers - - )} - - - - {detailsContent} - - - - {questionsContent} - - - {managersContent && ( - - {managersContent} - - )} - - ); -} diff --git a/components/features/position-managers-readonly.tsx b/components/features/position-managers-readonly.tsx new file mode 100644 index 00000000..4c7471e3 --- /dev/null +++ b/components/features/position-managers-readonly.tsx @@ -0,0 +1,49 @@ +import type { PositionManager } from '@/lib/types'; +import { getUserName } from '@/lib/utils'; + +interface PositionManagersReadonlyProps { + managers: PositionManager[]; +} + +// Archived twin of PositionManagersSection — same rows, no remove button and +// no search box, since membership stays admin-only to change once archived. +export function PositionManagersReadonly({ + managers, +}: PositionManagersReadonlyProps) { + return ( +
+ {managers.length === 0 && ( +

No managers assigned.

+ )} + + {managers.length > 0 && ( +
    + {managers.map((manager) => { + const managerName = getUserName(manager); + return ( +
  • +
    +

    + {managerName ?? manager.email} +

    + {managerName && ( +

    + {manager.email} +

    + )} +
    +
    • + ); + })} +
+ )} + +

+ Managers can't be changed while this position is archived. +

+
+ ); +} diff --git a/components/features/position-questions-section.tsx b/components/features/position-questions-section.tsx index 5cd18d33..a2f3367e 100644 --- a/components/features/position-questions-section.tsx +++ b/components/features/position-questions-section.tsx @@ -163,7 +163,7 @@ export function PositionQuestionsSection({ > {editingId === question.id ? ( -

Edit Question

+

Edit Question

-

Add Question

+

Add Question

( + null, + ); + const [confirmTarget, setConfirmTarget] = useState( + null, + ); + + const targets = getPositionTransitionTargets(isAdmin, currentStatus, { + hasApplications, + closesAtPast, + }); + + function performMove(target: PositionStatus) { + setPendingTarget(target); + startTransition(async () => { + try { + const result = await updatePositionStatus({ + id: positionId, + status: target, + }); + if (isError(result)) { + toast.error(result.error); + return; + } + const action = POSITION_TRANSITION_ACTIONS[currentStatus][target]; + toast.success(action?.successToast ?? 'Position updated'); + } catch (error) { + console.error(error); + toast.error('Something went wrong. Please try again.'); + } finally { + setPendingTarget(null); + setConfirmTarget(null); + } + }); + } + + if (targets.length === 0) { + // Precedence: non-admin draft -> reopen-past-close -> unpublish-blocked. + const note = + currentStatus === 'draft' && !isAdmin + ? POSITION_PUBLISH_REQUIRES_ADMIN_NOTE + : currentStatus === 'closed' && closesAtPast + ? POSITION_REOPEN_PAST_CLOSE_HINT + : hasApplications + ? POSITION_UNPUBLISH_BLOCKED_HINT + : null; + return note ? ( +

{note}

+ ) : null; + } + + const [primaryTarget, ...restTargets] = targets; + const primaryAction = primaryTarget + ? POSITION_TRANSITION_ACTIONS[currentStatus][primaryTarget] + : undefined; + const confirmAction = confirmTarget + ? POSITION_TRANSITION_ACTIONS[currentStatus][confirmTarget] + : undefined; + + if (!primaryTarget || !primaryAction) return null; + + return ( + <> +
+ + {restTargets.length > 0 && ( + + + + + + {restTargets.map((target) => { + const action = + POSITION_TRANSITION_ACTIONS[currentStatus][target]; + if (!action) return null; + return ( + setConfirmTarget(target)} + > + {action.label} + + ); + })} + + + )} +
+ + { + if (!open && !isPending) setConfirmTarget(null); + }} + title={confirmAction?.confirmTitle ?? ''} + description={ + confirmAction?.confirmDescription({ unresolvedApplicationCount }) ?? + '' + } + confirmLabel={confirmAction?.confirmLabel ?? ''} + pendingLabel={confirmAction?.pendingLabel ?? ''} + isPending={isPending} + onConfirm={() => { + if (confirmTarget) performMove(confirmTarget); + }} + /> + + ); +} diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index 0656a8b9..c8388eb6 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -124,7 +124,8 @@ type SectionCardSkeletonRowShape = | 'badge-meta' | 'stacked-action' | 'timeline' - | 'badge-stacked'; + | 'badge-stacked' + | 'form-field'; interface SectionCardSkeletonProps { rows?: number; @@ -217,6 +218,13 @@ function SectionCardSkeletonRow({
); + case 'form-field': + return ( +
+ + +
+ ); default: { const exhaustiveCheck: never = shape; return exhaustiveCheck; diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 5d5a67f2..e8fb8a0d 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -54,8 +54,8 @@ Any change to a brand/status token must keep ≥4.5:1 contrast against its paire | Tier | Container classes | Use for | | -------------- | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ | | **Full-bleed** | none (no `max-w`) | list, table and dashboard pages — `/`, `/positions`, `/manage/positions`, `/users`, `/manage/applications`, `/applications`, `/global-questions` | - | **Wide** | `mx-auto max-w-5xl` | two-column review/detail pages — `/manage/applications/[id]`, `/applications/[id]` | - | **Narrow** | `mx-auto max-w-2xl` | single-column forms and reading views — `/profile`, `/positions/[id]/apply`, `/manage/positions/[id]/edit` | + | **Wide** | `mx-auto max-w-5xl` | two-column review/detail pages — `/manage/applications/[id]`, `/applications/[id]`, `/manage/positions/[id]/edit` | + | **Narrow** | `mx-auto max-w-2xl` | single-column forms and reading views — `/profile`, `/positions/[id]/apply` | There is no fourth tier: `max-w-6xl`/`4xl`/`3xl` on a page container is a bug. Inside a full-bleed page, constraining an individual prose block (`max-w-2xl` on a description) is correct and not a tier violation — see `/positions/[id]`. A route's `loading.tsx` must use the same tier as its `page.tsx`, or the skeleton shifts on resolve. Routes outside the app shell (`(legal)`, `login`) set their own width. diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index d0494ad4..eaad33c5 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -18,7 +18,7 @@ Four principals, each derived rather than stored as a single role field: - **Action guards throw** (`requireAdmin`, `requireManagerOrAdmin`, `requirePositionAccess`, `requireOwnership`); **page guards call `notFound()`** (`requireAdminOr404`, `requireManagerOrAdminOr404`, `requireListedManagerOr404`) — an authenticated-but-not-permitted page renders identically to a missing one. `lib/auth/guards.ts`'s own header states the intent: "Action guards throw; page guards 404, so a denial never leaks existence." - The `denyWith` helper in `lib/auth/guards.ts` (`const denyWith = (message) => () => { throw new Error(message); }`) is why a guard and its `*Or404` twin cannot drift — a new authorization branch adds a `Deny` variant there, never an inline `if (!user.isAdmin)`. - `getOptionalManagerAccess` is the public-page twin: it uses `getOptionalUser()`, never forces auth, and returns `{ user, canManage }` — an anonymous visitor and a signed-in non-manager get the identical `notFound()` from the caller. -- **Authenticate before touching the DB.** `updatePosition`, `addPositionManager`, `removePositionManager` all call `getCurrentUser()` first, then the stale-link existence check, then the position guard — an existence check placed ahead of the guard would turn the action into an anonymous existence oracle. +- **Authenticate before touching the DB.** The four position-field actions (via `authorizePositionEdit`), `addPositionManager`, and `removePositionManager` all call `getCurrentUser()` first, then the stale-link existence check, then the position guard — an existence check placed ahead of the guard would turn the action into an anonymous existence oracle. - **A denial is never `{ error }`.** `{ error }` is for state the user can act on (`ARCHIVED_POSITION_EDIT_ERROR`, `POSITION_DELETE_BLOCKED_ERROR`); `throw` is for authorization (the decision test in `ENGINEERING.md` §4). Client call sites `catch` the throw and toast a generic message, never the thrown text. - **`redirect()` is routing, not denial** — three cases, named so they aren't mis-read as gates: the sign-in bounce (`getCurrentUser` for an anonymous caller), the name gate (`requireName`, redirecting to `/login` with a `redirectTo`), and a resource-state bounce (e.g. the apply page redirecting to `/positions` when the position no longer exists). @@ -61,9 +61,10 @@ Four principals, each derived rather than stored as a single role field: | `applications.ts` — `updateApplicationStatuses` (bulk) | authorization **folded into the `updateMany` where** (`buildApplicationScopeWhere(user)` + `status: { notIn: [...NON_REVIEWABLE_APPLICATION_STATUSES, status] }`) | any reviewer status but the target is eligible — forward, backward, or a final decision. When the target is `accepted`/`rejected`, the same status-change permission now also sends every affected applicant a decision email **immediately and irreversibly** — there is no email-specific gate, so a manager's bulk send behaves exactly like an admin's (`docs/WORKFLOWS.md` XC-9) | `{ updated, skipped }` — a skip count, never an error, for rows outside the caller's scope, `draft`/`withdrawn`, or already at the target | | `question-files.ts` — file answer actions | `getCurrentUser()` + `authorizeTarget` (ownership miss throws) | gated on `APPLICANT_EDITABLE_APPLICATION_STATUSES` (`draft`\|`withdrawn`) in **both** the pre-check and the in-transaction `findFirst`, matching the text-answer path | `{ error: APPLICATION_NOT_EDITABLE_MESSAGE }` | | `position-actions.ts` — `createPosition`, `searchUsers` | `requireManagerOrAdmin()` | creation is always `draft`, regardless of what's posted; at least one manager is required, and the caller is connected only if they're in that list | throw; `{ error: POSITION_MANAGERS_REQUIRED_ERROR }` for zero managers | -| `position-actions.ts` — `updatePosition` | `getCurrentUser()` → existence check → `requirePositionAccess(id)` → `checkPositionEditable` | archived positions rejected even for their own manager; a non-admin moving the stored status **to** `open` is refused (an already-`open` position stays freely editable) | `{ error: ARCHIVED_POSITION_EDIT_ERROR }` / `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }` | +| `position-actions.ts` — `updatePositionTitle`, `updatePositionDescription`, `updatePositionSchedule` | `getCurrentUser()` → existence check → `requirePositionAccess(id)` → `checkPositionEditable` | archived positions rejected even for their own manager; each is a single-field write, scoped by the shared `authorizePositionEdit` helper | `{ error: ARCHIVED_POSITION_EDIT_ERROR }` | +| `position-actions.ts` — `updatePositionStatus` | `getCurrentUser()` → existence check → `requirePositionAccess(id)` → `checkPositionEditable` | archived positions rejected even for their own manager; a non-admin moving the stored status **to** `open` is refused (an already-`open` position stays freely editable); `closesAtPast` is read from the stored row, never a submitted value | `{ error: ARCHIVED_POSITION_EDIT_ERROR }` / `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }` | | `position-actions.ts` — `deletePosition` | `requireAdmin()` | the "has applications" guard is **folded into the `updateMany` where** (`applications: { none: { deletedAt: null, status: { not: 'draft' } } }`), so check-and-write are atomic | `{ error: POSITION_DELETE_BLOCKED_ERROR }` or `{ error: 'no longer exists' }` | -| `position-actions.ts` — `addPositionManager`, `removePositionManager` | `getCurrentUser()` → existence check → `requirePositionAccess(positionId)` | `removePositionManager` blocks non-admin self-removal | `{ error: 'You cannot remove yourself as a manager. Ask an admin to do it.' }` | +| `position-actions.ts` — `addPositionManager`, `removePositionManager` | `getCurrentUser()` → existence check → `requirePositionAccess(positionId)` → `checkPositionEditable` | archived positions reject even their own manager (reversed from the previous policy — see [Archive](#archive)); `removePositionManager` also blocks non-admin self-removal | `{ error: ARCHIVED_POSITION_EDIT_ERROR }` / `{ error: 'You cannot remove yourself as a manager. Ask an admin to do it.' }` | | `position-question-actions.ts` — `createPositionQuestion`, `updatePositionQuestion`, `reorderPositionQuestions`, `deletePositionQuestion` | `requirePositionAccess(positionId)` → `checkPositionEditable` | `updatePositionQuestion` / `deletePositionQuestion` scope their write to `{ id, positionId }` to prevent cross-position IDOR | `{ error: ARCHIVED_POSITION_EDIT_ERROR }` | | `global-questions.ts` — all four actions | `requireAdmin()` | — | throw | | `users.ts` — `toggleUserAdmin` | `requireAdmin()` | self-guard: cannot change own admin role | `{ error: 'You cannot change your own admin role.' }`; throw if the scoped update hits 0 rows | @@ -115,13 +116,13 @@ Four principals, each derived rather than stored as a single role field: | Title, description | ✓ | ✓ | ✓ | ✗ | | `opensAt` / `closesAt` | ✓ | ✓ | ✓ | ✗ | | Questions (add, edit, delete) | ✓ | ✓ | ✓ | ✗ | -| Managers (add, remove others) | ✓ | ✓ | ✓ | ✓ | +| Managers (add, remove others) | ✓ | ✓ | ✓ | ✗ | | Status → `open` | ✗ (admin only) | ✓ (already open — no-op) | ✗ (admin only) | ✗ | | Status → `closed` | ✗ (never) | ✓ | ✓ (no-op) | ✗ | | Status → `draft` | ✓ | only with no applications | only with no applications | ✗ | | Delete the position | admin only | admin only | admin only | admin only | -Managers stay editable on an archived position: membership is how an admin hands oversight off, and `addPositionManager` / `removePositionManager` deliberately skip `checkPositionEditable`. +Managers lose access to membership on an archived position, same as every other field: `addPositionManager` / `removePositionManager` run `checkPositionEditable` like the rest of the edit surface. An admin still short-circuits the check, so oversight can always be handed off by an admin. ## Guardrails instead of freezes @@ -137,8 +138,8 @@ Confirmations carry the risk the freezes don't. - **Archived is derived, never stored.** `isPositionActive` (`lib/utils.ts`) is the single source of truth, fed by `positionActivitySelect` / `withPositionActivity` (`prisma/data/positions.ts`). A second implementation is an authorization bug, not a display bug. - A position is archived once it is closed (`status: 'closed'`, or `open` past `closesAt`) for more than `MANAGED_POSITIONS_WINDOW_DAYS` **and** no application status has changed in that same window. Unresolved applications no longer pin a position active indefinitely — only recent activity does. - **There is no manual archive or unarchive.** A position leaves archive when an admin reopens it with a future close date, or — since a status change is itself activity — the moment a reviewer moves one of its applications, admin or not. -- Managers are denied with `ARCHIVED_POSITION_EDIT_ERROR` (`lib/constants.ts`), returned by `updatePosition` and the three position-question actions. Admins short-circuit the check inside `checkPositionEditable` (`prisma/data/positions.ts`). -- **The edit page does not 404.** It renders `PositionDetailsReadonly` / `PositionQuestionsReadonly` behind an explanatory callout, so a manager can still read what they can no longer change. +- Managers are denied with `ARCHIVED_POSITION_EDIT_ERROR` (`lib/constants.ts`), returned by the four position-field actions (`updatePositionTitle`, `updatePositionDescription`, `updatePositionSchedule`, `updatePositionStatus`), the three position-question actions, and `addPositionManager`/`removePositionManager`. Admins short-circuit the check inside `checkPositionEditable` (`prisma/data/positions.ts`). +- **The edit page does not 404.** It renders Details/Availability as read-only text, `PositionQuestionsReadonly`, and `PositionManagersReadonly` behind an explanatory callout, so a manager can still read what they can no longer change. ## Manager vs admin diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index 03604235..f7121ffc 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -418,27 +418,29 @@ A user who manages at least one non-deleted position. Manager status is **derive ### PM-4 Edit position details -- **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`, Details tab). -- **Happy path** — the page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. `PositionDetailsForm` submits `updatePosition`, which authenticates, checks existence, checks access, then checks editability. Toast **"Position updated"**; the position, its detail page, the dashboard, `/applications` and `/manage/applications` are all revalidated because a status flip changes what every surface shows. The Status select is filtered by `getPositionStatusOptions` to moves both the role and the transition rules allow, so an option is never offered if the server would reject it — a manager on a `draft` position sees only **Draft** (disabled, nothing else is legal); on `open` or `closed`, **Open** stays admin-only. Closing (`open → closed`) with unresolved applications, and reopening (`closed → open`), each show a `ConfirmDialog` before the write; every other move saves immediately. +- **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`). +- **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: Details, Availability, Managers, an Applications summary, Questions, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. + - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. Admin · `draft` → **Publish**. Manager · `draft` → no button, a muted note: "Only an admin can publish this position." `open`, no applications → **Close applications** + caret → **Return to draft**; with applications, **Close applications** alone. Admin · `closed`, close date future or unset → **Reopen** + caret → **Return to draft**; manager · `closed` → **Return to draft** only. No legal move at all → the muted note alone. Archived → no header actions. Toasts: **"Position published"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. - **Failure / edge** - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. - Not a listed manager and not an admin → `notFound()`. - - Archived (closed >30 days with no application status change since) and the caller is not an admin → the form is replaced by `PositionDetailsReadonly` under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications. A stale tab that posts anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) - - A manager posting a transition **to** `open` from `draft` or `closed` (stale tab, hand-made request) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** The form keeps its values so they can pick Draft or Closed and resubmit. - - `draft → closed`, any caller (stale tab — the select never offers it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`; the select shows a hint explaining why Closed is absent. - - `open → draft` or `closed → draft` once any non-deleted application exists, at any status including `draft` (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; the select shows a hint and drops Draft from the list the moment an application exists. - - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; the select drops Open and shows a hint until `closesAt` is cleared or moved to the future — live, without a save. - - `opensAt` or `closesAt` **changed** to a date before today → **"The open date must be today or later."** / **"The close date must be today or later."**, checked against the position's own previous dates. An untouched past date on an already-open position saves normally — the rule only fires on a field the manager actually changed. - - Deleted between render and submit → **"This position no longer exists."** - - Unexpected throw → **"Something went wrong. Please try again."** - - Status `open` with a `closesAt` already past → below the Status field the form shows a warning callout ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or set status to Closed); the Status select still shows Open — displaying anything else would misrepresent what's stored. - - Status `draft` with an `opensAt` or `closesAt` already past → the same spot below the Status field shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or set Status to Open to publish now). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses), but both render from the same status-notice slot. -- **Confirmations** — **Close** (`open → closed`, only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable."; Cancel leaves the row untouched. **Reopen** (`closed → open`, always): "This position becomes listed and applyable again. Existing applications and decisions are unchanged." Both trap focus, Escape cancels (suppressed while saving), and focus returns to Save on close. -- **End state** — the position's details, status and window are updated; only an admin's draft/closed→open flip publishes it, a draft can never become closed directly, unpublishing is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. + - Archived (closed >30 days with no application status change since) and the caller is not an admin → Details and Availability render as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) + - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header never offers it) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** + - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. + - `open → draft` or `closed → draft` once any non-deleted application exists, at any status including `draft` (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. + - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it first) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; **Reopen** drops out of the header and the muted note explains why, reading the position's stored `closesAt` — never a value from the request. + - `opensAt` or `closesAt` **changed** to a date before today → the same past-date messages as [PM-3](#pm-3-create-a-position), checked against the position's own previous dates. An untouched past date on an already-open position saves normally — the rule only fires on a date the manager actually changed — and a partially-typed date never fires the check mid-keystroke. + - An empty title on blur → "Title is required" inline; nothing saves, the header keeps showing the last-saved title. + - Deleted between render and an autosave → **"This position no longer exists."**, shown inline on the field, not a toast. + - Unexpected throw during an autosave → the generic toast; a header-action throw does the same. + - Status `open` with a `closesAt` already past → a warning callout at the top of the Availability card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close applications** to make that explicit). + - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Publish to open it now", a manager sees "ask an admin to publish it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). +- **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Publish** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. +- **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, unpublishing is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. ### PM-5 Manage position questions -- **Trigger** — the Questions tab on `/manage/positions/[id]/edit`. +- **Trigger** — the Questions section on `/manage/positions/[id]/edit`, at the bottom of the page. - **Happy path** — `createPositionQuestion` appends at `max(order) + 1` inside a transaction (so concurrent inserts can't collide on order); `updatePositionQuestion` and `deletePositionQuestion` both scope their write to the `positionId` to prevent cross-position IDOR, and delete is a soft delete. Toasts **"Question added"**, **"Question updated"**, **"Question deleted"**. - **Failure / edge** - Validation — a missing label ("Label is required"), a choice question with no options ("At least one option is required for choice questions"), options or `allowOther` on a non-choice type, more than `QUESTION_MAX_OPTIONS` (50), an option over 200 characters, or a format on a non-`short_answer` type ("Format is only available for short-answer questions") → the first zod issue's message, verbatim. @@ -450,11 +452,12 @@ A user who manages at least one non-deleted position. Manager status is **derive ### PM-6 Add a manager -- **Trigger** — the Managers tab on `/manage/positions/[id]/edit`. -- **Happy path** — typing searches through `searchUsers` (gated to managers/admins, name or email, case-insensitive, capped at 10 results, and it returns display name + email only — never the user id). Picking a result calls `addPositionManager`, which authenticates, checks the position exists, checks access, resolves the target by email, and connects them. Toast **"Manager added"**; `/positions`, `/manage/positions` and `/users` are revalidated too, since membership drives all three. +- **Trigger** — the Managers section on `/manage/positions/[id]/edit`. +- **Happy path** — typing searches through `searchUsers` (gated to managers/admins, name or email, case-insensitive, capped at 10 results, and it returns display name + email only — never the user id). Picking a result calls `addPositionManager`, which authenticates, checks the position exists, checks access, checks editability, resolves the target by email, and connects them. Toast **"Manager added"**; `/positions`, `/manage/positions` and `/users` are revalidated too, since membership drives all three. - **Failure / edge** - Query over 200 characters → **"Search is limited to 200 characters."**; an empty query returns no results without querying. - Position deleted since render → **"This position no longer exists."** + - Archived position and the caller is not an admin → `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** The Managers section renders read-only for this position (no search box) instead of reaching the action at all ([AD-2](#ad-2-edit-an-archived-position)). - The picked user was deactivated in the meantime → **"That user is no longer available."** (never a raw Prisma error). - Caller lacks access to the position → the action throws. - Unexpected throw → **"Something went wrong. Please try again."** @@ -462,11 +465,12 @@ A user who manages at least one non-deleted position. Manager status is **derive ### PM-7 Remove a manager -- **Trigger** — the remove control next to a manager on the Managers tab. +- **Trigger** — the remove control next to a manager in the Managers section. - **Happy path** — `removePositionManager` disconnects them. Toast **"Manager removed"**. - **Failure / edge** - Removing yourself as a non-admin → **"You cannot remove yourself as a manager. Ask an admin to do it."** Admins are exempt. - Position deleted since render → **"This position no longer exists."** + - Archived position → `ARCHIVED_POSITION_EDIT_ERROR`, same gate as [PM-6](#pm-6-add-a-manager); the read-only view draws no remove buttons at all. - No access → throws. - **End state** — the user no longer manages this position. If it was their last, they lose manager status entirely — including the Manage nav and the ability to create positions. @@ -533,7 +537,7 @@ A user who manages at least one non-deleted position. Manager status is **derive ### PM-13 Reorder position questions -- **Trigger** — the drag handle on a question card, Questions tab on `/manage/positions/[id]/edit`. +- **Trigger** — the drag handle on a question card, Questions section on `/manage/positions/[id]/edit`. - **Happy path** — the list is wrapped in a shared `SortableProvider` (dnd-kit); dropping a card calls `reorderPositionQuestions` with the full ordered id list, which renumbers every live question `1..N` in one transaction. The new order shows immediately (`useOptimistic`) while the write is in flight. Toast **"Order saved"**. - **Failure / edge** - The id set changed since the page loaded (a question added or deleted in another tab) → **"The question list changed since this page loaded. Refresh and try reordering again."**; the list reverts to the server's order. @@ -579,7 +583,7 @@ An admin is a **manager on every position**: every [Position manager](#position- ### AD-2 Edit an archived position - **Trigger** — `/manage/positions/[id]/edit` for a position closed more than 30 days ago with no application status change since. -- **Happy path** — `checkPositionEditable` returns true for an admin, so the editable form and the question actions render normally where a manager would see the read-only view and the warning callout ([PM-4](#pm-4-edit-position-details), [PM-5](#pm-5-manage-position-questions)). +- **Happy path** — `checkPositionEditable` returns true for an admin, so the autosaving Details/Availability sections, the question actions, and the managers section all render normally where a manager would see read-only text, the read-only question list, and the read-only managers list, respectively, under the warning callout ([PM-4](#pm-4-edit-position-details), [PM-5](#pm-5-manage-position-questions), [PM-6](#pm-6-add-a-manager)). - **Failure / edge** — as [PM-4](#pm-4-edit-position-details); the archived branch simply does not fire. - **End state** — as [PM-4](#pm-4-edit-position-details). diff --git a/lib/constants.ts b/lib/constants.ts index d882e446..6e509240 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -634,11 +634,12 @@ export const RECENTLY_CLOSED_WINDOW_DAYS = 7; // Longer than the public window so managers and admins keep oversight during wrap-up. export const MANAGED_POSITIONS_WINDOW_DAYS = 30; -// Returned by updatePosition / the three position-question actions when isPositionActive is false. +// Returned by the position-field actions, addPositionManager/removePositionManager, +// and the three position-question actions when isPositionActive is false. export const ARCHIVED_POSITION_EDIT_ERROR = 'This position is archived. Ask an admin if it still needs changes.'; -// Returned by createPosition/updatePosition when a non-admin tries to set 'open'. +// Returned by createPosition/updatePositionStatus when a non-admin tries to set 'open'. export const POSITION_OPEN_REQUIRES_ADMIN_ERROR = 'Only an admin can open a position. Ask an admin to publish it for you.'; @@ -706,7 +707,7 @@ export function positionPastDateIssues( } // Ordering plus past-date, for the client form and createPosition — -// updatePosition runs positionPastDateIssues directly against its loaded row. +// updatePositionSchedule runs positionPastDateIssues directly against its loaded row. export function positionDatesRefinement( today: string, previous?: { opensAt?: string; closesAt?: string }, @@ -760,10 +761,12 @@ export function getStatusOptions( // Single source of truth for legal position status moves — draft -> closed is // deliberately absent, the map's only structural gap (see the resolver below). +// Order is header-action priority: the first legal target is the primary +// split-button action, the rest fall behind its caret. export const POSITION_STATUS_TRANSITIONS = { draft: ['open'], - open: ['draft', 'closed'], - closed: ['draft', 'open'], + open: ['closed', 'draft'], + closed: ['open', 'draft'], } as const satisfies Record; export const POSITION_DRAFT_CLOSE_BLOCKED_ERROR = @@ -773,14 +776,16 @@ export const POSITION_UNPUBLISH_BLOCKED_ERROR = export const POSITION_REOPEN_PAST_CLOSE_ERROR = "This position's close date has passed. Clear or extend the close date to reopen it."; -// The Status select's FormDescription twins of the errors above. -export const POSITION_DRAFT_CLOSE_HINT = - 'A draft has nothing to close — publish it first.'; +// The header note's twins of the errors above. export const POSITION_UNPUBLISH_BLOCKED_HINT = 'Someone has already started an application, so this position can no longer go back to Draft.'; export const POSITION_REOPEN_PAST_CLOSE_HINT = 'Clear or extend Closes At to reopen this position.'; +// Shown in place of a Publish button when a manager has no legal move to open. +export const POSITION_PUBLISH_REQUIRES_ADMIN_NOTE = + 'Only an admin can publish this position.'; + // null = legal. from === to always passes; then the map; then the two conditional rules. export function getPositionStatusTransitionError( from: PositionStatus, @@ -801,18 +806,84 @@ export function getPositionStatusTransitionError( return null; } -// getStatusOptions filtered to moves the resolver actually allows, so the -// select never offers a transition the server will reject. -export function getPositionStatusOptions( +// The header actions' legal targets, in split-button priority order (first = +// primary). `from` is never in the result. Folds in the admin-only `-> open` +// rule on top of getPositionStatusTransitionError, since that resolver only +// knows the transition graph, not who's allowed to publish. +export function getPositionTransitionTargets( isAdmin: boolean, from: PositionStatus, ctx: { hasApplications: boolean; closesAtPast: boolean }, -): typeof POSITION_STATUS_OPTIONS { - return getStatusOptions(isAdmin, from).filter( - (opt) => getPositionStatusTransitionError(from, opt.value, ctx) === null, - ); +): PositionStatus[] { + return POSITION_STATUS_TRANSITIONS[from].filter((to) => { + if (to === 'open' && !isAdmin) return false; + return getPositionStatusTransitionError(from, to, ctx) === null; + }); } +export interface PositionTransitionAction { + label: string; + confirmTitle: string; + confirmDescription: (ctx: { unresolvedApplicationCount: number }) => string; + confirmLabel: string; + pendingLabel: string; + successToast: string; +} + +const RETURN_TO_DRAFT_ACTION: PositionTransitionAction = { + label: 'Return to draft', + confirmTitle: 'Return this position to draft?', + confirmDescription: () => + 'Applicants stop seeing it entirely and no one can apply. You can publish it again later.', + confirmLabel: 'Return to draft', + pendingLabel: 'Returning to draft…', + successToast: 'Position returned to draft', +}; + +// Covers exactly the five legal (from, to) pairs in POSITION_STATUS_TRANSITIONS. +export const POSITION_TRANSITION_ACTIONS: Record< + PositionStatus, + Partial> +> = { + draft: { + open: { + label: 'Publish', + confirmTitle: 'Publish this position?', + confirmDescription: () => + 'It becomes visible on the positions list. Applications open on its open date, or immediately if it has none.', + confirmLabel: 'Publish', + pendingLabel: 'Publishing…', + successToast: 'Position published', + }, + }, + open: { + closed: { + label: 'Close applications', + confirmTitle: 'Close this position?', + confirmDescription: ({ unresolvedApplicationCount }) => + unresolvedApplicationCount > 0 + ? `${unresolvedApplicationCount} ${unresolvedApplicationCount === 1 ? 'application is' : 'applications are'} still in progress. Closing stops new applications; the ones you have stay reviewable.` + : 'New applications stop immediately. Everything already submitted stays reviewable.', + confirmLabel: 'Close position', + pendingLabel: 'Closing…', + successToast: 'Position closed', + }, + draft: RETURN_TO_DRAFT_ACTION, + }, + closed: { + open: { + label: 'Reopen', + confirmTitle: 'Reopen this position?', + confirmDescription: () => + 'This position becomes listed and applyable again. Existing applications and decisions are unchanged.', + confirmLabel: 'Reopen position', + pendingLabel: 'Reopening…', + successToast: 'Position reopened', + }, + draft: RETURN_TO_DRAFT_ACTION, + }, +}; + export const POSITION_DESCRIPTION_MAX_LENGTH = 10000; export const MARKDOWN_GUIDE_URL = 'https://www.markdownguide.org/basic-syntax/'; @@ -821,19 +892,26 @@ const orgDayInputSchema = z.union([z.iso.date(), z.literal('')], { error: 'Enter a valid date', }); +// Shared by makePositionFormSchema (both title/description fields at once) +// and each field's own autosave section (one field at a time). export const positionTitleSchema = z.string().min(1, 'Title is required'); +export const positionDescriptionSchema = z + .string() + .max( + POSITION_DESCRIPTION_MAX_LENGTH, + `Description must be ${POSITION_DESCRIPTION_MAX_LENGTH.toLocaleString()} characters or fewer.`, + ); + +export const positionScheduleShape = { + opensAt: orgDayInputSchema.optional(), + closesAt: orgDayInputSchema.optional(), +}; const positionFormShape = { title: positionTitleSchema, - description: z - .string() - .max( - POSITION_DESCRIPTION_MAX_LENGTH, - `Description must be ${POSITION_DESCRIPTION_MAX_LENGTH.toLocaleString()} characters or fewer.`, - ), + description: positionDescriptionSchema, status: z.enum(POSITION_STATUS_VALUES), - opensAt: orgDayInputSchema.optional(), - closesAt: orgDayInputSchema.optional(), + ...positionScheduleShape, }; // `today`/`previous` are injected — this module must not import lib/dates.ts @@ -1057,3 +1135,15 @@ export const EMAIL_FAILURE_STATUSES = [ 'complained', 'failed', ] as const satisfies $Enums.EmailStatus[]; + +// Statuses surfaced in a pipeline summary (excludes draft). Shared by the +// reviewer dashboard's PipelineSummary and the position edit page's +// applications summary. +export const APPLICATION_PIPELINE_STATUSES = [ + 'applied', + 'reached_out', + 'interview_scheduled', + 'reviewing', + 'accepted', + 'rejected', +] as const satisfies $Enums.ApplicationStatus[]; diff --git a/lib/use-autosave.ts b/lib/use-autosave.ts new file mode 100644 index 00000000..7a96fe6a --- /dev/null +++ b/lib/use-autosave.ts @@ -0,0 +1,89 @@ +'use client'; + +import { useCallback, useRef, useState } from 'react'; + +import { ActionError } from '@/lib/utils'; + +export type AutosaveStatus = 'idle' | 'saving' | 'saved' | 'error'; + +interface UseAutosaveOptions { + initialValue: T; + save: (value: T) => Promise; +} + +interface UseAutosaveResult { + status: AutosaveStatus; + error: string | null; + commit: (value: T) => void; + setSaved: (value: T) => void; +} + +// Generic field-level autosave: commits only a changed value, serializes +// writes through a per-instance promise chain so a fast second commit can't +// land before the first, and never advances the saved ref on failure so a +// retry re-sends. `save` should throw ActionError for a user-facing message +// (surfaced as `error`) — anything else reads as the generic failure. +export function useAutosave({ + initialValue, + save, +}: UseAutosaveOptions): UseAutosaveResult { + const savedValueRef = useRef(JSON.stringify(initialValue)); + const chainRef = useRef>(Promise.resolve()); + const [status, setStatus] = useState('idle'); + const [error, setError] = useState(null); + + const setSaved = useCallback((value: T) => { + savedValueRef.current = JSON.stringify(value); + setStatus('saved'); + setError(null); + }, []); + + const commit = useCallback( + (value: T) => { + const serialized = JSON.stringify(value); + if (serialized === savedValueRef.current) return; + + setStatus('saving'); + setError(null); + chainRef.current = chainRef.current + .then(() => save(value)) + .then(() => { + savedValueRef.current = serialized; + setStatus('saved'); + }) + .catch((err: unknown) => { + setStatus('error'); + setError( + err instanceof ActionError + ? err.message + : 'Something went wrong. Please try again.', + ); + }); + }, + [save], + ); + + return { status, error, commit, setSaved }; +} + +// Shared inline-status copy for every autosaved field (rendered inside its +// FormDescription so the aria-live/aria-describedby wiring stays free). +export function autosaveStatusText( + status: AutosaveStatus, + error: string | null, +): string | null { + switch (status) { + case 'idle': + return null; + case 'saving': + return 'Saving…'; + case 'saved': + return 'Saved'; + case 'error': + return error ?? 'Something went wrong. Please try again.'; + default: { + const exhaustiveCheck: never = status; + return exhaustiveCheck; + } + } +} diff --git a/prisma/actions/position-actions.ts b/prisma/actions/position-actions.ts index baf970fb..69d264e2 100644 --- a/prisma/actions/position-actions.ts +++ b/prisma/actions/position-actions.ts @@ -4,6 +4,7 @@ import { revalidatePath } from 'next/cache'; import { z } from 'zod/v4'; +import type { PositionStatus, User } from '@/prisma/client'; import { checkPositionEditable } from '@/prisma/data/positions'; import { @@ -17,15 +18,18 @@ import { POSITION_CLOSES_AT_ORDER_ERROR, POSITION_CLOSES_AT_PAST_ERROR, POSITION_DELETE_BLOCKED_ERROR, - POSITION_DESCRIPTION_MAX_LENGTH, POSITION_MANAGERS_REQUIRED_ERROR, POSITION_OPENS_AT_ORDER_ERROR, POSITION_OPENS_AT_PAST_ERROR, POSITION_OPEN_REQUIRES_ADMIN_ERROR, + POSITION_STATUS_VALUES, POSITION_UNPUBLISH_BLOCKED_ERROR, createPositionFormSchema, getPositionStatusTransitionError, + positionDescriptionSchema, positionPastDateIssues, + positionScheduleShape, + positionTitleSchema, validatePositionDates, } from '@/lib/constants'; import { orgDayEnd, orgDayStart, toOrgDayString } from '@/lib/dates'; @@ -33,23 +37,6 @@ import { prisma } from '@/lib/prisma'; import type { PositionManager, UserSearchResult } from '@/lib/types'; import { type ResponseType, displayUserName } from '@/lib/utils'; -// Past-date check runs separately in updatePosition, against the loaded row's -// previous dates — the schema itself stays ordering-only. -const updatePositionSchema = z - .object({ - id: z.string().min(1), - title: z.string().min(1), - description: z - .string() - .max(POSITION_DESCRIPTION_MAX_LENGTH) - .optional() - .default(''), - status: z.enum(['draft', 'open', 'closed']), - opensAt: z.iso.date().optional(), - closesAt: z.iso.date().optional(), - }) - .superRefine(validatePositionDates); - // The refinement's messages are the only user-actionable parse failures. const parseError = (error: z.ZodError) => error.issues.find( @@ -109,18 +96,23 @@ export async function createPosition( return { id: position.id }; } -export async function updatePosition( - input: unknown, -): Promise { - const parsed = updatePositionSchema.safeParse(input); - if (!parsed.success) return { error: parseError(parsed.error) }; - - const { id, title, description, status, opensAt, closesAt } = parsed.data; - +type PositionEditRow = { + id: string; + status: PositionStatus; + opensAt: Date | null; + closesAt: Date | null; +}; + +// Shared by every field-save action below: authenticate, check existence +// (before the access guard, so a stale link gives an actionable message), +// check access, then check editability. Returns the loaded row so callers +// don't re-fetch it. +async function authorizePositionEdit( + id: string, +): Promise<{ position: PositionEditRow; user: User } | { error: string }> { // Before any DB work: an earlier query would let an anonymous caller probe ids. await getCurrentUser(); - // Before the access guard, so a stale link gives an actionable message. const existing = await prisma.position.findFirst({ where: { id, deletedAt: null }, select: { id: true, status: true, opensAt: true, closesAt: true }, @@ -132,8 +124,88 @@ export async function updatePosition( if (!(await checkPositionEditable(id, user))) return { error: ARCHIVED_POSITION_EDIT_ERROR }; - if (status === 'open' && existing.status !== 'open' && !user.isAdmin) - return { error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }; + return { position: existing, user }; +} + +// title reaches every one of these — a status flip changes what every surface shows. +function revalidatePositionSurfaces(id: string) { + revalidatePath('/positions'); + revalidatePath('/manage/positions'); + revalidatePath(`/positions/${id}`); + revalidatePath(`/manage/positions/${id}/edit`); + revalidatePath('/'); + revalidatePath('/applications'); + revalidatePath('/manage/applications'); +} + +const updatePositionTitleSchema = z.object({ + id: z.string().min(1), + title: positionTitleSchema, +}); + +export async function updatePositionTitle( + input: unknown, +): Promise { + const parsed = updatePositionTitleSchema.safeParse(input); + if (!parsed.success) return { error: 'Invalid input' }; + + const auth = await authorizePositionEdit(parsed.data.id); + if ('error' in auth) return auth; + + await prisma.position.update({ + where: { id: parsed.data.id }, + data: { title: parsed.data.title, updatedById: auth.user.id }, + }); + + revalidatePositionSurfaces(parsed.data.id); +} + +const updatePositionDescriptionSchema = z.object({ + id: z.string().min(1), + description: positionDescriptionSchema, +}); + +export async function updatePositionDescription( + input: unknown, +): Promise { + const parsed = updatePositionDescriptionSchema.safeParse(input); + if (!parsed.success) return { error: 'Invalid input' }; + + const auth = await authorizePositionEdit(parsed.data.id); + if ('error' in auth) return auth; + + await prisma.position.update({ + where: { id: parsed.data.id }, + data: { description: parsed.data.description, updatedById: auth.user.id }, + }); + + revalidatePositionSurfaces(parsed.data.id); +} + +// Ordering only — past-date runs separately below, against the loaded row's +// previous dates, matching how updatePositionStatus reads closesAtPast. +const updatePositionScheduleSchema = z + .object({ id: z.string().min(1), ...positionScheduleShape }) + .superRefine(validatePositionDates); + +const scheduleParseError = (error: z.ZodError) => + error.issues.find( + (issue) => + issue.message === POSITION_OPENS_AT_ORDER_ERROR || + issue.message === POSITION_CLOSES_AT_ORDER_ERROR, + )?.message ?? 'Invalid input'; + +export async function updatePositionSchedule( + input: unknown, +): Promise { + const parsed = updatePositionScheduleSchema.safeParse(input); + if (!parsed.success) return { error: scheduleParseError(parsed.error) }; + + const { id, opensAt, closesAt } = parsed.data; + + const auth = await authorizePositionEdit(id); + if ('error' in auth) return auth; + const { position: existing, user } = auth; const previous = { opensAt: existing.opensAt ? toOrgDayString(existing.opensAt) : undefined, @@ -147,23 +219,56 @@ export async function updatePosition( if (pastDateIssues.length > 0) return { error: pastDateIssues[0]?.message ?? 'Invalid input' }; + await prisma.position.update({ + where: { id }, + data: { + opensAt: opensAt ? orgDayStart(opensAt) : null, + closesAt: closesAt ? orgDayEnd(closesAt) : null, + updatedById: user.id, + }, + }); + + revalidatePositionSurfaces(id); +} + +const updatePositionStatusSchema = z.object({ + id: z.string().min(1), + status: z.enum(POSITION_STATUS_VALUES), +}); + +export async function updatePositionStatus( + input: unknown, +): Promise { + const parsed = updatePositionStatusSchema.safeParse(input); + if (!parsed.success) return { error: 'Invalid input' }; + + const { id, status } = parsed.data; + + const auth = await authorizePositionEdit(id); + if ('error' in auth) return auth; + const { position: existing, user } = auth; + + if (status === 'open' && existing.status !== 'open' && !user.isAdmin) + return { error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }; + const isUnpublishing = status === 'draft' && existing.status !== 'draft'; - if (status !== existing.status) { - const hasApplications = - status === 'draft' - ? (await prisma.application.count({ - where: { positionId: id, deletedAt: null }, - })) > 0 - : false; - const closesAtPast = !!closesAt && closesAt < toOrgDayString(new Date()); - - const transitionError = getPositionStatusTransitionError( - existing.status, - status, - { hasApplications, closesAtPast }, - ); - if (transitionError) return { error: transitionError }; - } + const hasApplications = + status === 'draft' + ? (await prisma.application.count({ + where: { positionId: id, deletedAt: null }, + })) > 0 + : false; + // Read from the stored row, never a submitted value — the client never + // gets to assert its own closesAt is (or isn't) in the past. + const closesAtPast = + existing.closesAt !== null && existing.closesAt < new Date(); + + const transitionError = getPositionStatusTransitionError( + existing.status, + status, + { hasApplications, closesAtPast }, + ); + if (transitionError) return { error: transitionError }; // Folded into the where (not a separate count) so a concurrent first // application can't slip past the check above — same shape as deletePosition. @@ -175,14 +280,7 @@ export async function updatePosition( ? { applications: { none: { deletedAt: null } } } : {}), }, - data: { - title, - description, - status, - opensAt: opensAt ? orgDayStart(opensAt) : null, - closesAt: closesAt ? orgDayEnd(closesAt) : null, - updatedById: user.id, - }, + data: { status, updatedById: user.id }, }); if (updateResult.count === 0) { @@ -197,14 +295,7 @@ export async function updatePosition( }; } - revalidatePath('/positions'); - revalidatePath('/manage/positions'); - revalidatePath(`/positions/${id}`); - revalidatePath(`/manage/positions/${id}/edit`); - // status can flip open <-> draft, changing what every surface shows. - revalidatePath('/'); - revalidatePath('/applications'); - revalidatePath('/manage/applications'); + revalidatePositionSurfaces(id); } export async function deletePosition( @@ -258,7 +349,7 @@ export async function addPositionManager( const { positionId, email } = parsed.data; - // Authenticate before the existence query — see updatePosition. + // Authenticate before the existence query — see authorizePositionEdit. await getCurrentUser(); const exists = await prisma.position.findFirst({ @@ -269,6 +360,9 @@ export async function addPositionManager( const user = await requirePositionAccess(positionId); + if (!(await checkPositionEditable(positionId, user))) + return { error: ARCHIVED_POSITION_EDIT_ERROR }; + // A stale search result must not connect a deleted user via a raw P2025. const target = await prisma.user.findFirst({ where: { email, deletedAt: null }, @@ -298,7 +392,7 @@ export async function removePositionManager( const { positionId, userId } = parsed.data; - // Authenticate before the existence query — see updatePosition. + // Authenticate before the existence query — see authorizePositionEdit. await getCurrentUser(); const exists = await prisma.position.findFirst({ @@ -309,6 +403,9 @@ export async function removePositionManager( const user = await requirePositionAccess(positionId); + if (!(await checkPositionEditable(positionId, user))) + return { error: ARCHIVED_POSITION_EDIT_ERROR }; + // A manager may remove any other but never themselves; admins are exempt. if (userId === user.id && !user.isAdmin) return { diff --git a/tests/db/position-archive.test.ts b/tests/db/position-archive.test.ts index 7f1fbe10..1c7450b1 100644 --- a/tests/db/position-archive.test.ts +++ b/tests/db/position-archive.test.ts @@ -1,18 +1,28 @@ import { + TEST_PREFIX, cleanupFixtures, createTestApplication, createTestPosition, createTestUser, } from '@/tests/helpers/fixtures'; +import { actAs } from '@/tests/stubs/auth-server'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { + addPositionManager, + removePositionManager, +} from '@/prisma/actions/position-actions'; import type { Position, User } from '@/prisma/client'; import { checkPositionEditable, getManagedPositions, } from '@/prisma/data/positions'; -import { MANAGED_POSITIONS_WINDOW_DAYS } from '@/lib/constants'; +import { + ARCHIVED_POSITION_EDIT_ERROR, + MANAGED_POSITIONS_WINDOW_DAYS, +} from '@/lib/constants'; +import { prisma } from '@/lib/prisma'; import { isPositionActive } from '@/lib/utils'; const now = new Date(); @@ -134,3 +144,50 @@ describe('checkPositionEditable', () => { expect(editable).toBe(true); }); }); + +describe('managers on an archived position', () => { + it("refuses a manager's addPositionManager", async () => { + const target = await createTestUser(); + + actAs(manager); + const result = await addPositionManager({ + positionId: forgottenPosition.id, + email: target.email, + }); + expect(result).toEqual({ error: ARCHIVED_POSITION_EDIT_ERROR }); + }); + + it("refuses a manager's removePositionManager", async () => { + const extraManager = await createTestUser({ + name: `${TEST_PREFIX}extra-manager`, + }); + await prisma.position.update({ + where: { id: forgottenPosition.id }, + data: { managers: { connect: { id: extraManager.id } } }, + }); + + actAs(manager); + const result = await removePositionManager({ + positionId: forgottenPosition.id, + userId: extraManager.id, + }); + expect(result).toEqual({ error: ARCHIVED_POSITION_EDIT_ERROR }); + }); + + it("allows an admin's addPositionManager and removePositionManager", async () => { + const target = await createTestUser(); + + actAs(admin); + const addResult = await addPositionManager({ + positionId: forgottenPosition.id, + email: target.email, + }); + expect(addResult).not.toEqual({ error: ARCHIVED_POSITION_EDIT_ERROR }); + + const removeResult = await removePositionManager({ + positionId: forgottenPosition.id, + userId: target.id, + }); + expect(removeResult).toBeUndefined(); + }); +}); diff --git a/tests/db/position-field-saves.test.ts b/tests/db/position-field-saves.test.ts new file mode 100644 index 00000000..d9130766 --- /dev/null +++ b/tests/db/position-field-saves.test.ts @@ -0,0 +1,174 @@ +import { + TEST_PREFIX, + cleanupFixtures, + createTestPosition, + createTestUser, +} from '@/tests/helpers/fixtures'; +import { actAs } from '@/tests/stubs/auth-server'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { + updatePositionDescription, + updatePositionSchedule, + updatePositionTitle, +} from '@/prisma/actions/position-actions'; +import type { Position, User } from '@/prisma/client'; + +import { + POSITION_CLOSES_AT_PAST_ERROR, + POSITION_OPENS_AT_PAST_ERROR, +} from '@/lib/constants'; +import { orgDayEnd, orgDayStart, toOrgDayString } from '@/lib/dates'; +import { prisma } from '@/lib/prisma'; + +let admin: User; +let manager: User; + +function daysFromToday(offset: number): string { + const d = new Date(); + d.setDate(d.getDate() + offset); + return toOrgDayString(d); +} + +const futureOpen = daysFromToday(10); +const futureClose = daysFromToday(20); +const pastDay = daysFromToday(-5); + +async function makePosition( + overrides: Parameters[1] = {}, +): Promise { + return createTestPosition(admin, { managers: [manager], ...overrides }); +} + +async function loadPosition(id: string) { + return prisma.position.findUniqueOrThrow({ + where: { id }, + select: { + title: true, + description: true, + opensAt: true, + closesAt: true, + updatedById: true, + }, + }); +} + +beforeAll(async () => { + admin = await createTestUser({ isAdmin: true }); + manager = await createTestUser(); +}); + +afterAll(async () => { + await cleanupFixtures(); +}); + +describe('updatePositionTitle', () => { + it('persists only the title, and records updatedById', async () => { + const position = await makePosition({ + title: `${TEST_PREFIX}original-title`, + description: `${TEST_PREFIX}original-description`, + }); + + actAs(manager); + const result = await updatePositionTitle({ + id: position.id, + title: `${TEST_PREFIX}new-title`, + }); + expect(result).toBeUndefined(); + + const row = await loadPosition(position.id); + expect(row.title).toBe(`${TEST_PREFIX}new-title`); + expect(row.description).toBe(`${TEST_PREFIX}original-description`); + expect(row.updatedById).toBe(manager.id); + }); +}); + +describe('updatePositionDescription', () => { + it('persists only the description, and records updatedById', async () => { + const position = await makePosition({ + title: `${TEST_PREFIX}kept-title`, + description: `${TEST_PREFIX}original-description`, + }); + + actAs(manager); + const result = await updatePositionDescription({ + id: position.id, + description: `${TEST_PREFIX}new-description`, + }); + expect(result).toBeUndefined(); + + const row = await loadPosition(position.id); + expect(row.title).toBe(`${TEST_PREFIX}kept-title`); + expect(row.description).toBe(`${TEST_PREFIX}new-description`); + expect(row.updatedById).toBe(manager.id); + }); +}); + +describe('updatePositionSchedule', () => { + it('persists both dates as a pair, and records updatedById', async () => { + const position = await makePosition({}); + + actAs(manager); + const result = await updatePositionSchedule({ + id: position.id, + opensAt: futureOpen, + closesAt: futureClose, + }); + expect(result).toBeUndefined(); + + const row = await loadPosition(position.id); + expect(row.opensAt?.getTime()).toBe(orgDayStart(futureOpen).getTime()); + expect(row.closesAt?.getTime()).toBe(orgDayEnd(futureClose).getTime()); + expect(row.updatedById).toBe(manager.id); + }); + + it('writes null when a date is cleared', async () => { + const position = await makePosition({ + opensAt: orgDayStart(futureOpen), + closesAt: orgDayEnd(futureClose), + }); + + actAs(manager); + const result = await updatePositionSchedule({ + id: position.id, + opensAt: undefined, + closesAt: undefined, + }); + expect(result).toBeUndefined(); + + const row = await loadPosition(position.id); + expect(row.opensAt).toBeNull(); + expect(row.closesAt).toBeNull(); + }); + + it('refuses a changed past date and writes nothing', async () => { + const position = await makePosition({ opensAt: orgDayStart(futureOpen) }); + + actAs(manager); + const result = await updatePositionSchedule({ + id: position.id, + opensAt: pastDay, + closesAt: futureClose, + }); + expect(result).toEqual({ error: POSITION_OPENS_AT_PAST_ERROR }); + + const row = await loadPosition(position.id); + expect(row.opensAt?.getTime()).toBe(orgDayStart(futureOpen).getTime()); + expect(row.closesAt).toBeNull(); + }); + + it('leaves an unchanged past date alone, refusing only the newly-changed one', async () => { + const position = await makePosition({ opensAt: orgDayStart(pastDay) }); + + actAs(manager); + const result = await updatePositionSchedule({ + id: position.id, + opensAt: pastDay, + closesAt: pastDay, + }); + expect(result).toEqual({ error: POSITION_CLOSES_AT_PAST_ERROR }); + + const row = await loadPosition(position.id); + expect(row.closesAt).toBeNull(); + }); +}); diff --git a/tests/db/position-publish-permission.test.ts b/tests/db/position-publish-permission.test.ts index 44b54507..74bd0f46 100644 --- a/tests/db/position-publish-permission.test.ts +++ b/tests/db/position-publish-permission.test.ts @@ -7,7 +7,10 @@ import { import { actAs } from '@/tests/stubs/auth-server'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; -import { updatePosition } from '@/prisma/actions/position-actions'; +import { + updatePositionStatus, + updatePositionTitle, +} from '@/prisma/actions/position-actions'; import type { Position, User } from '@/prisma/client'; import { @@ -35,14 +38,12 @@ async function makePosition( return createTestPosition(admin, { managers: [manager], status }); } -describe('updatePosition — status-transition permission', () => { +describe('updatePositionStatus — status-transition permission', () => { it('refuses a manager moving draft to open, row unchanged', async () => { const position = await makePosition('draft'); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'open', }); expect(result).toEqual({ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }); @@ -57,10 +58,8 @@ describe('updatePosition — status-transition permission', () => { it('refuses a manager moving closed to open, row unchanged', async () => { const position = await makePosition('closed'); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'open', }); expect(result).toEqual({ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }); @@ -75,10 +74,8 @@ describe('updatePosition — status-transition permission', () => { it('allows a manager moving open to closed', async () => { const position = await makePosition('open'); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'closed', }); expect(result).toBeUndefined(); @@ -93,10 +90,8 @@ describe('updatePosition — status-transition permission', () => { it('refuses a manager moving draft to closed, row unchanged', async () => { const position = await makePosition('draft'); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'closed', }); expect(result).toEqual({ error: POSITION_DRAFT_CLOSE_BLOCKED_ERROR }); @@ -111,10 +106,8 @@ describe('updatePosition — status-transition permission', () => { it('allows a manager moving closed to draft', async () => { const position = await makePosition('closed'); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'draft', }); expect(result).toBeUndefined(); @@ -129,13 +122,16 @@ describe('updatePosition — status-transition permission', () => { it('lets a manager save an unchanged open status along with other edits', async () => { const position = await makePosition('open'); actAs(manager); - const result = await updatePosition({ + const titleResult = await updatePositionTitle({ id: position.id, title: `${TEST_PREFIX}manager-edited-title`, - description: '', + }); + expect(titleResult).toBeUndefined(); + const statusResult = await updatePositionStatus({ + id: position.id, status: 'open', }); - expect(result).toBeUndefined(); + expect(statusResult).toBeUndefined(); const row = await prisma.position.findUniqueOrThrow({ where: { id: position.id }, @@ -150,42 +146,22 @@ describe('updatePosition — status-transition permission', () => { const draftToOpen = await makePosition('draft'); expect( - await updatePosition({ - id: draftToOpen.id, - title: draftToOpen.title, - description: '', - status: 'open', - }), + await updatePositionStatus({ id: draftToOpen.id, status: 'open' }), ).toBeUndefined(); const closedToOpen = await makePosition('closed'); expect( - await updatePosition({ - id: closedToOpen.id, - title: closedToOpen.title, - description: '', - status: 'open', - }), + await updatePositionStatus({ id: closedToOpen.id, status: 'open' }), ).toBeUndefined(); const openToClosed = await makePosition('open'); expect( - await updatePosition({ - id: openToClosed.id, - title: openToClosed.title, - description: '', - status: 'closed', - }), + await updatePositionStatus({ id: openToClosed.id, status: 'closed' }), ).toBeUndefined(); const closedToDraft = await makePosition('closed'); expect( - await updatePosition({ - id: closedToDraft.id, - title: closedToDraft.title, - description: '', - status: 'draft', - }), + await updatePositionStatus({ id: closedToDraft.id, status: 'draft' }), ).toBeUndefined(); const rows = await prisma.position.findMany({ diff --git a/tests/db/position-transitions.test.ts b/tests/db/position-transitions.test.ts index d8a9ece6..435ffa69 100644 --- a/tests/db/position-transitions.test.ts +++ b/tests/db/position-transitions.test.ts @@ -8,7 +8,11 @@ import { import { actAs } from '@/tests/stubs/auth-server'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; -import { updatePosition } from '@/prisma/actions/position-actions'; +import { + updatePositionSchedule, + updatePositionStatus, + updatePositionTitle, +} from '@/prisma/actions/position-actions'; import type { Position, User } from '@/prisma/client'; import { @@ -62,10 +66,8 @@ describe('unpublish (open/closed -> draft)', () => { await createTestApplication(applicant, position, { status: 'draft' }); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'draft', }); expect(result).toEqual({ error: POSITION_UNPUBLISH_BLOCKED_ERROR }); @@ -76,10 +78,8 @@ describe('unpublish (open/closed -> draft)', () => { const position = await makePosition({ status: 'open' }); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'draft', }); expect(result).toBeUndefined(); @@ -88,19 +88,16 @@ describe('unpublish (open/closed -> draft)', () => { }); describe('reopen (closed -> open)', () => { - it('is blocked when the submitted closesAt is in the past', async () => { + it('is blocked when the stored closesAt is in the past', async () => { const position = await makePosition({ status: 'closed', closesAt: orgDayEnd(pastDay), }); actAs(admin); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'open', - closesAt: pastDay, }); expect(result).toEqual({ error: POSITION_REOPEN_PAST_CLOSE_ERROR }); expect(await status(position.id)).toBe('closed'); @@ -110,31 +107,32 @@ describe('reopen (closed -> open)', () => { const position = await makePosition({ status: 'closed' }); actAs(admin); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'open', }); expect(result).toBeUndefined(); expect(await status(position.id)).toBe('open'); }); - it('succeeds when the past closesAt is extended in the same save', async () => { + it('succeeds once the past closesAt is extended in an earlier save', async () => { const position = await makePosition({ status: 'closed', closesAt: orgDayEnd(pastDay), }); actAs(admin); - const result = await updatePosition({ + const scheduleResult = await updatePositionSchedule({ id: position.id, - title: position.title, - description: '', - status: 'open', closesAt: futureDay, }); - expect(result).toBeUndefined(); + expect(scheduleResult).toBeUndefined(); + + const statusResult = await updatePositionStatus({ + id: position.id, + status: 'open', + }); + expect(statusResult).toBeUndefined(); expect(await status(position.id)).toBe('open'); }); }); @@ -144,10 +142,8 @@ describe('draft -> closed', () => { const position = await makePosition({ status: 'draft' }); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', status: 'closed', }); expect(result).toEqual({ error: POSITION_DRAFT_CLOSE_BLOCKED_ERROR }); @@ -156,16 +152,14 @@ describe('draft -> closed', () => { }); describe('unchanged status', () => { - it('is never a transition — saving other fields with applications present still succeeds', async () => { + it('is never a transition — saving another field with applications present still succeeds', async () => { const position = await makePosition({ status: 'open' }); await createTestApplication(applicant, position, { status: 'applied' }); actAs(manager); - const result = await updatePosition({ + const result = await updatePositionTitle({ id: position.id, title: `${TEST_PREFIX}retitled`, - description: '', - status: 'open', }); expect(result).toBeUndefined(); @@ -176,4 +170,17 @@ describe('unchanged status', () => { expect(row.status).toBe('open'); expect(row.title).toBe(`${TEST_PREFIX}retitled`); }); + + it('saving the same status back is a no-op, even with applications present', async () => { + const position = await makePosition({ status: 'open' }); + await createTestApplication(applicant, position, { status: 'applied' }); + + actAs(manager); + const result = await updatePositionStatus({ + id: position.id, + status: 'open', + }); + expect(result).toBeUndefined(); + expect(await status(position.id)).toBe('open'); + }); }); diff --git a/tests/unit/constants.test.ts b/tests/unit/constants.test.ts index 97d67003..86cec010 100644 --- a/tests/unit/constants.test.ts +++ b/tests/unit/constants.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest'; +import type { PositionStatus } from '@/prisma/client'; + import { ANSWER_LONG_MAX_LENGTH, ANSWER_OTHER_MAX_LENGTH, @@ -18,6 +20,8 @@ import { POSITION_CLOSES_AT_PAST_ERROR, POSITION_OPENS_AT_ORDER_ERROR, POSITION_OPENS_AT_PAST_ERROR, + POSITION_STATUS_TRANSITIONS, + POSITION_TRANSITION_ACTIONS, REVIEWER_APPLICATION_STATUSES, TERMINAL_DECISION_STATUSES, UNRESOLVED_APPLICATION_STATUSES, @@ -26,6 +30,7 @@ import { getStatusOptions, makePositionFormSchema, matchesShortAnswerFormat, + positionPastDateIssues, } from '@/lib/constants'; import { toOrgDayString } from '@/lib/dates'; @@ -523,3 +528,51 @@ describe('getStatusOptions', () => { ); }); }); + +describe('positionPastDateIssues — the schedule autosave pair', () => { + const today = '2026-06-01'; + + it('flags only the date the caller actually changed', () => { + // opensAt unchanged from previous (still past); closesAt newly set to a past date. + const issues = positionPastDateIssues( + { opensAt: '2026-05-01', closesAt: '2026-05-15' }, + today, + { opensAt: '2026-05-01' }, + ); + expect(issues.map((i) => i.path)).toEqual(['closesAt']); + }); + + it('is silent on an unchanged past date with nothing else touched', () => { + const issues = positionPastDateIssues( + { opensAt: '2026-05-01', closesAt: undefined }, + today, + { opensAt: '2026-05-01' }, + ); + expect(issues).toEqual([]); + }); + + it('is silent on an empty/mid-typing value rather than erroring', () => { + const issues = positionPastDateIssues( + { opensAt: '', closesAt: '2026-06-15' }, + today, + {}, + ); + expect(issues).toEqual([]); + }); + + // The ordering case itself is covered above, via makePositionFormSchema's + // 'rejects an inverted pair' test — validatePositionDates is the same + // refinement makePositionFormSchema runs. +}); + +describe('POSITION_TRANSITION_ACTIONS', () => { + it('covers exactly the pairs POSITION_STATUS_TRANSITIONS allows', () => { + for (const from of Object.keys( + POSITION_STATUS_TRANSITIONS, + ) as PositionStatus[]) { + const allowedTargets = POSITION_STATUS_TRANSITIONS[from]; + const actionTargets = Object.keys(POSITION_TRANSITION_ACTIONS[from]); + expect(new Set(actionTargets)).toEqual(new Set(allowedTargets)); + } + }); +}); diff --git a/tests/unit/position-transitions.test.ts b/tests/unit/position-transitions.test.ts index 6dc73915..6fc10ee0 100644 --- a/tests/unit/position-transitions.test.ts +++ b/tests/unit/position-transitions.test.ts @@ -8,8 +8,8 @@ import { POSITION_STATUS_TRANSITIONS, POSITION_STATUS_VALUES, POSITION_UNPUBLISH_BLOCKED_ERROR, - getPositionStatusOptions, getPositionStatusTransitionError, + getPositionTransitionTargets, } from '@/lib/constants'; const NO_APPLICATIONS = { hasApplications: false, closesAtPast: false }; @@ -98,32 +98,59 @@ describe('getPositionStatusTransitionError', () => { }); }); -describe('getPositionStatusOptions', () => { - it('omits closed from a draft position for both roles', () => { - for (const isAdmin of [true, false]) { - const values = getPositionStatusOptions( - isAdmin, - 'draft', - NO_APPLICATIONS, - ).map((o) => o.value); - expect(values).not.toContain('closed'); - } +describe('getPositionTransitionTargets', () => { + it('offers nothing to a manager on a draft — publishing is admin-only', () => { + expect( + getPositionTransitionTargets(false, 'draft', NO_APPLICATIONS), + ).toEqual([]); + }); + + it('offers open to an admin on a draft', () => { + expect( + getPositionTransitionTargets(true, 'draft', NO_APPLICATIONS), + ).toEqual(['open']); }); - it('omits draft from an open position with applications', () => { - const values = getPositionStatusOptions(true, 'open', { - hasApplications: true, - closesAtPast: false, - }).map((o) => o.value); - expect(values).not.toContain('draft'); + it('orders an open position closed-first, then draft, matching the split button priority', () => { + expect(getPositionTransitionTargets(true, 'open', NO_APPLICATIONS)).toEqual( + ['closed', 'draft'], + ); + }); + + it('drops draft from an open position once applications exist', () => { + expect( + getPositionTransitionTargets(true, 'open', { + hasApplications: true, + closesAtPast: false, + }), + ).toEqual(['closed']); }); - it('omits open from a closed position past its close date', () => { - const values = getPositionStatusOptions(true, 'closed', { - hasApplications: false, - closesAtPast: true, - }).map((o) => o.value); - expect(values).not.toContain('open'); + it('orders a closed position open-first, then draft, matching the split button priority', () => { + expect( + getPositionTransitionTargets(true, 'closed', NO_APPLICATIONS), + ).toEqual(['open', 'draft']); + }); + + it('drops open from a closed position past its close date', () => { + expect( + getPositionTransitionTargets(true, 'closed', { + hasApplications: false, + closesAtPast: true, + }), + ).toEqual(['draft']); + }); + + it('never offers open to a non-admin, from any status', () => { + for (const from of POSITION_STATUS_VALUES) + for (const hasApplications of [true, false]) + for (const closesAtPast of [true, false]) + expect( + getPositionTransitionTargets(false, from, { + hasApplications, + closesAtPast, + }), + ).not.toContain('open'); }); it('never offers a move the resolver would reject', () => { @@ -132,10 +159,10 @@ describe('getPositionStatusOptions', () => { for (const hasApplications of [true, false]) for (const closesAtPast of [true, false]) { const ctx = { hasApplications, closesAtPast }; - const options = getPositionStatusOptions(isAdmin, from, ctx); - for (const opt of options) + const targets = getPositionTransitionTargets(isAdmin, from, ctx); + for (const to of targets) expect( - getPositionStatusTransitionError(from, opt.value, ctx), + getPositionStatusTransitionError(from, to, ctx), ).toBeNull(); } }); From 524f412eadb9d5910ef8ed89a5bd36ddcc83f0c1 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Mon, 7 Sep 2026 22:23:38 -0400 Subject: [PATCH 02/13] #666 address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebase onto dev to fix conflicting merge state and unblock CI; switch field-save actions to updateMany + count fallback for the stale-link race, and trim over-length comments per ENGINEERING §7. Co-Authored-By: Claude Sonnet 4.6 --- .../position-availability-section.tsx | 5 +--- .../position-status-header-actions.tsx | 5 +--- lib/constants.ts | 10 ++------ lib/use-autosave.ts | 6 +---- prisma/actions/position-actions.ts | 23 +++++++++++-------- 5 files changed, 18 insertions(+), 31 deletions(-) diff --git a/components/features/position-availability-section.tsx b/components/features/position-availability-section.tsx index 2816be55..ef83028e 100644 --- a/components/features/position-availability-section.tsx +++ b/components/features/position-availability-section.tsx @@ -39,10 +39,7 @@ interface PositionAvailabilitySectionProps { warnings?: ReactNode; } -// No zodResolver — the pair saves together, so validation runs inside the -// commit handler against the loaded row's previous values, mirroring -// validatePositionDates/positionPastDateIssues (lib/constants.ts) rather than -// a schema built for a single combined submit. +// No zodResolver — the pair saves together, so validation runs inside the commit handler instead. export function PositionAvailabilitySection({ positionId, opensAt, diff --git a/components/features/position-status-header-actions.tsx b/components/features/position-status-header-actions.tsx index 4071a229..96bcbabb 100644 --- a/components/features/position-status-header-actions.tsx +++ b/components/features/position-status-header-actions.tsx @@ -35,10 +35,7 @@ interface PositionStatusHeaderActionsProps { unresolvedApplicationCount: number; } -// Mirrors ApplicationStatusHeaderActions: a split button whose primary target -// is the first legal move (POSITION_STATUS_TRANSITIONS' priority order), the -// rest behind a caret. No legal move at all renders a muted explanation -// instead — never a disabled button that leaves the reason unstated. +// No legal move renders a muted explanation instead of a disabled button that leaves the reason unstated. export function PositionStatusHeaderActions({ positionId, currentStatus, diff --git a/lib/constants.ts b/lib/constants.ts index 6e509240..ed602492 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -759,10 +759,7 @@ export function getStatusOptions( return POSITION_STATUS_OPTIONS.filter((opt) => opt.value !== 'open'); } -// Single source of truth for legal position status moves — draft -> closed is -// deliberately absent, the map's only structural gap (see the resolver below). -// Order is header-action priority: the first legal target is the primary -// split-button action, the rest fall behind its caret. +// Legal position status moves — draft -> closed is deliberately absent. export const POSITION_STATUS_TRANSITIONS = { draft: ['open'], open: ['closed', 'draft'], @@ -806,10 +803,7 @@ export function getPositionStatusTransitionError( return null; } -// The header actions' legal targets, in split-button priority order (first = -// primary). `from` is never in the result. Folds in the admin-only `-> open` -// rule on top of getPositionStatusTransitionError, since that resolver only -// knows the transition graph, not who's allowed to publish. +// Folds the admin-only `-> open` rule on top of the transition graph above. export function getPositionTransitionTargets( isAdmin: boolean, from: PositionStatus, diff --git a/lib/use-autosave.ts b/lib/use-autosave.ts index 7a96fe6a..f82074c6 100644 --- a/lib/use-autosave.ts +++ b/lib/use-autosave.ts @@ -18,11 +18,7 @@ interface UseAutosaveResult { setSaved: (value: T) => void; } -// Generic field-level autosave: commits only a changed value, serializes -// writes through a per-instance promise chain so a fast second commit can't -// land before the first, and never advances the saved ref on failure so a -// retry re-sends. `save` should throw ActionError for a user-facing message -// (surfaced as `error`) — anything else reads as the generic failure. +// Serializes writes through a per-instance promise chain so a fast second commit can't outrace the first. export function useAutosave({ initialValue, save, diff --git a/prisma/actions/position-actions.ts b/prisma/actions/position-actions.ts index 69d264e2..34526f0f 100644 --- a/prisma/actions/position-actions.ts +++ b/prisma/actions/position-actions.ts @@ -103,10 +103,7 @@ type PositionEditRow = { closesAt: Date | null; }; -// Shared by every field-save action below: authenticate, check existence -// (before the access guard, so a stale link gives an actionable message), -// check access, then check editability. Returns the loaded row so callers -// don't re-fetch it. +// Existence check runs before the access guard so a stale link gives an actionable message. async function authorizePositionEdit( id: string, ): Promise<{ position: PositionEditRow; user: User } | { error: string }> { @@ -152,10 +149,12 @@ export async function updatePositionTitle( const auth = await authorizePositionEdit(parsed.data.id); if ('error' in auth) return auth; - await prisma.position.update({ - where: { id: parsed.data.id }, + const updateResult = await prisma.position.updateMany({ + where: { id: parsed.data.id, deletedAt: null }, data: { title: parsed.data.title, updatedById: auth.user.id }, }); + if (updateResult.count === 0) + return { error: 'This position no longer exists.' }; revalidatePositionSurfaces(parsed.data.id); } @@ -174,10 +173,12 @@ export async function updatePositionDescription( const auth = await authorizePositionEdit(parsed.data.id); if ('error' in auth) return auth; - await prisma.position.update({ - where: { id: parsed.data.id }, + const updateResult = await prisma.position.updateMany({ + where: { id: parsed.data.id, deletedAt: null }, data: { description: parsed.data.description, updatedById: auth.user.id }, }); + if (updateResult.count === 0) + return { error: 'This position no longer exists.' }; revalidatePositionSurfaces(parsed.data.id); } @@ -219,14 +220,16 @@ export async function updatePositionSchedule( if (pastDateIssues.length > 0) return { error: pastDateIssues[0]?.message ?? 'Invalid input' }; - await prisma.position.update({ - where: { id }, + const updateResult = await prisma.position.updateMany({ + where: { id, deletedAt: null }, data: { opensAt: opensAt ? orgDayStart(opensAt) : null, closesAt: closesAt ? orgDayEnd(closesAt) : null, updatedById: user.id, }, }); + if (updateResult.count === 0) + return { error: 'This position no longer exists.' }; revalidatePositionSurfaces(id); } From 79aee59c03fd1bdf62f5ce3baeb88d1762630668 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Mon, 7 Sep 2026 22:46:53 -0400 Subject: [PATCH 03/13] #666 apply layout feedback to the edit position page Merge Availability into the Details card (title, then opens/closes side by side, then description), put Managers and Questions side by side, drop the Applications summary section, tighten SectionCard's header padding, and inline the danger zone's delete button with state-dependent description text. Co-Authored-By: Claude Sonnet 4.6 --- .../manage/positions/[id]/edit/page.tsx | 156 ++++++++---------- .../position-applications-summary.tsx | 56 ------- .../position-availability-section.tsx | 10 +- components/features/position-danger-zone.tsx | 32 ++-- .../features/position-details-section.tsx | 11 +- components/ui/section-card.tsx | 2 +- docs/WORKFLOWS.md | 8 +- 7 files changed, 103 insertions(+), 172 deletions(-) delete mode 100644 components/features/position-applications-summary.tsx diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx index 9bdaf63f..e15b10e6 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -18,7 +18,6 @@ import { isPositionActive, } from '@/lib/utils'; -import { PositionApplicationsSummary } from '@/components/features/position-applications-summary'; import { PositionAvailabilitySection } from '@/components/features/position-availability-section'; import { PositionDangerZone } from '@/components/features/position-danger-zone'; import { PositionDetailsSection } from '@/components/features/position-details-section'; @@ -77,9 +76,7 @@ export default async function EditPositionPage({ getPositionApplicationStats([position.id]), ]); - // One groupBy, reused by the archived callout and the applications summary. const counts = stats.get(position.id)?.counts ?? {}; - const total = stats.get(position.id)?.total ?? 0; const unresolvedTotal = UNRESOLVED_APPLICATION_STATUSES.reduce( (sum, status) => sum + (counts[status] ?? 0), 0, @@ -174,19 +171,55 @@ export default async function EditPositionPage({ )} -
+
+ {canEdit && availabilityWarnings} {canEdit ? ( + > + + ) : ( -
+ <>

Title

{position.title}

+
+
+
Opens
+
+ {position.opensAt ? ( + + ) : ( + Not set + )} +
+
+
+
Closes
+
+ {position.closesAt ? ( + + ) : ( + Not set + )} +
+
+

Description

{position.description ? ( @@ -197,93 +230,40 @@ export default async function EditPositionPage({

)}
-
- )} -
- - - -
- {canEdit ? ( - - ) : ( -
-
-
Opens
-
- {position.opensAt ? ( - - ) : ( - Not set - )} -
-
-
-
Closes
-
- {position.closesAt ? ( - - ) : ( - Not set - )} -
-
-
+ )}
- -
- {canEdit ? ( - - ) : ( - - )} -
- - - -
- -
- +
+ +
+ {canEdit ? ( + + ) : ( + + )} +
+ - -
- {canEdit ? ( - - ) : ( - - )} -
- + +
+ {canEdit ? ( + + ) : ( + + )} +
+ +
{deletionSummary && ( >; - total: number; -} - -// getPositionApplicationStats is already fetched for this page to compute -// one number for a callout — this surfaces the full breakdown, nearly free. -export function PositionApplicationsSummary({ - counts, - total, -}: PositionApplicationsSummaryProps) { - if (total === 0) - return ( - - ); - - return ( -
- - {APPLICATION_PIPELINE_STATUSES.map((status) => { - const count = counts[status] ?? 0; - const variant = APPLICATION_STATUS_BADGE_VARIANT[status]; - return ( - - ); - })} -
- ); -} diff --git a/components/features/position-availability-section.tsx b/components/features/position-availability-section.tsx index ef83028e..c47a3449 100644 --- a/components/features/position-availability-section.tsx +++ b/components/features/position-availability-section.tsx @@ -1,6 +1,5 @@ 'use client'; -import type { ReactNode } from 'react'; import { useRef } from 'react'; import { useForm } from 'react-hook-form'; @@ -35,8 +34,6 @@ interface PositionAvailabilitySectionProps { positionId: string; opensAt: string | null; closesAt: string | null; - // Server-rendered warning callout(s) about a status/date divergence. - warnings?: ReactNode; } // No zodResolver — the pair saves together, so validation runs inside the commit handler instead. @@ -44,7 +41,6 @@ export function PositionAvailabilitySection({ positionId, opensAt, closesAt, - warnings, }: PositionAvailabilitySectionProps) { const initial: ScheduleValues = { opensAt: opensAt ?? '', @@ -102,9 +98,7 @@ export function PositionAvailabilitySection({ return (
- - {warnings} - +
)} - +
); } diff --git a/components/features/position-danger-zone.tsx b/components/features/position-danger-zone.tsx index 47d41657..ee025636 100644 --- a/components/features/position-danger-zone.tsx +++ b/components/features/position-danger-zone.tsx @@ -50,10 +50,24 @@ export function PositionDangerZone({

Delete Position

- -

- Deleting hides this position everywhere — the positions list, search - results and any direct link. This can't be undone from the app. + +

+ {blocked ? ( + <> + This position has {summary.submittedCount} application + {summary.submittedCount === 1 ? '' : 's'}, so it can't be + deleted. Close it instead — closed positions stay in the archive. + + ) : ( + <> + Deleting hides this position everywhere — the positions list, + search results and any direct link. This can't be undone from + the app. + + )}

@@ -63,7 +77,7 @@ export function PositionDangerZone({ disabled={blocked} aria-label={`Delete position ${positionTitle}`} aria-describedby={blocked ? blockedReasonId : undefined} - className="self-start" + className="shrink-0" > Delete position @@ -118,14 +132,6 @@ export function PositionDangerZone({ - - {blocked && ( -

- This position has {summary.submittedCount} application - {summary.submittedCount === 1 ? '' : 's'}, so it can't be - deleted. Close it instead — closed positions stay in the archive. -

- )} ); diff --git a/components/features/position-details-section.tsx b/components/features/position-details-section.tsx index 342b35c0..d8764031 100644 --- a/components/features/position-details-section.tsx +++ b/components/features/position-details-section.tsx @@ -1,5 +1,6 @@ 'use client'; +import type { ReactNode } from 'react'; import { useForm } from 'react-hook-form'; import { zodResolver } from '@hookform/resolvers/zod'; @@ -39,12 +40,16 @@ interface PositionDetailsSectionProps { positionId: string; title: string; description: string; + // Rendered between the title and description fields — e.g. the + // availability (opens/closes) fields, kept as their own component. + children?: ReactNode; } export function PositionDetailsSection({ positionId, title, description, + children, }: PositionDetailsSectionProps) { const form = useForm({ resolver: zodResolver(detailsSchema), @@ -76,7 +81,7 @@ export function PositionDetailsSection({ return (
- +
+ {children} + { @@ -147,7 +154,7 @@ export function PositionDetailsSection({ ) : undefined; })()} /> - +
); } diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index c8388eb6..c2dee176 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -9,7 +9,7 @@ import { cn } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -const HEADER_CLASS = 'border-b p-4'; +const HEADER_CLASS = 'border-b px-4 pt-4 pb-3'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink { diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index f7121ffc..86bdb4aa 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -419,12 +419,12 @@ A user who manages at least one non-deleted position. Manager status is **derive ### PM-4 Edit position details - **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`). -- **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: Details, Availability, Managers, an Applications summary, Questions, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. +- **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: one merged Details card (title, then opens/closes side by side, then description), Managers and Questions side by side, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. Admin · `draft` → **Publish**. Manager · `draft` → no button, a muted note: "Only an admin can publish this position." `open`, no applications → **Close applications** + caret → **Return to draft**; with applications, **Close applications** alone. Admin · `closed`, close date future or unset → **Reopen** + caret → **Return to draft**; manager · `closed` → **Return to draft** only. No legal move at all → the muted note alone. Archived → no header actions. Toasts: **"Position published"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. - **Failure / edge** - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. - Not a listed manager and not an admin → `notFound()`. - - Archived (closed >30 days with no application status change since) and the caller is not an admin → Details and Availability render as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) + - Archived (closed >30 days with no application status change since) and the caller is not an admin → the merged Details card renders as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header never offers it) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. - `open → draft` or `closed → draft` once any non-deleted application exists, at any status including `draft` (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. @@ -433,7 +433,7 @@ A user who manages at least one non-deleted position. Manager status is **derive - An empty title on blur → "Title is required" inline; nothing saves, the header keeps showing the last-saved title. - Deleted between render and an autosave → **"This position no longer exists."**, shown inline on the field, not a toast. - Unexpected throw during an autosave → the generic toast; a header-action throw does the same. - - Status `open` with a `closesAt` already past → a warning callout at the top of the Availability card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close applications** to make that explicit). + - Status `open` with a `closesAt` already past → a warning callout at the top of the Details card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close applications** to make that explicit). - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Publish to open it now", a manager sees "ask an admin to publish it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). - **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Publish** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. - **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, unpublishing is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. @@ -583,7 +583,7 @@ An admin is a **manager on every position**: every [Position manager](#position- ### AD-2 Edit an archived position - **Trigger** — `/manage/positions/[id]/edit` for a position closed more than 30 days ago with no application status change since. -- **Happy path** — `checkPositionEditable` returns true for an admin, so the autosaving Details/Availability sections, the question actions, and the managers section all render normally where a manager would see read-only text, the read-only question list, and the read-only managers list, respectively, under the warning callout ([PM-4](#pm-4-edit-position-details), [PM-5](#pm-5-manage-position-questions), [PM-6](#pm-6-add-a-manager)). +- **Happy path** — `checkPositionEditable` returns true for an admin, so the autosaving Details card (title, availability, description), the question actions, and the managers section all render normally where a manager would see read-only text, the read-only question list, and the read-only managers list, respectively, under the warning callout ([PM-4](#pm-4-edit-position-details), [PM-5](#pm-5-manage-position-questions), [PM-6](#pm-6-add-a-manager)). - **Failure / edge** — as [PM-4](#pm-4-edit-position-details); the archived branch simply does not fire. - **End state** — as [PM-4](#pm-4-edit-position-details). From 07f01b64847d04936997d02e85ffd67499091dae Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Tue, 8 Sep 2026 08:56:39 -0400 Subject: [PATCH 04/13] #666 fix edit-position loading skeleton to match current layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loading.tsx still modeled the old 5-card layout after the page was merged into a Details+Availability card, a Managers/Questions grid, and no Applications card — rebuilt it to match. Also picks up the SectionCard `icon` prop that became required on `dev` since this branch diverged. Co-Authored-By: Claude Sonnet 4.6 --- .../manage/positions/[id]/edit/loading.tsx | 18 ++++++++---------- .../(auth)/manage/positions/[id]/edit/page.tsx | 12 ++++++++---- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx index fb336b86..ae93ffb4 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx @@ -17,16 +17,14 @@ export default function EditPositionLoading() {
- {/* Details */} - - {/* Availability */} - - {/* Managers */} - - {/* Applications summary */} - - {/* Questions */} - + {/* Details + Availability merged card */} + + + {/* Managers / Questions grid */} +
+ + +
{/* Danger zone skeleton — compact, admin-only, shown speculatively to avoid layout shift */}
diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx index e15b10e6..598af6ea 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -11,7 +11,7 @@ import { import { requireListedManagerOr404 } from '@/lib/auth/guards'; import { UNRESOLVED_APPLICATION_STATUSES } from '@/lib/constants'; import { toOrgDayString } from '@/lib/dates'; -import { STATE_ICONS } from '@/lib/icons'; +import { CONCEPT_ICONS, STATE_ICONS } from '@/lib/icons'; import { getPositionDateInfo, isOpenPastCloseDate, @@ -170,7 +170,7 @@ export default async function EditPositionPage({ )} - +
{canEdit && availabilityWarnings} {canEdit ? ( @@ -236,7 +236,7 @@ export default async function EditPositionPage({
- +
{canEdit ? ( - +
{canEdit ? ( Date: Fri, 11 Sep 2026 22:01:42 -0400 Subject: [PATCH 05/13] #666 address review feedback Per the human decision on the R5-M1 escalation: closed -> draft is no longer a legal position transition (independent of closesAt or application count), and Reopen is now always rendered for a closed position, disabled with a keyboard-reachable tooltip explanation instead of disappearing. Also trims the R5-L1 stale comment on APPLICATION_PIPELINE_STATUSES. Co-Authored-By: Claude Sonnet 4.6 --- .../position-status-header-actions.tsx | 106 +++++++++++++----- docs/PERMISSIONS.md | 39 +++---- docs/WORKFLOWS.md | 9 +- lib/constants.ts | 18 +-- tests/db/position-publish-permission.test.ts | 42 +++---- tests/db/position-transitions.test.ts | 17 ++- tests/unit/position-transitions.test.ts | 40 ++++--- 7 files changed, 179 insertions(+), 92 deletions(-) diff --git a/components/features/position-status-header-actions.tsx b/components/features/position-status-header-actions.tsx index 96bcbabb..1a25a20a 100644 --- a/components/features/position-status-header-actions.tsx +++ b/components/features/position-status-header-actions.tsx @@ -10,6 +10,7 @@ import type { PositionStatus } from '@/prisma/client'; import { POSITION_PUBLISH_REQUIRES_ADMIN_NOTE, POSITION_REOPEN_PAST_CLOSE_HINT, + POSITION_REOPEN_REQUIRES_ADMIN_NOTE, POSITION_TRANSITION_ACTIONS, POSITION_UNPUBLISH_BLOCKED_HINT, getPositionTransitionTargets, @@ -25,6 +26,12 @@ import { DropdownMenuItem, DropdownMenuTrigger, } from '@/components/ui/dropdown-menu'; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from '@/components/ui/tooltip'; interface PositionStatusHeaderActionsProps { positionId: string; @@ -81,16 +88,82 @@ export function PositionStatusHeaderActions({ }); } + const confirmAction = confirmTarget + ? POSITION_TRANSITION_ACTIONS[currentStatus][confirmTarget] + : undefined; + + const confirmDialog = ( + { + if (!open && !isPending) setConfirmTarget(null); + }} + title={confirmAction?.confirmTitle ?? ''} + description={ + confirmAction?.confirmDescription({ unresolvedApplicationCount }) ?? '' + } + confirmLabel={confirmAction?.confirmLabel ?? ''} + pendingLabel={confirmAction?.pendingLabel ?? ''} + isPending={isPending} + onConfirm={() => { + if (confirmTarget) performMove(confirmTarget); + }} + /> + ); + + // Reopen is the only possible move from closed, so it's always rendered — + // disabled with a tooltip explanation, never hidden, when it isn't legal yet. + if (currentStatus === 'closed') { + const reopenAction = POSITION_TRANSITION_ACTIONS.closed.open; + if (!reopenAction) return null; + const canReopen = targets.includes('open'); + const disabledReason = canReopen + ? null + : isAdmin + ? POSITION_REOPEN_PAST_CLOSE_HINT + : POSITION_REOPEN_REQUIRES_ADMIN_NOTE; + + const reopenButton = ( + + ); + + return ( + <> + {disabledReason ? ( + + + + {reopenButton} + + {disabledReason} + + + ) : ( + reopenButton + )} + {confirmDialog} + + ); + } + if (targets.length === 0) { - // Precedence: non-admin draft -> reopen-past-close -> unpublish-blocked. const note = currentStatus === 'draft' && !isAdmin ? POSITION_PUBLISH_REQUIRES_ADMIN_NOTE - : currentStatus === 'closed' && closesAtPast - ? POSITION_REOPEN_PAST_CLOSE_HINT - : hasApplications - ? POSITION_UNPUBLISH_BLOCKED_HINT - : null; + : hasApplications + ? POSITION_UNPUBLISH_BLOCKED_HINT + : null; return note ? (

{note}

) : null; @@ -100,9 +173,6 @@ export function PositionStatusHeaderActions({ const primaryAction = primaryTarget ? POSITION_TRANSITION_ACTIONS[currentStatus][primaryTarget] : undefined; - const confirmAction = confirmTarget - ? POSITION_TRANSITION_ACTIONS[currentStatus][confirmTarget] - : undefined; if (!primaryTarget || !primaryAction) return null; @@ -156,23 +226,7 @@ export function PositionStatusHeaderActions({ )}
- { - if (!open && !isPending) setConfirmTarget(null); - }} - title={confirmAction?.confirmTitle ?? ''} - description={ - confirmAction?.confirmDescription({ unresolvedApplicationCount }) ?? - '' - } - confirmLabel={confirmAction?.confirmLabel ?? ''} - pendingLabel={confirmAction?.pendingLabel ?? ''} - isPending={isPending} - onConfirm={() => { - if (confirmTarget) performMove(confirmTarget); - }} - /> + {confirmDialog} ); } diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index eaad33c5..91588703 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -91,19 +91,20 @@ Four principals, each derived rather than stored as a single role field: `PositionStatus` is `draft | open | closed` (`POSITION_STATUS_VALUES`, `lib/constants.ts`). Archived is not a status — see [Archive](#archive). -| Transition | Allowed | Rule | -| -------------------------------------------- | ----------- | ----------------------------------------------------------------------- | -| `draft → open` (publish) | yes | Admin only, non-archived position | -| `open → closed` (close) | yes | Confirm first when unresolved applications exist | -| `closed → open` (reopen) | conditional | Admin only, and only when `closesAt` is null or in the future | -| `open → draft`, `closed → draft` (unpublish) | conditional | Blocked once any non-deleted application exists, at any status | -| `draft → closed` | no | A draft has never accepted applications — nothing to close | -| create as anything but `draft` | no | `createPosition` takes no status input — every position is born `draft` | +| Transition | Allowed | Rule | +| ---------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `draft → open` (publish) | yes | Admin only, non-archived position | +| `open → closed` (close) | yes | Confirm first when unresolved applications exist | +| `closed → open` (reopen) | conditional | Admin only, and only when `closesAt` is null or in the future. Reopen is always shown for a closed position and disabled with a tooltip when not yet legal, rather than hidden | +| `open → draft` (unpublish) | conditional | Blocked once any non-deleted application exists | +| `draft → closed`, `closed → draft` | no | A draft has never accepted applications — nothing to close. A closed position never returns to draft — reopen it instead | +| create as anything but `draft` | no | `createPosition` takes no status input — every position is born `draft` | - **Publishing is a permission, not a workflow.** There is no submit-for-approval step, no pending queue, no approve/reject with a reason, and no notification to the manager. A manager creates and shapes a draft; an admin performs the act of setting it `open`, from either `draft` or `closed`. Content edits after publishing stay unrestricted — approval gates the status change, not the position's fields. - **Reopening past `closesAt` is a silent no-op**, not a reopen — `getPositionAvailability` (`lib/utils.ts`) still returns `closed_by_date`, so the position reads Open and accepts nothing. Reject it: `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }` -- **Unpublishing hides a position out from under applicants who already have work in it**, so the first application — including a draft nobody has submitted — is one-way out of `draft`: `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }` +- **Unpublishing hides a position out from under applicants who already have work in it**, so the first application — including a draft nobody has submitted — is one-way out of `open`: `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }` - **A draft was never listed, so it has nothing to close.** `draft → closed` is rejected with `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }` — a manager who wants a draft off the board leaves it as a draft, or publishes and closes it instead. +- **A closed position never goes back to draft**, independent of `closesAt` or application count: `{ error: 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.' }` — reopen it (admin only) or leave it closed. - **Reopening changes nothing about existing applications.** Decisions stand; a reviewer reverses one through the application status control, not by reopening the position. - `Application` is unique on `[userId, positionId]` (`prisma/schema.prisma`), so a rejected or withdrawn applicant still cannot reapply to a reopened position. Known-open, not fixed by this policy. @@ -111,16 +112,16 @@ Four principals, each derived rather than stored as a single role field: **Publish freezes nothing. The first application freezes nothing.** The only hard freeze is archive. Do not re-derive a stricter rule from the fact that a position is live — freezing fields would be stricter than the settled "questions stay fully editable", which is incoherent. -| Capability (manager) | `draft` | `open` | `closed` | archived | -| ----------------------------- | -------------- | ------------------------- | ------------------------- | ---------- | -| Title, description | ✓ | ✓ | ✓ | ✗ | -| `opensAt` / `closesAt` | ✓ | ✓ | ✓ | ✗ | -| Questions (add, edit, delete) | ✓ | ✓ | ✓ | ✗ | -| Managers (add, remove others) | ✓ | ✓ | ✓ | ✗ | -| Status → `open` | ✗ (admin only) | ✓ (already open — no-op) | ✗ (admin only) | ✗ | -| Status → `closed` | ✗ (never) | ✓ | ✓ (no-op) | ✗ | -| Status → `draft` | ✓ | only with no applications | only with no applications | ✗ | -| Delete the position | admin only | admin only | admin only | admin only | +| Capability (manager) | `draft` | `open` | `closed` | archived | +| ----------------------------- | -------------- | ------------------------- | -------------- | ---------- | +| Title, description | ✓ | ✓ | ✓ | ✗ | +| `opensAt` / `closesAt` | ✓ | ✓ | ✓ | ✗ | +| Questions (add, edit, delete) | ✓ | ✓ | ✓ | ✗ | +| Managers (add, remove others) | ✓ | ✓ | ✓ | ✗ | +| Status → `open` | ✗ (admin only) | ✓ (already open — no-op) | ✗ (admin only) | ✗ | +| Status → `closed` | ✗ (never) | ✓ | ✓ (no-op) | ✗ | +| Status → `draft` | ✓ | only with no applications | ✗ (never) | ✗ | +| Delete the position | admin only | admin only | admin only | admin only | Managers lose access to membership on an archived position, same as every other field: `addPositionManager` / `removePositionManager` run `checkPositionEditable` like the rest of the edit surface. An admin still short-circuits the check, so oversight can always be handed off by an admin. diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index 86bdb4aa..0a302a9a 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -420,15 +420,16 @@ A user who manages at least one non-deleted position. Manager status is **derive - **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`). - **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: one merged Details card (title, then opens/closes side by side, then description), Managers and Questions side by side, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. - - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. Admin · `draft` → **Publish**. Manager · `draft` → no button, a muted note: "Only an admin can publish this position." `open`, no applications → **Close applications** + caret → **Return to draft**; with applications, **Close applications** alone. Admin · `closed`, close date future or unset → **Reopen** + caret → **Return to draft**; manager · `closed` → **Return to draft** only. No legal move at all → the muted note alone. Archived → no header actions. Toasts: **"Position published"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. + - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. Admin · `draft` → **Publish**. Manager · `draft` → no button, a muted note: "Only an admin can publish this position." `open`, no applications → **Close applications** + caret → **Return to draft**; with applications, **Close applications** alone. `closed` → **Reopen** is always rendered (never hidden), disabled with a tooltip when it isn't currently legal: a manager sees it disabled with "Only an admin can reopen this position."; an admin past the close date sees it disabled with "Clear or extend Closes At to reopen this position."; an admin with `closesAt` null or in the future sees it enabled. There is no **Return to draft** from `closed` — that move no longer exists. No legal move at all (draft, manager) → the muted note alone. Archived → no header actions. Toasts: **"Position published"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. - **Failure / edge** - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. - Not a listed manager and not an admin → `notFound()`. - Archived (closed >30 days with no application status change since) and the caller is not an admin → the merged Details card renders as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header never offers it) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. - - `open → draft` or `closed → draft` once any non-deleted application exists, at any status including `draft` (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. - - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it first) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; **Reopen** drops out of the header and the muted note explains why, reading the position's stored `closesAt` — never a value from the request. + - `closed → draft`, any caller (stale tab or hand-made request — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it, independent of application count or `closesAt`) → `{ error: 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.' }`. + - `open → draft` once any non-deleted application exists (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. + - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it first) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; **Reopen** stays visible but disabled, reading the position's stored `closesAt` — never a value from the request — with the same message in its tooltip. - `opensAt` or `closesAt` **changed** to a date before today → the same past-date messages as [PM-3](#pm-3-create-a-position), checked against the position's own previous dates. An untouched past date on an already-open position saves normally — the rule only fires on a date the manager actually changed — and a partially-typed date never fires the check mid-keystroke. - An empty title on blur → "Title is required" inline; nothing saves, the header keeps showing the last-saved title. - Deleted between render and an autosave → **"This position no longer exists."**, shown inline on the field, not a toast. @@ -436,7 +437,7 @@ A user who manages at least one non-deleted position. Manager status is **derive - Status `open` with a `closesAt` already past → a warning callout at the top of the Details card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close applications** to make that explicit). - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Publish to open it now", a manager sees "ask an admin to publish it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). - **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Publish** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. -- **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, unpublishing is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. +- **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, a closed position can never return to draft directly, unpublishing from `open` is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. ### PM-5 Manage position questions diff --git a/lib/constants.ts b/lib/constants.ts index ed602492..fa4f94ba 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -759,15 +759,17 @@ export function getStatusOptions( return POSITION_STATUS_OPTIONS.filter((opt) => opt.value !== 'open'); } -// Legal position status moves — draft -> closed is deliberately absent. +// Legal position status moves — draft <-> closed is deliberately absent both ways. export const POSITION_STATUS_TRANSITIONS = { draft: ['open'], open: ['closed', 'draft'], - closed: ['open', 'draft'], + closed: ['open'], } as const satisfies Record; export const POSITION_DRAFT_CLOSE_BLOCKED_ERROR = 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.'; +export const POSITION_CLOSED_DRAFT_BLOCKED_ERROR = + 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.'; export const POSITION_UNPUBLISH_BLOCKED_ERROR = 'Someone has already started an application, so this position cannot go back to draft. Close it instead.'; export const POSITION_REOPEN_PAST_CLOSE_ERROR = @@ -782,6 +784,9 @@ export const POSITION_REOPEN_PAST_CLOSE_HINT = // Shown in place of a Publish button when a manager has no legal move to open. export const POSITION_PUBLISH_REQUIRES_ADMIN_NOTE = 'Only an admin can publish this position.'; +// Tooltip on a disabled Reopen button for a manager viewing a closed position. +export const POSITION_REOPEN_REQUIRES_ADMIN_NOTE = + 'Only an admin can reopen this position.'; // null = legal. from === to always passes; then the map; then the two conditional rules. export function getPositionStatusTransitionError( @@ -790,6 +795,8 @@ export function getPositionStatusTransitionError( ctx: { hasApplications: boolean; closesAtPast: boolean }, ): string | null { if (from === to) return null; + if (from === 'closed' && to === 'draft') + return POSITION_CLOSED_DRAFT_BLOCKED_ERROR; if ( !(POSITION_STATUS_TRANSITIONS[from] as readonly PositionStatus[]).includes( to, @@ -834,7 +841,7 @@ const RETURN_TO_DRAFT_ACTION: PositionTransitionAction = { successToast: 'Position returned to draft', }; -// Covers exactly the five legal (from, to) pairs in POSITION_STATUS_TRANSITIONS. +// Covers exactly the four legal (from, to) pairs in POSITION_STATUS_TRANSITIONS. export const POSITION_TRANSITION_ACTIONS: Record< PositionStatus, Partial> @@ -874,7 +881,6 @@ export const POSITION_TRANSITION_ACTIONS: Record< pendingLabel: 'Reopening…', successToast: 'Position reopened', }, - draft: RETURN_TO_DRAFT_ACTION, }, }; @@ -1130,9 +1136,7 @@ export const EMAIL_FAILURE_STATUSES = [ 'failed', ] as const satisfies $Enums.EmailStatus[]; -// Statuses surfaced in a pipeline summary (excludes draft). Shared by the -// reviewer dashboard's PipelineSummary and the position edit page's -// applications summary. +// Statuses surfaced in the reviewer dashboard's PipelineSummary (excludes draft). export const APPLICATION_PIPELINE_STATUSES = [ 'applied', 'reached_out', diff --git a/tests/db/position-publish-permission.test.ts b/tests/db/position-publish-permission.test.ts index 74bd0f46..87909d96 100644 --- a/tests/db/position-publish-permission.test.ts +++ b/tests/db/position-publish-permission.test.ts @@ -14,6 +14,7 @@ import { import type { Position, User } from '@/prisma/client'; import { + POSITION_CLOSED_DRAFT_BLOCKED_ERROR, POSITION_DRAFT_CLOSE_BLOCKED_ERROR, POSITION_OPEN_REQUIRES_ADMIN_ERROR, } from '@/lib/constants'; @@ -103,20 +104,20 @@ describe('updatePositionStatus — status-transition permission', () => { expect(row.status).toBe('draft'); }); - it('allows a manager moving closed to draft', async () => { + it('refuses a manager moving closed to draft, row unchanged — never legal', async () => { const position = await makePosition('closed'); actAs(manager); const result = await updatePositionStatus({ id: position.id, status: 'draft', }); - expect(result).toBeUndefined(); + expect(result).toEqual({ error: POSITION_CLOSED_DRAFT_BLOCKED_ERROR }); const row = await prisma.position.findUniqueOrThrow({ where: { id: position.id }, select: { status: true }, }); - expect(row.status).toBe('draft'); + expect(row.status).toBe('closed'); }); it('lets a manager save an unchanged open status along with other edits', async () => { @@ -141,7 +142,7 @@ describe('updatePositionStatus — status-transition permission', () => { expect(row.title).toBe(`${TEST_PREFIX}manager-edited-title`); }); - it('allows an admin every transition, including to open', async () => { + it('allows an admin every legal transition, including to open', async () => { actAs(admin); const draftToOpen = await makePosition('draft'); @@ -159,28 +160,29 @@ describe('updatePositionStatus — status-transition permission', () => { await updatePositionStatus({ id: openToClosed.id, status: 'closed' }), ).toBeUndefined(); - const closedToDraft = await makePosition('closed'); - expect( - await updatePositionStatus({ id: closedToDraft.id, status: 'draft' }), - ).toBeUndefined(); - const rows = await prisma.position.findMany({ - where: { - id: { - in: [ - draftToOpen.id, - closedToOpen.id, - openToClosed.id, - closedToDraft.id, - ], - }, - }, + where: { id: { in: [draftToOpen.id, closedToOpen.id, openToClosed.id] } }, select: { id: true, status: true }, }); const statusById = new Map(rows.map((r) => [r.id, r.status])); expect(statusById.get(draftToOpen.id)).toBe('open'); expect(statusById.get(closedToOpen.id)).toBe('open'); expect(statusById.get(openToClosed.id)).toBe('closed'); - expect(statusById.get(closedToDraft.id)).toBe('draft'); + }); + + it('refuses even an admin moving closed to draft, row unchanged', async () => { + const position = await makePosition('closed'); + actAs(admin); + const result = await updatePositionStatus({ + id: position.id, + status: 'draft', + }); + expect(result).toEqual({ error: POSITION_CLOSED_DRAFT_BLOCKED_ERROR }); + + const row = await prisma.position.findUniqueOrThrow({ + where: { id: position.id }, + select: { status: true }, + }); + expect(row.status).toBe('closed'); }); }); diff --git a/tests/db/position-transitions.test.ts b/tests/db/position-transitions.test.ts index 435ffa69..68ab3daa 100644 --- a/tests/db/position-transitions.test.ts +++ b/tests/db/position-transitions.test.ts @@ -16,6 +16,7 @@ import { import type { Position, User } from '@/prisma/client'; import { + POSITION_CLOSED_DRAFT_BLOCKED_ERROR, POSITION_DRAFT_CLOSE_BLOCKED_ERROR, POSITION_REOPEN_PAST_CLOSE_ERROR, POSITION_UNPUBLISH_BLOCKED_ERROR, @@ -60,7 +61,7 @@ afterAll(async () => { await cleanupFixtures(); }); -describe('unpublish (open/closed -> draft)', () => { +describe('unpublish (open -> draft)', () => { it('is blocked once any non-deleted application exists, even a draft one', async () => { const position = await makePosition({ status: 'open' }); await createTestApplication(applicant, position, { status: 'draft' }); @@ -87,6 +88,20 @@ describe('unpublish (open/closed -> draft)', () => { }); }); +describe('closed -> draft', () => { + it('is always blocked — reopening is the only legal move out of closed', async () => { + const position = await makePosition({ status: 'closed' }); + + actAs(manager); + const result = await updatePositionStatus({ + id: position.id, + status: 'draft', + }); + expect(result).toEqual({ error: POSITION_CLOSED_DRAFT_BLOCKED_ERROR }); + expect(await status(position.id)).toBe('closed'); + }); +}); + describe('reopen (closed -> open)', () => { it('is blocked when the stored closesAt is in the past', async () => { const position = await makePosition({ diff --git a/tests/unit/position-transitions.test.ts b/tests/unit/position-transitions.test.ts index 6fc10ee0..51a13808 100644 --- a/tests/unit/position-transitions.test.ts +++ b/tests/unit/position-transitions.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import type { PositionStatus } from '@/prisma/client'; import { + POSITION_CLOSED_DRAFT_BLOCKED_ERROR, POSITION_DRAFT_CLOSE_BLOCKED_ERROR, POSITION_REOPEN_PAST_CLOSE_ERROR, POSITION_STATUS_TRANSITIONS, @@ -26,13 +27,25 @@ describe('getPositionStatusTransitionError', () => { } }); - it('rejects draft -> closed — the only structurally missing pair', () => { + it('rejects draft -> closed and closed -> draft — the two structurally missing pairs', () => { expect( getPositionStatusTransitionError('draft', 'closed', NO_APPLICATIONS), ).toBe(POSITION_DRAFT_CLOSE_BLOCKED_ERROR); + expect( + getPositionStatusTransitionError('closed', 'draft', NO_APPLICATIONS), + ).toBe(POSITION_CLOSED_DRAFT_BLOCKED_ERROR); + }); + + it('rejects closed -> draft regardless of applications — it is never legal', () => { + expect( + getPositionStatusTransitionError('closed', 'draft', { + hasApplications: true, + closesAtPast: false, + }), + ).toBe(POSITION_CLOSED_DRAFT_BLOCKED_ERROR); }); - it('is the only structurally missing pair — every other (from, to) is in the map', () => { + it('are the only structurally missing pairs — every other (from, to) is in the map', () => { const missing: [PositionStatus, PositionStatus][] = []; for (const from of POSITION_STATUS_VALUES) for (const to of POSITION_STATUS_VALUES) { @@ -42,26 +55,23 @@ describe('getPositionStatusTransitionError', () => { ).includes(to); if (!allowed) missing.push([from, to]); } - expect(missing).toEqual([['draft', 'closed']]); + expect(missing).toEqual([ + ['draft', 'closed'], + ['closed', 'draft'], + ]); }); - it('allows open/closed -> draft with no applications', () => { + it('allows open -> draft with no applications', () => { expect( getPositionStatusTransitionError('open', 'draft', NO_APPLICATIONS), ).toBeNull(); - expect( - getPositionStatusTransitionError('closed', 'draft', NO_APPLICATIONS), - ).toBeNull(); }); - it('rejects open/closed -> draft once any application exists', () => { + it('rejects open -> draft once any application exists', () => { const ctx = { hasApplications: true, closesAtPast: false }; expect(getPositionStatusTransitionError('open', 'draft', ctx)).toBe( POSITION_UNPUBLISH_BLOCKED_ERROR, ); - expect(getPositionStatusTransitionError('closed', 'draft', ctx)).toBe( - POSITION_UNPUBLISH_BLOCKED_ERROR, - ); }); it('allows closed -> open when closesAt is null or in the future', () => { @@ -126,19 +136,19 @@ describe('getPositionTransitionTargets', () => { ).toEqual(['closed']); }); - it('orders a closed position open-first, then draft, matching the split button priority', () => { + it('offers only open from a closed position — draft is never a target', () => { expect( getPositionTransitionTargets(true, 'closed', NO_APPLICATIONS), - ).toEqual(['open', 'draft']); + ).toEqual(['open']); }); - it('drops open from a closed position past its close date', () => { + it('drops open from a closed position past its close date, leaving no targets', () => { expect( getPositionTransitionTargets(true, 'closed', { hasApplications: false, closesAtPast: true, }), - ).toEqual(['draft']); + ).toEqual([]); }); it('never offers open to a non-admin, from any status', () => { From bf4b138aa04d29d716f8befe1f3e00963b2829ac Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Fri, 11 Sep 2026 23:49:19 -0400 Subject: [PATCH 06/13] #666 address review feedback Co-Authored-By: Claude Sonnet 4.6 --- lib/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/constants.ts b/lib/constants.ts index fa4f94ba..42956c59 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -788,7 +788,7 @@ export const POSITION_PUBLISH_REQUIRES_ADMIN_NOTE = export const POSITION_REOPEN_REQUIRES_ADMIN_NOTE = 'Only an admin can reopen this position.'; -// null = legal. from === to always passes; then the map; then the two conditional rules. +// null = legal. from === to, then closed->draft, then the map, then the two conditional rules. export function getPositionStatusTransitionError( from: PositionStatus, to: PositionStatus, From be0cc88114e4a24fbd8fee6d0beefebe04a4af8d Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Fri, 11 Sep 2026 23:58:21 -0400 Subject: [PATCH 07/13] #666 apply post-approval human feedback on edit position page Tighten SectionCard header padding site-wide, rename the draft/open and open/closed header buttons to Open position/Close position, extend the always-rendered-disabled-with-tooltip pattern to Open position for a manager, and add a full-width live-edit warning bar for open/closed positions. Co-Authored-By: Claude Sonnet 4.6 --- .../manage/positions/[id]/edit/page.tsx | 17 ++- .../position-status-header-actions.tsx | 109 +++++++++++------- components/ui/section-card.tsx | 2 +- docs/PERMISSIONS.md | 2 +- docs/WORKFLOWS.md | 11 +- lib/constants.ts | 26 +++-- 6 files changed, 101 insertions(+), 66 deletions(-) diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx index 598af6ea..6d5204d2 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -9,7 +9,10 @@ import { } from '@/prisma/data/positions'; import { requireListedManagerOr404 } from '@/lib/auth/guards'; -import { UNRESOLVED_APPLICATION_STATUSES } from '@/lib/constants'; +import { + POSITION_LIVE_EDIT_WARNING, + UNRESOLVED_APPLICATION_STATUSES, +} from '@/lib/constants'; import { toOrgDayString } from '@/lib/dates'; import { CONCEPT_ICONS, STATE_ICONS } from '@/lib/icons'; import { @@ -94,8 +97,8 @@ export default async function EditPositionPage({ Its close date passed on{' '} , so it stopped accepting applications even though its status is still - Open. Give it a future close date to reopen it, or choose Close to - make that explicit. + Open. Give it a future close date to reopen it, or choose Close + position to make that explicit.

@@ -113,8 +116,8 @@ export default async function EditPositionPage({ it's still a draft, so applicants can't see it. Give it a future {draftPastOpenDate ? 'open' : 'close'} date, or{' '} {user.isAdmin - ? 'choose Publish to open it now.' - : 'ask an admin to publish it.'} + ? 'choose Open position to open it now.' + : 'ask an admin to open it.'}

@@ -144,6 +147,10 @@ export default async function EditPositionPage({ } /> + {position.status !== 'draft' && ( + {POSITION_LIVE_EDIT_WARNING} + )} + {!canEdit && (
diff --git a/components/features/position-status-header-actions.tsx b/components/features/position-status-header-actions.tsx index 1a25a20a..b3795618 100644 --- a/components/features/position-status-header-actions.tsx +++ b/components/features/position-status-header-actions.tsx @@ -8,11 +8,10 @@ import { updatePositionStatus } from '@/prisma/actions/position-actions'; import type { PositionStatus } from '@/prisma/client'; import { - POSITION_PUBLISH_REQUIRES_ADMIN_NOTE, + POSITION_OPEN_REQUIRES_ADMIN_NOTE, POSITION_REOPEN_PAST_CLOSE_HINT, POSITION_REOPEN_REQUIRES_ADMIN_NOTE, POSITION_TRANSITION_ACTIONS, - POSITION_UNPUBLISH_BLOCKED_HINT, getPositionTransitionTargets, } from '@/lib/constants'; import { ACTION_ICONS } from '@/lib/icons'; @@ -42,7 +41,48 @@ interface PositionStatusHeaderActionsProps { unresolvedApplicationCount: number; } -// No legal move renders a muted explanation instead of a disabled button that leaves the reason unstated. +// Shared by the closed->open and draft->open cases: each has exactly one +// possible target, so it always renders, disabled with a tooltip if illegal. +function GatedActionButton({ + label, + disabled, + disabledReason, + pending, + onClick, +}: { + label: string; + disabled: boolean; + disabledReason: string | null; + pending: boolean; + onClick: () => void; +}) { + const button = ( + + ); + + if (!disabledReason) return button; + + return ( + + + + {button} + + {disabledReason} + + + ); +} + export function PositionStatusHeaderActions({ positionId, currentStatus, @@ -111,8 +151,6 @@ export function PositionStatusHeaderActions({ /> ); - // Reopen is the only possible move from closed, so it's always rendered — - // disabled with a tooltip explanation, never hidden, when it isn't legal yet. if (currentStatus === 'closed') { const reopenAction = POSITION_TRANSITION_ACTIONS.closed.open; if (!reopenAction) return null; @@ -123,50 +161,37 @@ export function PositionStatusHeaderActions({ ? POSITION_REOPEN_PAST_CLOSE_HINT : POSITION_REOPEN_REQUIRES_ADMIN_NOTE; - const reopenButton = ( - - ); - return ( <> - {disabledReason ? ( - - - - {reopenButton} - - {disabledReason} - - - ) : ( - reopenButton - )} + setConfirmTarget('open')} + /> {confirmDialog} ); } - if (targets.length === 0) { - const note = - currentStatus === 'draft' && !isAdmin - ? POSITION_PUBLISH_REQUIRES_ADMIN_NOTE - : hasApplications - ? POSITION_UNPUBLISH_BLOCKED_HINT - : null; - return note ? ( -

{note}

- ) : null; + if (currentStatus === 'draft') { + const openAction = POSITION_TRANSITION_ACTIONS.draft.open; + if (!openAction) return null; + const canOpen = targets.includes('open'); + + return ( + <> + setConfirmTarget('open')} + /> + {confirmDialog} + + ); } const [primaryTarget, ...restTargets] = targets; diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index c2dee176..65ab3876 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -9,7 +9,7 @@ import { cn } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -const HEADER_CLASS = 'border-b px-4 pt-4 pb-3'; +const HEADER_CLASS = 'border-b px-4 pt-3 pb-2'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink { diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index 91588703..c3d74e78 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -93,7 +93,7 @@ Four principals, each derived rather than stored as a single role field: | Transition | Allowed | Rule | | ---------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `draft → open` (publish) | yes | Admin only, non-archived position | +| `draft → open` (publish) | conditional | Admin only, non-archived position. **Open position** is always shown for a draft and disabled with a tooltip for a manager, rather than hidden | | `open → closed` (close) | yes | Confirm first when unresolved applications exist | | `closed → open` (reopen) | conditional | Admin only, and only when `closesAt` is null or in the future. Reopen is always shown for a closed position and disabled with a tooltip when not yet legal, rather than hidden | | `open → draft` (unpublish) | conditional | Blocked once any non-deleted application exists | diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index 0a302a9a..f133e13f 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -420,12 +420,13 @@ A user who manages at least one non-deleted position. Manager status is **derive - **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`). - **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: one merged Details card (title, then opens/closes side by side, then description), Managers and Questions side by side, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. - - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. Admin · `draft` → **Publish**. Manager · `draft` → no button, a muted note: "Only an admin can publish this position." `open`, no applications → **Close applications** + caret → **Return to draft**; with applications, **Close applications** alone. `closed` → **Reopen** is always rendered (never hidden), disabled with a tooltip when it isn't currently legal: a manager sees it disabled with "Only an admin can reopen this position."; an admin past the close date sees it disabled with "Clear or extend Closes At to reopen this position."; an admin with `closesAt` null or in the future sees it enabled. There is no **Return to draft** from `closed` — that move no longer exists. No legal move at all (draft, manager) → the muted note alone. Archived → no header actions. Toasts: **"Position published"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. + - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. `draft` → **Open position** is always rendered (never hidden): an admin sees it enabled; a manager sees it disabled with a tooltip, "Only an admin can open this position." `open`, no applications → **Close position** + caret → **Return to draft**; with applications, **Close position** alone. `closed` → **Reopen** is always rendered (never hidden), disabled with a tooltip when it isn't currently legal: a manager sees it disabled with "Only an admin can reopen this position."; an admin past the close date sees it disabled with "Clear or extend Closes At to reopen this position."; an admin with `closesAt` null or in the future sees it enabled. There is no **Return to draft** from `closed` — that move no longer exists. Archived → no header actions. Toasts: **"Position opened"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. + - **Live-edit warning.** An `open` or `closed` position — never `draft` — shows a full-width warning bar under the page title: "Changes made here are immediately visible in the live application." - **Failure / edge** - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. - Not a listed manager and not an admin → `notFound()`. - Archived (closed >30 days with no application status change since) and the caller is not an admin → the merged Details card renders as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) - - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header never offers it) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** + - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header's **Open position**/**Reopen** button stays visible but disabled, never actionable) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. - `closed → draft`, any caller (stale tab or hand-made request — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it, independent of application count or `closesAt`) → `{ error: 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.' }`. - `open → draft` once any non-deleted application exists (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. @@ -434,9 +435,9 @@ A user who manages at least one non-deleted position. Manager status is **derive - An empty title on blur → "Title is required" inline; nothing saves, the header keeps showing the last-saved title. - Deleted between render and an autosave → **"This position no longer exists."**, shown inline on the field, not a toast. - Unexpected throw during an autosave → the generic toast; a header-action throw does the same. - - Status `open` with a `closesAt` already past → a warning callout at the top of the Details card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close applications** to make that explicit). - - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Publish to open it now", a manager sees "ask an admin to publish it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). -- **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Publish** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. + - Status `open` with a `closesAt` already past → a warning callout at the top of the Details card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close position** to make that explicit). + - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Open position to open it now", a manager sees "ask an admin to open it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). +- **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Open position** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close position** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. - **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, a closed position can never return to draft directly, unpublishing from `open` is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. ### PM-5 Manage position questions diff --git a/lib/constants.ts b/lib/constants.ts index 42956c59..c898a4af 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -775,19 +775,21 @@ export const POSITION_UNPUBLISH_BLOCKED_ERROR = export const POSITION_REOPEN_PAST_CLOSE_ERROR = "This position's close date has passed. Clear or extend the close date to reopen it."; -// The header note's twins of the errors above. -export const POSITION_UNPUBLISH_BLOCKED_HINT = - 'Someone has already started an application, so this position can no longer go back to Draft.'; +// The header note's twin of the reopen error above. export const POSITION_REOPEN_PAST_CLOSE_HINT = 'Clear or extend Closes At to reopen this position.'; -// Shown in place of a Publish button when a manager has no legal move to open. -export const POSITION_PUBLISH_REQUIRES_ADMIN_NOTE = - 'Only an admin can publish this position.'; +// Tooltip on a disabled Open position button for a manager viewing a draft. +export const POSITION_OPEN_REQUIRES_ADMIN_NOTE = + 'Only an admin can open this position.'; // Tooltip on a disabled Reopen button for a manager viewing a closed position. export const POSITION_REOPEN_REQUIRES_ADMIN_NOTE = 'Only an admin can reopen this position.'; +// Shown on the edit page for a live (open or closed) position, never draft. +export const POSITION_LIVE_EDIT_WARNING = + 'Changes made here are immediately visible in the live application.'; + // null = legal. from === to, then closed->draft, then the map, then the two conditional rules. export function getPositionStatusTransitionError( from: PositionStatus, @@ -848,18 +850,18 @@ export const POSITION_TRANSITION_ACTIONS: Record< > = { draft: { open: { - label: 'Publish', - confirmTitle: 'Publish this position?', + label: 'Open position', + confirmTitle: 'Open this position?', confirmDescription: () => 'It becomes visible on the positions list. Applications open on its open date, or immediately if it has none.', - confirmLabel: 'Publish', - pendingLabel: 'Publishing…', - successToast: 'Position published', + confirmLabel: 'Open position', + pendingLabel: 'Opening…', + successToast: 'Position opened', }, }, open: { closed: { - label: 'Close applications', + label: 'Close position', confirmTitle: 'Close this position?', confirmDescription: ({ unresolvedApplicationCount }) => unresolvedApplicationCount > 0 From 06847732942c83245a3a85ee88e314dd67673bc3 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 00:05:09 -0400 Subject: [PATCH 08/13] #666 gate live-edit warning bar on canEdit Archived positions are always closed, so a non-admin manager saw the live-edit warning ("changes are immediately visible") directly above the archived callout saying the opposite. Gate on canEdit to match the header actions' existing gate. Co-Authored-By: Claude Sonnet 4.6 --- app/(main)/(auth)/manage/positions/[id]/edit/page.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx index 6d5204d2..5c1fa05d 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -147,7 +147,7 @@ export default async function EditPositionPage({ } /> - {position.status !== 'draft' && ( + {canEdit && position.status !== 'draft' && ( {POSITION_LIVE_EDIT_WARNING} )} From ab3d8f2ba6e47a7cbb47f55787c9211eed2a7ca1 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 00:13:46 -0400 Subject: [PATCH 09/13] #666 fix section card header padding and reopen button label Co-Authored-By: Claude Sonnet 4.6 --- components/ui/section-card.tsx | 2 +- lib/constants.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index 65ab3876..a88bc37e 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -9,7 +9,7 @@ import { cn } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -const HEADER_CLASS = 'border-b px-4 pt-3 pb-2'; +const HEADER_CLASS = 'border-b px-4 py-3'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink { diff --git a/lib/constants.ts b/lib/constants.ts index c898a4af..a4e9a208 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -875,7 +875,7 @@ export const POSITION_TRANSITION_ACTIONS: Record< }, closed: { open: { - label: 'Reopen', + label: 'Reopen position', confirmTitle: 'Reopen this position?', confirmDescription: () => 'This position becomes listed and applyable again. Existing applications and decisions are unchanged.', From 479b7aad5d327e993e19871dca0f6282e5e96936 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 00:20:12 -0400 Subject: [PATCH 10/13] #666 address review feedback Update docs/WORKFLOWS.md and docs/PERMISSIONS.md so the reopen transition's documented label matches the shipped "Reopen position" button/dialog/tooltip text. Co-Authored-By: Claude Sonnet 4.6 --- docs/PERMISSIONS.md | 16 ++++++++-------- docs/WORKFLOWS.md | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index c3d74e78..c43a82ee 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -91,14 +91,14 @@ Four principals, each derived rather than stored as a single role field: `PositionStatus` is `draft | open | closed` (`POSITION_STATUS_VALUES`, `lib/constants.ts`). Archived is not a status — see [Archive](#archive). -| Transition | Allowed | Rule | -| ---------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `draft → open` (publish) | conditional | Admin only, non-archived position. **Open position** is always shown for a draft and disabled with a tooltip for a manager, rather than hidden | -| `open → closed` (close) | yes | Confirm first when unresolved applications exist | -| `closed → open` (reopen) | conditional | Admin only, and only when `closesAt` is null or in the future. Reopen is always shown for a closed position and disabled with a tooltip when not yet legal, rather than hidden | -| `open → draft` (unpublish) | conditional | Blocked once any non-deleted application exists | -| `draft → closed`, `closed → draft` | no | A draft has never accepted applications — nothing to close. A closed position never returns to draft — reopen it instead | -| create as anything but `draft` | no | `createPosition` takes no status input — every position is born `draft` | +| Transition | Allowed | Rule | +| ---------------------------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `draft → open` (publish) | conditional | Admin only, non-archived position. **Open position** is always shown for a draft and disabled with a tooltip for a manager, rather than hidden | +| `open → closed` (close) | yes | Confirm first when unresolved applications exist | +| `closed → open` (reopen) | conditional | Admin only, and only when `closesAt` is null or in the future. Reopen position is always shown for a closed position and disabled with a tooltip when not yet legal, rather than hidden | +| `open → draft` (unpublish) | conditional | Blocked once any non-deleted application exists | +| `draft → closed`, `closed → draft` | no | A draft has never accepted applications — nothing to close. A closed position never returns to draft — reopen it instead | +| create as anything but `draft` | no | `createPosition` takes no status input — every position is born `draft` | - **Publishing is a permission, not a workflow.** There is no submit-for-approval step, no pending queue, no approve/reject with a reason, and no notification to the manager. A manager creates and shapes a draft; an admin performs the act of setting it `open`, from either `draft` or `closed`. Content edits after publishing stay unrestricted — approval gates the status change, not the position's fields. - **Reopening past `closesAt` is a silent no-op**, not a reopen — `getPositionAvailability` (`lib/utils.ts`) still returns `closed_by_date`, so the position reads Open and accepts nothing. Reject it: `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }` diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index f133e13f..9b169016 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -420,7 +420,7 @@ A user who manages at least one non-deleted position. Manager status is **derive - **Trigger** — **Edit** on `/positions/[id]`, or a managed position card (`/manage/positions/[id]/edit`). - **Happy path** — the page is a single `max-w-5xl` scroll, no tabs: one merged Details card (title, then opens/closes side by side, then description), Managers and Questions side by side, then the danger zone last. The page loads the position, then `requireListedManagerOr404` against the already-loaded managers list. Status is not a form field anywhere — it lives in the header as a badge (`PositionStatusBadge`, next to the title) plus a split-button/caret action control ([PM-3](#pm-3-create-a-position) covers create; the header actions below cover every later move). Title and description each autosave independently on blur — `updatePositionTitle`/`updatePositionDescription` — with inline **Saving…**/**Saved**/error text under the field instead of a toast (a toast per field per save would be spam); `opensAt`/`closesAt` autosave together as a pair through `updatePositionSchedule`, sharing one status line, since a single date can't be validated alone. Every autosave debounces to blur only — nothing fires mid-keystroke. Each write revalidates the position, its detail page, the dashboard, `/applications` and `/manage/applications`, since a status flip elsewhere changes what every one of those surfaces shows. - - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. `draft` → **Open position** is always rendered (never hidden): an admin sees it enabled; a manager sees it disabled with a tooltip, "Only an admin can open this position." `open`, no applications → **Close position** + caret → **Return to draft**; with applications, **Close position** alone. `closed` → **Reopen** is always rendered (never hidden), disabled with a tooltip when it isn't currently legal: a manager sees it disabled with "Only an admin can reopen this position."; an admin past the close date sees it disabled with "Clear or extend Closes At to reopen this position."; an admin with `closesAt` null or in the future sees it enabled. There is no **Return to draft** from `closed` — that move no longer exists. Archived → no header actions. Toasts: **"Position opened"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. + - **Status transitions (header actions).** The legal targets from the current status (`getPositionTransitionTargets`) render as a split button — first target primary, the rest behind a caret — always behind a confirm dialog, since a header button has none of the deliberateness a select-then-Save had. `draft` → **Open position** is always rendered (never hidden): an admin sees it enabled; a manager sees it disabled with a tooltip, "Only an admin can open this position." `open`, no applications → **Close position** + caret → **Return to draft**; with applications, **Close position** alone. `closed` → **Reopen position** is always rendered (never hidden), disabled with a tooltip when it isn't currently legal: a manager sees it disabled with "Only an admin can reopen this position."; an admin past the close date sees it disabled with "Clear or extend Closes At to reopen this position."; an admin with `closesAt` null or in the future sees it enabled. There is no **Return to draft** from `closed` — that move no longer exists. Archived → no header actions. Toasts: **"Position opened"** / **"Position closed"** / **"Position reopened"** / **"Position returned to draft"**. - **Live-edit warning.** An `open` or `closed` position — never `draft` — shows a full-width warning bar under the page title: "Changes made here are immediately visible in the live application." - **Failure / edge** - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. @@ -430,14 +430,14 @@ A user who manages at least one non-deleted position. Manager status is **derive - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. - `closed → draft`, any caller (stale tab or hand-made request — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it, independent of application count or `closesAt`) → `{ error: 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.' }`. - `open → draft` once any non-deleted application exists (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. - - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it first) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; **Reopen** stays visible but disabled, reading the position's stored `closesAt` — never a value from the request — with the same message in its tooltip. + - `closed → open` with `closesAt` still in the past (stale tab, or the manager didn't clear/extend it first) → `{ error: "This position's close date has passed. Clear or extend the close date to reopen it." }`; **Reopen position** stays visible but disabled, reading the position's stored `closesAt` — never a value from the request — with the same message in its tooltip. - `opensAt` or `closesAt` **changed** to a date before today → the same past-date messages as [PM-3](#pm-3-create-a-position), checked against the position's own previous dates. An untouched past date on an already-open position saves normally — the rule only fires on a date the manager actually changed — and a partially-typed date never fires the check mid-keystroke. - An empty title on blur → "Title is required" inline; nothing saves, the header keeps showing the last-saved title. - Deleted between render and an autosave → **"This position no longer exists."**, shown inline on the field, not a toast. - Unexpected throw during an autosave → the generic toast; a header-action throw does the same. - Status `open` with a `closesAt` already past → a warning callout at the top of the Details card ("Applicants see this position as Closed…"), naming the passed date and offering both remedies (extend `closesAt`, or use **Close position** to make that explicit). - Status `draft` with an `opensAt` or `closesAt` already past → the same spot shows a warning callout ("This position was scheduled to open/close…"), naming the passed date and offering both remedies (move that date to the future, or — role-aware — an admin sees "use Open position to open it now", a manager sees "ask an admin to open it"). Mutually exclusive with the `open`-past-`closesAt` case above (they gate on different statuses). -- **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Open position** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close position** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. +- **Confirmations** — every header transition confirms first (there is no "just Save" anymore): **Open position** — "It becomes visible on the positions list. Applications open on its open date, or immediately if it has none." **Close position** (only when unresolved applications exist): "N applications are still in progress. Closing stops new applications; the ones you have stay reviewable." — otherwise "New applications stop immediately. Everything already submitted stays reviewable." **Reopen position** — "This position becomes listed and applyable again. Existing applications and decisions are unchanged." **Return to draft** — "Applicants stop seeing it entirely and no one can apply. You can publish it again later." All trap focus, Escape cancels (suppressed while pending), and focus returns to the trigger on close. - **End state** — the position's fields, status and window are updated field by field; only an admin's flip to `open` publishes it, a draft can never become closed directly, a closed position can never return to draft directly, unpublishing from `open` is one-way once an application exists, and a stale `open` or `draft` position past its relevant date keeps its warning callout until the date is moved forward or the status is changed. ### PM-5 Manage position questions From ab1b5b49de535cf27ccfe5b0f450f2291da0a074 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 10:41:22 -0400 Subject: [PATCH 11/13] #666 fix missed reopen position rename in workflows doc Co-Authored-By: Claude Sonnet 4.6 --- docs/WORKFLOWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/WORKFLOWS.md b/docs/WORKFLOWS.md index 9b169016..6c6f4023 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -426,7 +426,7 @@ A user who manages at least one non-deleted position. Manager status is **derive - Position missing or soft-deleted → `notFound()`, checked **before** the access guard so both paths 404 identically. - Not a listed manager and not an admin → `notFound()`. - Archived (closed >30 days with no application status change since) and the caller is not an admin → the merged Details card renders as read-only text (description through `Markdown`, dates through `LocalTime`) under a warning callout ("This position is archived. It closed more than 30 days ago and no application status has changed since…"), plus a stalled-applications line and a **Review applications** link when the position still holds unresolved applications; no header actions render at all. A stale tab that autosaves anyway gets `ARCHIVED_POSITION_EDIT_ERROR`: **"This position is archived. Ask an admin if it still needs changes."** ([AD-2](#ad-2-edit-an-archived-position)) - - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header's **Open position**/**Reopen** button stays visible but disabled, never actionable) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** + - A manager triggering a transition **to** `open` (stale tab, hand-made request — the header's **Open position**/**Reopen position** button stays visible but disabled, never actionable) → `{ error: POSITION_OPEN_REQUIRES_ADMIN_ERROR }`: **"Only an admin can open a position. Ask an admin to publish it for you."** - `draft → closed`, any caller (stale tab — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it) → `{ error: 'A draft has never accepted applications, so there is nothing to close. Publish it first, or leave it as a draft.' }`. - `closed → draft`, any caller (stale tab or hand-made request — no header control ever offers it, since `POSITION_STATUS_TRANSITIONS` has no entry for it, independent of application count or `closesAt`) → `{ error: 'A closed position cannot go back to draft. Reopen it instead, or leave it closed.' }`. - `open → draft` once any non-deleted application exists (stale tab or a race with a concurrent first application) → `{ error: 'Someone has already started an application, so this position cannot go back to draft. Close it instead.' }`; **Return to draft** drops out of the header the moment an application exists. From ec04328248e34ece5cd0b55bcf0e4b7850cde777 Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 11:21:05 -0400 Subject: [PATCH 12/13] #666 force SectionCard header padding to beat CardHeader's border-b rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CardHeader ships `[.border-b]:pb-6` in its own default className, which beats the plain `py-3`/`pb-3` utility SectionCard passed via cn() — tailwind-merge doesn't dedupe an arbitrary-variant selector against a plain utility, so both classes ship and the arbitrary variant wins the cascade. Prior cycles (6/9/10) only ever edited the losing side. HEADER_CLASS now appends `pb-3!` (Tailwind v4 important suffix), which compiles to `!important` and unconditionally beats the non-important `[.border-b]:pb-6` rule regardless of selector specificity. Verified by compiling both classes through the actual @tailwindcss/postcss engine. Co-Authored-By: Claude Sonnet 4.6 --- components/ui/section-card.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index a88bc37e..bf8c289f 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -9,7 +9,7 @@ import { cn } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -const HEADER_CLASS = 'border-b px-4 py-3'; +const HEADER_CLASS = 'border-b px-4 py-3 pb-3!'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink { From 5dead6e6c703b658b05e3db21b1219fe24d0ba3b Mon Sep 17 00:00:00 2001 From: b-at-neu Date: Sat, 12 Sep 2026 11:35:51 -0400 Subject: [PATCH 13/13] #666 collapse SectionCard header to one grid row CardHeader's grid-rows-[auto_auto] reserved a second, empty row plus its 6px gap-1.5 row-gap below the title even with no second grid child, so the header stayed taller on the bottom than the top after the pb-3! padding fix. grid-rows-1 removes the phantom track and gap. Co-Authored-By: Claude Sonnet 4.6 --- components/ui/section-card.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index bf8c289f..f126d50a 100644 --- a/components/ui/section-card.tsx +++ b/components/ui/section-card.tsx @@ -9,7 +9,7 @@ import { cn } from '@/lib/utils'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Skeleton } from '@/components/ui/skeleton'; -const HEADER_CLASS = 'border-b px-4 py-3 pb-3!'; +const HEADER_CLASS = 'border-b px-4 py-3 pb-3! grid-rows-1'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink {