diff --git a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx index 9ed4519b..ae93ffb4 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/loading.tsx @@ -1,52 +1,34 @@ +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 */} -
- - - -
+ {/* Details + Availability merged card */} + - {/* Tab content skeleton — loose shape fits both the editable form and the archived read-only view */} -
-
- - -
-
- - -
-
- - -
-
- - -
-
- - -
- -
+ {/* Managers / Questions grid */} +
+ +
- {/* Danger zone skeleton — admin-only, shown speculatively to avoid layout shift */} -
- + {/* 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..5c1fa05d 100644 --- a/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx +++ b/app/(main)/(auth)/manage/positions/[id]/edit/page.tsx @@ -9,24 +9,31 @@ 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 { STATE_ICONS } from '@/lib/icons'; +import { CONCEPT_ICONS, STATE_ICONS } from '@/lib/icons'; import { getPositionDateInfo, isOpenPastCloseDate, isPositionActive, } from '@/lib/utils'; +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 +62,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,27 +74,83 @@ 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]); const counts = stats.get(position.id)?.counts ?? {}; 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 + position 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 Open position to open it now.' + : 'ask an admin to open it.'} +

+
+ + )} +
+ ); + return ( -
+
} + actions={ + canEdit ? ( + + ) : undefined + } /> + {canEdit && position.status !== 'draft' && ( + {POSITION_LIVE_EDIT_WARNING} + )} + {!canEdit && (
@@ -112,103 +177,104 @@ 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. -

-
- - )} -
- ) - } - isAdmin={user.isAdmin} - hasApplications={position.hasApplications} - unresolvedApplicationCount={unresolvedTotal} - /> - ) : ( - - ) - } - questionsContent={ - canEdit ? ( - +
+ {canEdit && availabilityWarnings} + {canEdit ? ( + + title={position.title} + description={position.description} + > + + ) : ( - - ) - } - managersContent={ - - } - /> + <> +
+

Title

+

{position.title}

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

Description

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

+ No description +

+ )} +
+ + )} +
+ + +
+ +
+ {canEdit ? ( + + ) : ( + + )} +
+ + + +
+ {canEdit ? ( + + ) : ( + + )} +
+ +
{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-availability-section.tsx b/components/features/position-availability-section.tsx new file mode 100644 index 00000000..c47a3449 --- /dev/null +++ b/components/features/position-availability-section.tsx @@ -0,0 +1,169 @@ +'use client'; + +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; +} + +// No zodResolver — the pair saves together, so validation runs inside the commit handler instead. +export function PositionAvailabilitySection({ + positionId, + opensAt, + closesAt, +}: 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 ( +
+
+
+ ( + + 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..ee025636 100644 --- a/components/features/position-danger-zone.tsx +++ b/components/features/position-danger-zone.tsx @@ -44,14 +44,30 @@ 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. + +

+ {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. + + )}

@@ -61,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 @@ -116,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-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..d8764031 --- /dev/null +++ b/components/features/position-details-section.tsx @@ -0,0 +1,160 @@ +'use client'; + +import type { ReactNode } from 'react'; +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; + // 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), + 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} + + )} + + ); + }} + /> + + {children} + + { + 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

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, + isAdmin, + hasApplications, + closesAtPast, + unresolvedApplicationCount, +}: PositionStatusHeaderActionsProps) { + const [isPending, startTransition] = useTransition(); + const [pendingTarget, setPendingTarget] = useState( + 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); + } + }); + } + + 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); + }} + /> + ); + + 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; + + return ( + <> + setConfirmTarget('open')} + /> + {confirmDialog} + + ); + } + + 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; + const primaryAction = primaryTarget + ? POSITION_TRANSITION_ACTIONS[currentStatus][primaryTarget] + : 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} + + ); + })} + + + )} +
+ + {confirmDialog} + + ); +} diff --git a/components/ui/section-card.tsx b/components/ui/section-card.tsx index 0656a8b9..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 p-4'; +const HEADER_CLASS = 'border-b px-4 py-3 pb-3! grid-rows-1'; const CONTENT_CLASS = 'p-0'; interface SectionCardLink { @@ -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..c43a82ee 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 | @@ -90,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) | 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." }` -- **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. @@ -110,18 +112,18 @@ 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 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 +139,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..6c6f4023 100644 --- a/docs/WORKFLOWS.md +++ b/docs/WORKFLOWS.md @@ -418,27 +418,31 @@ 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: 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 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. - 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 → 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 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. + - `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 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 -- **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 +454,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 +467,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 +539,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 +585,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 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). diff --git a/lib/constants.ts b/lib/constants.ts index d882e446..a4e9a208 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 }, @@ -758,36 +759,46 @@ 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). +// Legal position status moves — draft <-> closed is deliberately absent both ways. export const POSITION_STATUS_TRANSITIONS = { draft: ['open'], - open: ['draft', 'closed'], - closed: ['draft', 'open'], + open: ['closed', '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 = "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.'; -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.'; -// null = legal. from === to always passes; then the map; then the two conditional rules. +// 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, to: PositionStatus, 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, @@ -801,18 +812,80 @@ 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( +// Folds the admin-only `-> open` rule on top of the transition graph above. +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 four legal (from, to) pairs in POSITION_STATUS_TRANSITIONS. +export const POSITION_TRANSITION_ACTIONS: Record< + PositionStatus, + Partial> +> = { + draft: { + open: { + 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: 'Open position', + pendingLabel: 'Opening…', + successToast: 'Position opened', + }, + }, + open: { + closed: { + label: 'Close position', + 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 position', + 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', + }, + }, +}; + export const POSITION_DESCRIPTION_MAX_LENGTH = 10000; export const MARKDOWN_GUIDE_URL = 'https://www.markdownguide.org/basic-syntax/'; @@ -821,19 +894,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 +1137,13 @@ export const EMAIL_FAILURE_STATUSES = [ 'complained', 'failed', ] as const satisfies $Enums.EmailStatus[]; + +// Statuses surfaced in the reviewer dashboard's PipelineSummary (excludes draft). +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..f82074c6 --- /dev/null +++ b/lib/use-autosave.ts @@ -0,0 +1,85 @@ +'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; +} + +// Serializes writes through a per-instance promise chain so a fast second commit can't outrace the first. +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..34526f0f 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,20 @@ 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; +}; + +// 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 }> { // 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 +121,92 @@ 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; + + 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); +} + +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; + + 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); +} + +// 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 +220,58 @@ export async function updatePosition( if (pastDateIssues.length > 0) return { error: pastDateIssues[0]?.message ?? 'Invalid input' }; + 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); +} + +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 +283,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 +298,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 +352,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 +363,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 +395,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 +406,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..87909d96 100644 --- a/tests/db/position-publish-permission.test.ts +++ b/tests/db/position-publish-permission.test.ts @@ -7,10 +7,14 @@ 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 { + POSITION_CLOSED_DRAFT_BLOCKED_ERROR, POSITION_DRAFT_CLOSE_BLOCKED_ERROR, POSITION_OPEN_REQUIRES_ADMIN_ERROR, } from '@/lib/constants'; @@ -35,14 +39,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 +59,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 +75,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 +91,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 }); @@ -108,34 +104,35 @@ describe('updatePosition — 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 updatePosition({ + const result = await updatePositionStatus({ id: position.id, - title: position.title, - description: '', 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 () => { 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 }, @@ -145,66 +142,47 @@ describe('updatePosition — 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'); 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', - }), - ).toBeUndefined(); - - const closedToDraft = await makePosition('closed'); - expect( - await updatePosition({ - id: closedToDraft.id, - title: closedToDraft.title, - description: '', - status: 'draft', - }), + await updatePositionStatus({ id: openToClosed.id, status: 'closed' }), ).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 d8a9ece6..68ab3daa 100644 --- a/tests/db/position-transitions.test.ts +++ b/tests/db/position-transitions.test.ts @@ -8,10 +8,15 @@ 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 { + POSITION_CLOSED_DRAFT_BLOCKED_ERROR, POSITION_DRAFT_CLOSE_BLOCKED_ERROR, POSITION_REOPEN_PAST_CLOSE_ERROR, POSITION_UNPUBLISH_BLOCKED_ERROR, @@ -56,16 +61,14 @@ 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' }); 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 +79,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(); @@ -87,20 +88,31 @@ 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 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 +122,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 +157,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 +167,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 +185,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..51a13808 100644 --- a/tests/unit/position-transitions.test.ts +++ b/tests/unit/position-transitions.test.ts @@ -3,13 +3,14 @@ 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, POSITION_STATUS_VALUES, POSITION_UNPUBLISH_BLOCKED_ERROR, - getPositionStatusOptions, getPositionStatusTransitionError, + getPositionTransitionTargets, } from '@/lib/constants'; const NO_APPLICATIONS = { hasApplications: false, closesAtPast: false }; @@ -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('is the only structurally missing pair — every other (from, to) is in the map', () => { + 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('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', () => { @@ -98,32 +108,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('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('offers open to an admin on a draft', () => { + expect( + getPositionTransitionTargets(true, 'draft', NO_APPLICATIONS), + ).toEqual(['open']); }); - 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 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('offers only open from a closed position — draft is never a target', () => { + expect( + getPositionTransitionTargets(true, 'closed', NO_APPLICATIONS), + ).toEqual(['open']); + }); + + it('drops open from a closed position past its close date, leaving no targets', () => { + expect( + getPositionTransitionTargets(true, 'closed', { + hasApplications: false, + closesAtPast: true, + }), + ).toEqual([]); + }); + + 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 +169,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(); } });