diff --git a/actions/hireDrafts.ts b/actions/hireDrafts.ts index 130eab6..01fa922 100644 --- a/actions/hireDrafts.ts +++ b/actions/hireDrafts.ts @@ -1,10 +1,74 @@ "use server"; import { revalidatePath } from "next/cache"; -import { currentUserId } from "@/lib/auth/session"; +import { currentUserId, requireUserId } from "@/lib/auth/session"; import { withUser } from "@/lib/db"; import type { ActionResult } from "@/lib/db/rpc"; +/** + * Den Entwurf für mich belegen — beim Öffnen und danach im Takt. + * + * Die Regel hire_drafts_update lässt die Zeile nur durch, wenn die Sperre + * frei, meine oder abgelaufen ist. Trifft das Schreiben keine Zeile, hat sie + * jemand anderes offen; das ist kein Fehler, sondern die Auskunft. + * + * Dieselbe Funktion frischt die Sperre auf: sie schreibt locked_at neu, + * solange sie mir gehört. Der Assistent ruft sie darum im Takt, sonst liefe + * die Frist einem langen Ausfüllen davon. + * + * updated_at bleibt unberührt. Eine Sperre ist keine Änderung am Entwurf — + * bewegte sie den Zeitstempel, sortierte sich die Liste um und „Gespeichert + * am" logge. + */ +export async function entwurfSperren(id: string): Promise { + const userId = await requireUserId(); + try { + const ergebnis = await withUser(userId, (tx) => + tx + .updateTable("hire_drafts") + .set({ locked_by: userId, locked_at: new Date().toISOString() }) + .where("id", "=", id) + .executeTakeFirst() + ); + if (ergebnis.numUpdatedRows === 0n) { + return { success: false, error: "Dieser Entwurf wird gerade von jemand anderem bearbeitet." }; + } + } catch (err) { + return { success: false, error: err instanceof Error ? err.message : "Unbekannter Fehler." }; + } + revalidatePath("/"); + return { success: true }; +} + +/** + * Die Sperre zurückgeben. + * + * Die Bedingung auf locked_by steht hier und nicht nur in der Regel: die + * Regel liesse auch das Freigeben einer *abgelaufenen fremden* Sperre zu, + * und das wäre ein stiller Griff in die Arbeit von jemandem, der gerade + * wieder aufgefrischt hat. + * + * Ein Scheitern bleibt ohne Meldung: die Frist räumt die Sperre ohnehin ab, + * und es gäbe nichts, was die Person daraufhin tun könnte. + */ +export async function entwurfFreigeben(id: string): Promise { + const userId = await requireUserId(); + try { + await withUser(userId, (tx) => + tx + .updateTable("hire_drafts") + .set({ locked_by: null, locked_at: null }) + .where("id", "=", id) + .where("locked_by", "=", userId) + .execute() + ); + } catch (err) { + return { success: false, error: err instanceof Error ? err.message : "Unbekannter Fehler." }; + } + revalidatePath("/"); + return { success: true }; +} + export async function saveHireDraft(payload: { id?: string; step: number; @@ -16,21 +80,24 @@ export async function saveHireDraft(payload: { try { const id = await withUser(userId, async (tx) => { if (payload.id) { - // Ob die Zeile der aufrufenden Person gehört, entscheidet die Regel - // hire_drafts_update — nicht eine Prüfung hier. + // Wer schreiben darf, entscheidet die Regel hire_drafts_update: + // der Entwurf muss meiner oder von einer hinzugewählten Person sein, + // **und** die Sperre muss mir gehören. // - // Die Zahl der geänderten Zeilen wird trotzdem gelesen, und zwar - // seit fremde Entwürfe sichtbar sind: eine Regel weist ein UPDATE - // nicht mit einem Fehler ab, sie lässt es ins Leere laufen. Ohne - // diese Prüfung meldete die Anwendung „gespeichert", und gespeichert - // wäre nichts. + // Die Zahl der geänderten Zeilen wird trotzdem gelesen: eine Regel + // weist ein UPDATE nicht mit einem Fehler ab, sie lässt es ins Leere + // laufen. Ohne diese Prüfung meldete die Anwendung „gespeichert", und + // gespeichert wäre nichts — der ärgerlichste Fall überhaupt, weil die + // Person den Assistenten daraufhin beruhigt zumacht. const ergebnis = await tx .updateTable("hire_drafts") .set({ step: payload.step, payload: payload.data, updated_at: new Date().toISOString() }) .where("id", "=", payload.id) .executeTakeFirst(); if (ergebnis.numUpdatedRows === 0n) { - throw new Error("Der Entwurf liess sich nicht speichern: er gehört jemand anderem oder wurde inzwischen gelöscht."); + throw new Error( + "Der Entwurf liess sich nicht speichern: er wird gerade von jemand anderem bearbeitet oder wurde inzwischen gelöscht." + ); } return payload.id; } @@ -53,12 +120,15 @@ export async function saveHireDraft(payload: { export async function deleteHireDraft(id: string): Promise { try { // Auch hier die Zeilenzahl: die Regel hire_drafts_delete lässt ein - // fremdes Löschen leer laufen statt es abzuweisen. „Entwurf gelöscht" - // über einem Entwurf, der noch dasteht, wäre die schlechtere Auskunft. + // Löschen gegen eine fremde Sperre leer laufen statt es abzuweisen. + // „Entwurf gelöscht" über einem Entwurf, der noch dasteht, wäre die + // schlechtere Auskunft. await withUser(await currentUserId(), async (tx) => { const ergebnis = await tx.deleteFrom("hire_drafts").where("id", "=", id).executeTakeFirst(); if (ergebnis.numDeletedRows === 0n) { - throw new Error("Der Entwurf liess sich nicht löschen: er gehört jemand anderem oder war schon weg."); + throw new Error( + "Der Entwurf liess sich nicht löschen: er wird gerade von jemand anderem bearbeitet oder war schon weg." + ); } }); revalidatePath("/"); diff --git a/components/dashboard/DraftsCard.tsx b/components/dashboard/DraftsCard.tsx index e71f3cc..725e0b8 100644 --- a/components/dashboard/DraftsCard.tsx +++ b/components/dashboard/DraftsCard.tsx @@ -1,6 +1,6 @@ "use client"; -import { Trash2 } from "lucide-react"; +import { Lock, Trash2 } from "lucide-react"; import { useRouter } from "next/navigation"; import { useTransition } from "react"; import { deleteHireDraft } from "@/actions/hireDrafts"; @@ -12,10 +12,17 @@ import { fmtDate } from "@/lib/format"; // Angefangene Neueinstellungen — die eigenen und, wer in der Glocke // hinzugewählt ist, auch dessen. // -// Fremde Entwürfe stehen hier zum Ansehen. Fortsetzen und Löschen fehlen bei -// ihnen nicht aus Vorsicht, sondern weil die Datenbank sie ohnehin abwiese -// (hire_drafts_update, hire_drafts_delete): eine Schaltfläche anzubieten, die -// verlässlich in eine Fehlermeldung führt, wäre ein Versprechen ohne Deckung. +// Fortsetzen geht bei beiden: wer vertreten wird, soll die angefangene +// Einstellung zu Ende bringen können. Aber immer nur eine Person zugleich — +// der Entwurf ist ein einziges JSONB-Feld, zwei gleichzeitig Schreibende +// überschrieben einander vollständig, und die Zweite merkte nichts davon. +// Die Sperre dazu steht in der Datenbank (20260910160000); hier steht nur, +// was sie für die Zeile bedeutet. +// +// Löschen bleibt beim Eigentümer. Die Regel liesse mehr zu — sie muss, weil +// der Assistent den Entwurf nach dem Anlegen selbst wegräumt —, aber eine +// Schaltfläche, die fremde halbfertige Arbeit wegwirft, gehört nicht neben +// eine, die sie fortsetzt. export function DraftsCard({ drafts }: { drafts: Entwurf[] }) { const { openWizard } = useHireWizard(); @@ -56,8 +63,17 @@ export function DraftsCard({ drafts }: { drafts: Entwurf[] }) { Namens — den kennt man. */} {d.vonMir ? "von mir" : `von ${d.autor}`} - {d.vonMir ? ( -
+
+ {d.gesperrtVon ? ( + // Mit dem Grund daneben, nicht bloss abgeschaltet: eine + // graue Schaltfläche ohne Erklärung liest sich wie ein + // Fehler, und die nächste Handlung wäre, es gleich noch + // einmal zu versuchen. + + + {d.gesperrtVon} bearbeitet gerade + + ) : ( + )} + {d.vonMir && ( -
- ) : ( - // Warum hier nichts steht, ist sonst nicht zu erraten: die - // Zeile sähe aus wie eine, an der die Schaltflächen fehlen. - Nur zur Ansicht - )} + )} +
); })} diff --git a/components/hire/HireWizardContext.tsx b/components/hire/HireWizardContext.tsx index ca97097..1a0543a 100644 --- a/components/hire/HireWizardContext.tsx +++ b/components/hire/HireWizardContext.tsx @@ -1,11 +1,14 @@ "use client"; -import { createContext, useContext, useState, type ReactNode } from "react"; +import { useRouter } from "next/navigation"; +import { createContext, useContext, useEffect, useRef, useState, type ReactNode } from "react"; +import { entwurfFreigeben, entwurfSperren } from "@/actions/hireDrafts"; +import { useToast } from "@/components/ui/Toast"; +import type { Entwurf } from "@/lib/entwuerfe"; import type { OpenPositionResolved } from "@/lib/positions"; import { HireWizard } from "./HireWizard"; type Location = { id: string; name: string; country: string }; -type HireDraft = { id: string; step: number; payload: Record }; type OpenWizardOptions = { draftId?: string; positionId?: string }; @@ -15,6 +18,16 @@ type HireWizardContextValue = { const HireWizardContext = createContext(null); +/** + * Wie oft die Sperre aufgefrischt wird, solange der Assistent offen ist. + * + * Deutlich kürzer als die Frist in der Datenbank (15 Minuten, + * app_entwurf_sperrfrist): zwischen zwei Takten darf eine Antwort ausfallen, + * ohne dass die Sperre wegläuft. Wer eine halbe Stunde an einem Entwurf + * sitzt, soll ihn nicht auf halbem Weg an eine Kollegin verlieren. + */ +const TAKT_MS = 5 * 60 * 1000; + export function HireWizardProvider({ children, openPositions, @@ -24,10 +37,12 @@ export function HireWizardProvider({ children: ReactNode; openPositions: OpenPositionResolved[]; locations: Location[]; - drafts: HireDraft[]; + drafts: Entwurf[]; }) { + const { showToast } = useToast(); + const router = useRouter(); const [open, setOpen] = useState(false); - const [resumeDraft, setResumeDraft] = useState(null); + const [resumeDraft, setResumeDraft] = useState(null); const [initialPositionId, setInitialPositionId] = useState(undefined); // Forces HireWizard to remount fresh each time it's opened, so its // internal draft/step state is (re-)initialized directly from the current @@ -35,20 +50,73 @@ export function HireWizardProvider({ // effect needed inside HireWizard itself. const [openKey, setOpenKey] = useState(0); - function openWizard(options?: OpenWizardOptions) { + // Die Kennung des Entwurfs, dessen Sperre wir halten. Als Ref, weil sie im + // Aufräumen des Effekts gebraucht wird — über den Zustand gelesen wäre es + // dort der Stand von vorhin. + const gesperrt = useRef(null); + + async function openWizard(options?: OpenWizardOptions) { + // Ohne Entwurf gibt es nichts zu sperren: eine neue Einstellung entsteht + // erst beim Speichern. + if (options?.draftId) { + const result = await entwurfSperren(options.draftId); + if (!result.success) { + showToast(result.error ?? "Der Entwurf lässt sich gerade nicht öffnen.", "error"); + // Die Liste holt sich den Stand: wer die Sperre hält, steht danach + // an der Zeile, statt dass nur eine Meldung aufblitzt. + router.refresh(); + return; + } + gesperrt.current = options.draftId; + } setResumeDraft(options?.draftId ? (drafts.find((d) => d.id === options.draftId) ?? null) : null); setInitialPositionId(options?.positionId); setOpen(true); setOpenKey((k) => k + 1); } + function closeWizard() { + setOpen(false); + freigeben(); + } + + function freigeben() { + const id = gesperrt.current; + if (!id) return; + gesperrt.current = null; + void entwurfFreigeben(id); + } + + // Auffrischen, solange der Assistent offen ist. Ohne das liefe die Frist + // einem langen Ausfüllen davon, und das Speichern am Ende fände seine + // eigene Sperre abgelaufen. + // + // `!open` ist doppelt gemoppelt: freigeben() räumt beim Zumachen auch + // gesperrt.current weg, und daran scheitert der Takt ohnehin. Es steht + // trotzdem da, weil es die Bedingung ausspricht, um die es geht — dass + // nach dem Zumachen nichts mehr aufgefrischt wird, hängt sonst an einer + // Zuweisung drei Funktionen weiter oben. + useEffect(() => { + if (!open || !gesperrt.current) return; + const timer = setInterval(() => { + if (gesperrt.current) void entwurfSperren(gesperrt.current); + }, TAKT_MS); + return () => clearInterval(timer); + }, [open, openKey]); + + // Beim Verlassen der Seite: dieselbe Freigabe wie beim Schliessen. Sie + // erreicht den Server nicht immer — ein zugeklappter Laptop schickt nichts + // mehr. Deshalb ist sie die Höflichkeit und nicht die Absicherung; die + // Absicherung ist die Frist in der Datenbank. + useEffect(() => () => freigeben(), []); + return ( {children} setOpen(false)} + onClose={closeWizard} openPositions={openPositions} locations={locations} resumeDraft={resumeDraft} diff --git a/db/migrations/20260910160000_entwurf_sperre.sql b/db/migrations/20260910160000_entwurf_sperre.sql new file mode 100644 index 0000000..1c52bf6 --- /dev/null +++ b/db/migrations/20260910160000_entwurf_sperre.sql @@ -0,0 +1,167 @@ +-- Fremde Entwürfe fortsetzen — einer nach dem anderen. +-- +-- Seit 20260910120000 sieht man die Entwürfe der hinzugewählten +-- Kolleg:innen. Sehen genügt nicht: wer im Urlaub vertreten wird, soll die +-- angefangene Einstellung zu Ende bringen können. Also darf ab jetzt auch +-- geschrieben werden — aber nicht von zweien gleichzeitig. +-- +-- ═══ Warum überhaupt eine Sperre ═══════════════════════════════════ +-- +-- Ein Entwurf ist ein einziges JSONB-Feld. Wer speichert, schreibt den +-- **ganzen** Stand, nicht das geänderte Feld. Zwei Personen, die den +-- Assistenten offen haben, überschreiben einander also vollständig, und die +-- Zweite merkt nichts davon: sie sieht ihren eigenen Stand. Genau das war +-- der Grund, das Schreiben bisher beim Eigentümer zu lassen. +-- +-- Die Sperre ist die Bedingung dafür, das aufzugeben. +-- +-- ═══ Warum sie ablaufen muss ═══════════════════════════════════════ +-- +-- Freigegeben wird beim Schliessen des Assistenten. Ein zugeklappter Laptop, +-- ein geschlossener Reiter, ein Absturz — dann kommt kein Schliessen mehr. +-- Ohne Ablauf bliebe der Entwurf für immer gesperrt, und niemand käme je +-- wieder heran; das wäre schlimmer als das Problem, das die Sperre löst. +-- +-- Die Frist steht in einer Funktion und nicht als Zahl an drei Stellen: +-- die Regeln hier und die Abfrage in lib/entwuerfe.ts fragen dieselbe. +-- Der Assistent frischt die Sperre auf, solange er offen ist, damit ein +-- langes Ausfüllen sie nicht verliert. + +alter table hire_drafts + add column if not exists locked_by uuid references app_users(id) on delete set null, + add column if not exists locked_at timestamptz; + +comment on column hire_drafts.locked_by is + 'Wer den Entwurf gerade offen hat. Zusammen mit locked_at und app_entwurf_sperrfrist() entscheidet es, wer schreiben darf.'; +comment on column hire_drafts.locked_at is + 'Wann die Sperre gesetzt oder zuletzt aufgefrischt wurde. Älter als die Frist heisst: der Entwurf ist wieder frei.'; + +create or replace function app_entwurf_sperrfrist() + returns interval + language sql + immutable +as $$ select interval '15 minutes' $$; + +comment on function app_entwurf_sperrfrist() is + 'Wie lange eine Entwurfssperre ohne Auffrischen gilt. Eine Quelle für die Regeln und für die Anzeige.'; + +-- Ist der Entwurf für mich beschreibbar? Frei, meiner, oder abgelaufen. +create or replace function app_entwurf_frei(sperre uuid, seit timestamptz) + returns boolean + language sql + stable +as $$ + select sperre is null + or sperre = app_current_user_id() + or seit is null + or seit < now() - app_entwurf_sperrfrist() +$$; + +-- Darf ich diesen Entwurf überhaupt anfassen? Meiner, oder von jemandem, +-- den ich hinzugewählt habe. Dieselbe Frage wie beim Lesen. +create or replace function app_entwurf_zugriff(besitzer uuid) + returns boolean + language sql + stable +as $$ + select besitzer = app_current_user_id() + or exists ( + select 1 from colleague_subscriptions s + where s.user_id = app_current_user_id() and s.author_user_id = besitzer) +$$; + +-- ═══ Die Regeln ═══════════════════════════════════════════════════ +-- +-- Lesen bleibt, wie es war. Ändern und Löschen reichen jetzt so weit wie +-- das Lesen — **und** verlangen zusätzlich die Sperre. +-- +-- Löschen muss mitgehen, obwohl das nach mehr klingt, als gewollt war: der +-- Assistent löscht den Entwurf, sobald die Person angelegt ist +-- (HireWizard.tsx). Ohne Löschrecht liefe die Einstellung durch und der +-- Entwurf bliebe als Karteileiche stehen. +-- +-- Anlegen bleibt beim eigenen Namen: einen Entwurf auf fremden Namen zu +-- eröffnen, ergibt keinen Fall. + +drop policy if exists "hire_drafts_select" on hire_drafts; +create policy "hire_drafts_select" on hire_drafts + for select + using (is_hr_user() and app_entwurf_zugriff(created_by)); + +drop policy if exists "hire_drafts_update" on hire_drafts; +create policy "hire_drafts_update" on hire_drafts + for update + using (is_hr_user() and app_entwurf_zugriff(created_by) and app_entwurf_frei(locked_by, locked_at)) + with check (is_hr_user() and app_entwurf_zugriff(created_by)); + +drop policy if exists "hire_drafts_delete" on hire_drafts; +create policy "hire_drafts_delete" on hire_drafts + for delete + using (is_hr_user() and app_entwurf_zugriff(created_by) and app_entwurf_frei(locked_by, locked_at)); + +-- ═══ Was das der Anwendung abverlangt ══════════════════════════════ +-- +-- Eine Regel weist ein UPDATE nicht mit einem Fehler ab, sie lässt es ins +-- Leere laufen. Ein Speichern gegen eine fremde Sperre trifft also keine +-- Zeile und meldet nichts. actions/hireDrafts.ts liest deshalb die Zahl der +-- betroffenen Zeilen — sonst stünde „Entwurf gespeichert" über einer +-- Änderung, die niemand hat. + +-- ═══ Gegenprobe ═══════════════════════════════════════════════════ +do $$ +declare + anzahl int; +begin + if not exists ( + select 1 from information_schema.columns + where table_name = 'hire_drafts' and column_name in ('locked_by', 'locked_at') + having count(*) = 2 + ) then + raise exception 'hire_drafts fehlen die Sperrspalten.'; + end if; + + -- Ohne Frist wäre die Sperre endgültig: ein abgestürzter Reiter nähme den + -- Entwurf für immer mit. + if app_entwurf_sperrfrist() <= interval '0' then + raise exception 'Die Sperrfrist ist nicht positiv — eine Sperre ohne Ablauf.'; + end if; + + select count(*) into anzahl from pg_policies where tablename = 'hire_drafts'; + if anzahl <> 4 then + raise exception 'hire_drafts hat % Regeln, erwartet werden 4.', anzahl; + end if; + + -- Die alte Sammelregel darf nicht zurückkommen: Policies addieren sich, + -- eine mit `for all` hebelte die Sperre aus. + if exists (select 1 from pg_policies where tablename = 'hire_drafts' and cmd = 'ALL') then + raise exception 'Auf hire_drafts steht wieder eine Sammelregel — sie umginge die Sperre.'; + end if; + + -- Schreiben muss die Sperre prüfen, Lesen darf es nicht: wer zusieht, + -- sperrt nichts, und ein Entwurf, den man nicht mehr sähe, sobald ihn + -- jemand offen hat, wäre aus der Liste verschwunden. + if exists ( + select 1 from pg_policies + where tablename = 'hire_drafts' and cmd in ('UPDATE', 'DELETE') + and coalesce(qual, '') not like '%app_entwurf_frei%' + ) then + raise exception 'Eine Schreibregel auf hire_drafts prüft die Sperre nicht.'; + end if; + + if exists ( + select 1 from pg_policies + where tablename = 'hire_drafts' and cmd = 'SELECT' + and coalesce(qual, '') like '%app_entwurf_frei%' + ) then + raise exception 'Die Leseregel prüft die Sperre — ein gesperrter Entwurf verschwände aus der Liste.'; + end if; + + -- Anlegen bleibt eigentuemergebunden. + if exists ( + select 1 from pg_policies + where tablename = 'hire_drafts' and cmd = 'INSERT' + and coalesce(with_check, '') like '%colleague_subscriptions%' + ) then + raise exception 'Die Anlegeregel kennt die Abos — ein Entwurf auf fremden Namen waere moeglich.'; + end if; +end $$; diff --git a/lib/entwuerfe.ts b/lib/entwuerfe.ts index c6173f1..c3409fa 100644 --- a/lib/entwuerfe.ts +++ b/lib/entwuerfe.ts @@ -25,6 +25,10 @@ export type EntwurfZeile = { created_by: string | null; author_name: string | null; author_email: string | null; + /** Wahr, solange jemand **anderes** den Entwurf offen hat. Aus der Datenbank, nicht gerechnet. */ + gesperrt: boolean; + sperrer_name: string | null; + sperrer_email: string | null; }; export type Entwurf = { @@ -36,6 +40,13 @@ export type Entwurf = { vonMir: boolean; /** Wer ihn angefangen hat — steht an jeder Zeile, auch an den eigenen. */ autor: string; + /** + * Wer ihn gerade offen hat, wenn es jemand anderes ist — sonst leer. + * + * Nicht dasselbe wie „nicht meiner": ein fremder Entwurf ist bearbeitbar, + * ein gesperrter nicht. Die Karte hängt daran, ob „Fortsetzen" wählbar ist. + */ + gesperrtVon: string | null; }; /** @@ -56,9 +67,16 @@ export function entwuerfeAbfrage(eb: OrgEb, userId: string) { return eb .selectFrom("hire_drafts as d") .leftJoin("profiles as p", "p.id", "d.created_by") + .leftJoin("profiles as sp", "sp.id", "d.locked_by") .select(["d.id", "d.step", "d.payload", "d.created_by"]) .select(["p.full_name as author_name", "p.email as author_email"]) + .select(["sp.full_name as sperrer_name", "sp.email as sperrer_email"]) .select((x) => zeitstempel(x.ref("d.updated_at")).as("updated_at")) + // Ob die Sperre noch gilt, rechnet die Datenbank — mit **ihrer** Uhr und + // derselben Frist, die auch die Schreibregeln anwenden. Hier gerechnet + // wäre es die Uhr des Servers, der gerade rendert, und eine zweite + // Fassung der Frist. + .select(sql`not app_entwurf_frei("d"."locked_by", "d"."locked_at")`.as("gesperrt")) .where((e) => e.or([e("d.created_by", "=", userId), istHinzugewaehlt(userId, "d.created_by")])) // Die eigenen zuerst. Vor der Freigabe standen dort nur sie; wer seinen // halbfertigen Entwurf sucht, soll ihn nicht zwischen fremden suchen. @@ -75,6 +93,10 @@ export function baueEntwuerfe(rows: EntwurfZeile[], userId: string | null): Entw payload: row.payload, updated_at: row.updated_at, vonMir: userId !== null && row.created_by === userId, + // Ohne Namen die E-Mail. „Jemand" als letzter Ausweg: dass der Entwurf + // belegt ist, muss auch dann herauskommen, wenn nicht zu sagen ist, + // von wem — sonst wirkte „Fortsetzen" grundlos abgeschaltet. + gesperrtVon: row.gesperrt ? row.sperrer_name?.trim() || row.sperrer_email || "jemandem" : null, // Ohne Namen die E-Mail — sonst stünde an einem Entwurf nichts ausser // dem Hinweis, dass er von jemandem ist. autor: row.author_name?.trim() || row.author_email || "Unbekannt", diff --git a/lib/shell-data.ts b/lib/shell-data.ts index 7c543fd..23bbd41 100644 --- a/lib/shell-data.ts +++ b/lib/shell-data.ts @@ -1,6 +1,7 @@ import { sql } from "kysely"; import type { Tx } from "./db"; -import { jsonArrayFrom, jsonObjectFrom, zeitstempel } from "./db/json"; +import { jsonArrayFrom, jsonObjectFrom } from "./db/json"; +import { baueEntwuerfe, entwuerfeAbfrage, type Entwurf, type EntwurfZeile } from "./entwuerfe"; import { todayIso } from "./format"; import { baueOffeneNotizen, offeneNotizenAbfrage, type NotizZeile, type OpenNote } from "./notes"; import { buildOrgMaps, orgMapsAbfragen, type Location } from "./org"; @@ -26,7 +27,7 @@ export type ShellData = { profile: { full_name: string | null; email: string | null; role: string | null; is_active: boolean | null }; openPositions: OpenPositionResolved[]; locations: Location[]; - drafts: { id: string; step: number; payload: Record; updated_at: string }[]; + drafts: Entwurf[]; openNotes: OpenNote[]; /** * Die HR-Kolleg:innen für die Sichtbarkeitseinstellung der Glocke. @@ -90,20 +91,16 @@ export async function loadShellData(tx: Tx, userId: string): Promise zeitstempel(x.ref("updated_at")).as("updated_at")) - // Hier bewusst **nur** die eigenen, obwohl die Übersicht seit - // September 2026 auch fremde zeigt: diese Liste füttert den - // Einstellungsassistenten (HireWizardProvider), und was er darin - // findet, lässt sich fortsetzen. Ein fremder Entwurf gehört nicht - // hinein — die Regel hire_drafts_update wiese das Speichern ab, - // und die Person hätte den Assistenten umsonst durchlaufen. - .where("created_by", "=", userId) - .orderBy("updated_at", "desc") - ).as("drafts"), + // Dieselbe Liste wie auf der Übersicht, aus derselben Abfrage: die + // eigenen und die der hinzugewählten Kolleg:innen. + // + // Sie füttert den Einstellungsassistenten (HireWizardProvider), und was + // er darin findet, lässt sich fortsetzen. Bis zur Sperre standen hier + // nur die eigenen — ein fremder Entwurf wäre umsonst durchlaufen + // worden, weil das Speichern an der Regel gescheitert wäre. Seit es + // die Sperre gibt (20260910160000), ist Fortsetzen erlaubt, solange + // niemand anderes drin ist. + jsonArrayFrom(entwuerfeAbfrage(eb, userId)).as("drafts"), ]) .executeTakeFirstOrThrow(); @@ -128,7 +125,7 @@ export async function loadShellData(tx: Tx, userId: string): Promise; }; + // locked_by/locked_at: wer den Entwurf gerade offen hat. Die Regeln + // hire_drafts_update und _delete lassen nur den Halter schreiben, und + // die Sperre läuft ab (Migration 20260910160000) — sonst nähme ein + // geschlossener Reiter den Entwurf für immer mit. hire_drafts: { - Row: { id: string; created_by: string | null; step: number; payload: Record; updated_at: string }; - Insert: { id?: string; created_by?: string | null; step?: number; payload: Record; updated_at?: string }; + Row: { + id: string; + created_by: string | null; + step: number; + payload: Record; + updated_at: string; + locked_by: string | null; + locked_at: string | null; + }; + Insert: { + id?: string; + created_by?: string | null; + step?: number; + payload: Record; + updated_at?: string; + locked_by?: string | null; + locked_at?: string | null; + }; Update: Partial; }; saved_reports: { diff --git a/tests/components/DraftsCard.test.tsx b/tests/components/DraftsCard.test.tsx index f3a869b..5a2cbd0 100644 --- a/tests/components/DraftsCard.test.tsx +++ b/tests/components/DraftsCard.test.tsx @@ -28,6 +28,7 @@ function entwurf(teil: Partial = {}): Entwurf { updated_at: "2026-09-09T08:00:00.000Z", vonMir: true, autor: "Max Stubhan", + gesperrtVon: null, ...teil, }; } @@ -75,26 +76,42 @@ describe("DraftsCard", () => { expect(within(zeile("Eva Novak")).getByText("von Anna Berger")).toBeInTheDocument(); }); - it("bietet an fremden Entwürfen weder Fortsetzen noch Löschen", () => { - // Die Datenbank wiese beides ab (hire_drafts_update, hire_drafts_delete). - // Eine Schaltfläche, die verlässlich in einen Fehler führt, ist ein - // Versprechen ohne Deckung. + it("lässt auch fremde Entwürfe fortsetzen", () => { + // Der Sinn der ganzen Freigabe: wer vertreten wird, soll die angefangene + // Einstellung zu Ende bringen können. zeige([fremd]); - const z = zeile("Eva Novak"); - expect(within(z).queryByRole("button", { name: "Fortsetzen" })).not.toBeInTheDocument(); - expect(within(z).queryByRole("button", { name: "Entwurf löschen" })).not.toBeInTheDocument(); + expect(within(zeile("Eva Novak")).getByRole("button", { name: "Fortsetzen" })).toBeInTheDocument(); }); - it("sagt an fremden Entwürfen, warum dort nichts steht", () => { - // Ohne den Hinweis sähe die Zeile aus wie eine, an der die Schaltflächen - // fehlen. + it("bietet an fremden Entwürfen kein Löschen", () => { + // Die Regel liesse es zu — sie muss, weil der Assistent den Entwurf nach + // dem Anlegen selbst wegräumt. Eine Schaltfläche, die fremde halbfertige + // Arbeit wegwirft, gehört trotzdem nicht neben eine, die sie fortsetzt. zeige([fremd]); - expect(within(zeile("Eva Novak")).getByText("Nur zur Ansicht")).toBeInTheDocument(); + expect(within(zeile("Eva Novak")).queryByRole("button", { name: "Entwurf löschen" })).not.toBeInTheDocument(); }); - it("trennt die Zeilen: eigene bedienbar, fremde nicht", () => { + it("nimmt Fortsetzen weg, solange jemand anderes drin ist", () => { + zeige([entwurf({ gesperrtVon: "Bernd Huber" })]); + expect(within(zeile("Manuel Aigner")).queryByRole("button", { name: "Fortsetzen" })).not.toBeInTheDocument(); + }); + + it("sagt, wer den gesperrten Entwurf gerade bearbeitet", () => { + // Eine graue Schaltfläche ohne Erklärung liest sich wie ein Fehler, und + // die nächste Handlung wäre, es gleich noch einmal zu versuchen. + zeige([entwurf({ gesperrtVon: "Bernd Huber" })]); + expect(within(zeile("Manuel Aigner")).getByText(/Bernd Huber bearbeitet gerade/)).toBeInTheDocument(); + }); + + it("sperrt auch das Löschen des eigenen Entwurfs, solange jemand drin ist", () => { + // Sonst risse man jemandem den Entwurf unter den Händen weg. + zeige([entwurf({ gesperrtVon: "Bernd Huber" })]); + expect(within(zeile("Manuel Aigner")).getByRole("button", { name: "Entwurf löschen" })).toBeDisabled(); + }); + + it("trennt die Zeilen: freie bedienbar, gesperrte nicht", () => { // Beide nebeneinander — der Fall, in dem eine Verwechslung teuer wird. - zeige([entwurf(), fremd]); + zeige([entwurf(), entwurf({ id: "d9", payload: { firstName: "Eva", lastName: "Novak" }, gesperrtVon: "Bernd Huber" })]); expect(within(zeile("Manuel Aigner")).getByRole("button", { name: "Fortsetzen" })).toBeInTheDocument(); expect(within(zeile("Eva Novak")).queryByRole("button", { name: "Fortsetzen" })).not.toBeInTheDocument(); }); diff --git a/tests/components/HireWizardSperre.test.tsx b/tests/components/HireWizardSperre.test.tsx new file mode 100644 index 0000000..3063e80 --- /dev/null +++ b/tests/components/HireWizardSperre.test.tsx @@ -0,0 +1,198 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { HireWizardProvider, useHireWizard } from "@/components/hire/HireWizardContext"; +import { ToastProvider } from "@/components/ui/Toast"; +import type { Entwurf } from "@/lib/entwuerfe"; + +const entwurfSperren = vi.fn<(id: string) => Promise<{ success: boolean; error?: string }>>(async () => ({ + success: true, +})); +const entwurfFreigeben = vi.fn<(id: string) => Promise<{ success: boolean }>>(async () => ({ success: true })); +const refresh = vi.fn(); + +vi.mock("@/actions/hireDrafts", () => ({ + entwurfSperren: (id: string) => entwurfSperren(id), + entwurfFreigeben: (id: string) => entwurfFreigeben(id), + saveHireDraft: vi.fn(), + deleteHireDraft: vi.fn(), +})); +vi.mock("next/navigation", () => ({ useRouter: () => ({ refresh, push: vi.fn() }) })); + +// Der Assistent selbst spielt hier keine Rolle — geprüft wird, was um ihn +// herum passiert: die Sperre holen, bevor er aufgeht, und sie zurückgeben, +// wenn er zugeht. +vi.mock("@/components/hire/HireWizard", () => ({ + HireWizard: ({ open, onClose, resumeDraft }: { open: boolean; onClose: () => void; resumeDraft: Entwurf | null }) => + open ? ( +
+

Assistent offen{resumeDraft ? `: ${resumeDraft.id}` : ""}

+ +
+ ) : null, +})); + +const ENTWURF: Entwurf = { + id: "d1", + step: 2, + payload: { firstName: "Manuel", lastName: "Aigner" }, + updated_at: "2026-09-09T08:00:00.000Z", + vonMir: false, + autor: "Anna Berger", + gesperrtVon: null, +}; + +function Ausloeser() { + const { openWizard } = useHireWizard(); + return ( + <> + + + + ); +} + +function zeige() { + return render( + + + + + + ); +} + +beforeEach(() => { + entwurfSperren.mockClear(); + entwurfSperren.mockResolvedValue({ success: true }); + entwurfFreigeben.mockClear(); + refresh.mockClear(); +}); + +describe("Sperre um den Assistenten", () => { + it("holt die Sperre, bevor der Assistent aufgeht", async () => { + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + + expect(entwurfSperren).toHaveBeenCalledWith("d1"); + expect(await screen.findByText("Assistent offen: d1")).toBeInTheDocument(); + }); + + it("macht gar nicht erst auf, wenn die Sperre belegt ist", async () => { + // Aufzumachen und beim Speichern zu scheitern wäre der teure Weg: die + // Person hätte den ganzen Assistenten umsonst durchlaufen. + entwurfSperren.mockResolvedValue({ success: false, error: "Wird gerade bearbeitet." }); + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + + expect(screen.queryByText(/Assistent offen/)).not.toBeInTheDocument(); + expect(await screen.findByText("Wird gerade bearbeitet.")).toBeInTheDocument(); + }); + + it("holt die Liste nach, wenn die Sperre belegt war", async () => { + // Danach steht an der Zeile, wer sie hält — statt dass nur eine Meldung + // aufblitzt und die Karte weiter „Fortsetzen" anbietet. + entwurfSperren.mockResolvedValue({ success: false, error: "Wird gerade bearbeitet." }); + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + + expect(refresh).toHaveBeenCalled(); + }); + + it("gibt die Sperre zurück, wenn der Assistent zugeht", async () => { + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + await user.click(await screen.findByRole("button", { name: "Zumachen" })); + + expect(entwurfFreigeben).toHaveBeenCalledWith("d1"); + }); + + it("gibt nichts frei, was nie gesperrt war", async () => { + // Eine neue Einstellung hat noch keinen Entwurf — es gäbe keine Zeile, + // auf die sich die Freigabe beziehen könnte. + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Neu" })); + await user.click(await screen.findByRole("button", { name: "Zumachen" })); + + expect(entwurfSperren).not.toHaveBeenCalled(); + expect(entwurfFreigeben).not.toHaveBeenCalled(); + }); + + it("sperrt beim Anlegen ohne Entwurf nichts", async () => { + const user = userEvent.setup(); + zeige(); + await user.click(screen.getByRole("button", { name: "Neu" })); + + expect(entwurfSperren).not.toHaveBeenCalled(); + expect(await screen.findByText("Assistent offen")).toBeInTheDocument(); + }); + + it("gibt die Sperre nur einmal zurück", async () => { + // Ein zweites Freigeben träfe womöglich schon die Sperre der nächsten + // Person, die den Entwurf inzwischen geöffnet hat. + const user = userEvent.setup(); + const { unmount } = zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + await user.click(await screen.findByRole("button", { name: "Zumachen" })); + unmount(); + + expect(entwurfFreigeben).toHaveBeenCalledTimes(1); + }); + + it("gibt die Sperre auch beim Verlassen der Seite zurück", async () => { + // Zumachen ist der Normalfall, Wegnavigieren nicht der seltene. + const user = userEvent.setup(); + const { unmount } = zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + unmount(); + + expect(entwurfFreigeben).toHaveBeenCalledWith("d1"); + }); + + it("frischt die Sperre auf, solange der Assistent offen ist", async () => { + // Ohne das liefe die Frist einem langen Ausfüllen davon, und das + // Speichern am Ende fände seine eigene Sperre abgelaufen. + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + expect(entwurfSperren).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(11 * 60 * 1000); + expect(entwurfSperren.mock.calls.length).toBeGreaterThan(1); + expect(entwurfSperren).toHaveBeenLastCalledWith("d1"); + } finally { + vi.useRealTimers(); + } + }); + + it("hört mit dem Auffrischen auf, sobald zugemacht ist", async () => { + // Sonst hielte ein zugeklappter Assistent den Entwurf endlos belegt — + // genau das, was die Frist verhindern soll. + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); + zeige(); + await user.click(screen.getByRole("button", { name: "Fortsetzen" })); + await user.click(await screen.findByRole("button", { name: "Zumachen" })); + const nachher = entwurfSperren.mock.calls.length; + + await vi.advanceTimersByTimeAsync(30 * 60 * 1000); + expect(entwurfSperren.mock.calls.length).toBe(nachher); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/tests/unit/entwuerfe.test.ts b/tests/unit/entwuerfe.test.ts index 6772035..e501f44 100644 --- a/tests/unit/entwuerfe.test.ts +++ b/tests/unit/entwuerfe.test.ts @@ -71,6 +71,21 @@ describe("entwuerfeAbfrage", () => { expect(parameters).toContain("bösartig'; drop table hire_drafts; --"); }); + it("lässt die Datenbank entscheiden, ob die Sperre noch gilt", () => { + // Mit der Uhr des Servers, der gerade rendert, gerechnet wäre es eine + // zweite Uhr und eine zweite Fassung der Frist — die Regeln in der + // Datenbank benutzen dieselbe Funktion. + const s = text(); + expect(s).toContain('not app_entwurf_frei("d"."locked_by", "d"."locked_at")'); + expect(s).toContain('left join "profiles" as "sp" on "sp"."id" = "d"."locked_by"'); + }); + + it("filtert gesperrte Entwürfe nicht heraus", () => { + // Sie sollen dastehen, nur nicht anfassbar sein. Verschwänden sie, + // suchte man einen Entwurf, den es gibt. + expect(text()).not.toMatch(/where[^;]*locked_by/); + }); + it("formatiert den Zeitstempel innerhalb von JSON ausdrücklich", () => { // In JSON gibt Postgres ihn anders aus als der Treiber es täte, und der // Unterschied fällt erst beim Vergleichen zweier Listen auf. @@ -88,6 +103,9 @@ describe("baueEntwuerfe", () => { created_by: ANDERE, author_name: "Anna Berger", author_email: "a@example.test", + gesperrt: false, + sperrer_name: null, + sperrer_email: null, ...teil, }; } @@ -129,6 +147,37 @@ describe("baueEntwuerfe", () => { expect(e.vonMir).toBe(false); }); + it("lässt einen freien Entwurf ohne Sperrvermerk", () => { + // Leer heisst hier „bearbeitbar" — die Karte hängt daran, ob sie + // „Fortsetzen" anbietet. + expect(baueEntwuerfe([zeile()], ICH)[0].gesperrtVon).toBeNull(); + }); + + it("nennt, wer den Entwurf gerade offen hat", () => { + const [e] = baueEntwuerfe([zeile({ gesperrt: true, sperrer_name: "Bernd Huber" })], ICH); + expect(e.gesperrtVon).toBe("Bernd Huber"); + }); + + it("fällt beim Sperrer ohne Namen auf die E-Mail zurück", () => { + const [e] = baueEntwuerfe([zeile({ gesperrt: true, sperrer_name: " ", sperrer_email: "b@example.test" })], ICH); + expect(e.gesperrtVon).toBe("b@example.test"); + }); + + it("sagt auch ohne jeden Namen, dass gesperrt ist", () => { + // Sonst wirkte „Fortsetzen" grundlos abgeschaltet, und die nächste + // Handlung wäre, es gleich noch einmal zu versuchen. + const [e] = baueEntwuerfe([zeile({ gesperrt: true, sperrer_name: null, sperrer_email: null })], ICH); + expect(e.gesperrtVon).toBe("jemandem"); + }); + + it("unterscheidet fremd von gesperrt", () => { + // Ein fremder Entwurf ist bearbeitbar, ein gesperrter nicht — würden die + // beiden verwechselt, wäre die ganze Freigabe wirkungslos. + const [e] = baueEntwuerfe([zeile()], ICH); + expect(e.vonMir).toBe(false); + expect(e.gesperrtVon).toBeNull(); + }); + it("reicht Inhalt und Zeitpunkt unverändert weiter", () => { const [e] = baueEntwuerfe([zeile()], ICH); expect(e).toMatchObject({ id: "d1", step: 2, updated_at: "2026-09-09T08:00:00.000Z" });